Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

builtin:fetchurl: Ensure a fixed-output derivation #9902

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

edolstra
Copy link
Member

@edolstra edolstra commented Feb 1, 2024

Motivation

We accidentally allowed

builtins.derivation {
  name = "nix-cache-info";
  system = "x86_64-linux";
  builder = "builtin:fetchurl";
  url = https://cache.nixos.org/nix-cache-info;
  outputHashMode = "flat";
}

to succeed.

Also clean up the hashed mirror code a bit.

Context

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

Previously we didn't check that the derivation was fixed-output, so
you could use builtin:fetchurl to impurely fetch a file.
@edolstra edolstra added release-blocker backport 2.20-maintenance Automatically creates a PR against the branch labels Feb 1, 2024
@edolstra edolstra requested a review from thufschmitt as a code owner February 1, 2024 21:04
@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Feb 1, 2024
Copy link
Member

@thufschmitt thufschmitt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops 😬

Thanks for the fix

I think the test failure is due to the client not returning the right error code in daemon mode (there's an issue about this somewhere, but I can't find it any more)

@edolstra edolstra added the backport 2.19-maintenance Automatically creates a PR against the branch label Feb 2, 2024
@edolstra edolstra enabled auto-merge February 2, 2024 12:22
@edolstra edolstra disabled auto-merge February 2, 2024 12:28
@edolstra edolstra force-pushed the require-fixed-output-fetchurl branch from cbbe304 to e67458e Compare February 2, 2024 12:35
@edolstra edolstra enabled auto-merge February 2, 2024 12:35
@edolstra edolstra merged commit 081dc5d into master Feb 2, 2024
13 checks passed
@edolstra edolstra deleted the require-fixed-output-fetchurl branch February 2, 2024 13:00
Copy link

github-actions bot commented Feb 2, 2024

Backport failed for 2.19-maintenance, because it was unable to cherry-pick the commit(s).

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin 2.19-maintenance
git worktree add -d .worktree/backport-9902-to-2.19-maintenance origin/2.19-maintenance
cd .worktree/backport-9902-to-2.19-maintenance
git switch --create backport-9902-to-2.19-maintenance
git cherry-pick -x 1ee42c5b88eb0533ebcf8b2579ec82f2be80e4b2 b8b739e484078863c10c48d031fa8459081ba8b3 05535be03a1526061ea3a3ad25459c032e1f8f8c e67458e5b821e0a3a6839f4637eb96ff873f64ed

Copy link

github-actions bot commented Feb 2, 2024

Successfully created backport PR for 2.20-maintenance:

@puckipedia
Copy link
Contributor

puckipedia commented Feb 2, 2024

Hi,

This breaks some of my code. I consider this to be a stable feature. Note that this doesn't actually run builtin:fetchurl inside a FOD, which means that the example given only succeeds if run with sandboxing disabled; one could just use curl, or netcat, or even just bash, to do the same, and i'm not seeing an issue for that problem, seeing as it's inherent to not being in a sandbox.

puckipedia added a commit to puckipedia/nix that referenced this pull request Feb 2, 2024
…-fetchurl"

This reverts commit 081dc5d, reversing
changes made to ef6d055.
puckipedia added a commit to puckipedia/nix that referenced this pull request Feb 2, 2024
…-fetchurl"

This is not actually a bug on Nix (version 2.18, at least), and breaks code
I've written (with no other way to write that code.)

    nix-repl> :b builtins.derivation { name = "nix-cache-info"; system = "x86_64-linux"; builder = "builtin:fetchurl"; url = https://cache.nixos.org/nix-cache-info; outputHashMode = "flat"; }
    warning: error: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't resolve host name (6); retrying in 309 ms
    warning: error: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't resolve host name (6); retrying in 607 ms
    warning: error: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't resolve host name (6); retrying in 1002 ms
    warning: error: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't resolve host name (6); retrying in 2554 ms
    error: builder for '/nix/store/z89j6fdfm6k9k1qir99449wyvldpm2ma-nix-cache-info.drv' failed with exit code 1;
           last 4 log lines:
           > error:
           >        … writing file '/nix/store/l9ax62fdagg4cvw9wd4h84cf4pxqcmlg-nix-cache-info'
           >
           >        error: unable to download 'https://cache.nixos.org/nix-cache-info': Couldn't resolve host name (6)
           For full logs, run 'nix-store -l /nix/store/z89j6fdfm6k9k1qir99449wyvldpm2ma-nix-cache-info.drv'.

This reverts commit 081dc5d, reversing
changes made to ef6d055.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.19-maintenance Automatically creates a PR against the branch backport 2.20-maintenance Automatically creates a PR against the branch release-blocker with-tests Issues related to testing. PRs with tests have some priority
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants