From 0bfa0cdea1b4eb09405e35b338887c91a041d28c Mon Sep 17 00:00:00 2001 From: Martin Schwaighofer Date: Mon, 13 Dec 2021 21:31:15 +0100 Subject: [PATCH 01/83] git fetcher: improve check for valid repository The .git/refs/heads directory might be empty for a valid usable git repository. This often happens in CI environments, which might only fetch commits, not branches. Therefore instead we let git itself check if HEAD points to something that looks like a commit. fixes #5302 --- src/libfetchers/git.cc | 28 ++++++++++++++-------------- tests/fetchGit.sh | 8 ++++++++ 2 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index c3f0f8c8f..7479318e3 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -220,21 +220,21 @@ struct GitInputScheme : InputScheme if (!input.getRef() && !input.getRev() && isLocal) { bool clean = false; - /* Check whether this repo has any commits. There are - probably better ways to do this. */ - auto gitDir = actualUrl + "/.git"; - auto commonGitDir = chomp(runProgram( - "git", - true, - { "-C", actualUrl, "rev-parse", "--git-common-dir" } - )); - if (commonGitDir != ".git") - gitDir = commonGitDir; - - bool haveCommits = !readDirectory(gitDir + "/refs/heads").empty(); + /* Check whether HEAD points to something that looks like a commit, + since that is the refrence we want to use later on. */ + bool hasHead = false; + try { + runProgram("git", true, { "-C", actualUrl, "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }); + hasHead = true; + } catch (ExecError & e) { + // git exits with status 128 here if it does not detect a repository. + if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 128) { + throw Error("Git tree '%s' is broken.\n'git rev-parse --verify HEAD' failed with exit code %d.", actualUrl, WEXITSTATUS(e.status)); + } + } try { - if (haveCommits) { + if (hasHead) { runProgram("git", true, { "-C", actualUrl, "diff-index", "--quiet", "HEAD", "--" }); clean = true; } @@ -280,7 +280,7 @@ struct GitInputScheme : InputScheme // modified dirty file? input.attrs.insert_or_assign( "lastModified", - haveCommits ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); + hasHead ? std::stoull(runProgram("git", true, { "-C", actualUrl, "log", "-1", "--format=%ct", "--no-show-signature", "HEAD" })) : 0); return {std::move(storePath), input}; } diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 89294d8d2..dd0d98956 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -170,6 +170,14 @@ NIX=$(command -v nix) path5=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = $repo; ref = \"dev\"; }).outPath") [[ $path3 = $path5 ]] +# Fetching from a repo with only a specific revision and no branches should +# not fall back to copying files and record correct revision information. See: #5302 +mkdir $TEST_ROOT/minimal +git -C $TEST_ROOT/minimal init +git -C $TEST_ROOT/minimal fetch $repo $rev2 +git -C $TEST_ROOT/minimal checkout $rev2 +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit { url = $TEST_ROOT/minimal; }).rev") = $rev2 ]] + # Fetching a shallow repo shouldn't work by default, because we can't # return a revCount. git clone --depth 1 file://$repo $TEST_ROOT/shallow From c7e527b82b3dafed5f0da2ec0e14a47cf8e65def Mon Sep 17 00:00:00 2001 From: Martin Schwaighofer Date: Mon, 13 Dec 2021 21:31:20 +0100 Subject: [PATCH 02/83] git fetcher: invoke diff instead of diff-index diff-index operates on the view that git has of the working tree, which might be outdated. The higher-level diff command does this automatically. This change also adds handling for submodules. fixes #4140 Alternative fixes would be invoking update-index before diff-index or matching more closely what require_clean_work_tree from git-sh-setup.sh does, but both those options make it more difficult to reason about correctness. --- src/libfetchers/git.cc | 12 +++++++++++- tests/fetchGit.sh | 7 ++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 7479318e3..d241eb67a 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -235,7 +235,17 @@ struct GitInputScheme : InputScheme try { if (hasHead) { - runProgram("git", true, { "-C", actualUrl, "diff-index", "--quiet", "HEAD", "--" }); + // Using git diff is preferrable over lower-level operations here, + // because its conceptually simpler and we only need the exit code anyways. + auto gitDiffOpts = Strings({ "-C", actualUrl, "diff", "HEAD", "--quiet"}); + if (!submodules) { + // Changes in submodules should only make the tree dirty + // when those submodules will be copied as well. + gitDiffOpts.emplace_back("--ignore-submodules"); + } + gitDiffOpts.emplace_back("--"); + runProgram("git", true, gitDiffOpts); + clean = true; } } catch (ExecError & e) { diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index dd0d98956..628d96924 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -11,7 +11,7 @@ repo=$TEST_ROOT/git export _NIX_FORCE_HTTP=1 -rm -rf $repo ${repo}-tmp $TEST_HOME/.cache/nix $TEST_ROOT/worktree $TEST_ROOT/shallow +rm -rf $repo ${repo}-tmp $TEST_HOME/.cache/nix $TEST_ROOT/worktree $TEST_ROOT/shallow $TEST_ROOT/minimal git init $repo git -C $repo config user.email "foobar@example.com" @@ -147,8 +147,13 @@ path3=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") # (check dirty-tree handling was used) [[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = 0000000000000000000000000000000000000000 ]] [[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).shortRev") = 0000000 ]] +# Making a dirty tree clean again and fetching it should +# record correct revision information. See: #4140 +echo world > $repo/hello +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit $repo).rev") = $rev2 ]] # Committing shouldn't change store path, or switch to using 'master' +echo dev > $repo/hello git -C $repo commit -m 'Bla5' -a path4=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [[ $(cat $path4/hello) = dev ]] From 9504445cab095fe3869c5a68342fb2cf23ac0f28 Mon Sep 17 00:00:00 2001 From: Martin Schwaighofer Date: Sat, 1 Jan 2022 21:26:41 +0100 Subject: [PATCH 03/83] git fetcher: distinguish errors more precisely --- src/libfetchers/git.cc | 26 +++++++++++++++++--------- tests/fetchGit.sh | 8 ++++++++ 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index d241eb67a..ad877eacc 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -222,17 +222,25 @@ struct GitInputScheme : InputScheme /* Check whether HEAD points to something that looks like a commit, since that is the refrence we want to use later on. */ - bool hasHead = false; - try { - runProgram("git", true, { "-C", actualUrl, "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }); - hasHead = true; - } catch (ExecError & e) { - // git exits with status 128 here if it does not detect a repository. - if (!WIFEXITED(e.status) || WEXITSTATUS(e.status) != 128) { - throw Error("Git tree '%s' is broken.\n'git rev-parse --verify HEAD' failed with exit code %d.", actualUrl, WEXITSTATUS(e.status)); - } + auto result = runProgram(RunOptions { + .program = "git", + .args = { "-C", actualUrl, "--git-dir=.git", "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, + .mergeStderrToStdout = true + }); + auto exitCode = WEXITSTATUS(result.first); + auto errorMessage = result.second; + + if (errorMessage.find("fatal: not a git repository") != std::string::npos) { + throw Error("'%s' is not a git repository.", actualUrl); + } else if (errorMessage.find("fatal: Needed a single revision") != std::string::npos) { + // indicates that the repo does not have any commits + // we want to proceed and will consider it dirty later + } else if (exitCode != 0) { + // any other errors should lead to a failure + throw Error("Getting the HEAD of the git tree '%s' failed with exit code %d:\n%s", actualUrl, exitCode, errorMessage); } + bool hasHead = exitCode == 0; try { if (hasHead) { // Using git diff is preferrable over lower-level operations here, diff --git a/tests/fetchGit.sh b/tests/fetchGit.sh index 628d96924..ac23be5c0 100644 --- a/tests/fetchGit.sh +++ b/tests/fetchGit.sh @@ -206,3 +206,11 @@ rev4_nix=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$ # The name argument should be handled path9=$(nix eval --impure --raw --expr "(builtins.fetchGit { url = \"file://$repo\"; ref = \"HEAD\"; name = \"foo\"; }).outPath") [[ $path9 =~ -foo$ ]] + +# should fail if there is no repo +rm -rf $repo/.git +(! nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath") + +# should succeed for a repo without commits +git init $repo +path10=$(nix eval --impure --raw --expr "(builtins.fetchGit \"file://$repo\").outPath") From 53523c0ab834416e38a15cf7be6f71d8f68d1c99 Mon Sep 17 00:00:00 2001 From: Martin Schwaighofer Date: Mon, 7 Feb 2022 20:36:39 +0100 Subject: [PATCH 04/83] git fetcher: set locale for rev-parse --- src/libfetchers/git.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index ad877eacc..6571a9d02 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -220,11 +220,17 @@ struct GitInputScheme : InputScheme if (!input.getRef() && !input.getRev() && isLocal) { bool clean = false; + auto env = getEnv(); + // Set LC_ALL to C: because we rely on the error messages from git rev-parse to determine what went wrong + // that way unknown errors can lead to a failure instead of continuing through the wrong code path + env["LC_ALL"] = "C"; + /* Check whether HEAD points to something that looks like a commit, since that is the refrence we want to use later on. */ auto result = runProgram(RunOptions { .program = "git", .args = { "-C", actualUrl, "--git-dir=.git", "rev-parse", "--verify", "--no-revs", "HEAD^{commit}" }, + .environment = env, .mergeStderrToStdout = true }); auto exitCode = WEXITSTATUS(result.first); From 400d70a3a9ab494b2eebf5522789f89d16d000b2 Mon Sep 17 00:00:00 2001 From: toonn Date: Tue, 22 Feb 2022 16:28:24 +0100 Subject: [PATCH 05/83] doc: Add detailed uninstall section for macOS The multi-user installation on macOS, which is now the only option, has gotten complicated enough that it discourages some users from checking Nix out for fear of being left with a "dirty" system. Detailed uninstallation instructions should make this less of an issue. --- .../src/installation/installing-binary.md | 84 +++++++++++++++++-- 1 file changed, 79 insertions(+), 5 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index 4367654a2..ba90ce280 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -84,7 +84,9 @@ The installer will modify `/etc/bashrc`, and `/etc/zshrc` if they exist. The installer will first back up these files with a `.backup-before-nix` extension. The installer will also create `/etc/profile.d/nix.sh`. -You can uninstall Nix with the following commands: +## Uninstalling + +### Linux ```console sudo rm -rf /etc/profile/nix.sh /etc/nix /nix ~root/.nix-profile ~root/.nix-defexpr ~root/.nix-channels ~/.nix-profile ~/.nix-defexpr ~/.nix-channels @@ -95,15 +97,87 @@ sudo systemctl stop nix-daemon.service sudo systemctl disable nix-daemon.socket sudo systemctl disable nix-daemon.service sudo systemctl daemon-reload - -# If you are on macOS, you will need to run: -sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist -sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist ``` There may also be references to Nix in `/etc/profile`, `/etc/bashrc`, and `/etc/zshrc` which you may remove. +### macOS + +1. Edit `/etc/zshrc` and `/etc/bashrc` to remove the lines sourcing + `nix-daemon.sh`, which should look like this: + + ```bash + # Nix + if [ -e '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' ]; then + . '/nix/var/nix/profiles/default/etc/profile.d/nix-daemon.sh' + fi + # End Nix + ``` + + If these files haven't been altered since installing Nix you can simply put + the backups back in place: + + ```console + sudo mv /etc/zshrc.backup-before-nix /etc/zshrc + sudo mv /etc/bashrc.backup-before-nix /etc/bashrc + ``` + + This will stop shells from sourcing the file and bringing everything you + installed using Nix in scope. + +2. Stop and remove the Nix daemon services: + + ```console + sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo launchctl unload /Library/LaunchDaemons/org.nixos.activate-system.plist + sudo rm /Library/LaunchDaemons/org.nixos.activate-system.plist + ``` + + This stops the Nix daemon and prevents it from being started next time you + boot the system. + +3. Remove the `nixbld` group and the `_nixbuildN` users: + + ```console + sudo dscl . -delete /Groups/nixbld + for u in $(sudo dscl . -list /Users | grep _nixbld); do sudo dscl . -delete /Users/$u; done + ``` + + This will remove all the build users that no longer serve a purpose. + +4. Edit fstab using `sudo vifs` to remove the line mounting the Nix Store + volume on `/nix`, which looks like this, + `LABEL=Nix\040Store /nix apfs rw,nobrowse`. This will prevent automatic + mounting of the Nix Store volume. + +5. Edit `/etc/synthetic.conf` to remove the `nix` line. If this is the only + line in the file you can remove it entirely, `sudo rm /etc/synthetic.conf`. + This will prevent the creation of the empty `/nix` directory to provide a + mountpoint for the Nix Store volume. + +6. Remove the files Nix added to your system: + + ```console + sudo rm -rf /etc/nix /var/root/.nix-profile /var/root/.nix-defexpr /var/root/.nix-channels ~/.nix-profile ~/.nix-defexpr ~/.nix-channels + ``` + + This gets rid of any data Nix may have created except for the store which is + removed next. + +7. Remove the Nix Store volume: + + ```console + sudo diskutil apfs deleteVolume /nix + ``` + + This will remove the Nix Store volume and everything that was added to the + store. + +The change to `/etc/synthetic.conf` will only take effect after a reboot but +you shouldn't have any traces of Nix left on your system. + # macOS Installation From 064cad7e9fec88a2906dd0f87084278ec119f3af Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 25 Feb 2022 10:32:45 +0100 Subject: [PATCH 06/83] doc: Add macOS uninstall note about /nix Clarify that `/nix` being present after the uninstall is normal and it will only disappear after a reboot. Co-authored-by: Travis A. Everett --- doc/manual/src/installation/installing-binary.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index ba90ce280..ea5c9febe 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -175,8 +175,16 @@ and `/etc/zshrc` which you may remove. This will remove the Nix Store volume and everything that was added to the store. -The change to `/etc/synthetic.conf` will only take effect after a reboot but -you shouldn't have any traces of Nix left on your system. +> **Note** +> +> After you complete the steps here, you will still have an empty `/nix` +> directory. This is an expected sign of a successful uninstall. The empty +> `/nix` directory will disappear the next time you reboot. +> +> You do not have to reboot to finish uninstalling Nix. The uninstall is +> complete. macOS (Catalina+) directly controls root directories and its +> read-only root will prevent you from manually deleting the empty `/nix` +> mountpoint. # macOS Installation From 2df23e2b3e9dd456a42b93264ccd5390e88d8542 Mon Sep 17 00:00:00 2001 From: toonn Date: Fri, 25 Feb 2022 10:50:01 +0100 Subject: [PATCH 07/83] doc: Drop nix-darwin service from macOS uninstall --- doc/manual/src/installation/installing-binary.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index ea5c9febe..eb184c07b 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -131,8 +131,6 @@ and `/etc/zshrc` which you may remove. ```console sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist - sudo launchctl unload /Library/LaunchDaemons/org.nixos.activate-system.plist - sudo rm /Library/LaunchDaemons/org.nixos.activate-system.plist ``` This stops the Nix daemon and prevents it from being started next time you From 947d4761b35ffa5f689ad22d631696d35d28b941 Mon Sep 17 00:00:00 2001 From: toonn Date: Sat, 26 Feb 2022 14:16:35 +0100 Subject: [PATCH 08/83] doc: Add removal of darwin-store LaunchDaemon The uninstall instructions used to accidentally remove the nix-darwin LaunchDaemon, this was dropped. However, the original intent was to remove the Store volume mounting LaunchDaemon. --- doc/manual/src/installation/installing-binary.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/manual/src/installation/installing-binary.md b/doc/manual/src/installation/installing-binary.md index eb184c07b..e5fb50088 100644 --- a/doc/manual/src/installation/installing-binary.md +++ b/doc/manual/src/installation/installing-binary.md @@ -131,6 +131,8 @@ and `/etc/zshrc` which you may remove. ```console sudo launchctl unload /Library/LaunchDaemons/org.nixos.nix-daemon.plist sudo rm /Library/LaunchDaemons/org.nixos.nix-daemon.plist + sudo launchctl unload /Library/LaunchDaemons/org.nixos.darwin-store.plist + sudo rm /Library/LaunchDaemons/org.nixos.darwin-store.plist ``` This stops the Nix daemon and prevents it from being started next time you From 0c6e46e34965ba0db4e0b755ee473868a4fef21f Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Mar 2022 06:16:51 +0100 Subject: [PATCH 09/83] Add some suggestions to the evaluator MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the evaluator show some suggestions when trying to access an invalid field from an attrset. ```console $ nix eval --expr '{ foo = 1; }.foa' error: attribute 'foa' missing at «string»:1:1: 1| { foo = 1; }.foa | ^ Did you mean foo? ``` --- src/libexpr/eval.cc | 20 ++++++++++++++++++-- tests/suggestions.sh | 4 ++++ 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 5bf161cc0..1d88e8709 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -727,6 +727,15 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const std::string & s2 throw EvalError(s, s2); } +LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const Suggestions & suggestions, const char * s, const std::string & s2)) +{ + throw EvalError({ + .msg = hintfmt(s, s2), + .errPos = pos, + .suggestions = suggestions, + }); +} + LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const std::string & s2)) { throw EvalError({ @@ -1281,8 +1290,15 @@ void ExprSelect::eval(EvalState & state, Env & env, Value & v) } } else { state.forceAttrs(*vAttrs, pos); - if ((j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) - throwEvalError(pos, "attribute '%1%' missing", name); + if ((j = vAttrs->attrs->find(name)) == vAttrs->attrs->end()) { + std::set allAttrNames; + for (auto & attr : *vAttrs->attrs) + allAttrNames.insert(attr.name); + throwEvalError( + pos, + Suggestions::bestMatches(allAttrNames, name), + "attribute '%1%' missing", name); + } } vAttrs = j->value; pos2 = j->pos; diff --git a/tests/suggestions.sh b/tests/suggestions.sh index 16a5a7004..29d5b364b 100644 --- a/tests/suggestions.sh +++ b/tests/suggestions.sh @@ -34,3 +34,7 @@ NIX_BUILD_STDERR_WITH_SUGGESTIONS=$(! nix build .\#fob 2>&1 1>/dev/null) NIX_BUILD_STDERR_WITH_NO_CLOSE_SUGGESTION=$(! nix build .\#bar 2>&1 1>/dev/null) [[ ! "$NIX_BUILD_STDERR_WITH_NO_CLOSE_SUGGESTION" =~ "Did you mean" ]] || \ fail "The nix build stderr shouldn’t suggest anything if there’s nothing relevant to suggest" + +NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '(builtins.getFlake (builtins.toPath ./.)).packages.'$system'.fob' 2>&1 1>/dev/null) +[[ "$NIX_EVAL_STDERR_WITH_SUGGESTIONS" =~ "Did you mean one of fo1, fo2, foo or fooo?" ]] || \ + fail "The evaluator should suggest the three closest possiblities" From 33b7514035a967df2ab61ab9770627157aa4f5c5 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Mar 2022 16:07:17 +0100 Subject: [PATCH 10/83] Try and make the darwin build happy --- src/libexpr/eval.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1d88e8709..3bfb82b16 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -729,7 +729,7 @@ LocalNoInlineNoReturn(void throwEvalError(const char * s, const std::string & s2 LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const Suggestions & suggestions, const char * s, const std::string & s2)) { - throw EvalError({ + throw EvalError(ErrorInfo { .msg = hintfmt(s, s2), .errPos = pos, .suggestions = suggestions, @@ -738,7 +738,7 @@ LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const Suggestions & s LocalNoInlineNoReturn(void throwEvalError(const Pos & pos, const char * s, const std::string & s2)) { - throw EvalError({ + throw EvalError(ErrorInfo { .msg = hintfmt(s, s2), .errPos = pos }); From f6078e474d5fc41c8a7f683865d60490bf0c7040 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Mar 2022 16:20:01 +0100 Subject: [PATCH 11/83] Also display some suggestions for invalid formal arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ```console $ nix eval --expr '({ foo ? 1 }: foo) { fob = 2; }' error: anonymous function at (string):1:2 called with unexpected argument 'fob' at «string»:1:1: 1| ({ foo ? 1 }: foo) { fob = 2; } | ^ Did you mean foo? ``` Not that because Nix will first check for _missing_ arguments before checking for extra arguments, `({ foo }: foo) { fob = 1; }` will complain about the missing `foo` argument (rather than extra `fob`) and so won’t display a suggestion. --- src/libexpr/eval.cc | 23 +++++++++++++++++++++-- tests/suggestions.sh | 4 ++++ 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3bfb82b16..a5e9dc286 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -782,6 +782,16 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const }); } +LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const Suggestions & suggestions, const char * s, const ExprLambda & fun, const Symbol & s2)) +{ + throw TypeError(ErrorInfo { + .msg = hintfmt(s, fun.showNamePos(), s2), + .errPos = pos, + .suggestions = suggestions, + }); +} + + LocalNoInlineNoReturn(void throwTypeError(const char * s, const Value & v)) { throw TypeError(s, showType(v)); @@ -1414,8 +1424,17 @@ void EvalState::callFunction(Value & fun, size_t nrArgs, Value * * args, Value & /* Nope, so show the first unexpected argument to the user. */ for (auto & i : *args[0]->attrs) - if (!lambda.formals->has(i.name)) - throwTypeError(pos, "%1% called with unexpected argument '%2%'", lambda, i.name); + if (!lambda.formals->has(i.name)) { + std::set formalNames; + for (auto & formal : lambda.formals->formals) + formalNames.insert(formal.name); + throwTypeError( + pos, + Suggestions::bestMatches(formalNames, i.name), + "%1% called with unexpected argument '%2%'", + lambda, + i.name); + } abort(); // can't happen } } diff --git a/tests/suggestions.sh b/tests/suggestions.sh index 29d5b364b..f18fefef9 100644 --- a/tests/suggestions.sh +++ b/tests/suggestions.sh @@ -38,3 +38,7 @@ NIX_BUILD_STDERR_WITH_NO_CLOSE_SUGGESTION=$(! nix build .\#bar 2>&1 1>/dev/null) NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '(builtins.getFlake (builtins.toPath ./.)).packages.'$system'.fob' 2>&1 1>/dev/null) [[ "$NIX_EVAL_STDERR_WITH_SUGGESTIONS" =~ "Did you mean one of fo1, fo2, foo or fooo?" ]] || \ fail "The evaluator should suggest the three closest possiblities" + +NIX_EVAL_STDERR_WITH_SUGGESTIONS=$(! nix build --impure --expr '({ foo }: foo) { foo = 1; fob = 2; }' 2>&1 1>/dev/null) +[[ "$NIX_EVAL_STDERR_WITH_SUGGESTIONS" =~ "Did you mean foo?" ]] || \ + fail "The evaluator should suggest the three closest possiblities" From 4b2b0d3a5528133898a15f3210c79162ec993823 Mon Sep 17 00:00:00 2001 From: pennae Date: Wed, 29 Dec 2021 01:28:58 +0100 Subject: [PATCH 12/83] remove GC_PTR_STORE_AND_DIRTY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit turns out it's only necessary for MANUAL_VDB, which nix doesn't use. omitting them gives a slight performance improvement on eval. before: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.988 s ± 0.061 s [User: 5.935 s, System: 0.845 s] Range (min … max): 6.865 s … 7.075 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 332.6 ms ± 3.9 ms [User: 299.6 ms, System: 32.9 ms] Range (min … max): 328.1 ms … 339.1 ms 20 runs Benchmark 3: nix eval --raw --impure --expr 'with import {}; system' Time (mean ± σ): 2.681 s ± 0.049 s [User: 2.382 s, System: 0.228 s] Range (min … max): 2.607 s … 2.776 s 20 runs after: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.946 s ± 0.041 s [User: 5.875 s, System: 0.835 s] Range (min … max): 6.834 s … 7.005 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 330.3 ms ± 2.5 ms [User: 299.2 ms, System: 30.9 ms] Range (min … max): 327.5 ms … 337.7 ms 20 runs Benchmark 3: nix eval --raw --impure --expr 'with import {}; system' Time (mean ± σ): 2.671 s ± 0.035 s [User: 2.370 s, System: 0.232 s] Range (min … max): 2.597 s … 2.749 s 20 runs --- src/libexpr/eval.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 193358161..777f6a4ec 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -879,7 +879,7 @@ Value * EvalState::allocValue() /* GC_NEXT is a convenience macro for accessing the first word of an object. Take the first list item, advance the list to the next item, and clear the next pointer. */ void * p = *valueAllocCache; - GC_PTR_STORE_AND_DIRTY(&*valueAllocCache, GC_NEXT(p)); + *valueAllocCache = GC_NEXT(p); GC_NEXT(p) = nullptr; nrValues++; From 60ed4e908a59f258f82356a9dd47e43361d39f2f Mon Sep 17 00:00:00 2001 From: pennae Date: Sun, 26 Dec 2021 19:32:08 +0100 Subject: [PATCH 13/83] cache singleton Envs just like Values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit vast majority of envs is this size. before: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.946 s ± 0.041 s [User: 5.875 s, System: 0.835 s] Range (min … max): 6.834 s … 7.005 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 330.3 ms ± 2.5 ms [User: 299.2 ms, System: 30.9 ms] Range (min … max): 327.5 ms … 337.7 ms 20 runs Benchmark 3: nix eval --raw --impure --expr 'with import {}; system' Time (mean ± σ): 2.671 s ± 0.035 s [User: 2.370 s, System: 0.232 s] Range (min … max): 2.597 s … 2.749 s 20 runs after: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.935 s ± 0.052 s [User: 5.852 s, System: 0.853 s] Range (min … max): 6.808 s … 7.026 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 329.8 ms ± 2.7 ms [User: 299.0 ms, System: 30.8 ms] Range (min … max): 326.6 ms … 336.5 ms 20 runs Benchmark 3: nix flakes eval --raw --impure --file expr.nix Time (mean ± σ): 2.655 s ± 0.038 s [User: 2.364 s, System: 0.220 s] Range (min … max): 2.574 s … 2.737 s 20 runs --- src/libexpr/eval.cc | 21 ++++++++++++++++++++- src/libexpr/eval.hh | 3 +++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 777f6a4ec..ddbbdeaa6 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -438,8 +438,10 @@ EvalState::EvalState( , regexCache(makeRegexCache()) #if HAVE_BOEHMGC , valueAllocCache(std::allocate_shared(traceable_allocator(), nullptr)) + , env1AllocCache(std::allocate_shared(traceable_allocator(), nullptr)) #else , valueAllocCache(std::make_shared(nullptr)) + , env1AllocCache(std::make_shared(nullptr)) #endif , baseEnv(allocEnv(128)) , staticBaseEnv(false, 0) @@ -892,7 +894,24 @@ Env & EvalState::allocEnv(size_t size) { nrEnvs++; nrValuesInEnvs += size; - Env * env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); + + Env * env; + + if (size != 1) + env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); + else { + /* see allocValue for explanations. */ + if (!*env1AllocCache) { + *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *)); + if (!*env1AllocCache) throw std::bad_alloc(); + } + + void * p = *env1AllocCache; + *env1AllocCache = GC_NEXT(p); + GC_NEXT(p) = nullptr; + env = (Env *) p; + } + env->type = Env::Plain; /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 800b00eef..41384f044 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -136,6 +136,9 @@ private: /* Allocation cache for GC'd Value objects. */ std::shared_ptr valueAllocCache; + /* Allocation cache for size-1 Env objects. */ + std::shared_ptr env1AllocCache; + public: EvalState( From c96460f3520862d52b7bf3108f609e20384878e7 Mon Sep 17 00:00:00 2001 From: pennae Date: Tue, 28 Dec 2021 19:18:17 +0100 Subject: [PATCH 14/83] force-inline a few much-used functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit these functions are called a whole lot, and they're all comparatively small. always inlining them gives ~0.7% performance boost on eval. before: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.935 s ± 0.052 s [User: 5.852 s, System: 0.853 s] Range (min … max): 6.808 s … 7.026 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 329.8 ms ± 2.7 ms [User: 299.0 ms, System: 30.8 ms] Range (min … max): 326.6 ms … 336.5 ms 20 runs Benchmark 3: nix flakes eval --raw --impure --file expr.nix Time (mean ± σ): 2.655 s ± 0.038 s [User: 2.364 s, System: 0.220 s] Range (min … max): 2.574 s … 2.737 s 20 runs after: Benchmark 1: nix flakes search --no-eval-cache --offline ../nixpkgs hello Time (mean ± σ): 6.912 s ± 0.036 s [User: 5.823 s, System: 0.856 s] Range (min … max): 6.849 s … 6.980 s 20 runs Benchmark 2: nix flakes eval -f ../nixpkgs/pkgs/development/haskell-modules/hackage-packages.nix Time (mean ± σ): 325.1 ms ± 2.5 ms [User: 293.2 ms, System: 31.8 ms] Range (min … max): 322.2 ms … 332.8 ms 20 runs Benchmark 3: nix flakes eval --raw --impure --file expr.nix Time (mean ± σ): 2.636 s ± 0.024 s [User: 2.352 s, System: 0.226 s] Range (min … max): 2.574 s … 2.681 s 20 runs --- src/libexpr/eval-inline.hh | 86 ++++++++++++++++++++++++++++++++------ src/libexpr/eval.cc | 53 ----------------------- src/libexpr/eval.hh | 6 ++- 3 files changed, 77 insertions(+), 68 deletions(-) diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index aef1f6351..3331a7643 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -24,6 +24,76 @@ LocalNoInlineNoReturn(void throwTypeError(const Pos & pos, const char * s, const } +/* Note: Various places expect the allocated memory to be zeroed. */ +[[gnu::always_inline]] +inline void * allocBytes(size_t n) +{ + void * p; +#if HAVE_BOEHMGC + p = GC_MALLOC(n); +#else + p = calloc(n, 1); +#endif + if (!p) throw std::bad_alloc(); + return p; +} + + +[[gnu::always_inline]] +Value * EvalState::allocValue() +{ + /* We use the boehm batch allocator to speed up allocations of Values (of which there are many). + GC_malloc_many returns a linked list of objects of the given size, where the first word + of each object is also the pointer to the next object in the list. This also means that we + have to explicitly clear the first word of every object we take. */ + if (!*valueAllocCache) { + *valueAllocCache = GC_malloc_many(sizeof(Value)); + if (!*valueAllocCache) throw std::bad_alloc(); + } + + /* GC_NEXT is a convenience macro for accessing the first word of an object. + Take the first list item, advance the list to the next item, and clear the next pointer. */ + void * p = *valueAllocCache; + *valueAllocCache = GC_NEXT(p); + GC_NEXT(p) = nullptr; + + nrValues++; + return (Value *) p; +} + + +[[gnu::always_inline]] +Env & EvalState::allocEnv(size_t size) +{ + nrEnvs++; + nrValuesInEnvs += size; + + Env * env; + + if (size != 1) + env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); + else { + /* see allocValue for explanations. */ + if (!*env1AllocCache) { + *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *)); + if (!*env1AllocCache) throw std::bad_alloc(); + } + + void * p = *env1AllocCache; + *env1AllocCache = GC_NEXT(p); + GC_NEXT(p) = nullptr; + env = (Env *) p; + } + + env->type = Env::Plain; + + /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ + + return *env; +} + + +[[gnu::always_inline]] void EvalState::forceValue(Value & v, const Pos & pos) { forceValue(v, [&]() { return pos; }); @@ -52,6 +122,7 @@ void EvalState::forceValue(Value & v, Callable getPos) } +[[gnu::always_inline]] inline void EvalState::forceAttrs(Value & v, const Pos & pos) { forceAttrs(v, [&]() { return pos; }); @@ -59,6 +130,7 @@ inline void EvalState::forceAttrs(Value & v, const Pos & pos) template +[[gnu::always_inline]] inline void EvalState::forceAttrs(Value & v, Callable getPos) { forceValue(v, getPos); @@ -67,6 +139,7 @@ inline void EvalState::forceAttrs(Value & v, Callable getPos) } +[[gnu::always_inline]] inline void EvalState::forceList(Value & v, const Pos & pos) { forceValue(v, pos); @@ -74,18 +147,5 @@ inline void EvalState::forceList(Value & v, const Pos & pos) throwTypeError(pos, "value is %1% while a list was expected", v); } -/* Note: Various places expect the allocated memory to be zeroed. */ -inline void * allocBytes(size_t n) -{ - void * p; -#if HAVE_BOEHMGC - p = GC_MALLOC(n); -#else - p = calloc(n, 1); -#endif - if (!p) throw std::bad_alloc(); - return p; -} - } diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index ddbbdeaa6..038b6bb7c 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -867,59 +867,6 @@ inline Value * EvalState::lookupVar(Env * env, const ExprVar & var, bool noEval) } -Value * EvalState::allocValue() -{ - /* We use the boehm batch allocator to speed up allocations of Values (of which there are many). - GC_malloc_many returns a linked list of objects of the given size, where the first word - of each object is also the pointer to the next object in the list. This also means that we - have to explicitly clear the first word of every object we take. */ - if (!*valueAllocCache) { - *valueAllocCache = GC_malloc_many(sizeof(Value)); - if (!*valueAllocCache) throw std::bad_alloc(); - } - - /* GC_NEXT is a convenience macro for accessing the first word of an object. - Take the first list item, advance the list to the next item, and clear the next pointer. */ - void * p = *valueAllocCache; - *valueAllocCache = GC_NEXT(p); - GC_NEXT(p) = nullptr; - - nrValues++; - auto v = (Value *) p; - return v; -} - - -Env & EvalState::allocEnv(size_t size) -{ - nrEnvs++; - nrValuesInEnvs += size; - - Env * env; - - if (size != 1) - env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); - else { - /* see allocValue for explanations. */ - if (!*env1AllocCache) { - *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *)); - if (!*env1AllocCache) throw std::bad_alloc(); - } - - void * p = *env1AllocCache; - *env1AllocCache = GC_NEXT(p); - GC_NEXT(p) = nullptr; - env = (Env *) p; - } - - env->type = Env::Plain; - - /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ - - return *env; -} - - void EvalState::mkList(Value & v, size_t size) { v.mkList(size); diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 41384f044..d2efe8a47 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -350,8 +350,8 @@ public: void autoCallFunction(Bindings & args, Value & fun, Value & res); /* Allocation primitives. */ - Value * allocValue(); - Env & allocEnv(size_t size); + inline Value * allocValue(); + inline Env & allocEnv(size_t size); Value * allocAttr(Value & vAttrs, const Symbol & name); Value * allocAttr(Value & vAttrs, std::string_view name); @@ -512,3 +512,5 @@ extern EvalSettings evalSettings; static const std::string corepkgsPrefix{"/__corepkgs__/"}; } + +#include "eval-inline.hh" From 47baa9d43c0339b0a738b9b75c5ddcfb07d7131d Mon Sep 17 00:00:00 2001 From: pennae Date: Sat, 1 Jan 2022 19:24:20 +0100 Subject: [PATCH 15/83] make Pos smaller reduces peak hep memory use on eval of our test system from 264.4MB to 242.3MB, possibly also a slight performance boost. theoretically memory use could be cut down by another eight bytes per Pos on average by turning it into a tuple containing an index into a global base position table with row and column offsets, but that doesn't seem worth the effort at this point. --- src/libexpr/nixexpr.hh | 13 ++++++------- src/libutil/error.hh | 1 + 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 12b54b8eb..4dbe31510 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -23,14 +23,13 @@ MakeError(RestrictedPathError, Error); struct Pos { - FileOrigin origin; Symbol file; - unsigned int line, column; - - Pos() : origin(foString), line(0), column(0) { } - Pos(FileOrigin origin, const Symbol & file, unsigned int line, unsigned int column) - : origin(origin), file(file), line(line), column(column) { } - + uint32_t line; + FileOrigin origin:2; + uint32_t column:30; + Pos() : line(0), origin(foString), column(0) { }; + Pos(FileOrigin origin, const Symbol & file, uint32_t line, uint32_t column) + : file(file), line(line), origin(origin), column(column) { }; operator bool() const { return line != 0; diff --git a/src/libutil/error.hh b/src/libutil/error.hh index d55e1d701..bb43aa53b 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -53,6 +53,7 @@ typedef enum { lvlVomit } Verbosity; +/* adjust Pos::origin bit width when adding stuff here */ typedef enum { foFile, foStdin, From 8e2eaaaf69d9e216fce3ca6f7913bd0e2048e4b2 Mon Sep 17 00:00:00 2001 From: pennae Date: Fri, 31 Dec 2021 00:50:23 +0100 Subject: [PATCH 16/83] make Finally more local no need for function<> with c++17 deduction. this saves allocations and virtual calls, but has the same semantics otherwise. not going through function has the side effect of giving compilers more insight into the cleanup code, so we need a few local warning disables. --- src/libutil/finally.hh | 7 +++---- src/libutil/util.cc | 15 ++++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh index 7760cfe9a..dee2e8d2f 100644 --- a/src/libutil/finally.hh +++ b/src/libutil/finally.hh @@ -1,14 +1,13 @@ #pragma once -#include - /* A trivial class to run a function at the end of a scope. */ +template class Finally { private: - std::function fun; + Fn fun; public: - Finally(std::function fun) : fun(fun) { } + Finally(Fn fun) : fun(std::move(fun)) { } ~Finally() { fun(); } }; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index b833038a9..9f13d5f02 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -682,7 +682,14 @@ std::string drainFD(int fd, bool block, const size_t reserveSize) void drainFD(int fd, Sink & sink, bool block) { - int saved; + // silence GCC maybe-uninitialized warning in finally + int saved = 0; + + if (!block) { + saved = fcntl(fd, F_GETFL); + if (fcntl(fd, F_SETFL, saved | O_NONBLOCK) == -1) + throw SysError("making file descriptor non-blocking"); + } Finally finally([&]() { if (!block) { @@ -691,12 +698,6 @@ void drainFD(int fd, Sink & sink, bool block) } }); - if (!block) { - saved = fcntl(fd, F_GETFL); - if (fcntl(fd, F_SETFL, saved | O_NONBLOCK) == -1) - throw SysError("making file descriptor non-blocking"); - } - std::vector buf(64 * 1024); while (1) { checkInterrupt(); From 4d629c4f7abbbe58dfe6d9d2b37541cdf2331606 Mon Sep 17 00:00:00 2001 From: pennae Date: Wed, 5 Jan 2022 01:48:26 +0100 Subject: [PATCH 17/83] add HAVE_BOEHMGC guards to batched allocation functions --- src/libexpr/eval-inline.hh | 13 +++++++++---- src/libexpr/eval.hh | 2 ++ 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 3331a7643..08a419923 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -42,6 +42,7 @@ inline void * allocBytes(size_t n) [[gnu::always_inline]] Value * EvalState::allocValue() { +#if HAVE_BOEHMGC /* We use the boehm batch allocator to speed up allocations of Values (of which there are many). GC_malloc_many returns a linked list of objects of the given size, where the first word of each object is also the pointer to the next object in the list. This also means that we @@ -56,6 +57,9 @@ Value * EvalState::allocValue() void * p = *valueAllocCache; *valueAllocCache = GC_NEXT(p); GC_NEXT(p) = nullptr; +#else + void * p = allocBytes(sizeof(Value)); +#endif nrValues++; return (Value *) p; @@ -70,9 +74,8 @@ Env & EvalState::allocEnv(size_t size) Env * env; - if (size != 1) - env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); - else { +#if HAVE_BOEHMGC + if (size == 1) { /* see allocValue for explanations. */ if (!*env1AllocCache) { *env1AllocCache = GC_malloc_many(sizeof(Env) + sizeof(Value *)); @@ -83,7 +86,9 @@ Env & EvalState::allocEnv(size_t size) *env1AllocCache = GC_NEXT(p); GC_NEXT(p) = nullptr; env = (Env *) p; - } + } else +#endif + env = (Env *) allocBytes(sizeof(Env) + size * sizeof(Value *)); env->type = Env::Plain; diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index d2efe8a47..f1e00bae7 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -133,11 +133,13 @@ private: /* Cache used by prim_match(). */ std::shared_ptr regexCache; +#if HAVE_BOEHMGC /* Allocation cache for GC'd Value objects. */ std::shared_ptr valueAllocCache; /* Allocation cache for size-1 Env objects. */ std::shared_ptr env1AllocCache; +#endif public: From 167766b65ca203238f733efd0b5df92d28ab66c4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 11 Mar 2022 11:19:21 +0100 Subject: [PATCH 18/83] Style --- src/libfetchers/git.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 6884e8de7..d75c5d3ae 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -239,13 +239,13 @@ struct GitInputScheme : InputScheme auto errorMessage = result.second; if (errorMessage.find("fatal: not a git repository") != std::string::npos) { - throw Error("'%s' is not a git repository.", actualUrl); + throw Error("'%s' is not a Git repository", actualUrl); } else if (errorMessage.find("fatal: Needed a single revision") != std::string::npos) { // indicates that the repo does not have any commits // we want to proceed and will consider it dirty later } else if (exitCode != 0) { // any other errors should lead to a failure - throw Error("Getting the HEAD of the git tree '%s' failed with exit code %d:\n%s", actualUrl, exitCode, errorMessage); + throw Error("getting the HEAD of the Git tree '%s' failed with exit code %d:\n%s", actualUrl, exitCode, errorMessage); } bool hasHead = exitCode == 0; From 073e134de6260de7d90b135b7e173157741d4853 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 8 Mar 2022 17:45:19 +0000 Subject: [PATCH 19/83] Rename `requireGcStore` to `GcStore::require` I should have done this to begin with. This will be nicer once more Store sub-interfaces exist too, to illustrate the pattern. --- src/libstore/daemon.cc | 6 +++--- src/libstore/gc-store.cc | 2 +- src/libstore/gc-store.hh | 4 ++-- src/nix-collect-garbage/nix-collect-garbage.cc | 2 +- src/nix-store/nix-store.cc | 6 +++--- src/nix/store-delete.cc | 2 +- src/nix/store-gc.cc | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index e6760664c..ffc605410 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -645,7 +645,7 @@ static void performOp(TunnelLogger * logger, ref store, Path path = absPath(readString(from)); logger->startWork(); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); gcStore.addIndirectRoot(path); logger->stopWork(); @@ -663,7 +663,7 @@ static void performOp(TunnelLogger * logger, ref store, case wopFindRoots: { logger->startWork(); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); Roots roots = gcStore.findRoots(!trusted); logger->stopWork(); @@ -695,7 +695,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (options.ignoreLiveness) throw Error("you are not allowed to ignore liveness"); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); gcStore.collectGarbage(options, results); logger->stopWork(); diff --git a/src/libstore/gc-store.cc b/src/libstore/gc-store.cc index 3dbdec53b..4516cc744 100644 --- a/src/libstore/gc-store.cc +++ b/src/libstore/gc-store.cc @@ -2,7 +2,7 @@ namespace nix { -GcStore & requireGcStore(Store & store) +GcStore & GcStore::require(Store & store) { auto * gcStore = dynamic_cast(&store); if (!gcStore) diff --git a/src/libstore/gc-store.hh b/src/libstore/gc-store.hh index 829f70dc4..462cc097b 100644 --- a/src/libstore/gc-store.hh +++ b/src/libstore/gc-store.hh @@ -77,8 +77,8 @@ struct GcStore : public virtual Store /* Perform a garbage collection. */ virtual void collectGarbage(const GCOptions & options, GCResults & results) = 0; + + static GcStore & require(Store & store); }; -GcStore & requireGcStore(Store & store); - } diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index 4b28ea6a4..b61f0e640 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -81,7 +81,7 @@ static int main_nix_collect_garbage(int argc, char * * argv) // Run the actual garbage collector. if (!dryRun) { auto store = openStore(); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); options.action = GCOptions::gcDeleteDead; GCResults results; PrintFreed freed(true, results); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 8ebaf9387..33493500c 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -429,7 +429,7 @@ static void opQuery(Strings opFlags, Strings opArgs) store->computeFSClosure( args, referrers, true, settings.gcKeepOutputs, settings.gcKeepDerivations); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); Roots roots = gcStore.findRoots(false); for (auto & [target, links] : roots) if (referrers.find(target) != referrers.end()) @@ -590,7 +590,7 @@ static void opGC(Strings opFlags, Strings opArgs) if (!opArgs.empty()) throw UsageError("no arguments expected"); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); if (printRoots) { Roots roots = gcStore.findRoots(false); @@ -629,7 +629,7 @@ static void opDelete(Strings opFlags, Strings opArgs) for (auto & i : opArgs) options.pathsToDelete.insert(store->followLinksToStorePath(i)); - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); GCResults results; PrintFreed freed(true, results); diff --git a/src/nix/store-delete.cc b/src/nix/store-delete.cc index aa7a8b12f..0de7efaba 100644 --- a/src/nix/store-delete.cc +++ b/src/nix/store-delete.cc @@ -33,7 +33,7 @@ struct CmdStoreDelete : StorePathsCommand void run(ref store, std::vector && storePaths) override { - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); for (auto & path : storePaths) options.pathsToDelete.insert(path); diff --git a/src/nix/store-gc.cc b/src/nix/store-gc.cc index 21718dc0c..515f4ffdb 100644 --- a/src/nix/store-gc.cc +++ b/src/nix/store-gc.cc @@ -34,7 +34,7 @@ struct CmdStoreGC : StoreCommand, MixDryRun void run(ref store) override { - auto & gcStore = requireGcStore(*store); + auto & gcStore = GcStore::require(*store); options.action = dryRun ? GCOptions::gcReturnDead : GCOptions::gcDeleteDead; GCResults results; From 89effe9d4abde2ea8970736f79e0a6a499777692 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 8 Mar 2022 18:28:29 +0000 Subject: [PATCH 20/83] `GcStore::resolve` should print the URI --- src/libstore/gc-store.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/gc-store.cc b/src/libstore/gc-store.cc index 4516cc744..dc2b82166 100644 --- a/src/libstore/gc-store.cc +++ b/src/libstore/gc-store.cc @@ -6,7 +6,7 @@ GcStore & GcStore::require(Store & store) { auto * gcStore = dynamic_cast(&store); if (!gcStore) - throw UsageError("Garbage collection not supported by this store"); + throw UsageError("Garbage collection not supported by store '%s'", store.getUri()); return *gcStore; } From 678d1c2aa0f499466c723d3461277dc197515f57 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 8 Mar 2022 18:20:39 +0000 Subject: [PATCH 21/83] Factor out a `LogStore` interface Continue progress on #5729. Just as I hoped, this uncovered an issue: the daemon protocol is missing a way to query build logs. This doesn't effect `unix://`, but does effect `ssh://`. A FIXME is left for this, so we come back to it later. --- src/libstore/binary-cache-store.hh | 5 ++++- src/libstore/build/local-derivation-goal.cc | 6 ++++++ src/libstore/daemon.cc | 4 +++- src/libstore/local-fs-store.hh | 6 +++++- src/libstore/log-store.cc | 13 +++++++++++++ src/libstore/log-store.hh | 19 +++++++++++++++++++ src/libstore/remote-store.hh | 6 +++++- src/libstore/ssh-store.cc | 4 ++++ src/libstore/store-api.hh | 8 -------- src/nix-store/nix-store.cc | 9 ++++++--- src/nix/log.cc | 14 +++++++++++--- src/nix/repl.cc | 12 ++++++++++-- src/nix/store-copy-log.cc | 8 ++++++-- 13 files changed, 92 insertions(+), 22 deletions(-) create mode 100644 src/libstore/log-store.cc create mode 100644 src/libstore/log-store.hh diff --git a/src/libstore/binary-cache-store.hh b/src/libstore/binary-cache-store.hh index 9603a8caa..ca538b3cb 100644 --- a/src/libstore/binary-cache-store.hh +++ b/src/libstore/binary-cache-store.hh @@ -2,6 +2,7 @@ #include "crypto.hh" #include "store-api.hh" +#include "log-store.hh" #include "pool.hh" @@ -28,7 +29,9 @@ struct BinaryCacheStoreConfig : virtual StoreConfig "other than -1 which we reserve to indicate Nix defaults should be used"}; }; -class BinaryCacheStore : public virtual BinaryCacheStoreConfig, public virtual Store +class BinaryCacheStore : public virtual BinaryCacheStoreConfig, + public virtual Store, + public virtual LogStore { private: diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a372728f5..4e763e570 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1340,6 +1340,12 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo next->queryMissing(allowed, willBuild, willSubstitute, unknown, downloadSize, narSize); } + + virtual std::optional getBuildLog(const StorePath & path) override + { return std::nullopt; } + + virtual void addBuildLog(const StorePath & path, std::string_view log) override + { unsupported("addBuildLog"); } }; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index ffc605410..1e24a1c79 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -4,6 +4,7 @@ #include "build-result.hh" #include "store-api.hh" #include "gc-store.hh" +#include "log-store.hh" #include "path-with-outputs.hh" #include "finally.hh" #include "archive.hh" @@ -953,11 +954,12 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (!trusted) throw Error("you are not privileged to add logs"); + auto & logStore = LogStore::require(*store); { FramedSource source(from); StringSink sink; source.drainInto(sink); - store->addBuildLog(path, sink.s); + logStore.addBuildLog(path, sink.s); } logger->stopWork(); to << 1; diff --git a/src/libstore/local-fs-store.hh b/src/libstore/local-fs-store.hh index fbd49dc2c..e6fb3201a 100644 --- a/src/libstore/local-fs-store.hh +++ b/src/libstore/local-fs-store.hh @@ -2,6 +2,7 @@ #include "store-api.hh" #include "gc-store.hh" +#include "log-store.hh" namespace nix { @@ -24,7 +25,10 @@ struct LocalFSStoreConfig : virtual StoreConfig "physical path to the Nix store"}; }; -class LocalFSStore : public virtual LocalFSStoreConfig, public virtual Store, virtual GcStore +class LocalFSStore : public virtual LocalFSStoreConfig, + public virtual Store, + public virtual GcStore, + public virtual LogStore { public: diff --git a/src/libstore/log-store.cc b/src/libstore/log-store.cc new file mode 100644 index 000000000..56aec91f2 --- /dev/null +++ b/src/libstore/log-store.cc @@ -0,0 +1,13 @@ +#include "log-store.hh" + +namespace nix { + +LogStore & LogStore::require(Store & store) +{ + auto * gcStore = dynamic_cast(&store); + if (!gcStore) + throw UsageError("Build log storage and retrieval not supported by store '%s'", store.getUri()); + return *gcStore; +} + +} diff --git a/src/libstore/log-store.hh b/src/libstore/log-store.hh new file mode 100644 index 000000000..ad17cbe6e --- /dev/null +++ b/src/libstore/log-store.hh @@ -0,0 +1,19 @@ +#pragma once + +#include "store-api.hh" + + +namespace nix { + +struct LogStore : public virtual Store +{ + /* Return the build log of the specified store path, if available, + or null otherwise. */ + virtual std::optional getBuildLog(const StorePath & path) = 0; + + virtual void addBuildLog(const StorePath & path, std::string_view log) = 0; + + static LogStore & require(Store & store); +}; + +} diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index 9f6f50593..8493be6fc 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -5,6 +5,7 @@ #include "store-api.hh" #include "gc-store.hh" +#include "log-store.hh" namespace nix { @@ -30,7 +31,10 @@ struct RemoteStoreConfig : virtual StoreConfig /* FIXME: RemoteStore is a misnomer - should be something like DaemonStore. */ -class RemoteStore : public virtual RemoteStoreConfig, public virtual Store, public virtual GcStore +class RemoteStore : public virtual RemoteStoreConfig, + public virtual Store, + public virtual GcStore, + public virtual LogStore { public: diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index bb03daef4..62daa838c 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -52,6 +52,10 @@ public: bool sameMachine() override { return false; } + // FIXME extend daemon protocol, move implementation to RemoteStore + std::optional getBuildLog(const StorePath & path) override + { unsupported("getBuildLog"); } + private: struct Connection : RemoteStore::Connection diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index e99a3f2cb..635a82a2a 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -605,14 +605,6 @@ public: */ StorePathSet exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths); - /* Return the build log of the specified store path, if available, - or null otherwise. */ - virtual std::optional getBuildLog(const StorePath & path) - { return std::nullopt; } - - virtual void addBuildLog(const StorePath & path, std::string_view log) - { unsupported("addBuildLog"); } - /* Hack to allow long-running processes like hydra-queue-runner to occasionally flush their path info cache. */ void clearPathInfoCache() diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 33493500c..b3f28bcc2 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -4,6 +4,7 @@ #include "globals.hh" #include "build-result.hh" #include "gc-store.hh" +#include "log-store.hh" #include "local-store.hh" #include "monitor-fd.hh" #include "serve-protocol.hh" @@ -474,13 +475,15 @@ static void opReadLog(Strings opFlags, Strings opArgs) { if (!opFlags.empty()) throw UsageError("unknown flag"); + auto & logStore = LogStore::require(*store); + RunPager pager; for (auto & i : opArgs) { - auto path = store->followLinksToStorePath(i); - auto log = store->getBuildLog(path); + auto path = logStore.followLinksToStorePath(i); + auto log = logStore.getBuildLog(path); if (!log) - throw Error("build log of derivation '%s' is not available", store->printStorePath(path)); + throw Error("build log of derivation '%s' is not available", logStore.printStorePath(path)); std::cout << *log; } } diff --git a/src/nix/log.cc b/src/nix/log.cc index fd3c1d787..72d02ef11 100644 --- a/src/nix/log.cc +++ b/src/nix/log.cc @@ -2,6 +2,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "log-store.hh" #include "progress-bar.hh" using namespace nix; @@ -34,17 +35,24 @@ struct CmdLog : InstallableCommand RunPager pager; for (auto & sub : subs) { + auto * logSubP = dynamic_cast(&*sub); + if (!logSubP) { + printInfo("Skipped '%s' which does not support retrieving build logs", sub->getUri()); + continue; + } + auto & logSub = *logSubP; + auto log = std::visit(overloaded { [&](const DerivedPath::Opaque & bo) { - return sub->getBuildLog(bo.path); + return logSub.getBuildLog(bo.path); }, [&](const DerivedPath::Built & bfd) { - return sub->getBuildLog(bfd.drvPath); + return logSub.getBuildLog(bfd.drvPath); }, }, b.raw()); if (!log) continue; stopProgressBar(); - printInfo("got build log for '%s' from '%s'", installable->what(), sub->getUri()); + printInfo("got build log for '%s' from '%s'", installable->what(), logSub.getUri()); std::cout << *log; return; } diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 3a51a13e6..916353d8c 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -25,6 +25,7 @@ extern "C" { #include "eval-inline.hh" #include "attr-path.hh" #include "store-api.hh" +#include "log-store.hh" #include "common-eval-args.hh" #include "get-drvs.hh" #include "derivations.hh" @@ -526,9 +527,16 @@ bool NixRepl::processLine(std::string line) bool foundLog = false; RunPager pager; for (auto & sub : subs) { - auto log = sub->getBuildLog(drvPath); + auto * logSubP = dynamic_cast(&*sub); + if (!logSubP) { + printInfo("Skipped '%s' which does not support retrieving build logs", sub->getUri()); + continue; + } + auto & logSub = *logSubP; + + auto log = logSub.getBuildLog(drvPath); if (log) { - printInfo("got build log for '%s' from '%s'", drvPathRaw, sub->getUri()); + printInfo("got build log for '%s' from '%s'", drvPathRaw, logSub.getUri()); logger->writeToStdout(*log); foundLog = true; break; diff --git a/src/nix/store-copy-log.cc b/src/nix/store-copy-log.cc index 079cd6b3e..22b3980c0 100644 --- a/src/nix/store-copy-log.cc +++ b/src/nix/store-copy-log.cc @@ -1,6 +1,7 @@ #include "command.hh" #include "shared.hh" #include "store-api.hh" +#include "log-store.hh" #include "sync.hh" #include "thread-pool.hh" @@ -26,7 +27,10 @@ struct CmdCopyLog : virtual CopyCommand, virtual InstallablesCommand void run(ref srcStore) override { + auto & srcLogStore = LogStore::require(*srcStore); + auto dstStore = getDstStore(); + auto & dstLogStore = LogStore::require(*dstStore); StorePathSet drvPaths; @@ -35,8 +39,8 @@ struct CmdCopyLog : virtual CopyCommand, virtual InstallablesCommand drvPaths.insert(drvPath); for (auto & drvPath : drvPaths) { - if (auto log = srcStore->getBuildLog(drvPath)) - dstStore->addBuildLog(drvPath, *log); + if (auto log = srcLogStore.getBuildLog(drvPath)) + dstLogStore.addBuildLog(drvPath, *log); else throw Error("build log for '%s' is not available", srcStore->printStorePath(drvPath)); } From a03b1fd7f60788304f358d5f4dc063c7c9e650a9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 9 Mar 2022 15:27:48 +0000 Subject: [PATCH 22/83] Deduplicate the Store downcasting with a template --- src/libstore/daemon.cc | 9 +++++---- src/libstore/gc-store.cc | 13 ------------- src/libstore/gc-store.hh | 4 ++-- src/libstore/log-store.cc | 13 ------------- src/libstore/log-store.hh | 2 ++ src/libstore/store-cast.hh | 16 ++++++++++++++++ src/nix-collect-garbage/nix-collect-garbage.cc | 3 ++- src/nix-store/nix-store.cc | 9 +++++---- src/nix/store-copy-log.cc | 5 +++-- src/nix/store-delete.cc | 3 ++- src/nix/store-gc.cc | 3 ++- 11 files changed, 39 insertions(+), 41 deletions(-) delete mode 100644 src/libstore/gc-store.cc delete mode 100644 src/libstore/log-store.cc create mode 100644 src/libstore/store-cast.hh diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 1e24a1c79..9f21ecf36 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -3,6 +3,7 @@ #include "worker-protocol.hh" #include "build-result.hh" #include "store-api.hh" +#include "store-cast.hh" #include "gc-store.hh" #include "log-store.hh" #include "path-with-outputs.hh" @@ -646,7 +647,7 @@ static void performOp(TunnelLogger * logger, ref store, Path path = absPath(readString(from)); logger->startWork(); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); gcStore.addIndirectRoot(path); logger->stopWork(); @@ -664,7 +665,7 @@ static void performOp(TunnelLogger * logger, ref store, case wopFindRoots: { logger->startWork(); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); Roots roots = gcStore.findRoots(!trusted); logger->stopWork(); @@ -696,7 +697,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (options.ignoreLiveness) throw Error("you are not allowed to ignore liveness"); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); gcStore.collectGarbage(options, results); logger->stopWork(); @@ -954,7 +955,7 @@ static void performOp(TunnelLogger * logger, ref store, logger->startWork(); if (!trusted) throw Error("you are not privileged to add logs"); - auto & logStore = LogStore::require(*store); + auto & logStore = require(*store); { FramedSource source(from); StringSink sink; diff --git a/src/libstore/gc-store.cc b/src/libstore/gc-store.cc deleted file mode 100644 index dc2b82166..000000000 --- a/src/libstore/gc-store.cc +++ /dev/null @@ -1,13 +0,0 @@ -#include "gc-store.hh" - -namespace nix { - -GcStore & GcStore::require(Store & store) -{ - auto * gcStore = dynamic_cast(&store); - if (!gcStore) - throw UsageError("Garbage collection not supported by store '%s'", store.getUri()); - return *gcStore; -} - -} diff --git a/src/libstore/gc-store.hh b/src/libstore/gc-store.hh index 462cc097b..b3cbbad74 100644 --- a/src/libstore/gc-store.hh +++ b/src/libstore/gc-store.hh @@ -61,6 +61,8 @@ struct GCResults struct GcStore : public virtual Store { + inline static std::string operationName = "Garbage collection"; + /* Add an indirect root, which is merely a symlink to `path' from /nix/var/nix/gcroots/auto/. `path' is supposed to be a symlink to a store path. The garbage collector will @@ -77,8 +79,6 @@ struct GcStore : public virtual Store /* Perform a garbage collection. */ virtual void collectGarbage(const GCOptions & options, GCResults & results) = 0; - - static GcStore & require(Store & store); }; } diff --git a/src/libstore/log-store.cc b/src/libstore/log-store.cc deleted file mode 100644 index 56aec91f2..000000000 --- a/src/libstore/log-store.cc +++ /dev/null @@ -1,13 +0,0 @@ -#include "log-store.hh" - -namespace nix { - -LogStore & LogStore::require(Store & store) -{ - auto * gcStore = dynamic_cast(&store); - if (!gcStore) - throw UsageError("Build log storage and retrieval not supported by store '%s'", store.getUri()); - return *gcStore; -} - -} diff --git a/src/libstore/log-store.hh b/src/libstore/log-store.hh index ad17cbe6e..ff1b92e17 100644 --- a/src/libstore/log-store.hh +++ b/src/libstore/log-store.hh @@ -7,6 +7,8 @@ namespace nix { struct LogStore : public virtual Store { + inline static std::string operationName = "Build log storage and retrieval"; + /* Return the build log of the specified store path, if available, or null otherwise. */ virtual std::optional getBuildLog(const StorePath & path) = 0; diff --git a/src/libstore/store-cast.hh b/src/libstore/store-cast.hh new file mode 100644 index 000000000..ff62fc359 --- /dev/null +++ b/src/libstore/store-cast.hh @@ -0,0 +1,16 @@ +#pragma once + +#include "store-api.hh" + +namespace nix { + +template +T & require(Store & store) +{ + auto * castedStore = dynamic_cast(&store); + if (!castedStore) + throw UsageError("%s not supported by store '%s'", T::operationName, store.getUri()); + return *castedStore; +} + +} diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index b61f0e640..af6f1c88c 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -1,4 +1,5 @@ #include "store-api.hh" +#include "store-cast.hh" #include "gc-store.hh" #include "profiles.hh" #include "shared.hh" @@ -81,7 +82,7 @@ static int main_nix_collect_garbage(int argc, char * * argv) // Run the actual garbage collector. if (!dryRun) { auto store = openStore(); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); options.action = GCOptions::gcDeleteDead; GCResults results; PrintFreed freed(true, results); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index b3f28bcc2..153b84137 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -3,6 +3,7 @@ #include "dotgraph.hh" #include "globals.hh" #include "build-result.hh" +#include "store-cast.hh" #include "gc-store.hh" #include "log-store.hh" #include "local-store.hh" @@ -430,7 +431,7 @@ static void opQuery(Strings opFlags, Strings opArgs) store->computeFSClosure( args, referrers, true, settings.gcKeepOutputs, settings.gcKeepDerivations); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); Roots roots = gcStore.findRoots(false); for (auto & [target, links] : roots) if (referrers.find(target) != referrers.end()) @@ -475,7 +476,7 @@ static void opReadLog(Strings opFlags, Strings opArgs) { if (!opFlags.empty()) throw UsageError("unknown flag"); - auto & logStore = LogStore::require(*store); + auto & logStore = require(*store); RunPager pager; @@ -593,7 +594,7 @@ static void opGC(Strings opFlags, Strings opArgs) if (!opArgs.empty()) throw UsageError("no arguments expected"); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); if (printRoots) { Roots roots = gcStore.findRoots(false); @@ -632,7 +633,7 @@ static void opDelete(Strings opFlags, Strings opArgs) for (auto & i : opArgs) options.pathsToDelete.insert(store->followLinksToStorePath(i)); - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); GCResults results; PrintFreed freed(true, results); diff --git a/src/nix/store-copy-log.cc b/src/nix/store-copy-log.cc index 22b3980c0..2e288f743 100644 --- a/src/nix/store-copy-log.cc +++ b/src/nix/store-copy-log.cc @@ -1,6 +1,7 @@ #include "command.hh" #include "shared.hh" #include "store-api.hh" +#include "store-cast.hh" #include "log-store.hh" #include "sync.hh" #include "thread-pool.hh" @@ -27,10 +28,10 @@ struct CmdCopyLog : virtual CopyCommand, virtual InstallablesCommand void run(ref srcStore) override { - auto & srcLogStore = LogStore::require(*srcStore); + auto & srcLogStore = require(*srcStore); auto dstStore = getDstStore(); - auto & dstLogStore = LogStore::require(*dstStore); + auto & dstLogStore = require(*dstStore); StorePathSet drvPaths; diff --git a/src/nix/store-delete.cc b/src/nix/store-delete.cc index 0de7efaba..ca43f1530 100644 --- a/src/nix/store-delete.cc +++ b/src/nix/store-delete.cc @@ -2,6 +2,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "store-cast.hh" #include "gc-store.hh" using namespace nix; @@ -33,7 +34,7 @@ struct CmdStoreDelete : StorePathsCommand void run(ref store, std::vector && storePaths) override { - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); for (auto & path : storePaths) options.pathsToDelete.insert(path); diff --git a/src/nix/store-gc.cc b/src/nix/store-gc.cc index 515f4ffdb..8b9b5d164 100644 --- a/src/nix/store-gc.cc +++ b/src/nix/store-gc.cc @@ -2,6 +2,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "store-cast.hh" #include "gc-store.hh" using namespace nix; @@ -34,7 +35,7 @@ struct CmdStoreGC : StoreCommand, MixDryRun void run(ref store) override { - auto & gcStore = GcStore::require(*store); + auto & gcStore = require(*store); options.action = dryRun ? GCOptions::gcReturnDead : GCOptions::gcDeleteDead; GCResults results; From 015d2ad507959b06987ba7ed4566689aba316766 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 11 Mar 2022 14:42:09 +0000 Subject: [PATCH 23/83] Desugar `StorePathWithOutputs` in nix-build implementation `DerivedPath` has replaced `StorePathWithOutputs` internally, so shrinking the usage of `StorePathWithOutputs` to just the boundary is good. --- src/nix-build/nix-build.cc | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 795a4f7bd..faa8c078f 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -325,8 +325,7 @@ static void main_nix_build(int argc, char * * argv) state->printStats(); - auto buildPaths = [&](const std::vector & paths0) { - auto paths = toDerivedPaths(paths0); + auto buildPaths = [&](const std::vector & paths) { /* Note: we do this even when !printMissing to efficiently fetch binary cache data. */ uint64_t downloadSize, narSize; @@ -348,7 +347,7 @@ static void main_nix_build(int argc, char * * argv) auto & drvInfo = drvs.front(); auto drv = evalStore->derivationFromPath(drvInfo.requireDrvPath()); - std::vector pathsToBuild; + std::vector pathsToBuild; RealisedPath::Set pathsToCopy; /* Figure out what bash shell to use. If $NIX_BUILD_SHELL @@ -370,7 +369,10 @@ static void main_nix_build(int argc, char * * argv) throw Error("the 'bashInteractive' attribute in did not evaluate to a derivation"); auto bashDrv = drv->requireDrvPath(); - pathsToBuild.push_back({bashDrv}); + pathsToBuild.push_back(DerivedPath::Built { + .drvPath = bashDrv, + .outputs = {}, + }); pathsToCopy.insert(bashDrv); shellDrv = bashDrv; @@ -382,17 +384,24 @@ static void main_nix_build(int argc, char * * argv) } // Build or fetch all dependencies of the derivation. - for (const auto & input : drv.inputDrvs) + for (const auto & [inputDrv0, inputOutputs] : drv.inputDrvs) { + // To get around lambda capturing restrictions in the + // standard. + const auto & inputDrv = inputDrv0; if (std::all_of(envExclude.cbegin(), envExclude.cend(), [&](const std::string & exclude) { - return !std::regex_search(store->printStorePath(input.first), std::regex(exclude)); + return !std::regex_search(store->printStorePath(inputDrv), std::regex(exclude)); })) { - pathsToBuild.push_back({input.first, input.second}); - pathsToCopy.insert(input.first); + pathsToBuild.push_back(DerivedPath::Built { + .drvPath = inputDrv, + .outputs = inputOutputs + }); + pathsToCopy.insert(inputDrv); } + } for (const auto & src : drv.inputSrcs) { - pathsToBuild.push_back({src}); + pathsToBuild.push_back(DerivedPath::Opaque{src}); pathsToCopy.insert(src); } @@ -543,7 +552,7 @@ static void main_nix_build(int argc, char * * argv) else { - std::vector pathsToBuild; + std::vector pathsToBuild; std::vector> pathsToBuildOrdered; RealisedPath::Set drvsToCopy; @@ -556,7 +565,7 @@ static void main_nix_build(int argc, char * * argv) if (outputName == "") throw Error("derivation '%s' lacks an 'outputName' attribute", store->printStorePath(drvPath)); - pathsToBuild.push_back({drvPath, {outputName}}); + pathsToBuild.push_back(DerivedPath::Built{drvPath, {outputName}}); pathsToBuildOrdered.push_back({drvPath, {outputName}}); drvsToCopy.insert(drvPath); From 0948b8e94dfbf87ed2a695c6c7d8dac250e2c293 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 1 Oct 2021 18:05:53 +0000 Subject: [PATCH 24/83] Reduce variants for derivation hash modulo This changes was taken from dynamic derivation (#4628). It` somewhat undoes the refactors I first did for floating CA derivations, as the benefit of hindsight + requirements of dynamic derivations made me reconsider some things. They aren't to consequential, but I figured they might be good to land first, before the more profound changes @thufschmitt has in the works. --- src/libexpr/primops.cc | 45 ++++++++------- src/libstore/derivations.cc | 111 ++++++++++++++++++++++-------------- src/libstore/derivations.hh | 40 +++++++++++-- src/libstore/local-store.cc | 4 +- src/nix/develop.cc | 3 +- 5 files changed, 131 insertions(+), 72 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3124025aa..0980e75f4 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1207,32 +1207,37 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * // hash per output. auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); std::visit(overloaded { - [&](Hash & h) { - for (auto & i : outputs) { - auto outPath = state.store->makeOutputPath(i, h, drvName); - drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign(i, - DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = std::move(outPath), - }, - }); + [&](const DrvHash & drvHash) { + auto & h = drvHash.hash; + switch (drvHash.kind) { + case DrvHash::Kind::Deferred: + for (auto & i : outputs) { + drv.outputs.insert_or_assign(i, + DerivationOutput { + .output = DerivationOutputDeferred{}, + }); + } + break; + case DrvHash::Kind::Regular: + for (auto & i : outputs) { + auto outPath = state.store->makeOutputPath(i, h, drvName); + drv.env[i] = state.store->printStorePath(outPath); + drv.outputs.insert_or_assign(i, + DerivationOutput { + .output = DerivationOutputInputAddressed { + .path = std::move(outPath), + }, + }); + } + break; } }, - [&](CaOutputHashes &) { + [&](const CaOutputHashes &) { // Shouldn't happen as the toplevel derivation is not CA. assert(false); }, - [&](DeferredHash &) { - for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, - DerivationOutput { - .output = DerivationOutputDeferred{}, - }); - } - }, }, - hashModulo); + hashModulo.raw()); } diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index a49be0057..1fe45bd87 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -510,7 +510,7 @@ static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & */ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { - bool isDeferred = false; + auto kind = DrvHash::Kind::Regular; /* Return a fixed hash for fixed-output derivations. */ switch (drv.type()) { case DerivationType::CAFixed: { @@ -526,7 +526,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m return outputHashes; } case DerivationType::CAFloating: - isDeferred = true; + kind = DrvHash::Kind::Deferred; break; case DerivationType::InputAddressed: break; @@ -537,21 +537,20 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m /* For other derivations, replace the inputs paths with recursive calls to this function. */ std::map inputs2; - for (auto & i : drv.inputDrvs) { - const auto & res = pathDerivationModulo(store, i.first); + for (auto & [drvPath, inputOutputs0] : drv.inputDrvs) { + // Avoid lambda capture restriction with standard / Clang + auto & inputOutputs = inputOutputs0; + const auto & res = pathDerivationModulo(store, drvPath); std::visit(overloaded { // Regular non-CA derivation, replace derivation - [&](const Hash & drvHash) { - inputs2.insert_or_assign(drvHash.to_string(Base16, false), i.second); - }, - [&](const DeferredHash & deferredHash) { - isDeferred = true; - inputs2.insert_or_assign(deferredHash.hash.to_string(Base16, false), i.second); + [&](const DrvHash & drvHash) { + kind |= drvHash.kind; + inputs2.insert_or_assign(drvHash.hash.to_string(Base16, false), inputOutputs); }, // CA derivation's output hashes [&](const CaOutputHashes & outputHashes) { std::set justOut = { "out" }; - for (auto & output : i.second) { + for (auto & output : inputOutputs) { /* Put each one in with a single "out" output.. */ const auto h = outputHashes.at(output); inputs2.insert_or_assign( @@ -559,15 +558,24 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m justOut); } }, - }, res); + }, res.raw()); } auto hash = hashString(htSHA256, drv.unparse(store, maskOutputs, &inputs2)); - if (isDeferred) - return DeferredHash { hash }; - else - return hash; + return DrvHash { .hash = hash, .kind = kind }; +} + + +void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept +{ + switch (other) { + case DrvHash::Kind::Regular: + break; + case DrvHash::Kind::Deferred: + self = other; + break; + } } @@ -575,20 +583,15 @@ std::map staticOutputHashes(Store & store, const Derivation & { std::map res; std::visit(overloaded { - [&](const Hash & drvHash) { + [&](const DrvHash & drvHash) { for (auto & outputName : drv.outputNames()) { - res.insert({outputName, drvHash}); - } - }, - [&](const DeferredHash & deferredHash) { - for (auto & outputName : drv.outputNames()) { - res.insert({outputName, deferredHash.hash}); + res.insert({outputName, drvHash.hash}); } }, [&](const CaOutputHashes & outputHashes) { res = outputHashes; }, - }, hashDerivationModulo(store, drv, true)); + }, hashDerivationModulo(store, drv, true).raw()); return res; } @@ -738,7 +741,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { if (std::holds_alternative(output.output)) { - Hash h = std::get(hashModulo); + auto & h = hashModulo.requireNoFixedNonDeferred(); auto outPath = store.makeOutputPath(outputName, h, drv.name); drv.env[outputName] = store.printStorePath(outPath); output = DerivationOutput { @@ -751,31 +754,51 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } +const Hash & DrvHashModulo::requireNoFixedNonDeferred() const { + auto * drvHashOpt = std::get_if(&raw()); + assert(drvHashOpt); + assert(drvHashOpt->kind == DrvHash::Kind::Regular); + return drvHashOpt->hash; +} + +static bool tryResolveInput( + Store & store, StorePathSet & inputSrcs, StringMap & inputRewrites, + const StorePath & inputDrv, const StringSet & inputOutputs) +{ + auto inputDrvOutputs = store.queryPartialDerivationOutputMap(inputDrv); + + auto getOutput = [&](const std::string & outputName) { + auto & actualPathOpt = inputDrvOutputs.at(outputName); + if (!actualPathOpt) + warn("output %s of input %s missing, aborting the resolving", + outputName, + store.printStorePath(inputDrv) + ); + return actualPathOpt; + }; + + for (auto & outputName : inputOutputs) { + auto actualPathOpt = getOutput(outputName); + if (!actualPathOpt) return false; + auto actualPath = *actualPathOpt; + inputRewrites.emplace( + downstreamPlaceholder(store, inputDrv, outputName), + store.printStorePath(actualPath)); + inputSrcs.insert(std::move(actualPath)); + } + + return true; +} + std::optional Derivation::tryResolve(Store & store) { BasicDerivation resolved { *this }; // Input paths that we'll want to rewrite in the derivation StringMap inputRewrites; - for (auto & input : inputDrvs) { - auto inputDrvOutputs = store.queryPartialDerivationOutputMap(input.first); - StringSet newOutputNames; - for (auto & outputName : input.second) { - auto actualPathOpt = inputDrvOutputs.at(outputName); - if (!actualPathOpt) { - warn("output %s of input %s missing, aborting the resolving", - outputName, - store.printStorePath(input.first) - ); - return std::nullopt; - } - auto actualPath = *actualPathOpt; - inputRewrites.emplace( - downstreamPlaceholder(store, input.first, outputName), - store.printStorePath(actualPath)); - resolved.inputSrcs.insert(std::move(actualPath)); - } - } + for (auto & [inputDrv, inputOutputs] : inputDrvs) + if (!tryResolveInput(store, resolved.inputSrcs, inputRewrites, inputDrv, inputOutputs)) + return std::nullopt; rewriteDerivation(store, resolved, inputRewrites); diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 132de82b6..2fb18d7f7 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -175,13 +175,43 @@ std::string outputPathName(std::string_view drvName, std::string_view outputName // whose output hashes are always known since they are fixed up-front. typedef std::map CaOutputHashes; -struct DeferredHash { Hash hash; }; +struct DrvHash { + Hash hash; + + enum struct Kind { + // Statically determined derivations. + // This hash will be directly used to compute the output paths + Regular, + // Floating-output derivations (and their dependencies). + Deferred, + }; + + Kind kind; +}; + +void operator |= (DrvHash::Kind & self, const DrvHash::Kind & other) noexcept; typedef std::variant< - Hash, // regular DRV normalized hash - CaOutputHashes, // Fixed-output derivation hashes - DeferredHash // Deferred hashes for floating outputs drvs and their dependencies -> DrvHashModulo; + // Regular normalized derivation hash, and whether it was deferred (because + // an ancestor derivation is a floating content addressed derivation). + DrvHash, + // Fixed-output derivation hashes + CaOutputHashes +> DrvHashModuloRaw; + +struct DrvHashModulo : DrvHashModuloRaw { + using Raw = DrvHashModuloRaw; + using Raw::Raw; + + /* Get hash, throwing if it is per-output CA hashes or a + deferred Drv hash. + */ + const Hash & requireNoFixedNonDeferred() const; + + inline const Raw & raw() const { + return static_cast(*this); + } +}; /* Returns hashes with the details of fixed-output subderivations expunged. diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 1ee71b1c0..9230be15a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -701,8 +701,8 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat [&](const DerivationOutputInputAddressed & doia) { if (!h) { // somewhat expensive so we do lazily - auto temp = hashDerivationModulo(*this, drv, true); - h = std::get(temp); + auto h0 = hashDerivationModulo(*this, drv, true); + h = h0.requireNoFixedNonDeferred(); } StorePath recomputed = makeOutputPath(i.first, *h, drvName); if (doia.path != recomputed) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 8af5da9d0..dafcafd79 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -206,7 +206,8 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore output.second = { .output = DerivationOutputInputAddressed { .path = StorePath::dummy } }; drv.env[output.first] = ""; } - Hash h = std::get<0>(hashDerivationModulo(*evalStore, drv, true)); + auto h0 = hashDerivationModulo(*evalStore, drv, true); + const Hash & h = h0.requireNoFixedNonDeferred(); for (auto & output : drv.outputs) { auto outPath = store->makeOutputPath(output.first, h, drv.name); From 91adfb8894b4b8183c2948712d56a97bb9d93d9f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Wed, 10 Mar 2021 02:20:32 +0000 Subject: [PATCH 25/83] Create some type aliases for string Contexts --- src/libexpr/eval-cache.cc | 2 +- src/libexpr/eval-cache.hh | 2 +- src/libexpr/eval.cc | 6 +++--- src/libexpr/eval.hh | 2 +- src/libexpr/primops/context.cc | 2 +- src/libexpr/value.hh | 4 +++- 6 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 188223957..7e5b5c9c4 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -254,7 +254,7 @@ struct AttrDb return {{rowId, attrs}}; } case AttrType::String: { - std::vector> context; + NixStringContext context; if (!queryAttribute.isNull(3)) for (auto & s : tokenizeString>(queryAttribute.getStr(3), ";")) context.push_back(decodeContext(s)); diff --git a/src/libexpr/eval-cache.hh b/src/libexpr/eval-cache.hh index 40f1d4ffc..c9a9bf471 100644 --- a/src/libexpr/eval-cache.hh +++ b/src/libexpr/eval-cache.hh @@ -52,7 +52,7 @@ struct misc_t {}; struct failed_t {}; typedef uint64_t AttrId; typedef std::pair AttrKey; -typedef std::pair>> string_t; +typedef std::pair string_t; typedef std::variant< std::vector, diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index c65ef9738..126e0af8c 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1903,7 +1903,7 @@ std::string_view EvalState::forceString(Value & v, const Pos & pos) /* Decode a context string ‘!!’ into a pair . */ -std::pair decodeContext(std::string_view s) +NixStringContextElem decodeContext(std::string_view s) { if (s.at(0) == '!') { size_t index = s.find("!", 1); @@ -1921,9 +1921,9 @@ void copyContext(const Value & v, PathSet & context) } -std::vector> Value::getContext() +NixStringContext Value::getContext() { - std::vector> res; + NixStringContext res; assert(internalType == tString); if (string.context) for (const char * * p = string.context; *p; ++p) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index f1e00bae7..18480f290 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -430,7 +430,7 @@ std::string showType(const Value & v); /* Decode a context string ‘!!’ into a pair . */ -std::pair decodeContext(std::string_view s); +NixStringContextElem decodeContext(std::string_view s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 3701bd442..ad4de3840 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -82,7 +82,7 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, drv = std::string(p, 1); path = &drv; } else if (p.at(0) == '!') { - std::pair ctx = decodeContext(p); + NixStringContextElem ctx = decodeContext(p); drv = ctx.first; output = ctx.second; path = &drv; diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index d0fa93e92..ee9cb832a 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -64,6 +64,8 @@ class JSONPlaceholder; typedef int64_t NixInt; typedef double NixFloat; +typedef std::pair NixStringContextElem; +typedef std::vector NixStringContext; /* External values must descend from ExternalValueBase, so that * type-agnostic nix functions (e.g. showType) can be implemented @@ -368,7 +370,7 @@ public: non-trivial. */ bool isTrivial() const; - std::vector> getContext(); + NixStringContext getContext(); auto listItems() { From cb1a76112eb77883038ebad8d3b0e39330bdf1b3 Mon Sep 17 00:00:00 2001 From: Artturin Date: Sat, 12 Mar 2022 23:58:29 +0200 Subject: [PATCH 26/83] nix-env: Add a suggestion for when there's a name collision in channels help new users find a solution to their problem ./result/bin/nix-env -qa hello warning: name collision in input Nix expressions, skipping '/home/artturin/.nix-defexpr/channels_root/master' suggestion: remove 'master' from either the root channels or the user channels hello-2.12 hello-2.12 --- src/nix-env/nix-env.cc | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 40c3c5d65..eb5fbc08f 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -128,7 +128,12 @@ static void getAllExprs(EvalState & state, if (hasSuffix(attrName, ".nix")) attrName = std::string(attrName, 0, attrName.size() - 4); if (!seen.insert(attrName).second) { - printError("warning: name collision in input Nix expressions, skipping '%1%'", path2); + std::string suggestionMessage = ""; + if (path2.find("channels") != std::string::npos && path.find("channels") != std::string::npos) { + suggestionMessage = fmt("\nsuggestion: remove '%s' from either the root channels or the user channels", attrName); + } + printError("warning: name collision in input Nix expressions, skipping '%1%'" + "%2%", path2, suggestionMessage); continue; } /* Load the expression on demand. */ From 6b1872312f1b505334acb67b8bf7990b0a0fdbd8 Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sat, 12 Mar 2022 21:56:08 +0000 Subject: [PATCH 27/83] nix store gc: account for auto-optimised store Before the change on a system with `auto-optimise-store = true`: $ nix store gc --verbose --max 1 deleted all the paths instead of one path (we requested 1 byte limit). It happens because every file in `auto-optimise-store = true` has at least 2 links: file itself and a link in /nix/store/.links/ directory. The change conservatively assumes that any file that has one (as before) or two links (assume auto-potimise mode) will free space. Co-authored-by: Sandro --- src/libstore/gc.cc | 3 ++- src/libutil/util.cc | 25 +++++++++++++++++++++++-- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 024da66c1..69755bd19 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -841,7 +841,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) if (unlink(path.c_str()) == -1) throw SysError("deleting '%1%'", path); - results.bytesFreed += st.st_size; + /* Do not accound for deleted file here. Rely on deletePath() + accounting. */ } struct stat st; diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 9f13d5f02..70eaf4f9c 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -406,8 +406,29 @@ static void _deletePath(int parentfd, const Path & path, uint64_t & bytesFreed) throw SysError("getting status of '%1%'", path); } - if (!S_ISDIR(st.st_mode) && st.st_nlink == 1) - bytesFreed += st.st_size; + if (!S_ISDIR(st.st_mode)) { + /* We are about to delete a file. Will it likely free space? */ + + switch (st.st_nlink) { + /* Yes: last link. */ + case 1: + bytesFreed += st.st_size; + break; + /* Maybe: yes, if 'auto-optimise-store' or manual optimisation + was performed. Instead of checking for real let's assume + it's an optimised file and space will be freed. + + In worst case we will double count on freed space for files + with exactly two hardlinks for unoptimised packages. + */ + case 2: + bytesFreed += st.st_size; + break; + /* No: 3+ links. */ + default: + break; + } + } if (S_ISDIR(st.st_mode)) { /* Make the directory accessible. */ From e06b264f944226cfede0b3fc0394a9c3bbde4a78 Mon Sep 17 00:00:00 2001 From: thkoch2001 <94641+thkoch2001@users.noreply.github.com> Date: Sun, 13 Mar 2022 18:42:26 +0200 Subject: [PATCH 28/83] Add documentation= entry to systemd unit file Closes: #6246 --- misc/systemd/nix-daemon.service.in | 1 + 1 file changed, 1 insertion(+) diff --git a/misc/systemd/nix-daemon.service.in b/misc/systemd/nix-daemon.service.in index 25655204d..b4badf2ba 100644 --- a/misc/systemd/nix-daemon.service.in +++ b/misc/systemd/nix-daemon.service.in @@ -1,5 +1,6 @@ [Unit] Description=Nix Daemon +Documentation=man:nix-daemon https://nixos.org/manual RequiresMountsFor=@storedir@ RequiresMountsFor=@localstatedir@ ConditionPathIsReadWrite=@localstatedir@/nix/daemon-socket From 34e20c164cd512c10af99803215e32889a6d965b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Fri, 18 Feb 2022 16:56:35 +0100 Subject: [PATCH 29/83] libfetchers/path: set `lastModified` to path's mtime When importing e.g. a local `nixpkgs` in a flake to test a change like { inputs.nixpkgs.url = path:/home/ma27/Projects/nixpkgs; outputs = /* ... */ } then the input is missing a `lastModified`-field that's e.g. used in `nixpkgs.lib.nixosSystem`. Due to the missing `lastMoified`-field, the mtime is set to 19700101: result -> /nix/store/b7dg1lmmsill2rsgyv2w7b6cnmixkvc1-nixos-system-nixos-22.05.19700101.dirty With this change, the `path`-fetcher now sets a `lastModified` attribute to the `mtime` just like it's the case in the `tarball`-fetcher already. When building NixOS systems with `nixpkgs` being a `path`-input and this patch, the output-path now looks like this: result -> /nix/store/ld2qf9c1s98dxmiwcaq5vn9k5ylzrm1s-nixos-system-nixos-22.05.20220217.dirty --- src/libfetchers/path.cc | 15 ++++++++++++--- src/libutil/archive.cc | 19 +++++++++++++++---- src/libutil/archive.hh | 4 ++++ 3 files changed, 31 insertions(+), 7 deletions(-) diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 59e228e97..2d5d7d58d 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -1,5 +1,7 @@ #include "fetchers.hh" #include "store-api.hh" +#include +#include "archive.hh" namespace nix::fetchers { @@ -80,8 +82,9 @@ struct PathInputScheme : InputScheme // nothing to do } - std::pair fetch(ref store, const Input & input) override + std::pair fetch(ref store, const Input & _input) override { + Input input(_input); std::string absPath; auto path = getStrAttr(input.attrs, "path"); @@ -111,9 +114,15 @@ struct PathInputScheme : InputScheme if (storePath) store->addTempRoot(*storePath); - if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath)) + time_t mtime = 0; + if (!storePath || storePath->name() != "source" || !store->isValidPath(*storePath)) { // FIXME: try to substitute storePath. - storePath = store->addToStore("source", absPath); + auto src = sinkToSource([&](Sink & sink) { + mtime = dumpPathAndGetMtime(absPath, sink, defaultPathFilter); + }); + storePath = store->addToStoreFromDump(*src, "source"); + } + input.attrs.insert_or_assign("lastModified", uint64_t(mtime)); return {std::move(*storePath), input}; } diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index eda004756..30b471af5 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -64,11 +64,12 @@ static void dumpContents(const Path & path, off_t size, } -static void dump(const Path & path, Sink & sink, PathFilter & filter) +static time_t dump(const Path & path, Sink & sink, PathFilter & filter) { checkInterrupt(); auto st = lstat(path); + time_t result = st.st_mtime; sink << "("; @@ -103,7 +104,10 @@ static void dump(const Path & path, Sink & sink, PathFilter & filter) for (auto & i : unhacked) if (filter(path + "/" + i.first)) { sink << "entry" << "(" << "name" << i.first << "node"; - dump(path + "/" + i.second, sink, filter); + auto tmp_mtime = dump(path + "/" + i.second, sink, filter); + if (tmp_mtime > result) { + result = tmp_mtime; + } sink << ")"; } } @@ -114,13 +118,20 @@ static void dump(const Path & path, Sink & sink, PathFilter & filter) else throw Error("file '%1%' has an unsupported type", path); sink << ")"; + + return result; } -void dumpPath(const Path & path, Sink & sink, PathFilter & filter) +time_t dumpPathAndGetMtime(const Path & path, Sink & sink, PathFilter & filter) { sink << narVersionMagic1; - dump(path, sink, filter); + return dump(path, sink, filter); +} + +void dumpPath(const Path & path, Sink & sink, PathFilter & filter) +{ + dumpPathAndGetMtime(path, sink, filter); } diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index fca351605..79ce08df0 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -48,6 +48,10 @@ namespace nix { void dumpPath(const Path & path, Sink & sink, PathFilter & filter = defaultPathFilter); +/* Same as `void dumpPath()`, but returns the last modified date of the path */ +time_t dumpPathAndGetMtime(const Path & path, Sink & sink, + PathFilter & filter = defaultPathFilter); + void dumpString(std::string_view s, Sink & sink); /* FIXME: fix this API, it sucks. */ From 244baff2c7bc287b83377f1ed807a861fc705dac Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 28 Feb 2022 11:56:50 +0100 Subject: [PATCH 30/83] libfetchers: remove obsolete filesystem #include --- src/libfetchers/path.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 2d5d7d58d..f0ef97da5 100644 --- a/src/libfetchers/path.cc +++ b/src/libfetchers/path.cc @@ -1,6 +1,5 @@ #include "fetchers.hh" #include "store-api.hh" -#include #include "archive.hh" namespace nix::fetchers { From 975bade7f0adb0d122eabeeca6ad44bf1d1f4366 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 15 Mar 2022 12:55:32 +0100 Subject: [PATCH 31/83] Implement simple test for `path`-fetcher setting a correct `lastModifiedDate` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Théophane Hufschmitt <7226587+thufschmitt@users.noreply.github.com> --- tests/fetchPath.sh | 6 ++++++ tests/local.mk | 1 + 2 files changed, 7 insertions(+) create mode 100644 tests/fetchPath.sh diff --git a/tests/fetchPath.sh b/tests/fetchPath.sh new file mode 100644 index 000000000..0cfdf68cf --- /dev/null +++ b/tests/fetchPath.sh @@ -0,0 +1,6 @@ +source common.sh + +touch foo -t 222211111111 +# We only check whether 2222-11-1* **:**:** is the last modified date since +# `lastModified` is transformed into UTC in `builtins.fetchTarball`. +[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2222111.* ]] diff --git a/tests/local.mk b/tests/local.mk index 8032fc38a..b69fb9391 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -21,6 +21,7 @@ nix_tests = \ tarball.sh \ fetchGit.sh \ fetchurl.sh \ + fetchPath.sh \ simple.sh \ referrers.sh \ optimise-store.sh \ From 2d5c43f210694033eb0d1ddfb0131e18a159b790 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 15 Mar 2022 21:05:01 +0100 Subject: [PATCH 32/83] Fix the tests on 32bits machines year 2222 is too much for a 32 bit timestamp. So replace it by something smaller --- tests/fetchPath.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/fetchPath.sh b/tests/fetchPath.sh index 0cfdf68cf..0e065cae2 100644 --- a/tests/fetchPath.sh +++ b/tests/fetchPath.sh @@ -1,6 +1,6 @@ source common.sh -touch foo -t 222211111111 +touch foo -t 202211111111 # We only check whether 2222-11-1* **:**:** is the last modified date since # `lastModified` is transformed into UTC in `builtins.fetchTarball`. -[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2222111.* ]] +[[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2022111.* ]] From 3cea6f569e300c92d23e46eb4e2039302f58518f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= <7226587+thufschmitt@users.noreply.github.com> Date: Wed, 16 Mar 2022 08:56:01 +0100 Subject: [PATCH 33/83] =?UTF-8?q?Fix=20the=20date=20in=20the=20comment=20o?= =?UTF-8?q?f=20fetchPath=E2=80=99s=20test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: pennae <82953136+pennae@users.noreply.github.com> --- tests/fetchPath.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fetchPath.sh b/tests/fetchPath.sh index 0e065cae2..8f17638e9 100644 --- a/tests/fetchPath.sh +++ b/tests/fetchPath.sh @@ -1,6 +1,6 @@ source common.sh touch foo -t 202211111111 -# We only check whether 2222-11-1* **:**:** is the last modified date since +# We only check whether 2022-11-1* **:**:** is the last modified date since # `lastModified` is transformed into UTC in `builtins.fetchTarball`. [[ "$(nix eval --impure --raw --expr "(builtins.fetchTree \"path://$PWD/foo\").lastModifiedDate")" =~ 2022111.* ]] From af013281c9212e49709ef4ecb429a4307f48bea0 Mon Sep 17 00:00:00 2001 From: Joachim Breitner Date: Wed, 16 Mar 2022 12:53:38 +0100 Subject: [PATCH 34/83] distributed-builds.md: fixing typo of the most minor sort --- doc/manual/src/advanced-topics/distributed-builds.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/src/advanced-topics/distributed-builds.md b/doc/manual/src/advanced-topics/distributed-builds.md index c4c60db15..b0d7fbf1a 100644 --- a/doc/manual/src/advanced-topics/distributed-builds.md +++ b/doc/manual/src/advanced-topics/distributed-builds.md @@ -110,7 +110,7 @@ default, set it to `-`. 7. A comma-separated list of *mandatory features*. A machine will only be used to build a derivation if all of the machine’s mandatory features appear in the derivation’s `requiredSystemFeatures` - attribute.. + attribute. 8. The (base64-encoded) public host key of the remote machine. If omitted, SSH will use its regular known-hosts file. Specifically, the field is calculated From a5c969db496843b9a42b3aee494a02714ee9ca78 Mon Sep 17 00:00:00 2001 From: Artturin Date: Wed, 16 Mar 2022 20:48:50 +0200 Subject: [PATCH 35/83] nix: allow using --file - to read from stdin --- doc/manual/src/release-notes/rl-next.md | 2 ++ src/libcmd/installables.cc | 9 +++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index c869b5e2f..c9753f9aa 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -1 +1,3 @@ # Release X.Y (202?-??-??) + +* Various nix commands can now read expressions from stdin with `--file -`. diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index b7623d4ba..784117569 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -134,7 +134,9 @@ SourceExprCommand::SourceExprCommand() addFlag({ .longName = "file", .shortName = 'f', - .description = "Interpret installables as attribute paths relative to the Nix expression stored in *file*.", + .description = + "Interpret installables as attribute paths relative to the Nix expression stored in *file*. " + "If *file* is the character -, then a Nix expression will be read from standard input.", .category = installablesCategory, .labels = {"file"}, .handler = {&file}, @@ -695,7 +697,10 @@ std::vector> SourceExprCommand::parseInstallables( auto state = getEvalState(); auto vFile = state->allocValue(); - if (file) + if (file == "-") { + auto e = state->parseStdin(); + state->eval(e, *vFile); + } else if (file) state->evalFile(lookupFileArg(*state, *file), *vFile); else { auto e = state->parseExprFromString(*expr, absPath(".")); From 4f8ad41d4eb2012f01cd09ad745e78c5b8d8bee6 Mon Sep 17 00:00:00 2001 From: Artturin Date: Wed, 16 Mar 2022 20:57:18 +0200 Subject: [PATCH 36/83] add tests for nix eval and nix-instantiate --- tests/eval.nix | 5 +++++ tests/eval.sh | 29 +++++++++++++++++++++++++++++ tests/local.mk | 1 + 3 files changed, 35 insertions(+) create mode 100644 tests/eval.nix create mode 100644 tests/eval.sh diff --git a/tests/eval.nix b/tests/eval.nix new file mode 100644 index 000000000..befbd17a9 --- /dev/null +++ b/tests/eval.nix @@ -0,0 +1,5 @@ +{ + int = 123; + str = "foo"; + attr.foo = "bar"; +} diff --git a/tests/eval.sh b/tests/eval.sh new file mode 100644 index 000000000..2e5ceb969 --- /dev/null +++ b/tests/eval.sh @@ -0,0 +1,29 @@ +source common.sh + +clearStore + +testStdinHeredoc=$(nix eval -f - < Date: Sat, 12 Mar 2022 13:47:01 +0100 Subject: [PATCH 37/83] nix-env: always print output names in JSON and XML The current `--out-path` flag has two disadvantages when one is only concerned with querying the names of outputs: - it requires evaluating every output's `outPath`, which takes significantly more resources and runs into more failures - it destroys the information of the order of outputs so we can't tell which one is the main output This patch makes the output names always present (replacing paths with `null` in JSON if `--out-path` isn't given), and adds an `outputName` field. --- src/libexpr/get-drvs.cc | 28 +++++---- src/libexpr/get-drvs.hh | 7 ++- src/nix-env/nix-env.cc | 136 +++++++++++++++++++++------------------- src/nix-env/user-env.cc | 10 +-- 4 files changed, 95 insertions(+), 86 deletions(-) diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 7630c5ff4..7f2ecb4f7 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -102,7 +102,7 @@ StorePath DrvInfo::queryOutPath() const } -DrvInfo::Outputs DrvInfo::queryOutputs(bool onlyOutputsToInstall) +DrvInfo::Outputs DrvInfo::queryOutputs(bool withPaths, bool onlyOutputsToInstall) { if (outputs.empty()) { /* Get the ‘outputs’ list. */ @@ -112,20 +112,24 @@ DrvInfo::Outputs DrvInfo::queryOutputs(bool onlyOutputsToInstall) /* For each output... */ for (auto elem : i->value->listItems()) { - /* Evaluate the corresponding set. */ - std::string name(state->forceStringNoCtx(*elem, *i->pos)); - Bindings::iterator out = attrs->find(state->symbols.create(name)); - if (out == attrs->end()) continue; // FIXME: throw error? - state->forceAttrs(*out->value, *i->pos); + std::string output(state->forceStringNoCtx(*elem, *i->pos)); - /* And evaluate its ‘outPath’ attribute. */ - Bindings::iterator outPath = out->value->attrs->find(state->sOutPath); - if (outPath == out->value->attrs->end()) continue; // FIXME: throw error? - PathSet context; - outputs.emplace(name, state->coerceToStorePath(*outPath->pos, *outPath->value, context)); + if (withPaths) { + /* Evaluate the corresponding set. */ + Bindings::iterator out = attrs->find(state->symbols.create(output)); + if (out == attrs->end()) continue; // FIXME: throw error? + state->forceAttrs(*out->value, *i->pos); + + /* And evaluate its ‘outPath’ attribute. */ + Bindings::iterator outPath = out->value->attrs->find(state->sOutPath); + if (outPath == out->value->attrs->end()) continue; // FIXME: throw error? + PathSet context; + outputs.emplace(output, state->coerceToStorePath(*outPath->pos, *outPath->value, context)); + } else + outputs.emplace(output, std::nullopt); } } else - outputs.emplace("out", queryOutPath()); + outputs.emplace("out", withPaths ? std::optional{queryOutPath()} : std::nullopt); } if (!onlyOutputsToInstall || !attrs) return outputs; diff --git a/src/libexpr/get-drvs.hh b/src/libexpr/get-drvs.hh index 3ca6f1fca..7cc1abef2 100644 --- a/src/libexpr/get-drvs.hh +++ b/src/libexpr/get-drvs.hh @@ -13,7 +13,7 @@ namespace nix { struct DrvInfo { public: - typedef std::map Outputs; + typedef std::map> Outputs; private: EvalState * state; @@ -46,8 +46,9 @@ public: StorePath requireDrvPath() const; StorePath queryOutPath() const; std::string queryOutputName() const; - /** Return the list of outputs. The "outputs to install" are determined by `meta.outputsToInstall`. */ - Outputs queryOutputs(bool onlyOutputsToInstall = false); + /** Return the unordered map of output names to (optional) output paths. + * The "outputs to install" are determined by `meta.outputsToInstall`. */ + Outputs queryOutputs(bool withPaths = true, bool onlyOutputsToInstall = false); StringSet queryMetaNames(); Value * queryMeta(const std::string & name); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 40c3c5d65..e68218d1c 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -918,12 +918,17 @@ static void queryJSON(Globals & globals, std::vector & elems, bool prin pkgObj.attr("pname", drvName.name); pkgObj.attr("version", drvName.version); pkgObj.attr("system", i.querySystem()); + pkgObj.attr("outputName", i.queryOutputName()); - if (printOutPath) { - DrvInfo::Outputs outputs = i.queryOutputs(); + { + DrvInfo::Outputs outputs = i.queryOutputs(printOutPath); JSONObject outputObj = pkgObj.object("outputs"); - for (auto & j : outputs) - outputObj.attr(j.first, globals.state->store->printStorePath(j.second)); + for (auto & j : outputs) { + if (j.second) + outputObj.attr(j.first, globals.state->store->printStorePath(*j.second)); + else + outputObj.attr(j.first, nullptr); + } } if (printMeta) { @@ -1154,13 +1159,16 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) columns.push_back(drvPath ? store.printStorePath(*drvPath) : "-"); } + if (xmlOutput) + attrs["outputName"] = i.queryOutputName(); + if (printOutPath && !xmlOutput) { DrvInfo::Outputs outputs = i.queryOutputs(); std::string s; for (auto & j : outputs) { if (!s.empty()) s += ';'; if (j.first != "out") { s += j.first; s += "="; } - s += store.printStorePath(j.second); + s += store.printStorePath(*j.second); } columns.push_back(s); } @@ -1174,71 +1182,67 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) } if (xmlOutput) { - if (printOutPath || printMeta) { - XMLOpenElement item(xml, "item", attrs); - if (printOutPath) { - DrvInfo::Outputs outputs = i.queryOutputs(); - for (auto & j : outputs) { - XMLAttrs attrs2; - attrs2["name"] = j.first; - attrs2["path"] = store.printStorePath(j.second); - xml.writeEmptyElement("output", attrs2); - } - } - if (printMeta) { - StringSet metaNames = i.queryMetaNames(); - for (auto & j : metaNames) { - XMLAttrs attrs2; - attrs2["name"] = j; - Value * v = i.queryMeta(j); - if (!v) - printError( - "derivation '%s' has invalid meta attribute '%s'", - i.queryName(), j); - else { - if (v->type() == nString) { - attrs2["type"] = "string"; - attrs2["value"] = v->string.s; - xml.writeEmptyElement("meta", attrs2); - } else if (v->type() == nInt) { - attrs2["type"] = "int"; - attrs2["value"] = (format("%1%") % v->integer).str(); - xml.writeEmptyElement("meta", attrs2); - } else if (v->type() == nFloat) { - attrs2["type"] = "float"; - attrs2["value"] = (format("%1%") % v->fpoint).str(); - xml.writeEmptyElement("meta", attrs2); - } else if (v->type() == nBool) { - attrs2["type"] = "bool"; - attrs2["value"] = v->boolean ? "true" : "false"; - xml.writeEmptyElement("meta", attrs2); - } else if (v->type() == nList) { - attrs2["type"] = "strings"; - XMLOpenElement m(xml, "meta", attrs2); - for (auto elem : v->listItems()) { - if (elem->type() != nString) continue; - XMLAttrs attrs3; - attrs3["value"] = elem->string.s; - xml.writeEmptyElement("string", attrs3); - } - } else if (v->type() == nAttrs) { - attrs2["type"] = "strings"; - XMLOpenElement m(xml, "meta", attrs2); - Bindings & attrs = *v->attrs; - for (auto &i : attrs) { - Attr & a(*attrs.find(i.name)); - if(a.value->type() != nString) continue; - XMLAttrs attrs3; - attrs3["type"] = i.name; - attrs3["value"] = a.value->string.s; - xml.writeEmptyElement("string", attrs3); + XMLOpenElement item(xml, "item", attrs); + DrvInfo::Outputs outputs = i.queryOutputs(printOutPath); + for (auto & j : outputs) { + XMLAttrs attrs2; + attrs2["name"] = j.first; + if (j.second) + attrs2["path"] = store.printStorePath(*j.second); + xml.writeEmptyElement("output", attrs2); + } + if (printMeta) { + StringSet metaNames = i.queryMetaNames(); + for (auto & j : metaNames) { + XMLAttrs attrs2; + attrs2["name"] = j; + Value * v = i.queryMeta(j); + if (!v) + printError( + "derivation '%s' has invalid meta attribute '%s'", + i.queryName(), j); + else { + if (v->type() == nString) { + attrs2["type"] = "string"; + attrs2["value"] = v->string.s; + xml.writeEmptyElement("meta", attrs2); + } else if (v->type() == nInt) { + attrs2["type"] = "int"; + attrs2["value"] = (format("%1%") % v->integer).str(); + xml.writeEmptyElement("meta", attrs2); + } else if (v->type() == nFloat) { + attrs2["type"] = "float"; + attrs2["value"] = (format("%1%") % v->fpoint).str(); + xml.writeEmptyElement("meta", attrs2); + } else if (v->type() == nBool) { + attrs2["type"] = "bool"; + attrs2["value"] = v->boolean ? "true" : "false"; + xml.writeEmptyElement("meta", attrs2); + } else if (v->type() == nList) { + attrs2["type"] = "strings"; + XMLOpenElement m(xml, "meta", attrs2); + for (auto elem : v->listItems()) { + if (elem->type() != nString) continue; + XMLAttrs attrs3; + attrs3["value"] = elem->string.s; + xml.writeEmptyElement("string", attrs3); } - } + } else if (v->type() == nAttrs) { + attrs2["type"] = "strings"; + XMLOpenElement m(xml, "meta", attrs2); + Bindings & attrs = *v->attrs; + for (auto &i : attrs) { + Attr & a(*attrs.find(i.name)); + if(a.value->type() != nString) continue; + XMLAttrs attrs3; + attrs3["type"] = i.name; + attrs3["value"] = a.value->string.s; + xml.writeEmptyElement("string", attrs3); + } } } } - } else - xml.writeEmptyElement("item", attrs); + } } else table.push_back(columns); diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index af4f350ff..bcc7736ac 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -56,7 +56,7 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, output paths, and optionally the derivation path, as well as the meta attributes. */ std::optional drvPath = keepDerivations ? i.queryDrvPath() : std::nullopt; - DrvInfo::Outputs outputs = i.queryOutputs(true); + DrvInfo::Outputs outputs = i.queryOutputs(true, true); StringSet metaNames = i.queryMetaNames(); auto attrs = state.buildBindings(7 + outputs.size()); @@ -76,15 +76,15 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, for (const auto & [m, j] : enumerate(outputs)) { (vOutputs.listElems()[m] = state.allocValue())->mkString(j.first); auto outputAttrs = state.buildBindings(2); - outputAttrs.alloc(state.sOutPath).mkString(state.store->printStorePath(j.second)); + outputAttrs.alloc(state.sOutPath).mkString(state.store->printStorePath(*j.second)); attrs.alloc(j.first).mkAttrs(outputAttrs); /* This is only necessary when installing store paths, e.g., `nix-env -i /nix/store/abcd...-foo'. */ - state.store->addTempRoot(j.second); - state.store->ensurePath(j.second); + state.store->addTempRoot(*j.second); + state.store->ensurePath(*j.second); - references.insert(j.second); + references.insert(*j.second); } // Copy the meta attributes. From 8dcecc07387e6a782b4831dcc0d846eceb396191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Na=C3=AFm=20Favier?= Date: Wed, 16 Mar 2022 16:15:11 +0100 Subject: [PATCH 38/83] nix-env: print a final newline after JSON --- src/nix-env/nix-env.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index e68218d1c..a1cd8fd40 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1057,6 +1057,7 @@ static void opQuery(Globals & globals, Strings opFlags, Strings opArgs) /* Print the desired columns, or XML output. */ if (jsonOutput) { queryJSON(globals, elems, printOutPath, printMeta); + cout << '\n'; return; } From c20e07763d84c9dc6f8d06993335c765ba514aca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Wed, 16 Mar 2022 18:39:46 +0100 Subject: [PATCH 39/83] Add some tests for `nix-env -q --json` --- tests/user-envs.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/user-envs.sh b/tests/user-envs.sh index 430688de1..d63fe780a 100644 --- a/tests/user-envs.sh +++ b/tests/user-envs.sh @@ -17,6 +17,16 @@ outPath10=$(nix-env -f ./user-envs.nix -qa --out-path --no-name '*' | grep foo-1 drvPath10=$(nix-env -f ./user-envs.nix -qa --drv-path --no-name '*' | grep foo-1.0) [ -n "$outPath10" -a -n "$drvPath10" ] +# Query with json +nix-env -f ./user-envs.nix -qa --json | jq -e '.[] | select(.name == "bar-0.1") | [ + .outputName == "out", + .outputs.out == null +] | all' +nix-env -f ./user-envs.nix -qa --json --out-path | jq -e '.[] | select(.name == "bar-0.1") | [ + .outputName == "out", + (.outputs.out | test("'$NIX_STORE_DIR'.*-0\\.1")) +] | all' + # Query descriptions. nix-env -f ./user-envs.nix -qa '*' --description | grep -q silly rm -rf $HOME/.nix-defexpr From 3fc4c612fbde332d66b78dcc5b17b7d0d5235484 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 17 Mar 2022 11:34:31 +0100 Subject: [PATCH 40/83] Fix `nix build --dry-run` with CA derivations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Don’t try and assume that we know the output paths when we’ve just built with `--dry-run`. Instead make `--dry-run` follow a different code path that won’t assume the knowledge of the output paths at all. Fix #6275 --- src/libstore/derived-path.cc | 31 ++++++++++++++++++++++++++----- src/libstore/derived-path.hh | 2 ++ src/nix/build.cc | 17 ++++++++++++++--- tests/build-dry.sh | 19 +++++++++++++++++++ tests/ca/build-dry.sh | 6 ++++++ 5 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 tests/ca/build-dry.sh diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 194489580..0183bda35 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -11,6 +11,21 @@ nlohmann::json DerivedPath::Opaque::toJSON(ref store) const { return res; } +nlohmann::json DerivedPath::Built::toJSON(ref store) const { + nlohmann::json res; + res["drvPath"] = store->printStorePath(drvPath); + // Fallback for the input-addressed derivation case: We expect to always be + // able to print the output paths, so let’s do it + auto knownOutputs = store->queryPartialDerivationOutputMap(drvPath); + for (const auto& output : outputs) { + if (knownOutputs.at(output)) + res["outputs"][output] = store->printStorePath(knownOutputs.at(output).value()); + else + res["outputs"][output] = nullptr; + } + return res; +} + nlohmann::json BuiltPath::Built::toJSON(ref store) const { nlohmann::json res; res["drvPath"] = store->printStorePath(drvPath); @@ -35,16 +50,22 @@ StorePathSet BuiltPath::outPaths() const ); } -nlohmann::json derivedPathsWithHintsToJSON(const BuiltPaths & buildables, ref store) { +template +nlohmann::json stuffToJSON(const std::vector & ts, ref store) { auto res = nlohmann::json::array(); - for (const BuiltPath & buildable : buildables) { - std::visit([&res, store](const auto & buildable) { - res.push_back(buildable.toJSON(store)); - }, buildable.raw()); + for (const T & t : ts) { + std::visit([&res, store](const auto & t) { + res.push_back(t.toJSON(store)); + }, t.raw()); } return res; } +nlohmann::json derivedPathsWithHintsToJSON(const BuiltPaths & buildables, ref store) +{ return stuffToJSON(buildables, store); } +nlohmann::json derivedPathsToJSON(const DerivedPaths & paths, ref store) +{ return stuffToJSON(paths, store); } + std::string DerivedPath::Opaque::to_string(const Store & store) const { return store.printStorePath(path); diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index 9d6ace069..8ca0882a4 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -45,6 +45,7 @@ struct DerivedPathBuilt { std::string to_string(const Store & store) const; static DerivedPathBuilt parse(const Store & store, std::string_view); + nlohmann::json toJSON(ref store) const; }; using _DerivedPathRaw = std::variant< @@ -119,5 +120,6 @@ typedef std::vector DerivedPaths; typedef std::vector BuiltPaths; nlohmann::json derivedPathsWithHintsToJSON(const BuiltPaths & buildables, ref store); +nlohmann::json derivedPathsToJSON(const DerivedPaths & , ref store); } diff --git a/src/nix/build.cc b/src/nix/build.cc index 680db1c60..840c7ca38 100644 --- a/src/nix/build.cc +++ b/src/nix/build.cc @@ -52,15 +52,26 @@ struct CmdBuild : InstallablesCommand, MixDryRun, MixJSON, MixProfile void run(ref store) override { + if (dryRun) { + std::vector pathsToBuild; + + for (auto & i : installables) { + auto b = i->toDerivedPaths(); + pathsToBuild.insert(pathsToBuild.end(), b.begin(), b.end()); + } + printMissing(store, pathsToBuild, lvlError); + if (json) + logger->cout("%s", derivedPathsToJSON(pathsToBuild, store).dump()); + return; + } + auto buildables = Installable::build( getEvalStore(), store, - dryRun ? Realise::Derivation : Realise::Outputs, + Realise::Outputs, installables, buildMode); if (json) logger->cout("%s", derivedPathsWithHintsToJSON(buildables, store).dump()); - if (dryRun) return; - if (outLink != "") if (auto store2 = store.dynamic_pointer_cast()) for (const auto & [_i, buildable] : enumerate(buildables)) { diff --git a/tests/build-dry.sh b/tests/build-dry.sh index e72533e70..f0f38e9a0 100644 --- a/tests/build-dry.sh +++ b/tests/build-dry.sh @@ -50,3 +50,22 @@ nix build -f dependencies.nix -o $RESULT --dry-run nix build -f dependencies.nix -o $RESULT [[ -h $RESULT ]] + +################################################### +# Check the JSON output +clearStore +clearCache + +RES=$(nix build -f dependencies.nix --dry-run --json) + +if [[ -z "$NIX_TESTS_CA_BY_DEFAULT" ]]; then + echo "$RES" | jq '.[0] | [ + (.drvPath | test("'$NIX_STORE_DIR'.*\\.drv")), + (.outputs.out | test("'$NIX_STORE_DIR'")) + ] | all' +else + echo "$RES" | jq '.[0] | [ + (.drvPath | test("'$NIX_STORE_DIR'.*\\.drv")), + .outputs.out == null + ] | all' +fi diff --git a/tests/ca/build-dry.sh b/tests/ca/build-dry.sh new file mode 100644 index 000000000..9a72075ec --- /dev/null +++ b/tests/ca/build-dry.sh @@ -0,0 +1,6 @@ +source common.sh + +export NIX_TESTS_CA_BY_DEFAULT=1 + +cd .. && source build-dry.sh + From d58453f72ea584cac2e3362fd6a73fcf0e3b615e Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Thu, 17 Mar 2022 18:36:45 +0000 Subject: [PATCH 41/83] gc: don't visit implicit referrers on garbage collection Before the change garbage collector was not considering `.drv` and outputs as alive even if configuration says otherwise. As a result `nix store gc --dry-run` could visit (and parse) `.drv` files multiple times (worst case it's quadratic). It happens because `alive` set was populating only runtime closure without regard for actual configuration. The change fixes it. Benchmark: my system has about 139MB, 40K `.drv` files. Performance before the change: $ time nix store gc --dry-run real 4m22,148s Performance after the change: $ time nix store gc --dry-run real 0m14,178s --- src/libstore/gc.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 69755bd19..f65fb1b2e 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -678,7 +678,8 @@ void LocalStore::collectGarbage(const GCOptions & options, GCResults & results) alive.insert(start); try { StorePathSet closure; - computeFSClosure(*path, closure); + computeFSClosure(*path, closure, + /* flipDirection */ false, gcKeepOutputs, gcKeepDerivations); for (auto & p : closure) alive.insert(p); } catch (InvalidPath &) { } From 197feed51dd770929ba8f6f12aec50ed8597a747 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Thu, 17 Mar 2022 22:29:15 +0000 Subject: [PATCH 42/83] Clean up `DerivationOutput`, and headers 1. `DerivationOutput` now as the `std::variant` as a base class. And the variants are given hierarchical names under `DerivationOutput`. In 8e0d0689be797f9e42f9b43b06f50c1af7f20b4a @matthewbauer and I didn't know a better idiom, and so we made it a field. But this sort of "newtype" is anoying for literals downstream. Since then we leaned the base class, inherit the constructors trick, e.g. used in `DerivedPath`. Switching to use that makes this more ergonomic, and consistent. 2. `store-api.hh` and `derivations.hh` are now independent. In bcde5456cc3295061a0726881c3e441444dd6680 I swapped the dependency, but I now know it is better to just keep on using incomplete types as much as possible for faster compilation and good separation of concerns. --- src/libexpr/get-drvs.cc | 1 + src/libexpr/primops.cc | 36 ++++------ src/libexpr/primops/context.cc | 1 + src/libstore/build/local-derivation-goal.cc | 14 ++-- src/libstore/derivations.cc | 80 +++++++++------------ src/libstore/derivations.hh | 35 +++++---- src/libstore/derived-path.cc | 1 + src/libstore/local-store.cc | 10 +-- src/libstore/misc.cc | 2 +- src/libstore/parsed-derivations.hh | 1 + src/libstore/repair-flag.hh | 7 ++ src/libstore/store-api.cc | 1 + src/libstore/store-api.hh | 4 +- src/nix/app.cc | 1 + src/nix/develop.cc | 12 ++-- src/nix/show-derivation.cc | 10 +-- 16 files changed, 113 insertions(+), 103 deletions(-) create mode 100644 src/libstore/repair-flag.hh diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 7f2ecb4f7..bb7e77b61 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -1,6 +1,7 @@ #include "get-drvs.hh" #include "util.hh" #include "eval-inline.hh" +#include "derivations.hh" #include "store-api.hh" #include "path-with-outputs.hh" diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 0980e75f4..4f9b6f0e0 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1163,26 +1163,24 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeFixedOutputPath(ingestionMethod, h, drvName); drv.env["out"] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign("out", DerivationOutput { - .output = DerivationOutputCAFixed { - .hash = FixedOutputHash { - .method = ingestionMethod, - .hash = std::move(h), - }, + drv.outputs.insert_or_assign("out", + DerivationOutput::CAFixed { + .hash = FixedOutputHash { + .method = ingestionMethod, + .hash = std::move(h), }, - }); + }); } else if (contentAddressed) { HashType ht = parseHashType(outputHashAlgo); for (auto & i : outputs) { drv.env[i] = hashPlaceholder(i); - drv.outputs.insert_or_assign(i, DerivationOutput { - .output = DerivationOutputCAFloating { + drv.outputs.insert_or_assign(i, + DerivationOutput::CAFloating { .method = ingestionMethod, .hashType = ht, - }, - }); + }); } } @@ -1196,10 +1194,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * for (auto & i : outputs) { drv.env[i] = ""; drv.outputs.insert_or_assign(i, - DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = StorePath::dummy, - }, + DerivationOutput::InputAddressed { + .path = StorePath::dummy, }); } @@ -1213,9 +1209,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * case DrvHash::Kind::Deferred: for (auto & i : outputs) { drv.outputs.insert_or_assign(i, - DerivationOutput { - .output = DerivationOutputDeferred{}, - }); + DerivationOutput::Deferred { }); } break; case DrvHash::Kind::Regular: @@ -1223,10 +1217,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto outPath = state.store->makeOutputPath(i, h, drvName); drv.env[i] = state.store->printStorePath(outPath); drv.outputs.insert_or_assign(i, - DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = std::move(outPath), - }, + DerivationOutput::InputAddressed { + .path = std::move(outPath), }); } break; diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index 3701bd442..324394d10 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -1,5 +1,6 @@ #include "primops.hh" #include "eval-inline.hh" +#include "derivations.hh" #include "store-api.hh" namespace nix { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4e763e570..8a301d4ac 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2279,7 +2279,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() return res; }; - auto newInfoFromCA = [&](const DerivationOutputCAFloating outputHash) -> ValidPathInfo { + auto newInfoFromCA = [&](const DerivationOutput::CAFloating outputHash) -> ValidPathInfo { auto & st = outputStats.at(outputName); if (outputHash.method == FileIngestionMethod::Flat) { /* The output path should be a regular file without execute permission. */ @@ -2346,7 +2346,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() ValidPathInfo newInfo = std::visit(overloaded { - [&](const DerivationOutputInputAddressed & output) { + [&](const DerivationOutput::InputAddressed & output) { /* input-addressed case */ auto requiredFinalPath = output.path; /* Preemptively add rewrite rule for final hash, as that is @@ -2366,8 +2366,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs() return newInfo0; }, - [&](const DerivationOutputCAFixed & dof) { - auto newInfo0 = newInfoFromCA(DerivationOutputCAFloating { + [&](const DerivationOutput::CAFixed & dof) { + auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating { .method = dof.hash.method, .hashType = dof.hash.hash.type, }); @@ -2389,17 +2389,17 @@ DrvOutputs LocalDerivationGoal::registerOutputs() return newInfo0; }, - [&](DerivationOutputCAFloating & dof) { + [&](const DerivationOutput::CAFloating & dof) { return newInfoFromCA(dof); }, - [&](DerivationOutputDeferred) -> ValidPathInfo { + [&](const DerivationOutput::Deferred &) -> ValidPathInfo { // No derivation should reach that point without having been // rewritten first assert(false); }, - }, output.output); + }, output.raw()); /* FIXME: set proper permissions in restorePath() so we don't have to do another traversal. */ diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 1fe45bd87..3dd3a78d8 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -11,25 +11,25 @@ namespace nix { std::optional DerivationOutput::path(const Store & store, std::string_view drvName, std::string_view outputName) const { return std::visit(overloaded { - [](const DerivationOutputInputAddressed & doi) -> std::optional { + [](const DerivationOutput::InputAddressed & doi) -> std::optional { return { doi.path }; }, - [&](const DerivationOutputCAFixed & dof) -> std::optional { + [&](const DerivationOutput::CAFixed & dof) -> std::optional { return { dof.path(store, drvName, outputName) }; }, - [](const DerivationOutputCAFloating & dof) -> std::optional { + [](const DerivationOutput::CAFloating & dof) -> std::optional { return std::nullopt; }, - [](const DerivationOutputDeferred &) -> std::optional { + [](const DerivationOutput::Deferred &) -> std::optional { return std::nullopt; }, - }, output); + }, raw()); } -StorePath DerivationOutputCAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const { +StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view drvName, std::string_view outputName) const { return store.makeFixedOutputPath( hash.method, hash.hash, outputPathName(drvName, outputName)); @@ -179,35 +179,27 @@ static DerivationOutput parseDerivationOutput(const Store & store, const auto hashType = parseHashType(hashAlgo); if (hash != "") { validatePath(pathS); - return DerivationOutput { - .output = DerivationOutputCAFixed { - .hash = FixedOutputHash { - .method = std::move(method), - .hash = Hash::parseNonSRIUnprefixed(hash, hashType), - }, + return DerivationOutput::CAFixed { + .hash = FixedOutputHash { + .method = std::move(method), + .hash = Hash::parseNonSRIUnprefixed(hash, hashType), }, }; } else { settings.requireExperimentalFeature(Xp::CaDerivations); assert(pathS == ""); - return DerivationOutput { - .output = DerivationOutputCAFloating { - .method = std::move(method), - .hashType = std::move(hashType), - }, + return DerivationOutput::CAFloating { + .method = std::move(method), + .hashType = std::move(hashType), }; } } else { if (pathS == "") { - return DerivationOutput { - .output = DerivationOutputDeferred { } - }; + return DerivationOutput::Deferred { }; } validatePath(pathS); - return DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = store.parseStorePath(pathS), - } + return DerivationOutput::InputAddressed { + .path = store.parseStorePath(pathS), }; } } @@ -335,27 +327,27 @@ std::string Derivation::unparse(const Store & store, bool maskOutputs, if (first) first = false; else s += ','; s += '('; printUnquotedString(s, i.first); std::visit(overloaded { - [&](const DerivationOutputInputAddressed & doi) { + [&](const DerivationOutput::InputAddressed & doi) { s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(doi.path)); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); }, - [&](const DerivationOutputCAFixed & dof) { + [&](const DerivationOutput::CAFixed & dof) { s += ','; printUnquotedString(s, maskOutputs ? "" : store.printStorePath(dof.path(store, name, i.first))); s += ','; printUnquotedString(s, dof.hash.printMethodAlgo()); s += ','; printUnquotedString(s, dof.hash.hash.to_string(Base16, false)); }, - [&](const DerivationOutputCAFloating & dof) { + [&](const DerivationOutput::CAFloating & dof) { s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); s += ','; printUnquotedString(s, ""); }, - [&](const DerivationOutputDeferred &) { + [&](const DerivationOutput::Deferred &) { s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); s += ','; printUnquotedString(s, ""); } - }, i.second.output); + }, i.second.raw()); s += ')'; } @@ -423,13 +415,13 @@ DerivationType BasicDerivation::type() const std::optional floatingHashType; for (auto & i : outputs) { std::visit(overloaded { - [&](const DerivationOutputInputAddressed &) { + [&](const DerivationOutput::InputAddressed &) { inputAddressedOutputs.insert(i.first); }, - [&](const DerivationOutputCAFixed &) { + [&](const DerivationOutput::CAFixed &) { fixedCAOutputs.insert(i.first); }, - [&](const DerivationOutputCAFloating & dof) { + [&](const DerivationOutput::CAFloating & dof) { floatingCAOutputs.insert(i.first); if (!floatingHashType) { floatingHashType = dof.hashType; @@ -438,10 +430,10 @@ DerivationType BasicDerivation::type() const throw Error("All floating outputs must use the same hash type"); } }, - [&](const DerivationOutputDeferred &) { + [&](const DerivationOutput::Deferred &) { deferredIAOutputs.insert(i.first); }, - }, i.second.output); + }, i.second.raw()); } if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { @@ -516,7 +508,7 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m case DerivationType::CAFixed: { std::map outputHashes; for (const auto & i : drv.outputs) { - auto & dof = std::get(i.second.output); + auto & dof = std::get(i.second.raw()); auto hash = hashString(htSHA256, "fixed:out:" + dof.hash.printMethodAlgo() + ":" + dof.hash.hash.to_string(Base16, false) + ":" @@ -672,27 +664,27 @@ void writeDerivation(Sink & out, const Store & store, const BasicDerivation & dr for (auto & i : drv.outputs) { out << i.first; std::visit(overloaded { - [&](const DerivationOutputInputAddressed & doi) { + [&](const DerivationOutput::InputAddressed & doi) { out << store.printStorePath(doi.path) << "" << ""; }, - [&](const DerivationOutputCAFixed & dof) { + [&](const DerivationOutput::CAFixed & dof) { out << store.printStorePath(dof.path(store, drv.name, i.first)) << dof.hash.printMethodAlgo() << dof.hash.hash.to_string(Base16, false); }, - [&](const DerivationOutputCAFloating & dof) { + [&](const DerivationOutput::CAFloating & dof) { out << "" << (makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)) << ""; }, - [&](const DerivationOutputDeferred &) { + [&](const DerivationOutput::Deferred &) { out << "" << "" << ""; }, - }, i.second.output); + }, i.second.raw()); } worker_proto::write(store, out, drv.inputSrcs); out << drv.platform << drv.builder << drv.args; @@ -740,14 +732,12 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String auto hashModulo = hashDerivationModulo(store, Derivation(drv), true); for (auto & [outputName, output] : drv.outputs) { - if (std::holds_alternative(output.output)) { + if (std::holds_alternative(output.raw())) { auto & h = hashModulo.requireNoFixedNonDeferred(); auto outPath = store.makeOutputPath(outputName, h, drv.name); drv.env[outputName] = store.printStorePath(outPath); - output = DerivationOutput { - .output = DerivationOutputInputAddressed { - .path = std::move(outPath), - }, + output = DerivationOutput::InputAddressed { + .path = std::move(outPath), }; } } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 2fb18d7f7..6b92b2f21 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -4,6 +4,7 @@ #include "types.hh" #include "hash.hh" #include "content-address.hh" +#include "repair-flag.hh" #include "sync.hh" #include @@ -44,19 +45,31 @@ struct DerivationOutputCAFloating */ struct DerivationOutputDeferred {}; -struct DerivationOutput +typedef std::variant< + DerivationOutputInputAddressed, + DerivationOutputCAFixed, + DerivationOutputCAFloating, + DerivationOutputDeferred +> _DerivationOutputRaw; + +struct DerivationOutput : _DerivationOutputRaw { - std::variant< - DerivationOutputInputAddressed, - DerivationOutputCAFixed, - DerivationOutputCAFloating, - DerivationOutputDeferred - > output; + using Raw = _DerivationOutputRaw; + using Raw::Raw; + + using InputAddressed = DerivationOutputInputAddressed; + using CAFixed = DerivationOutputCAFixed; + using CAFloating = DerivationOutputCAFloating; + using Deferred = DerivationOutputDeferred; /* Note, when you use this function you should make sure that you're passing the right derivation name. When in doubt, you should use the safer interface provided by BasicDerivation::outputsAndOptPaths */ std::optional path(const Store & store, std::string_view drvName, std::string_view outputName) const; + + inline const Raw & raw() const { + return static_cast(*this); + } }; typedef std::map DerivationOutputs; @@ -150,8 +163,6 @@ struct Derivation : BasicDerivation class Store; -enum RepairFlag : bool { NoRepair = false, Repair = true }; - /* Write a derivation to the Nix store, and return its path. */ StorePath writeDerivation(Store & store, const Derivation & drv, @@ -197,10 +208,10 @@ typedef std::variant< DrvHash, // Fixed-output derivation hashes CaOutputHashes -> DrvHashModuloRaw; +> _DrvHashModuloRaw; -struct DrvHashModulo : DrvHashModuloRaw { - using Raw = DrvHashModuloRaw; +struct DrvHashModulo : _DrvHashModuloRaw { + using Raw = _DrvHashModuloRaw; using Raw::Raw; /* Get hash, throwing if it is per-output CA hashes or a diff --git a/src/libstore/derived-path.cc b/src/libstore/derived-path.cc index 0183bda35..319b1c790 100644 --- a/src/libstore/derived-path.cc +++ b/src/libstore/derived-path.cc @@ -1,4 +1,5 @@ #include "derived-path.hh" +#include "derivations.hh" #include "store-api.hh" #include diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 9230be15a..75726a1ab 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -698,7 +698,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat std::optional h; for (auto & i : drv.outputs) { std::visit(overloaded { - [&](const DerivationOutputInputAddressed & doia) { + [&](const DerivationOutput::InputAddressed & doia) { if (!h) { // somewhat expensive so we do lazily auto h0 = hashDerivationModulo(*this, drv, true); @@ -710,16 +710,16 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat printStorePath(drvPath), printStorePath(doia.path), printStorePath(recomputed)); envHasRightPath(doia.path, i.first); }, - [&](const DerivationOutputCAFixed & dof) { + [&](const DerivationOutput::CAFixed & dof) { StorePath path = makeFixedOutputPath(dof.hash.method, dof.hash.hash, drvName); envHasRightPath(path, i.first); }, - [&](const DerivationOutputCAFloating &) { + [&](const DerivationOutput::CAFloating &) { /* Nothing to check */ }, - [&](const DerivationOutputDeferred &) { + [&](const DerivationOutput::Deferred &) { }, - }, i.second.output); + }, i.second.raw()); } } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 6409874ff..1f0bae7fe 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -87,7 +87,7 @@ std::optional getDerivationCA(const BasicDerivation & drv) { auto out = drv.outputs.find("out"); if (out != drv.outputs.end()) { - if (auto v = std::get_if(&out->second.output)) + if (const auto * v = std::get_if(&out->second.raw())) return v->hash; } return std::nullopt; diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index effcf099d..95bec21e8 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -1,5 +1,6 @@ #pragma once +#include "derivations.hh" #include "store-api.hh" #include diff --git a/src/libstore/repair-flag.hh b/src/libstore/repair-flag.hh new file mode 100644 index 000000000..a13cda312 --- /dev/null +++ b/src/libstore/repair-flag.hh @@ -0,0 +1,7 @@ +#pragma once + +namespace nix { + +enum RepairFlag : bool { NoRepair = false, Repair = true }; + +} diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 86fa6a211..59937be4d 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -1,6 +1,7 @@ #include "crypto.hh" #include "fs-accessor.hh" #include "globals.hh" +#include "derivations.hh" #include "store-api.hh" #include "util.hh" #include "nar-info-disk-cache.hh" diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 635a82a2a..0c8a4db56 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -10,8 +10,8 @@ #include "sync.hh" #include "globals.hh" #include "config.hh" -#include "derivations.hh" #include "path-info.hh" +#include "repair-flag.hh" #include #include @@ -62,6 +62,8 @@ MakeError(BadStorePath, Error); MakeError(InvalidStoreURI, Error); +struct BasicDerivation; +struct Derivation; class FSAccessor; class NarInfoDiskCache; class Store; diff --git a/src/nix/app.cc b/src/nix/app.cc index 2563180fb..30e138a96 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -4,6 +4,7 @@ #include "eval-cache.hh" #include "names.hh" #include "command.hh" +#include "derivations.hh" namespace nix { diff --git a/src/nix/develop.cc b/src/nix/develop.cc index dafcafd79..6baf539c3 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -196,14 +196,14 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore drv.inputSrcs.insert(std::move(getEnvShPath)); if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { for (auto & output : drv.outputs) { - output.second = { - .output = DerivationOutputDeferred{}, - }; + output.second = DerivationOutput::Deferred {}, drv.env[output.first] = hashPlaceholder(output.first); } } else { for (auto & output : drv.outputs) { - output.second = { .output = DerivationOutputInputAddressed { .path = StorePath::dummy } }; + output.second = DerivationOutput::InputAddressed { + .path = StorePath::dummy, + }; drv.env[output.first] = ""; } auto h0 = hashDerivationModulo(*evalStore, drv, true); @@ -211,7 +211,9 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore for (auto & output : drv.outputs) { auto outPath = store->makeOutputPath(output.first, h, drv.name); - output.second = { .output = DerivationOutputInputAddressed { .path = outPath } }; + output.second = DerivationOutput::InputAddressed { + .path = outPath, + }; drv.env[output.first] = store->printStorePath(outPath); } } diff --git a/src/nix/show-derivation.cc b/src/nix/show-derivation.cc index 61a02c9b3..0d9655732 100644 --- a/src/nix/show-derivation.cc +++ b/src/nix/show-derivation.cc @@ -65,19 +65,19 @@ struct CmdShowDerivation : InstallablesCommand auto & outputName = _outputName; // work around clang bug auto outputObj { outputsObj.object(outputName) }; std::visit(overloaded { - [&](const DerivationOutputInputAddressed & doi) { + [&](const DerivationOutput::InputAddressed & doi) { outputObj.attr("path", store->printStorePath(doi.path)); }, - [&](const DerivationOutputCAFixed & dof) { + [&](const DerivationOutput::CAFixed & dof) { outputObj.attr("path", store->printStorePath(dof.path(*store, drv.name, outputName))); outputObj.attr("hashAlgo", dof.hash.printMethodAlgo()); outputObj.attr("hash", dof.hash.hash.to_string(Base16, false)); }, - [&](const DerivationOutputCAFloating & dof) { + [&](const DerivationOutput::CAFloating & dof) { outputObj.attr("hashAlgo", makeFileIngestionPrefix(dof.method) + printHashType(dof.hashType)); }, - [&](const DerivationOutputDeferred &) {}, - }, output.output); + [&](const DerivationOutput::Deferred &) {}, + }, output.raw()); } } From 8496be7defed3584c9fcd3b5d905fb84a9515d6f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 18 Mar 2022 02:07:31 +0000 Subject: [PATCH 43/83] Use Deferred when building an input-addressed drv Easier than using dummy path with input addressed. --- src/libexpr/primops.cc | 22 ++++++++-------------- src/nix/develop.cc | 4 +--- 2 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 4f9b6f0e0..489bee826 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1194,9 +1194,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * for (auto & i : outputs) { drv.env[i] = ""; drv.outputs.insert_or_assign(i, - DerivationOutput::InputAddressed { - .path = StorePath::dummy, - }); + DerivationOutput::Deferred { }); } // Regular, non-CA derivation should always return a single hash and not @@ -1207,19 +1205,15 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * auto & h = drvHash.hash; switch (drvHash.kind) { case DrvHash::Kind::Deferred: - for (auto & i : outputs) { - drv.outputs.insert_or_assign(i, - DerivationOutput::Deferred { }); - } + /* Outputs already deferred, nothing to do */ break; case DrvHash::Kind::Regular: - for (auto & i : outputs) { - auto outPath = state.store->makeOutputPath(i, h, drvName); - drv.env[i] = state.store->printStorePath(outPath); - drv.outputs.insert_or_assign(i, - DerivationOutput::InputAddressed { - .path = std::move(outPath), - }); + for (auto & [outputName, output] : drv.outputs) { + auto outPath = state.store->makeOutputPath(outputName, h, drvName); + drv.env[outputName] = state.store->printStorePath(outPath); + output = DerivationOutput::InputAddressed { + .path = std::move(outPath), + }; } break; } diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 6baf539c3..d2f9b5a6a 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -201,9 +201,7 @@ static StorePath getDerivationEnvironment(ref store, ref evalStore } } else { for (auto & output : drv.outputs) { - output.second = DerivationOutput::InputAddressed { - .path = StorePath::dummy, - }; + output.second = DerivationOutput::Deferred { }; drv.env[output.first] = ""; } auto h0 = hashDerivationModulo(*evalStore, drv, true); From 049fae155a18784ca59d194bf3e579fadbc3b48f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 18 Mar 2022 02:15:32 +0000 Subject: [PATCH 44/83] Avoid some pointless copying of drvs --- src/libexpr/primops.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 489bee826..2dc33103d 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1199,7 +1199,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * // Regular, non-CA derivation should always return a single hash and not // hash per output. - auto hashModulo = hashDerivationModulo(*state.store, Derivation(drv), true); + auto hashModulo = hashDerivationModulo(*state.store, drv, true); std::visit(overloaded { [&](const DrvHash & drvHash) { auto & h = drvHash.hash; @@ -1240,7 +1240,7 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * However, we don't bother doing this for floating CA derivations because their "hash modulo" is indeterminate until built. */ if (drv.type() != DerivationType::CAFloating) { - auto h = hashDerivationModulo(*state.store, Derivation(drv), false); + auto h = hashDerivationModulo(*state.store, drv, false); drvHashes.lock()->insert_or_assign(drvPath, h); } From a544ed7684bdf5fa3c0b78d40913f5be3f73f5a7 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 18 Mar 2022 00:36:52 +0000 Subject: [PATCH 45/83] Generalize `DerivationType` in preparation for impure derivations --- src/build-remote/build-remote.cc | 2 +- src/libexpr/primops.cc | 7 +- src/libstore/build/derivation-goal.cc | 35 ++++-- src/libstore/build/local-derivation-goal.cc | 14 +-- src/libstore/daemon.cc | 8 +- src/libstore/derivations.cc | 112 +++++++++++--------- src/libstore/derivations.hh | 62 +++++++---- src/libstore/local-store.cc | 1 + src/libstore/parsed-derivations.cc | 2 +- 9 files changed, 148 insertions(+), 95 deletions(-) diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 8c9133c17..ff8ba2724 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -300,7 +300,7 @@ connected: std::set missingRealisations; StorePathSet missingPaths; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !drv.type().hasKnownOutputPaths()) { for (auto & outputName : wantedOutputs) { auto thisOutputHash = outputHashes.at(outputName); auto thisOutputId = DrvOutput{ thisOutputHash, outputName }; diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 2dc33103d..bfa18f898 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -1235,11 +1235,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Optimisation, but required in read-only mode! because in that case we don't actually write store derivations, so we can't - read them later. - - However, we don't bother doing this for floating CA derivations because - their "hash modulo" is indeterminate until built. */ - if (drv.type() != DerivationType::CAFloating) { + read them later. */ + { auto h = hashDerivationModulo(*state.store, drv, false); drvHashes.lock()->insert_or_assign(drvPath, h); } diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index afed9bf16..40c445836 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -204,7 +204,7 @@ void DerivationGoal::haveDerivation() { trace("have derivation"); - if (drv->type() == DerivationType::CAFloating) + if (!drv->type().hasKnownOutputPaths()) settings.requireExperimentalFeature(Xp::CaDerivations); retrySubstitution = false; @@ -440,9 +440,28 @@ void DerivationGoal::inputsRealised() if (useDerivation) { auto & fullDrv = *dynamic_cast(drv.get()); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && - ((!fullDrv.inputDrvs.empty() && derivationIsCA(fullDrv.type())) - || fullDrv.type() == DerivationType::DeferredInputAddressed)) { + auto drvType = fullDrv.type(); + bool resolveDrv = std::visit(overloaded { + [&](const DerivationType::InputAddressed & ia) { + /* must resolve if deferred. */ + return ia.deferred; + }, + [&](const DerivationType::ContentAddressed & ca) { + return !fullDrv.inputDrvs.empty() && ( + ca.fixed + /* Can optionally resolve if fixed, which is good + for avoiding unnecessary rebuilds. */ + ? settings.isExperimentalFeatureEnabled(Xp::CaDerivations) + /* Must resolve if floating and there are any inputs + drvs. */ + : true); + }, + }, drvType.raw()); + + if (resolveDrv) + { + settings.requireExperimentalFeature(Xp::CaDerivations); + /* We are be able to resolve this derivation based on the now-known results of dependencies. If so, we become a stub goal aliasing that resolved derivation goal */ @@ -501,7 +520,7 @@ void DerivationGoal::inputsRealised() /* Don't repeat fixed-output derivations since they're already verified by their output hash.*/ - nrRounds = derivationIsFixed(derivationType) ? 1 : settings.buildRepeat + 1; + nrRounds = derivationType.isFixed() ? 1 : settings.buildRepeat + 1; /* Okay, try to build. Note that here we don't wait for a build slot to become available, since we don't need one if there is a @@ -908,7 +927,7 @@ void DerivationGoal::buildDone() st = dynamic_cast(&e) ? BuildResult::NotDeterministic : statusOk(status) ? BuildResult::OutputRejected : - derivationIsImpure(derivationType) || diskFull ? BuildResult::TransientFailure : + derivationType.isImpure() || diskFull ? BuildResult::TransientFailure : BuildResult::PermanentFailure; } @@ -1221,7 +1240,7 @@ void DerivationGoal::flushLine() std::map> DerivationGoal::queryPartialDerivationOutputMap() { - if (!useDerivation || drv->type() != DerivationType::CAFloating) { + if (!useDerivation || drv->type().hasKnownOutputPaths()) { std::map> res; for (auto & [name, output] : drv->outputs) res.insert_or_assign(name, output.path(worker.store, drv->name, name)); @@ -1233,7 +1252,7 @@ std::map> DerivationGoal::queryPartialDeri OutputPathMap DerivationGoal::queryDerivationOutputMap() { - if (!useDerivation || drv->type() != DerivationType::CAFloating) { + if (!useDerivation || drv->type().hasKnownOutputPaths()) { OutputPathMap res; for (auto & [name, output] : drv->outputsAndOptPaths(worker.store)) res.insert_or_assign(name, *output.second); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8a301d4ac..75e7e6ca3 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder() else if (settings.sandboxMode == smDisabled) useChroot = false; else if (settings.sandboxMode == smRelaxed) - useChroot = !(derivationIsImpure(derivationType)) && !noChroot; + useChroot = !(derivationType.isImpure()) && !noChroot; } auto & localStore = getLocalStore(); @@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder() "nogroup:x:65534:\n", sandboxGid())); /* Create /etc/hosts with localhost entry. */ - if (!(derivationIsImpure(derivationType))) + if (!(derivationType.isImpure())) writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n"); /* Make the closure of the inputs available in the chroot, @@ -796,7 +796,7 @@ void LocalDerivationGoal::startBuilder() us. */ - if (!(derivationIsImpure(derivationType))) + if (!(derivationType.isImpure())) privateNetwork = true; userNamespaceSync.create(); @@ -1049,7 +1049,7 @@ void LocalDerivationGoal::initEnv() derivation, tell the builder, so that for instance `fetchurl' can skip checking the output. On older Nixes, this environment variable won't be set, so `fetchurl' will do the check. */ - if (derivationIsFixed(derivationType)) env["NIX_OUTPUT_CHECKED"] = "1"; + if (derivationType.isFixed()) env["NIX_OUTPUT_CHECKED"] = "1"; /* *Only* if this is a fixed-output derivation, propagate the values of the environment variables specified in the @@ -1060,7 +1060,7 @@ void LocalDerivationGoal::initEnv() to the builder is generally impure, but the output of fixed-output derivations is by definition pure (since we already know the cryptographic hash of the output). */ - if (derivationIsImpure(derivationType)) { + if (derivationType.isImpure()) { for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings())) env[i] = getEnv(i).value_or(""); } @@ -1674,7 +1674,7 @@ void LocalDerivationGoal::runChild() /* Fixed-output derivations typically need to access the network, so give them access to /etc/resolv.conf and so on. */ - if (derivationIsImpure(derivationType)) { + if (derivationType.isImpure()) { // Only use nss functions to resolve hosts and // services. Don’t use it for anything else that may // be configured for this system. This limits the @@ -1918,7 +1918,7 @@ void LocalDerivationGoal::runChild() sandboxProfile += "(import \"sandbox-defaults.sb\")\n"; - if (derivationIsImpure(derivationType)) + if (derivationType.isImpure()) sandboxProfile += "(import \"sandbox-network.sb\")\n"; /* Add the output paths we'll use at build-time to the chroot */ diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 9f21ecf36..de69b50ee 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -560,6 +560,8 @@ static void performOp(TunnelLogger * logger, ref store, BuildMode buildMode = (BuildMode) readInt(from); logger->startWork(); + auto drvType = drv.type(); + /* Content-addressed derivations are trustless because their output paths are verified by their content alone, so any derivation is free to try to produce such a path. @@ -592,12 +594,12 @@ static void performOp(TunnelLogger * logger, ref store, derivations, we throw out the precomputed output paths and just store the hashes, so there aren't two competing sources of truth an attacker could exploit. */ - if (drv.type() == DerivationType::InputAddressed && !trusted) + if (!(drvType.isCA() || trusted)) throw Error("you are not privileged to build input-addressed derivations"); /* Make sure that the non-input-addressed derivations that got this far are in fact content-addressed if we don't trust them. */ - assert(derivationIsCA(drv.type()) || trusted); + assert(drvType.isCA() || trusted); /* Recompute the derivation path when we cannot trust the original. */ if (!trusted) { @@ -606,7 +608,7 @@ static void performOp(TunnelLogger * logger, ref store, original not-necessarily-resolved derivation to verify the drv derivation as adequate claim to the input-addressed output paths. */ - assert(derivationIsCA(drv.type())); + assert(drvType.isCA()); Derivation drv2; static_cast(drv2) = drv; diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 3dd3a78d8..7fed80387 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -36,47 +36,46 @@ StorePath DerivationOutput::CAFixed::path(const Store & store, std::string_view } -bool derivationIsCA(DerivationType dt) { - switch (dt) { - case DerivationType::InputAddressed: return false; - case DerivationType::CAFixed: return true; - case DerivationType::CAFloating: return true; - case DerivationType::DeferredInputAddressed: return false; - }; - // Since enums can have non-variant values, but making a `default:` would - // disable exhaustiveness warnings. - assert(false); +bool DerivationType::isCA() const { + /* Normally we do the full `std::visit` to make sure we have + exhaustively handled all variants, but so long as there is a + variant called `ContentAddressed`, it must be the only one for + which `isCA` is true for this to make sense!. */ + return std::holds_alternative(raw()); } -bool derivationIsFixed(DerivationType dt) { - switch (dt) { - case DerivationType::InputAddressed: return false; - case DerivationType::CAFixed: return true; - case DerivationType::CAFloating: return false; - case DerivationType::DeferredInputAddressed: return false; - }; - assert(false); +bool DerivationType::isFixed() const { + return std::visit(overloaded { + [](const InputAddressed & ia) { + return false; + }, + [](const ContentAddressed & ca) { + return ca.fixed; + }, + }, raw()); } -bool derivationHasKnownOutputPaths(DerivationType dt) { - switch (dt) { - case DerivationType::InputAddressed: return true; - case DerivationType::CAFixed: return true; - case DerivationType::CAFloating: return false; - case DerivationType::DeferredInputAddressed: return false; - }; - assert(false); +bool DerivationType::hasKnownOutputPaths() const { + return std::visit(overloaded { + [](const InputAddressed & ia) { + return !ia.deferred; + }, + [](const ContentAddressed & ca) { + return ca.fixed; + }, + }, raw()); } -bool derivationIsImpure(DerivationType dt) { - switch (dt) { - case DerivationType::InputAddressed: return false; - case DerivationType::CAFixed: return true; - case DerivationType::CAFloating: return false; - case DerivationType::DeferredInputAddressed: return false; - }; - assert(false); +bool DerivationType::isImpure() const { + return std::visit(overloaded { + [](const InputAddressed & ia) { + return false; + }, + [](const ContentAddressed & ca) { + return !ca.pure; + }, + }, raw()); } @@ -439,18 +438,28 @@ DerivationType BasicDerivation::type() const if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { throw Error("Must have at least one output"); } else if (! inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { - return DerivationType::InputAddressed; + return DerivationType::InputAddressed { + .deferred = false, + }; } else if (inputAddressedOutputs.empty() && ! fixedCAOutputs.empty() && floatingCAOutputs.empty() && deferredIAOutputs.empty()) { if (fixedCAOutputs.size() > 1) // FIXME: Experimental feature? throw Error("Only one fixed output is allowed for now"); if (*fixedCAOutputs.begin() != "out") throw Error("Single fixed output must be named \"out\""); - return DerivationType::CAFixed; + return DerivationType::ContentAddressed { + .pure = false, + .fixed = true, + }; } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && ! floatingCAOutputs.empty() && deferredIAOutputs.empty()) { - return DerivationType::CAFloating; + return DerivationType::ContentAddressed { + .pure = true, + .fixed = false, + }; } else if (inputAddressedOutputs.empty() && fixedCAOutputs.empty() && floatingCAOutputs.empty() && !deferredIAOutputs.empty()) { - return DerivationType::DeferredInputAddressed; + return DerivationType::InputAddressed { + .deferred = true, + }; } else { throw Error("Can't mix derivation output types"); } @@ -502,10 +511,10 @@ static const DrvHashModulo pathDerivationModulo(Store & store, const StorePath & */ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool maskOutputs) { - auto kind = DrvHash::Kind::Regular; + auto type = drv.type(); + /* Return a fixed hash for fixed-output derivations. */ - switch (drv.type()) { - case DerivationType::CAFixed: { + if (type.isFixed()) { std::map outputHashes; for (const auto & i : drv.outputs) { auto & dof = std::get(i.second.raw()); @@ -517,14 +526,19 @@ DrvHashModulo hashDerivationModulo(Store & store, const Derivation & drv, bool m } return outputHashes; } - case DerivationType::CAFloating: - kind = DrvHash::Kind::Deferred; - break; - case DerivationType::InputAddressed: - break; - case DerivationType::DeferredInputAddressed: - break; - } + + auto kind = std::visit(overloaded { + [](const DerivationType::InputAddressed & ia) { + /* This might be a "pesimistically" deferred output, so we don't + "taint" the kind yet. */ + return DrvHash::Kind::Regular; + }, + [](const DerivationType::ContentAddressed & ca) { + return ca.fixed + ? DrvHash::Kind::Regular + : DrvHash::Kind::Deferred; + }, + }, drv.type().raw()); /* For other derivations, replace the inputs paths with recursive calls to this function. */ diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 6b92b2f21..8dea90abf 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -85,30 +85,50 @@ typedef std::map DerivationInputs; -enum struct DerivationType : uint8_t { - InputAddressed, - DeferredInputAddressed, - CAFixed, - CAFloating, +struct DerivationType_InputAddressed { + bool deferred; }; -/* Do the outputs of the derivation have paths calculated from their content, - or from the derivation itself? */ -bool derivationIsCA(DerivationType); +struct DerivationType_ContentAddressed { + bool pure; + bool fixed; +}; -/* Is the content of the outputs fixed a-priori via a hash? Never true for - non-CA derivations. */ -bool derivationIsFixed(DerivationType); +typedef std::variant< + DerivationType_InputAddressed, + DerivationType_ContentAddressed +> _DerivationTypeRaw; -/* Is the derivation impure and needs to access non-deterministic resources, or - pure and can be sandboxed? Note that whether or not we actually sandbox the - derivation is controlled separately. Never true for non-CA derivations. */ -bool derivationIsImpure(DerivationType); +struct DerivationType : _DerivationTypeRaw { + using Raw = _DerivationTypeRaw; + using Raw::Raw; + using InputAddressed = DerivationType_InputAddressed; + using ContentAddressed = DerivationType_ContentAddressed; -/* Does the derivation knows its own output paths? - * Only true when there's no floating-ca derivation involved in the closure. - */ -bool derivationHasKnownOutputPaths(DerivationType); + + /* Do the outputs of the derivation have paths calculated from their content, + or from the derivation itself? */ + bool isCA() const; + + /* Is the content of the outputs fixed a-priori via a hash? Never true for + non-CA derivations. */ + bool isFixed() const; + + /* Is the derivation impure and needs to access non-deterministic resources, or + pure and can be sandboxed? Note that whether or not we actually sandbox the + derivation is controlled separately. Never true for non-CA derivations. */ + bool isImpure() const; + + /* Does the derivation knows its own output paths? + Only true when there's no floating-ca derivation involved in the + closure, or if fixed output. + */ + bool hasKnownOutputPaths() const; + + inline const Raw & raw() const { + return static_cast(*this); + } +}; struct BasicDerivation { @@ -189,11 +209,11 @@ typedef std::map CaOutputHashes; struct DrvHash { Hash hash; - enum struct Kind { + enum struct Kind: bool { // Statically determined derivations. // This hash will be directly used to compute the output paths Regular, - // Floating-output derivations (and their dependencies). + // Floating-output derivations (and their reverse dependencies). Deferred, }; diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 75726a1ab..46a547db1 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -718,6 +718,7 @@ void LocalStore::checkDerivationOutputs(const StorePath & drvPath, const Derivat /* Nothing to check */ }, [&](const DerivationOutput::Deferred &) { + /* Nothing to check */ }, }, i.second.raw()); } diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 8c65053e4..f2288a04e 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -93,7 +93,7 @@ StringSet ParsedDerivation::getRequiredSystemFeatures() const StringSet res; for (auto & i : getStringsAttr("requiredSystemFeatures").value_or(Strings())) res.insert(i); - if (!derivationHasKnownOutputPaths(drv.type())) + if (!drv.type().hasKnownOutputPaths()) res.insert("ca-derivations"); return res; } From d60f3cf6e9c904912199ea64156fea295494430a Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Thu, 17 Mar 2022 22:59:43 +0100 Subject: [PATCH 46/83] nix-daemon.conf.in: add tmpfiles file to create nix/daemon-socket directory nix-daemon.socket is used to socket-activate nix-daemon.service when /nix/var/nix/daemon-socket/socket is accessed. In container usecases, sometimes /nix/var/nix/daemon-socket is bind-mounted read-only into the container. In these cases, we want to skip starting nix-daemon.socket. However, since systemd 250, `ConditionPathIsReadWrite` is also not met if /nix/var/nix/daemon-socket doesn't exist at all. This means, a regular NixOS system will skip starting nix-daemon.socket: > [ 237.187747] systemd[1]: Nix Daemon Socket was skipped because of a failed condition check (ConditionPathIsReadWrite=/nix/var/nix/daemon-socket). To prevent this from happening, ship a tmpfiles file that'll cause the directory to be created if it doesn't exist already. In the case of NixOS, we can just add Nix to `systemd.tmpfiles.packages` and have these files picked up automatically. --- misc/systemd/local.mk | 3 ++- misc/systemd/nix-daemon.conf.in | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 misc/systemd/nix-daemon.conf.in diff --git a/misc/systemd/local.mk b/misc/systemd/local.mk index 1fa037485..76121a0f9 100644 --- a/misc/systemd/local.mk +++ b/misc/systemd/local.mk @@ -1,7 +1,8 @@ ifdef HOST_LINUX $(foreach n, nix-daemon.socket nix-daemon.service, $(eval $(call install-file-in, $(d)/$(n), $(prefix)/lib/systemd/system, 0644))) + $(foreach n, nix-daemon.conf, $(eval $(call install-file-in, $(d)/$(n), $(prefix)/lib/tmpfiles.d, 0644))) - clean-files += $(d)/nix-daemon.socket $(d)/nix-daemon.service + clean-files += $(d)/nix-daemon.socket $(d)/nix-daemon.service $(d)/nix-daemon.conf endif diff --git a/misc/systemd/nix-daemon.conf.in b/misc/systemd/nix-daemon.conf.in new file mode 100644 index 000000000..e7b264234 --- /dev/null +++ b/misc/systemd/nix-daemon.conf.in @@ -0,0 +1 @@ +d @localstatedir@/nix/daemon-socket 0755 root root - - From 4d6a3806d24b54f06ddc0cf234ac993db028cf29 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sat, 12 Mar 2022 00:28:00 +0000 Subject: [PATCH 47/83] Decode string context straight to using `StorePath`s I gather decoding happens on demand, so I hope don't think this should have any perf implications one way or the other. --- src/libexpr/eval-cache.cc | 19 +++++++++++-------- src/libexpr/eval.cc | 19 ++++++++++++++----- src/libexpr/eval.hh | 4 ++-- src/libexpr/primops.cc | 8 ++++---- src/libexpr/primops/context.cc | 4 ++-- src/libexpr/value.hh | 6 ++++-- src/nix/app.cc | 2 +- 7 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/libexpr/eval-cache.cc b/src/libexpr/eval-cache.cc index 7e5b5c9c4..54fa9b741 100644 --- a/src/libexpr/eval-cache.cc +++ b/src/libexpr/eval-cache.cc @@ -21,6 +21,8 @@ struct AttrDb { std::atomic_bool failed{false}; + const Store & cfg; + struct State { SQLite db; @@ -33,8 +35,9 @@ struct AttrDb std::unique_ptr> _state; - AttrDb(const Hash & fingerprint) - : _state(std::make_unique>()) + AttrDb(const Store & cfg, const Hash & fingerprint) + : cfg(cfg) + , _state(std::make_unique>()) { auto state(_state->lock()); @@ -257,7 +260,7 @@ struct AttrDb NixStringContext context; if (!queryAttribute.isNull(3)) for (auto & s : tokenizeString>(queryAttribute.getStr(3), ";")) - context.push_back(decodeContext(s)); + context.push_back(decodeContext(cfg, s)); return {{rowId, string_t{queryAttribute.getStr(2), context}}}; } case AttrType::Bool: @@ -274,10 +277,10 @@ struct AttrDb } }; -static std::shared_ptr makeAttrDb(const Hash & fingerprint) +static std::shared_ptr makeAttrDb(const Store & cfg, const Hash & fingerprint) { try { - return std::make_shared(fingerprint); + return std::make_shared(cfg, fingerprint); } catch (SQLiteError &) { ignoreException(); return nullptr; @@ -288,7 +291,7 @@ EvalCache::EvalCache( std::optional> useCache, EvalState & state, RootLoader rootLoader) - : db(useCache ? makeAttrDb(*useCache) : nullptr) + : db(useCache ? makeAttrDb(*state.store, *useCache) : nullptr) , state(state) , rootLoader(rootLoader) { @@ -546,7 +549,7 @@ string_t AttrCursor::getStringWithContext() if (auto s = std::get_if(&cachedValue->second)) { bool valid = true; for (auto & c : s->second) { - if (!root->state.store->isValidPath(root->state.store->parseStorePath(c.first))) { + if (!root->state.store->isValidPath(c.first)) { valid = false; break; } @@ -563,7 +566,7 @@ string_t AttrCursor::getStringWithContext() auto & v = forceValue(); if (v.type() == nString) - return {v.string.s, v.getContext()}; + return {v.string.s, v.getContext(*root->state.store)}; else if (v.type() == nPath) return {v.path, {}}; else diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 126e0af8c..f7911e32b 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1903,13 +1903,22 @@ std::string_view EvalState::forceString(Value & v, const Pos & pos) /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s) +NixStringContextElem decodeContext(const Store & store, std::string_view s) { if (s.at(0) == '!') { size_t index = s.find("!", 1); - return {std::string(s.substr(index + 1)), std::string(s.substr(1, index - 1))}; + return { + store.parseStorePath(s.substr(index + 1)), + std::string(s.substr(1, index - 1)), + }; } else - return {s.at(0) == '/' ? std::string(s) : std::string(s.substr(1)), ""}; + return { + store.parseStorePath( + s.at(0) == '/' + ? s + : s.substr(1)), + "", + }; } @@ -1921,13 +1930,13 @@ void copyContext(const Value & v, PathSet & context) } -NixStringContext Value::getContext() +NixStringContext Value::getContext(const Store & store) { NixStringContext res; assert(internalType == tString); if (string.context) for (const char * * p = string.context; *p; ++p) - res.push_back(decodeContext(*p)); + res.push_back(decodeContext(store, *p)); return res; } diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 18480f290..198a62ad2 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -17,7 +17,7 @@ namespace nix { -class Store; +struct Store; class EvalState; class StorePath; enum RepairFlag : bool; @@ -430,7 +430,7 @@ std::string showType(const Value & v); /* Decode a context string ‘!!’ into a pair . */ -NixStringContextElem decodeContext(std::string_view s); +NixStringContextElem decodeContext(const Store & store, std::string_view s); /* If `path' refers to a directory, then append "/default.nix". */ Path resolveExprPath(Path path); diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 3124025aa..566cbdfdb 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -43,8 +43,8 @@ StringMap EvalState::realiseContext(const PathSet & context) StringMap res; for (auto & i : context) { - auto [ctxS, outputName] = decodeContext(i); - auto ctx = store->parseStorePath(ctxS); + auto [ctx, outputName] = decodeContext(*store, i); + auto ctxS = store->printStorePath(ctx); if (!store->isValidPath(ctx)) throw InvalidPathError(store->printStorePath(ctx)); if (!outputName.empty() && ctx.isDerivation()) { @@ -1118,8 +1118,8 @@ static void prim_derivationStrict(EvalState & state, const Pos & pos, Value * * /* Handle derivation outputs of the form ‘!!’. */ else if (path.at(0) == '!') { - auto ctx = decodeContext(path); - drv.inputDrvs[state.store->parseStorePath(ctx.first)].insert(ctx.second); + auto ctx = decodeContext(*state.store, path); + drv.inputDrvs[ctx.first].insert(ctx.second); } /* Otherwise it's a source file. */ diff --git a/src/libexpr/primops/context.cc b/src/libexpr/primops/context.cc index ad4de3840..6ec5e4a74 100644 --- a/src/libexpr/primops/context.cc +++ b/src/libexpr/primops/context.cc @@ -82,8 +82,8 @@ static void prim_getContext(EvalState & state, const Pos & pos, Value * * args, drv = std::string(p, 1); path = &drv; } else if (p.at(0) == '!') { - NixStringContextElem ctx = decodeContext(p); - drv = ctx.first; + NixStringContextElem ctx = decodeContext(*state.store, p); + drv = state.store->printStorePath(ctx.first); output = ctx.second; path = &drv; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index ee9cb832a..d413060f9 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -57,6 +57,8 @@ struct ExprLambda; struct PrimOp; class Symbol; struct Pos; +class StorePath; +class Store; class EvalState; class XMLWriter; class JSONPlaceholder; @@ -64,7 +66,7 @@ class JSONPlaceholder; typedef int64_t NixInt; typedef double NixFloat; -typedef std::pair NixStringContextElem; +typedef std::pair NixStringContextElem; typedef std::vector NixStringContext; /* External values must descend from ExternalValueBase, so that @@ -370,7 +372,7 @@ public: non-trivial. */ bool isTrivial() const; - NixStringContext getContext(); + NixStringContext getContext(const Store &); auto listItems() { diff --git a/src/nix/app.cc b/src/nix/app.cc index 2563180fb..a4e0df06a 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -70,7 +70,7 @@ UnresolvedApp Installable::toApp(EvalState & state) std::vector context2; for (auto & [path, name] : context) - context2.push_back({state.store->parseStorePath(path), {name}}); + context2.push_back({path, {name}}); return UnresolvedApp{App { .context = std::move(context2), From 345a8ee0cb56775c57c3b7da99b0726290241e18 Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 10:56:13 -0300 Subject: [PATCH 48/83] Fix sourcehut tag ref resolving --- src/libfetchers/github.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index a1430f087..d52ea649d 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -390,7 +390,7 @@ struct SourceHutInputScheme : GitArchiveInputScheme ref_uri = line.substr(ref_index+5, line.length()-1); } else - ref_uri = fmt("refs/heads/%s", ref); + ref_uri = fmt("refs/(heads|tags)/%s", ref); auto file = store->toRealPath( downloadFile(store, fmt("%s/info/refs", base_url), "source", false, headers).storePath); @@ -399,9 +399,9 @@ struct SourceHutInputScheme : GitArchiveInputScheme std::string line; std::string id; while(getline(is, line)) { - auto index = line.find(ref_uri); - if (index != std::string::npos) { - id = line.substr(0, index-1); + std::regex pattern(ref_uri); + if (std::regex_search(line, pattern)) { + id = line.substr(0, line.find('\t')); break; } } From 9720797f695f7646744e9cd020fa79679441814f Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 11:04:04 -0300 Subject: [PATCH 49/83] Don't partial match sourcehut refs --- src/libfetchers/github.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index d52ea649d..58b6e7c04 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -399,7 +399,9 @@ struct SourceHutInputScheme : GitArchiveInputScheme std::string line; std::string id; while(getline(is, line)) { - std::regex pattern(ref_uri); + // Append $ to avoid partial name matches + std::regex pattern(fmt("%s$", ref_uri)); + if (std::regex_search(line, pattern)) { id = line.substr(0, line.find('\t')); break; From 31544b93ffa985661541309ec2360dfb7b527a80 Mon Sep 17 00:00:00 2001 From: Gabriel Fontes Date: Sat, 19 Mar 2022 11:38:45 -0300 Subject: [PATCH 50/83] Fix sourcehut integration test The new implementation relies on tab separting the hash and ref (this is how sourcehut does it). This fixes the integration test to use a tab instead of a space. --- tests/sourcehut-flakes.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/sourcehut-flakes.nix b/tests/sourcehut-flakes.nix index d1d89d149..6a1930904 100644 --- a/tests/sourcehut-flakes.nix +++ b/tests/sourcehut-flakes.nix @@ -59,7 +59,7 @@ let echo 'ref: refs/heads/master' > $out/HEAD mkdir -p $out/info - echo '${nixpkgs.rev} refs/heads/master' > $out/info/refs + echo -e '${nixpkgs.rev}\trefs/heads/master' > $out/info/refs ''; in From 0b42afe02794a446d20066c0e9931b9c759d36e2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 10:48:02 +0100 Subject: [PATCH 51/83] buildProfile(): Ignore manifest.{nix,json} If a package installs a file named manifest.json, it caused nix-env to consider the profile a new-style profile created by 'nix profile'. Fixes #6032. --- src/libstore/builtins/buildenv.cc | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 25d015cb9..6f6ad57cb 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -47,9 +47,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, throw; } - /* The files below are special-cased to that they don't show up - * in user profiles, either because they are useless, or - * because they would cauase pointless collisions (e.g., each + /* The files below are special-cased to that they don't show + * up in user profiles, either because they are useless, or + * because they would cause pointless collisions (e.g., each * Python package brings its own * `$out/lib/pythonX.Y/site-packages/easy-install.pth'.) */ @@ -57,7 +57,9 @@ static void createLinks(State & state, const Path & srcDir, const Path & dstDir, hasSuffix(srcFile, "/nix-support") || hasSuffix(srcFile, "/perllocal.pod") || hasSuffix(srcFile, "/info/dir") || - hasSuffix(srcFile, "/log")) + hasSuffix(srcFile, "/log") || + hasSuffix(srcFile, "/manifest.nix") || + hasSuffix(srcFile, "/manifest.json")) continue; else if (S_ISDIR(srcSt.st_mode)) { From 732296ddc078f9cce8ffeb3131fef8898330b6ae Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 13:00:27 +0100 Subject: [PATCH 52/83] =?UTF-8?q?printValue():=20=20->=20=C2=ABrep?= =?UTF-8?q?eated=C2=BB?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures that it doesn't get parsed as a valid Nix expression. --- src/libexpr/eval.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index f7911e32b..333ad68eb 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -126,7 +126,7 @@ void printValue(std::ostream & str, std::set & seen, const Value & break; case tAttrs: { if (!v.attrs->empty() && !seen.insert(v.attrs).second) - str << ""; + str << "«repeated»"; else { str << "{ "; for (auto & i : v.attrs->lexicographicOrder()) { @@ -142,7 +142,7 @@ void printValue(std::ostream & str, std::set & seen, const Value & case tList2: case tListN: if (v.listSize() && !seen.insert(v.listElems()).second) - str << ""; + str << "«repeated»"; else { str << "[ "; for (auto v2 : v.listItems()) { From a0259a21a416301573d9d6b994d6b5fb62a1bcf3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 13:18:11 +0100 Subject: [PATCH 53/83] Don't hide repeated values while generating manifest.nix Fixes #6243. --- src/libexpr/eval.cc | 38 ++++++++++++++++++++++---------------- src/libexpr/value.hh | 5 ++++- src/nix-env/user-env.cc | 4 +++- tests/user-envs.nix | 3 +++ 4 files changed, 32 insertions(+), 18 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 333ad68eb..3ec5ed202 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -96,20 +96,20 @@ RootValue allocRootValue(Value * v) } -void printValue(std::ostream & str, std::set & seen, const Value & v) +void Value::print(std::ostream & str, std::set * seen) const { checkInterrupt(); - switch (v.internalType) { + switch (internalType) { case tInt: - str << v.integer; + str << integer; break; case tBool: - str << (v.boolean ? "true" : "false"); + str << (boolean ? "true" : "false"); break; case tString: str << "\""; - for (const char * i = v.string.s; *i; i++) + for (const char * i = string.s; *i; i++) if (*i == '\"' || *i == '\\') str << "\\" << *i; else if (*i == '\n') str << "\\n"; else if (*i == '\r') str << "\\r"; @@ -119,19 +119,19 @@ void printValue(std::ostream & str, std::set & seen, const Value & str << "\""; break; case tPath: - str << v.path; // !!! escaping? + str << path; // !!! escaping? break; case tNull: str << "null"; break; case tAttrs: { - if (!v.attrs->empty() && !seen.insert(v.attrs).second) + if (seen && !attrs->empty() && !seen->insert(attrs).second) str << "«repeated»"; else { str << "{ "; - for (auto & i : v.attrs->lexicographicOrder()) { + for (auto & i : attrs->lexicographicOrder()) { str << i->name << " = "; - printValue(str, seen, *i->value); + i->value->print(str, seen); str << "; "; } str << "}"; @@ -141,12 +141,12 @@ void printValue(std::ostream & str, std::set & seen, const Value & case tList1: case tList2: case tListN: - if (v.listSize() && !seen.insert(v.listElems()).second) + if (seen && listSize() && !seen->insert(listElems()).second) str << "«repeated»"; else { str << "[ "; - for (auto v2 : v.listItems()) { - printValue(str, seen, *v2); + for (auto v2 : listItems()) { + v2->print(str, seen); str << " "; } str << "]"; @@ -166,10 +166,10 @@ void printValue(std::ostream & str, std::set & seen, const Value & str << ""; break; case tExternal: - str << *v.external; + str << *external; break; case tFloat: - str << v.fpoint; + str << fpoint; break; default: abort(); @@ -177,10 +177,16 @@ void printValue(std::ostream & str, std::set & seen, const Value & } -std::ostream & operator << (std::ostream & str, const Value & v) +void Value::print(std::ostream & str, bool showRepeated) const { std::set seen; - printValue(str, seen, v); + print(str, showRepeated ? nullptr : &seen); +} + + +std::ostream & operator << (std::ostream & str, const Value & v) +{ + v.print(str, false); return str; } diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index d413060f9..3d07c3198 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -119,10 +119,13 @@ private: InternalType internalType; friend std::string showType(const Value & v); - friend void printValue(std::ostream & str, std::set & seen, const Value & v); + + void print(std::ostream & str, std::set * seen) const; public: + void print(std::ostream & str, bool showRepeated = false) const; + // Functions needed to distinguish the type // These should be removed eventually, by putting the functionality that's // needed by callers into methods of this type diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index bcc7736ac..78692b9c6 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -105,8 +105,10 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, /* Also write a copy of the list of user environment elements to the store; we need it for future modifications of the environment. */ + std::ostringstream str; + manifest.print(str, true); auto manifestFile = state.store->addTextToStore("env-manifest.nix", - fmt("%s", manifest), references); + str.str(), references); /* Get the environment builder expression. */ Value envBuilder; diff --git a/tests/user-envs.nix b/tests/user-envs.nix index 6ac896ed8..46f8b51dd 100644 --- a/tests/user-envs.nix +++ b/tests/user-envs.nix @@ -8,6 +8,8 @@ assert foo == "foo"; let + platforms = let x = "foobar"; in [ x x ]; + makeDrv = name: progName: (mkDerivation { name = assert progName != "fail"; name; inherit progName system; @@ -15,6 +17,7 @@ let } // { meta = { description = "A silly test package with some \${escaped anti-quotation} in it"; + inherit platforms; }; }); From 3b776cb0a7574bf11c20fe0a1d91ec1d21d8ebeb Mon Sep 17 00:00:00 2001 From: Hideaki Kawai Date: Tue, 22 Mar 2022 23:18:02 +0900 Subject: [PATCH 54/83] nix edit: support kakoune --- src/libcmd/command.cc | 3 ++- src/nix/edit.md | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index dc8fa9e5a..a53b029b7 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -204,7 +204,8 @@ Strings editorFor(const Pos & pos) if (pos.line > 0 && ( editor.find("emacs") != std::string::npos || editor.find("nano") != std::string::npos || - editor.find("vim") != std::string::npos)) + editor.find("vim") != std::string::npos || + editor.find("kak") != std::string::npos)) args.push_back(fmt("+%d", pos.line)); args.push_back(pos.file); return args; diff --git a/src/nix/edit.md b/src/nix/edit.md index 80563d06b..89bd09abf 100644 --- a/src/nix/edit.md +++ b/src/nix/edit.md @@ -24,8 +24,8 @@ this attribute to the location of the definition of the `meta.description`, `version` or `name` derivation attributes. The editor to invoke is specified by the `EDITOR` environment -variable. It defaults to `cat`. If the editor is `emacs`, `nano` or -`vim`, it is passed the line number of the derivation using the -argument `+`. +variable. It defaults to `cat`. If the editor is `emacs`, `nano`, +`vim` or `kak`, it is passed the line number of the derivation using +the argument `+`. )"" From 67af5f7eda1dade544cc15397a7d0d35d0913cb2 Mon Sep 17 00:00:00 2001 From: Florian Klink Date: Tue, 22 Mar 2022 17:06:09 +0100 Subject: [PATCH 55/83] scripts/install-systemd-multi-user.sh: install /etc/tmpfiles.d/nix-daemon.conf, too While `create_directories()` from install-multi-user.sh seems to already create parts of the directory structure, it's marked as deprecated, and it won't hurt also copying over the tmpfiles config and have it execute once. --- scripts/install-systemd-multi-user.sh | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scripts/install-systemd-multi-user.sh b/scripts/install-systemd-multi-user.sh index f4a2dfc5d..24884a023 100755 --- a/scripts/install-systemd-multi-user.sh +++ b/scripts/install-systemd-multi-user.sh @@ -9,6 +9,8 @@ readonly SERVICE_DEST=/etc/systemd/system/nix-daemon.service readonly SOCKET_SRC=/lib/systemd/system/nix-daemon.socket readonly SOCKET_DEST=/etc/systemd/system/nix-daemon.socket +readonly TMPFILES_SRC=/lib/tmpfiles.d/nix-daemon.conf +readonly TMPFILES_DEST=/etc/tmpfiles.d/nix-daemon.conf # Path for the systemd override unit file to contain the proxy settings readonly SERVICE_OVERRIDE=${SERVICE_DEST}.d/override.conf @@ -83,6 +85,13 @@ EOF poly_configure_nix_daemon_service() { if [ -e /run/systemd/system ]; then task "Setting up the nix-daemon systemd service" + + _sudo "to create the nix-daemon tmpfiles config" \ + ln -sfn /nix/var/nix/profiles/default/$TMPFILES_SRC $TMPFILES_DEST + + _sudo "to run systemd-tmpfiles once to pick that path up" \ + sytemd-tmpfiles create --prefix=/nix/var/nix + _sudo "to set up the nix-daemon service" \ systemctl link "/nix/var/nix/profiles/default$SERVICE_SRC" From 9174d884d750b7b49a571bd55275f0883c2dabda Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Thu, 24 Mar 2022 08:10:33 +0000 Subject: [PATCH 56/83] lexer: add error location to lexer errors Before the change lexter errors did not report the location: $ nix build -f. mc error: path has a trailing slash (use '--show-trace' to show detailed location information) Note that it's not clear what file generates the error. After the change location is reported: $ src/nix/nix --extra-experimental-features nix-command build -f ~/nm mc error: path has a trailing slash at .../pkgs/development/libraries/glib/default.nix:54:18: 53| }; 54| src = /tmp/foo/; | ^ 55| (use '--show-trace' to show detailed location information) Here we see both problematic file and the string itself. --- src/libexpr/lexer.l | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/libexpr/lexer.l b/src/libexpr/lexer.l index e276b0467..d574121b0 100644 --- a/src/libexpr/lexer.l +++ b/src/libexpr/lexer.l @@ -28,6 +28,13 @@ using namespace nix; namespace nix { +static inline Pos makeCurPos(const YYLTYPE & loc, ParseData * data) +{ + return Pos(data->origin, data->file, loc.first_line, loc.first_column); +} + +#define CUR_POS makeCurPos(*yylloc, data) + // backup to recover from yyless(0) YYLTYPE prev_yylloc; @@ -37,7 +44,6 @@ static void initLoc(YYLTYPE * loc) loc->first_column = loc->last_column = 1; } - static void adjustLoc(YYLTYPE * loc, const char * s, size_t len) { prev_yylloc = *loc; @@ -147,14 +153,20 @@ or { return OR_KW; } try { yylval->n = boost::lexical_cast(yytext); } catch (const boost::bad_lexical_cast &) { - throw ParseError("invalid integer '%1%'", yytext); + throw ParseError({ + .msg = hintfmt("invalid integer '%1%'", yytext), + .errPos = CUR_POS, + }); } return INT; } {FLOAT} { errno = 0; yylval->nf = strtod(yytext, 0); if (errno != 0) - throw ParseError("invalid float '%1%'", yytext); + throw ParseError({ + .msg = hintfmt("invalid float '%1%'", yytext), + .errPos = CUR_POS, + }); return FLOAT; } @@ -280,7 +292,10 @@ or { return OR_KW; } {ANY} | <> { - throw ParseError("path has a trailing slash"); + throw ParseError({ + .msg = hintfmt("path has a trailing slash"), + .errPos = CUR_POS, + }); } {SPATH} { yylval->path = {yytext, (size_t) yyleng}; return SPATH; } From 4546a007a46fe12b77a49ae8753ab186d0b55fdd Mon Sep 17 00:00:00 2001 From: Rok Garbas Date: Thu, 24 Mar 2022 12:28:38 +0100 Subject: [PATCH 57/83] Fix flake profile use of originalUrl vs. originalUri Fixes #5872 --- src/nix/profile.cc | 5 +++-- src/nix/profile.md | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/nix/profile.cc b/src/nix/profile.cc index a8ff9c78a..da990ddc8 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -107,8 +107,9 @@ struct ProfileManifest element.storePaths.insert(state.store->parseStorePath((std::string) p)); element.active = e["active"]; if (e.value("uri", "") != "") { + auto originalUrl = e.value("originalUrl", e["originalUri"]); element.source = ProfileElementSource{ - parseFlakeRef(e["originalUri"]), + parseFlakeRef(originalUrl), parseFlakeRef(e["uri"]), e["attrPath"] }; @@ -143,7 +144,7 @@ struct ProfileManifest obj["storePaths"] = paths; obj["active"] = element.active; if (element.source) { - obj["originalUri"] = element.source->originalRef.to_string(); + obj["originalUrl"] = element.source->originalRef.to_string(); obj["uri"] = element.source->resolvedRef.to_string(); obj["attrPath"] = element.source->attrPath; } diff --git a/src/nix/profile.md b/src/nix/profile.md index 0a4ff2fa9..8dade051d 100644 --- a/src/nix/profile.md +++ b/src/nix/profile.md @@ -70,7 +70,7 @@ are installed in this version of the profile. It looks like this: { "active": true, "attrPath": "legacyPackages.x86_64-linux.zoom-us", - "originalUri": "flake:nixpkgs", + "originalUrl": "flake:nixpkgs", "storePaths": [ "/nix/store/wbhg2ga8f3h87s9h5k0slxk0m81m4cxl-zoom-us-5.3.469451.0927" ], @@ -84,11 +84,11 @@ are installed in this version of the profile. It looks like this: Each object in the array `elements` denotes an installed package and has the following fields: -* `originalUri`: The [flake reference](./nix3-flake.md) specified by +* `originalUrl`: The [flake reference](./nix3-flake.md) specified by the user at the time of installation (e.g. `nixpkgs`). This is also the flake reference that will be used by `nix profile upgrade`. -* `uri`: The immutable flake reference to which `originalUri` +* `uri`: The immutable flake reference to which `originalUrl` resolved. * `attrPath`: The flake output attribute that provided this From bb0c4b9f25b6d064d91aedd71940f14b1b624291 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 12:46:46 +0100 Subject: [PATCH 58/83] install-multi-user.sh: Preserve symlinks We need to pass -P to ensure that symlinks are copied correctly. Fixes #6303. --- scripts/install-multi-user.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index 5f2c93b7f..69b6676ea 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -739,7 +739,7 @@ install_from_extracted_nix() { cd "$EXTRACTED_NIX_PATH" _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ - cp -RLp ./store/* "$NIX_ROOT/store/" + cp -RPp ./store/* "$NIX_ROOT/store/" _sudo "to make the new store non-writable at $NIX_ROOT/store" \ chmod -R ugo-w "$NIX_ROOT/store/" From 0736f3651d56d3e0c3b742fe0d90958c90e9c968 Mon Sep 17 00:00:00 2001 From: Tom Bereknyei Date: Wed, 23 Mar 2022 22:54:43 -0400 Subject: [PATCH 59/83] docs: genericClosure --- src/libexpr/primops.cc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index bfa18f898..e7e224b3a 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -694,7 +694,32 @@ static void prim_genericClosure(EvalState & state, const Pos & pos, Value * * ar static RegisterPrimOp primop_genericClosure(RegisterPrimOp::Info { .name = "__genericClosure", + .args = {"attrset"}, .arity = 1, + .doc = R"( + Take an *attrset* with values named `startSet` and `operator` in order to + return a *list of attrsets* by starting with the `startSet`, recursively + applying the `operator` function to each element. The *attrsets* in the + `startSet` and produced by the `operator` must each contain value named + `key` which are comparable to each other. The result is produced by + repeatedly calling the operator for each element encountered with a + unique key, terminating when no new elements are produced. For example, + + ``` + builtins.genericClosure { + startSet = [ {key = 5;} ]; + operator = item: [{ + key = if (item.key / 2 ) * 2 == item.key + then item.key / 2 + else 3 * item.key + 1; + }]; + } + ``` + evaluates to + ``` + [ { key = 5; } { key = 16; } { key = 8; } { key = 4; } { key = 2; } { key = 1; } ] + ``` + )", .fun = prim_genericClosure, }); From f4bafc412fac79ce07c89f8d3ab9bd1c32f7b9cd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 11:31:01 +0100 Subject: [PATCH 60/83] Add builtins.fetchClosure This allows closures to be imported at evaluation time, without requiring the user to configure substituters. E.g. builtins.fetchClosure { storePath = /nix/store/f89g6yi63m1ywfxj96whv5sxsm74w5ka-python3.9-sqlparse-0.4.2; from = "https://cache.ngi0.nixos.org"; } --- src/libexpr/primops/fetchClosure.cc | 62 +++++++++++++++++++++++++++++ src/libexpr/primops/fetchTree.cc | 4 +- 2 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 src/libexpr/primops/fetchClosure.cc diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc new file mode 100644 index 000000000..4bcc716db --- /dev/null +++ b/src/libexpr/primops/fetchClosure.cc @@ -0,0 +1,62 @@ +#include "primops.hh" +#include "store-api.hh" + +namespace nix { + +static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) +{ + state.forceAttrs(*args[0], pos); + + std::optional storePath; + std::optional from; + + for (auto & attr : *args[0]->attrs) { + if (attr.name == "storePath") { + PathSet context; + storePath = state.coerceToStorePath(*attr.pos, *attr.value, context); + } + + else if (attr.name == "from") + from = state.forceStringNoCtx(*attr.value, *attr.pos); + + else + throw Error({ + .msg = hintfmt("attribute '%s' isn't supported in call to 'fetchClosure'", attr.name), + .errPos = pos + }); + } + + if (!storePath) + throw Error({ + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "storePath"), + .errPos = pos + }); + + if (!from) + throw Error({ + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "from"), + .errPos = pos + }); + + // FIXME: only allow some "trusted" store types (like BinaryCacheStore). + auto srcStore = openStore(*from); + + copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + + auto storePathS = state.store->printStorePath(*storePath); + + v.mkString(storePathS, {storePathS}); + + // FIXME: in pure mode, require a CA path or a NAR hash of the + // top-level path. +} + +static RegisterPrimOp primop_fetchClosure({ + .name = "__fetchClosure", + .args = {"args"}, + .doc = R"( + )", + .fun = prim_fetchClosure, +}); + +} diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index 9c2da2178..42c98e312 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -145,7 +145,7 @@ static void fetchTree( if (!params.allowNameArgument) if (auto nameIter = attrs.find("name"); nameIter != attrs.end()) throw Error({ - .msg = hintfmt("attribute 'name' isn’t supported in call to 'fetchTree'"), + .msg = hintfmt("attribute 'name' isn't supported in call to 'fetchTree'"), .errPos = pos }); @@ -329,7 +329,7 @@ static RegisterPrimOp primop_fetchTarball({ .fun = prim_fetchTarball, }); -static void prim_fetchGit(EvalState &state, const Pos &pos, Value **args, Value &v) +static void prim_fetchGit(EvalState & state, const Pos & pos, Value * * args, Value & v) { fetchTree(state, pos, args, v, "git", FetchTreeParams { .emptyRevFallback = true, .allowNameArgument = true }); } From 41659418cfeb7a33fc76716a1847f79ae13d9b0c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 14:00:54 +0100 Subject: [PATCH 61/83] fetchClosure: Require a CA path in pure mode --- src/libexpr/primops/fetchClosure.cc | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 4bcc716db..22a4649a4 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -43,12 +43,19 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + /* In pure mode, require a CA path. */ + if (evalSettings.pureEval) { + auto info = state.store->queryPathInfo(*storePath); + if (!info->isContentAddressed(*state.store)) + throw Error({ + .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", + state.store->printStorePath(*storePath)), + .errPos = pos + }); + } + auto storePathS = state.store->printStorePath(*storePath); - v.mkString(storePathS, {storePathS}); - - // FIXME: in pure mode, require a CA path or a NAR hash of the - // top-level path. } static RegisterPrimOp primop_fetchClosure({ From 7f6fe8ca1d41bceef32790aa0313aa62ae2b65fb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Mar 2022 14:34:45 +0100 Subject: [PATCH 62/83] Rename --- src/libexpr/primops/fetchClosure.cc | 32 ++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 22a4649a4..8e60b2ccc 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,17 +7,17 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args { state.forceAttrs(*args[0], pos); - std::optional storePath; - std::optional from; + std::optional fromStoreUrl; + std::optional fromPath; for (auto & attr : *args[0]->attrs) { - if (attr.name == "storePath") { + if (attr.name == "fromPath") { PathSet context; - storePath = state.coerceToStorePath(*attr.pos, *attr.value, context); + fromPath = state.coerceToStorePath(*attr.pos, *attr.value, context); } - else if (attr.name == "from") - from = state.forceStringNoCtx(*attr.value, *attr.pos); + else if (attr.name == "fromStore") + fromStoreUrl = state.forceStringNoCtx(*attr.value, *attr.pos); else throw Error({ @@ -26,36 +26,36 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args }); } - if (!storePath) + if (!fromPath) throw Error({ - .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "storePath"), + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromPath"), .errPos = pos }); - if (!from) + if (!fromStoreUrl) throw Error({ - .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "from"), + .msg = hintfmt("attribute '%s' is missing in call to 'fetchClosure'", "fromStore"), .errPos = pos }); // FIXME: only allow some "trusted" store types (like BinaryCacheStore). - auto srcStore = openStore(*from); + auto fromStore = openStore(*fromStoreUrl); - copyClosure(*srcStore, *state.store, RealisedPath::Set { *storePath }); + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); /* In pure mode, require a CA path. */ if (evalSettings.pureEval) { - auto info = state.store->queryPathInfo(*storePath); + auto info = state.store->queryPathInfo(*fromPath); if (!info->isContentAddressed(*state.store)) throw Error({ .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", - state.store->printStorePath(*storePath)), + state.store->printStorePath(*fromPath)), .errPos = pos }); } - auto storePathS = state.store->printStorePath(*storePath); - v.mkString(storePathS, {storePathS}); + auto fromPathS = state.store->printStorePath(*fromPath); + v.mkString(fromPathS, {fromPathS}); } static RegisterPrimOp primop_fetchClosure({ From 545c2d0d8cbac86c169a6dd049c1ed9c3913774d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:14:58 +0100 Subject: [PATCH 63/83] fetchClosure: Allow a path to be rewritten to CA on the fly The advantage is that the resulting closure doesn't need to be signed, so you don't need to configure any binary cache keys on the client. --- src/libexpr/primops/fetchClosure.cc | 46 ++++++++++++-- src/libstore/make-content-addressed.cc | 79 ++++++++++++++++++++++++ src/libstore/make-content-addressed.hh | 12 ++++ src/nix/make-content-addressable.cc | 85 ++++++-------------------- 4 files changed, 150 insertions(+), 72 deletions(-) create mode 100644 src/libstore/make-content-addressed.cc create mode 100644 src/libstore/make-content-addressed.hh diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 8e60b2ccc..56fd53ed5 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -1,5 +1,6 @@ #include "primops.hh" #include "store-api.hh" +#include "make-content-addressed.hh" namespace nix { @@ -9,6 +10,8 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args std::optional fromStoreUrl; std::optional fromPath; + bool toCA = false; + std::optional toPath; for (auto & attr : *args[0]->attrs) { if (attr.name == "fromPath") { @@ -16,6 +19,15 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args fromPath = state.coerceToStorePath(*attr.pos, *attr.value, context); } + else if (attr.name == "toPath") { + state.forceValue(*attr.value, *attr.pos); + toCA = true; + if (attr.value->type() != nString || attr.value->string.s != std::string("")) { + PathSet context; + toPath = state.coerceToStorePath(*attr.pos, *attr.value, context); + } + } + else if (attr.name == "fromStore") fromStoreUrl = state.forceStringNoCtx(*attr.value, *attr.pos); @@ -41,21 +53,45 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args // FIXME: only allow some "trusted" store types (like BinaryCacheStore). auto fromStore = openStore(*fromStoreUrl); - copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + if (toCA) { + auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); + auto i = remappings.find(*fromPath); + assert(i != remappings.end()); + if (toPath && *toPath != i->second) + throw Error({ + .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second), + state.store->printStorePath(*toPath)), + .errPos = pos + }); + if (!toPath) + throw Error({ + .msg = hintfmt( + "rewriting '%s' to content-addressed form yielded '%s'; " + "please set this in the 'toPath' attribute passed to 'fetchClosure'", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second)), + .errPos = pos + }); + } else { + copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); + toPath = fromPath; + } /* In pure mode, require a CA path. */ if (evalSettings.pureEval) { - auto info = state.store->queryPathInfo(*fromPath); + auto info = state.store->queryPathInfo(*toPath); if (!info->isContentAddressed(*state.store)) throw Error({ .msg = hintfmt("in pure mode, 'fetchClosure' requires a content-addressed path, which '%s' isn't", - state.store->printStorePath(*fromPath)), + state.store->printStorePath(*toPath)), .errPos = pos }); } - auto fromPathS = state.store->printStorePath(*fromPath); - v.mkString(fromPathS, {fromPathS}); + auto toPathS = state.store->printStorePath(*toPath); + v.mkString(toPathS, {toPathS}); } static RegisterPrimOp primop_fetchClosure({ diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc new file mode 100644 index 000000000..0b95ff37c --- /dev/null +++ b/src/libstore/make-content-addressed.cc @@ -0,0 +1,79 @@ +#include "make-content-addressed.hh" +#include "references.hh" + +namespace nix { + +std::map makeContentAddressed( + Store & srcStore, + Store & dstStore, + const StorePathSet & storePaths) +{ + // FIXME: use closure of storePaths. + + auto paths = srcStore.topoSortPaths(storePaths); + + std::reverse(paths.begin(), paths.end()); + + std::map remappings; + + for (auto & path : paths) { + auto pathS = srcStore.printStorePath(path); + auto oldInfo = srcStore.queryPathInfo(path); + std::string oldHashPart(path.hashPart()); + + StringSink sink; + srcStore.narFromPath(path, sink); + + StringMap rewrites; + + StorePathSet references; + bool hasSelfReference = false; + for (auto & ref : oldInfo->references) { + if (ref == path) + hasSelfReference = true; + else { + auto i = remappings.find(ref); + auto replacement = i != remappings.end() ? i->second : ref; + // FIXME: warn about unremapped paths? + if (replacement != ref) + rewrites.insert_or_assign(srcStore.printStorePath(ref), srcStore.printStorePath(replacement)); + references.insert(std::move(replacement)); + } + } + + sink.s = rewriteStrings(sink.s, rewrites); + + HashModuloSink hashModuloSink(htSHA256, oldHashPart); + hashModuloSink(sink.s); + + auto narHash = hashModuloSink.finish().first; + + ValidPathInfo info { + dstStore.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), + narHash, + }; + info.references = std::move(references); + if (hasSelfReference) info.references.insert(info.path); + info.narSize = sink.s.size(); + info.ca = FixedOutputHash { + .method = FileIngestionMethod::Recursive, + .hash = info.narHash, + }; + + printInfo("rewrote '%s' to '%s'", pathS, srcStore.printStorePath(info.path)); + + auto source = sinkToSource([&](Sink & nextSink) { + RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); + rsink2(sink.s); + rsink2.flush(); + }); + + dstStore.addToStore(info, *source); + + remappings.insert_or_assign(std::move(path), std::move(info.path)); + } + + return remappings; +} + +} diff --git a/src/libstore/make-content-addressed.hh b/src/libstore/make-content-addressed.hh new file mode 100644 index 000000000..c4a66ed41 --- /dev/null +++ b/src/libstore/make-content-addressed.hh @@ -0,0 +1,12 @@ +#pragma once + +#include "store-api.hh" + +namespace nix { + +std::map makeContentAddressed( + Store & srcStore, + Store & dstStore, + const StorePathSet & storePaths); + +} diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressable.cc index 2e75a3b61..a8579ea7c 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressable.cc @@ -1,6 +1,6 @@ #include "command.hh" #include "store-api.hh" -#include "references.hh" +#include "make-content-addressed.hh" #include "common-args.hh" #include "json.hh" @@ -27,74 +27,25 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON void run(ref store, StorePaths && storePaths) override { - auto paths = store->topoSortPaths(StorePathSet(storePaths.begin(), storePaths.end())); + auto remappings = makeContentAddressed(*store, *store, + StorePathSet(storePaths.begin(), storePaths.end())); - std::reverse(paths.begin(), paths.end()); - - std::map remappings; - - auto jsonRoot = json ? std::make_unique(std::cout) : nullptr; - auto jsonRewrites = json ? std::make_unique(jsonRoot->object("rewrites")) : nullptr; - - for (auto & path : paths) { - auto pathS = store->printStorePath(path); - auto oldInfo = store->queryPathInfo(path); - std::string oldHashPart(path.hashPart()); - - StringSink sink; - store->narFromPath(path, sink); - - StringMap rewrites; - - StorePathSet references; - bool hasSelfReference = false; - for (auto & ref : oldInfo->references) { - if (ref == path) - hasSelfReference = true; - else { - auto i = remappings.find(ref); - auto replacement = i != remappings.end() ? i->second : ref; - // FIXME: warn about unremapped paths? - if (replacement != ref) - rewrites.insert_or_assign(store->printStorePath(ref), store->printStorePath(replacement)); - references.insert(std::move(replacement)); - } + if (json) { + JSONObject jsonRoot(std::cout); + JSONObject jsonRewrites(jsonRoot.object("rewrites")); + for (auto & path : storePaths) { + auto i = remappings.find(path); + assert(i != remappings.end()); + jsonRewrites.attr(store->printStorePath(path), store->printStorePath(i->second)); + } + } else { + for (auto & path : storePaths) { + auto i = remappings.find(path); + assert(i != remappings.end()); + notice("rewrote '%s' to '%s'", + store->printStorePath(path), + store->printStorePath(i->second)); } - - sink.s = rewriteStrings(sink.s, rewrites); - - HashModuloSink hashModuloSink(htSHA256, oldHashPart); - hashModuloSink(sink.s); - - auto narHash = hashModuloSink.finish().first; - - ValidPathInfo info { - store->makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), - narHash, - }; - info.references = std::move(references); - if (hasSelfReference) info.references.insert(info.path); - info.narSize = sink.s.size(); - info.ca = FixedOutputHash { - .method = FileIngestionMethod::Recursive, - .hash = info.narHash, - }; - - if (!json) - notice("rewrote '%s' to '%s'", pathS, store->printStorePath(info.path)); - - auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); - rsink2(sink.s); - rsink2.flush(); - }); - - store->addToStore(info, *source); - - if (json) - jsonRewrites->attr(store->printStorePath(path), store->printStorePath(info.path)); - - remappings.insert_or_assign(std::move(path), std::move(info.path)); } } }; From f18607549ce38545b1d754ed93f3b7c5417970d8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:47:50 +0100 Subject: [PATCH 64/83] Fix makeContentAddressed() on self-references LocalStore::addToStore() since 79ae9e4558cbefd743f28a5e73110c2303b03a85 expects a regular NAR hash, rather than a NAR hash modulo self-references. Fixes #6300. Also, makeContentAddressed() now rewrites the entire closure (so 'nix store make-content-addressable' no longer needs '-r'). See #6301. --- src/libstore/make-content-addressed.cc | 35 +++++++++++++------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index 0b95ff37c..fc11fcb27 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -8,9 +8,10 @@ std::map makeContentAddressed( Store & dstStore, const StorePathSet & storePaths) { - // FIXME: use closure of storePaths. + StorePathSet closure; + srcStore.computeFSClosure(storePaths, closure); - auto paths = srcStore.topoSortPaths(storePaths); + auto paths = srcStore.topoSortPaths(closure); std::reverse(paths.begin(), paths.end()); @@ -46,29 +47,29 @@ std::map makeContentAddressed( HashModuloSink hashModuloSink(htSHA256, oldHashPart); hashModuloSink(sink.s); - auto narHash = hashModuloSink.finish().first; + auto narModuloHash = hashModuloSink.finish().first; - ValidPathInfo info { - dstStore.makeFixedOutputPath(FileIngestionMethod::Recursive, narHash, path.name(), references, hasSelfReference), - narHash, - }; + auto dstPath = dstStore.makeFixedOutputPath( + FileIngestionMethod::Recursive, narModuloHash, path.name(), references, hasSelfReference); + + printInfo("rewroting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); + + StringSink sink2; + RewritingSink rsink2(oldHashPart, std::string(dstPath.hashPart()), sink2); + rsink2(sink.s); + rsink2.flush(); + + ValidPathInfo info { dstPath, hashString(htSHA256, sink2.s) }; info.references = std::move(references); if (hasSelfReference) info.references.insert(info.path); info.narSize = sink.s.size(); info.ca = FixedOutputHash { .method = FileIngestionMethod::Recursive, - .hash = info.narHash, + .hash = narModuloHash, }; - printInfo("rewrote '%s' to '%s'", pathS, srcStore.printStorePath(info.path)); - - auto source = sinkToSource([&](Sink & nextSink) { - RewritingSink rsink2(oldHashPart, std::string(info.path.hashPart()), nextSink); - rsink2(sink.s); - rsink2.flush(); - }); - - dstStore.addToStore(info, *source); + StringSource source(sink2.s); + dstStore.addToStore(info, source); remappings.insert_or_assign(std::move(path), std::move(info.path)); } From 5acaf13d3564f689e5461f29a9cc5958809d5e93 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 21:54:49 +0100 Subject: [PATCH 65/83] Rename 'nix store make-content-addressable' to 'nix store make-content-addressed' --- doc/manual/src/release-notes/rl-next.md | 3 +++ src/nix/main.cc | 2 +- ...e-content-addressable.cc => make-content-addressed.cc} | 8 ++++---- ...e-content-addressable.md => make-content-addressed.md} | 4 ++-- 4 files changed, 10 insertions(+), 7 deletions(-) rename src/nix/{make-content-addressable.cc => make-content-addressed.cc} (83%) rename src/nix/{make-content-addressable.md => make-content-addressed.md} (94%) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index c9753f9aa..8fbc605e7 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -1,3 +1,6 @@ # Release X.Y (202?-??-??) * Various nix commands can now read expressions from stdin with `--file -`. + +* `nix store make-content-addressable` has been renamed to `nix store + make-content-addressed`. diff --git a/src/nix/main.cc b/src/nix/main.cc index b923f2535..9bc6c15fa 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -117,7 +117,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs {"hash-path", {"hash", "path"}}, {"ls-nar", {"nar", "ls"}}, {"ls-store", {"store", "ls"}}, - {"make-content-addressable", {"store", "make-content-addressable"}}, + {"make-content-addressable", {"store", "make-content-addressed"}}, {"optimise-store", {"store", "optimise"}}, {"ping-store", {"store", "ping"}}, {"sign-paths", {"store", "sign"}}, diff --git a/src/nix/make-content-addressable.cc b/src/nix/make-content-addressed.cc similarity index 83% rename from src/nix/make-content-addressable.cc rename to src/nix/make-content-addressed.cc index a8579ea7c..dc0447cb8 100644 --- a/src/nix/make-content-addressable.cc +++ b/src/nix/make-content-addressed.cc @@ -6,9 +6,9 @@ using namespace nix; -struct CmdMakeContentAddressable : StorePathsCommand, MixJSON +struct CmdMakeContentAddressed : StorePathsCommand, MixJSON { - CmdMakeContentAddressable() + CmdMakeContentAddressed() { realiseMode = Realise::Outputs; } @@ -21,7 +21,7 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON std::string doc() override { return - #include "make-content-addressable.md" + #include "make-content-addressed.md" ; } @@ -50,4 +50,4 @@ struct CmdMakeContentAddressable : StorePathsCommand, MixJSON } }; -static auto rCmdMakeContentAddressable = registerCommand2({"store", "make-content-addressable"}); +static auto rCmdMakeContentAddressed = registerCommand2({"store", "make-content-addressed"}); diff --git a/src/nix/make-content-addressable.md b/src/nix/make-content-addressed.md similarity index 94% rename from src/nix/make-content-addressable.md rename to src/nix/make-content-addressed.md index 3dd847edc..215683e6d 100644 --- a/src/nix/make-content-addressable.md +++ b/src/nix/make-content-addressed.md @@ -5,7 +5,7 @@ R""( * Create a content-addressed representation of the closure of GNU Hello: ```console - # nix store make-content-addressable -r nixpkgs#hello + # nix store make-content-addressed nixpkgs#hello … rewrote '/nix/store/v5sv61sszx301i0x6xysaqzla09nksnd-hello-2.10' to '/nix/store/5skmmcb9svys5lj3kbsrjg7vf2irid63-hello-2.10' ``` @@ -29,7 +29,7 @@ R""( system closure: ```console - # nix store make-content-addressable -r /run/current-system + # nix store make-content-addressed /run/current-system ``` # Description From 7ffda0af6effbf32c8668f34cc3f0448c58bc3c1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 22:01:20 +0100 Subject: [PATCH 66/83] fetchClosure: Skip makeContentAddressed() if toPath is already valid --- src/libexpr/primops/fetchClosure.cc | 42 +++++++++++++++-------------- 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 56fd53ed5..c3f07b6d6 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -54,26 +54,28 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args auto fromStore = openStore(*fromStoreUrl); if (toCA) { - auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); - auto i = remappings.find(*fromPath); - assert(i != remappings.end()); - if (toPath && *toPath != i->second) - throw Error({ - .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", - state.store->printStorePath(*fromPath), - state.store->printStorePath(i->second), - state.store->printStorePath(*toPath)), - .errPos = pos - }); - if (!toPath) - throw Error({ - .msg = hintfmt( - "rewriting '%s' to content-addressed form yielded '%s'; " - "please set this in the 'toPath' attribute passed to 'fetchClosure'", - state.store->printStorePath(*fromPath), - state.store->printStorePath(i->second)), - .errPos = pos - }); + if (!toPath || !state.store->isValidPath(*toPath)) { + auto remappings = makeContentAddressed(*fromStore, *state.store, { *fromPath }); + auto i = remappings.find(*fromPath); + assert(i != remappings.end()); + if (toPath && *toPath != i->second) + throw Error({ + .msg = hintfmt("rewriting '%s' to content-addressed form yielded '%s', while '%s' was expected", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second), + state.store->printStorePath(*toPath)), + .errPos = pos + }); + if (!toPath) + throw Error({ + .msg = hintfmt( + "rewriting '%s' to content-addressed form yielded '%s'; " + "please set this in the 'toPath' attribute passed to 'fetchClosure'", + state.store->printStorePath(*fromPath), + state.store->printStorePath(i->second)), + .errPos = pos + }); + } } else { copyClosure(*fromStore, *state.store, RealisedPath::Set { *fromPath }); toPath = fromPath; From 4120930ac19ab7296818fdc1d1389e7799168867 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 22:47:33 +0100 Subject: [PATCH 67/83] fetchClosure: Only allow some "safe" store types --- src/libexpr/primops/fetchClosure.cc | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index c3f07b6d6..247bceb07 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -1,6 +1,7 @@ #include "primops.hh" #include "store-api.hh" #include "make-content-addressed.hh" +#include "url.hh" namespace nix { @@ -50,8 +51,15 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args .errPos = pos }); - // FIXME: only allow some "trusted" store types (like BinaryCacheStore). - auto fromStore = openStore(*fromStoreUrl); + auto parsedURL = parseURL(*fromStoreUrl); + + if (parsedURL.scheme != "http" && parsedURL.scheme != "https") + throw Error({ + .msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"), + .errPos = pos + }); + + auto fromStore = openStore(parsedURL.to_string()); if (toCA) { if (!toPath || !state.store->isValidPath(*toPath)) { From 28186b7044dca513e6e07c3e66b7de2143543ae4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:19:21 +0100 Subject: [PATCH 68/83] Add a test for fetchClosure and 'nix store make-content-addressed' --- src/libexpr/primops/fetchClosure.cc | 4 +- src/libstore/make-content-addressed.cc | 2 +- tests/binary-cache.sh | 2 +- tests/fetchClosure.sh | 57 ++++++++++++++++++++++++++ tests/local.mk | 3 +- 5 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 tests/fetchClosure.sh diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 247bceb07..47e2d2bf2 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -53,7 +53,9 @@ static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args auto parsedURL = parseURL(*fromStoreUrl); - if (parsedURL.scheme != "http" && parsedURL.scheme != "https") + if (parsedURL.scheme != "http" && + parsedURL.scheme != "https" && + !(getEnv("_NIX_IN_TEST").has_value() && parsedURL.scheme == "file")) throw Error({ .msg = hintfmt("'fetchClosure' only supports http:// and https:// stores"), .errPos = pos diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index fc11fcb27..64d172918 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -52,7 +52,7 @@ std::map makeContentAddressed( auto dstPath = dstStore.makeFixedOutputPath( FileIngestionMethod::Recursive, narModuloHash, path.name(), references, hasSelfReference); - printInfo("rewroting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); + printInfo("rewriting '%s' to '%s'", pathS, srcStore.printStorePath(dstPath)); StringSink sink2; RewritingSink rsink2(oldHashPart, std::string(dstPath.hashPart()), sink2); diff --git a/tests/binary-cache.sh b/tests/binary-cache.sh index 2368884f7..0361ac6a8 100644 --- a/tests/binary-cache.sh +++ b/tests/binary-cache.sh @@ -1,6 +1,6 @@ source common.sh -needLocalStore "“--no-require-sigs” can’t be used with the daemon" +needLocalStore "'--no-require-sigs' can’t be used with the daemon" # We can produce drvs directly into the binary cache clearStore diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh new file mode 100644 index 000000000..811d44af9 --- /dev/null +++ b/tests/fetchClosure.sh @@ -0,0 +1,57 @@ +source common.sh + +needLocalStore "'--no-require-sigs' can’t be used with the daemon" + +clearStore +clearCacheCache + +# Initialize binary cache. +nonCaPath=$(nix build --json --file ./dependencies.nix | jq -r .[].outputs.out) +caPath=$(nix store make-content-addressed --json $nonCaPath | jq -r '.rewrites | map(.) | .[]') +nix copy --to file://$cacheDir $nonCaPath + +# Test basic fetchClosure rewriting from non-CA to CA. +clearStore + +[ ! -e $nonCaPath ] +[ ! -e $caPath ] + +[[ $(nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + toPath = $caPath; + } +") = $caPath ]] + +[ ! -e $nonCaPath ] +[ -e $caPath ] + +# In impure mode, we can use non-CA paths. +[[ $(nix eval --raw --no-require-sigs --impure --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + } +") = $nonCaPath ]] + +[ -e $nonCaPath ] + +# 'toPath' set to empty string should fail but print the expected path. +nix eval -v --json --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $nonCaPath; + toPath = \"\"; + } +" 2>&1 | grep "error: rewriting.*$nonCaPath.*yielded.*$caPath" + +# If fromPath is CA, then toPath isn't needed. +nix copy --to file://$cacheDir $caPath + +[[ $(nix eval -v --raw --expr " + builtins.fetchClosure { + fromStore = \"file://$cacheDir\"; + fromPath = $caPath; + } +") = $caPath ]] diff --git a/tests/local.mk b/tests/local.mk index c686be049..97971dd76 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -96,7 +96,8 @@ nix_tests = \ describe-stores.sh \ nix-profile.sh \ suggestions.sh \ - store-ping.sh + store-ping.sh \ + fetchClosure.sh ifeq ($(HAVE_LIBCPUID), 1) nix_tests += compute-levels.sh From 98658ae9d25e5e715f9349fbbb79d0ba31bb810c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:31:48 +0100 Subject: [PATCH 69/83] Document fetchClosure --- doc/manual/src/release-notes/rl-next.md | 7 ++++++ src/libexpr/primops/fetchClosure.cc | 33 +++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 8fbc605e7..33eaa8a2c 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -4,3 +4,10 @@ * `nix store make-content-addressable` has been renamed to `nix store make-content-addressed`. + +* New builtin function `builtins.fetchClosure` that copies a closure + from a binary cache at evaluation time and rewrites it to + content-addressed form (if it isn't already). Like + `builtins.storePath`, this allows importing pre-built store paths; + the difference is that it doesn't require the user to configure + binary caches and trusted public keys. diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 47e2d2bf2..045e97dbd 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -110,6 +110,39 @@ static RegisterPrimOp primop_fetchClosure({ .name = "__fetchClosure", .args = {"args"}, .doc = R"( + Fetch a Nix store closure from a binary cache, rewriting it into + content-addressed form. For example, + + ```nix + builtins.fetchClosure { + fromStore = "https://cache.nixos.org"; + fromPath = /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1; + toPath = /nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1; + } + ``` + + fetches `/nix/store/r2jd...` from the specified binary cache, + and rewrites it into the content-addressed store path + `/nix/store/ldbh...`. + + If `fromPath` is already content-addressed, or if you are + allowing impure evaluation (`--impure`), then `toPath` may be + omitted. + + To find out the correct value for `toPath` given a `fromPath`, + you can use `nix store make-content-addressed`: + + ```console + # nix store make-content-addressed /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 + rewrote '/nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1' to '/nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1' + ``` + + This function is similar to `builtins.storePath` in that it + allows you to use a previously built store path in a Nix + expression. However, it is more reproducible because it requires + specifying a binary cache from which the path can be fetched. + Also, requiring a content-addressed final store path avoids the + need for users to configure binary cache public keys. )", .fun = prim_fetchClosure, }); From e5f7029ba49a486c1b0c8011be79b8b41be20935 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 22 Mar 2022 23:40:04 +0100 Subject: [PATCH 70/83] nix store make-content-addressed: Support --from / --to --- src/libexpr/primops/fetchClosure.cc | 2 +- src/nix/make-content-addressed.cc | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 045e97dbd..3e07d1d18 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -133,7 +133,7 @@ static RegisterPrimOp primop_fetchClosure({ you can use `nix store make-content-addressed`: ```console - # nix store make-content-addressed /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 + # nix store make-content-addressed --from https://cache.nixos.org /nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1 rewrote '/nix/store/r2jd6ygnmirm2g803mksqqjm4y39yi6i-git-2.33.1' to '/nix/store/ldbhlwhh39wha58rm61bkiiwm6j7211j-git-2.33.1' ``` diff --git a/src/nix/make-content-addressed.cc b/src/nix/make-content-addressed.cc index dc0447cb8..34860c38f 100644 --- a/src/nix/make-content-addressed.cc +++ b/src/nix/make-content-addressed.cc @@ -6,7 +6,7 @@ using namespace nix; -struct CmdMakeContentAddressed : StorePathsCommand, MixJSON +struct CmdMakeContentAddressed : virtual CopyCommand, virtual StorePathsCommand, MixJSON { CmdMakeContentAddressed() { @@ -25,9 +25,11 @@ struct CmdMakeContentAddressed : StorePathsCommand, MixJSON ; } - void run(ref store, StorePaths && storePaths) override + void run(ref srcStore, StorePaths && storePaths) override { - auto remappings = makeContentAddressed(*store, *store, + auto dstStore = dstUri.empty() ? openStore() : openStore(dstUri); + + auto remappings = makeContentAddressed(*srcStore, *dstStore, StorePathSet(storePaths.begin(), storePaths.end())); if (json) { @@ -36,15 +38,15 @@ struct CmdMakeContentAddressed : StorePathsCommand, MixJSON for (auto & path : storePaths) { auto i = remappings.find(path); assert(i != remappings.end()); - jsonRewrites.attr(store->printStorePath(path), store->printStorePath(i->second)); + jsonRewrites.attr(srcStore->printStorePath(path), srcStore->printStorePath(i->second)); } } else { for (auto & path : storePaths) { auto i = remappings.find(path); assert(i != remappings.end()); notice("rewrote '%s' to '%s'", - store->printStorePath(path), - store->printStorePath(i->second)); + srcStore->printStorePath(path), + srcStore->printStorePath(i->second)); } } } From f902f3c2cbfa1a78bec8d135297abe244452e794 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 21:33:20 +0100 Subject: [PATCH 71/83] Add experimental feature 'fetch-closure' --- src/libexpr/primops/fetchClosure.cc | 2 ++ src/libutil/experimental-features.cc | 1 + src/libutil/experimental-features.hh | 3 ++- tests/fetchClosure.sh | 1 + 4 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 3e07d1d18..47f40ef33 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,6 +7,8 @@ namespace nix { static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) { + settings.requireExperimentalFeature(Xp::FetchClosure); + state.forceAttrs(*args[0], pos); std::optional fromStoreUrl; diff --git a/src/libutil/experimental-features.cc b/src/libutil/experimental-features.cc index b49f47e1d..01f318fa3 100644 --- a/src/libutil/experimental-features.cc +++ b/src/libutil/experimental-features.cc @@ -11,6 +11,7 @@ std::map stringifiedXpFeatures = { { Xp::NixCommand, "nix-command" }, { Xp::RecursiveNix, "recursive-nix" }, { Xp::NoUrlLiterals, "no-url-literals" }, + { Xp::FetchClosure, "fetch-closure" }, }; const std::optional parseExperimentalFeature(const std::string_view & name) diff --git a/src/libutil/experimental-features.hh b/src/libutil/experimental-features.hh index 291a58e32..b5140dcfe 100644 --- a/src/libutil/experimental-features.hh +++ b/src/libutil/experimental-features.hh @@ -19,7 +19,8 @@ enum struct ExperimentalFeature Flakes, NixCommand, RecursiveNix, - NoUrlLiterals + NoUrlLiterals, + FetchClosure, }; /** diff --git a/tests/fetchClosure.sh b/tests/fetchClosure.sh index 811d44af9..0c905ac43 100644 --- a/tests/fetchClosure.sh +++ b/tests/fetchClosure.sh @@ -1,5 +1,6 @@ source common.sh +enableFeatures "fetch-closure" needLocalStore "'--no-require-sigs' can’t be used with the daemon" clearStore From c85467a1b65d2a8907fcab3303572f76c071776d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 6 Feb 2022 14:47:07 +0100 Subject: [PATCH 72/83] Revert "TarArchive: Small refactoring" This reverts commit 50a35860ee9237d341948437c5f70a7f0987d393. With this change Nix fails to open bzip2 logfiles that were created from builds with no stdout/stderr. --- src/libutil/tarfile.cc | 26 ++++++++++++++------------ src/libutil/tarfile.hh | 3 --- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/src/libutil/tarfile.cc b/src/libutil/tarfile.cc index 790bc943a..a7db58559 100644 --- a/src/libutil/tarfile.cc +++ b/src/libutil/tarfile.cc @@ -39,30 +39,32 @@ void TarArchive::check(int err, const std::string & reason) throw Error(reason, archive_error_string(this->archive)); } -TarArchive::TarArchive(Source & source, bool raw) - : source(&source), buffer(4096) +TarArchive::TarArchive(Source & source, bool raw) : buffer(4096) { - init(); - if (!raw) + this->archive = archive_read_new(); + this->source = &source; + + if (!raw) { + archive_read_support_filter_all(archive); archive_read_support_format_all(archive); - else + } else { + archive_read_support_filter_all(archive); archive_read_support_format_raw(archive); + archive_read_support_format_empty(archive); + } check(archive_read_open(archive, (void *)this, callback_open, callback_read, callback_close), "Failed to open archive (%s)"); } + TarArchive::TarArchive(const Path & path) { - init(); + this->archive = archive_read_new(); + + archive_read_support_filter_all(archive); archive_read_support_format_all(archive); check(archive_read_open_filename(archive, path.c_str(), 16384), "failed to open archive: %s"); } -void TarArchive::init() -{ - archive = archive_read_new(); - archive_read_support_filter_all(archive); -} - void TarArchive::close() { check(archive_read_close(this->archive), "Failed to close archive (%s)"); diff --git a/src/libutil/tarfile.hh b/src/libutil/tarfile.hh index f107a7e2e..4d9141fd4 100644 --- a/src/libutil/tarfile.hh +++ b/src/libutil/tarfile.hh @@ -17,13 +17,10 @@ struct TarArchive { // disable copy constructor TarArchive(const TarArchive &) = delete; - void init(); - void close(); ~TarArchive(); }; - void unpackTarfile(Source & source, const Path & destDir); void unpackTarfile(const Path & tarFile, const Path & destDir); From d02e34ef06e24f98404d7a50130ddced15a12279 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 6 Feb 2022 14:52:04 +0100 Subject: [PATCH 73/83] Implement regression test for empty logs loaded via `nix log` --- tests/logging.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/logging.sh b/tests/logging.sh index c894ad3ff..1481b9b36 100644 --- a/tests/logging.sh +++ b/tests/logging.sh @@ -13,3 +13,14 @@ rm -rf $NIX_LOG_DIR (! nix-store -l $path) nix-build dependencies.nix --no-out-link --compress-build-log [ "$(nix-store -l $path)" = FOO ] + +# test whether empty logs work fine with `nix log`. +builder="$(mktemp)" +echo -e "#!/bin/sh\nmkdir \$out" > "$builder" +outp="$(nix-build -E \ + 'with import ./config.nix; mkDerivation { name = "fnord"; builder = '"$builder"'; }' \ + --out-link "$(mktemp -d)/result")" + +test -d "$outp" + +nix log "$outp" From 175c78591b1877c280936cc663b76e47f207dd77 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:09:43 +0100 Subject: [PATCH 74/83] Random cleanup --- src/libstore/build/goal.hh | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 07c752bb9..35121c5d9 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -40,21 +40,21 @@ struct Goal : public std::enable_shared_from_this WeakGoals waiters; /* Number of goals we are/were waiting for that have failed. */ - unsigned int nrFailed; + size_t nrFailed = 0; /* Number of substitution goals we are/were waiting for that failed because there are no substituters. */ - unsigned int nrNoSubstituters; + size_t nrNoSubstituters = 0; /* Number of substitution goals we are/were waiting for that failed because they had unsubstitutable references. */ - unsigned int nrIncompleteClosure; + size_t nrIncompleteClosure = 0; /* Name of this goal for debugging purposes. */ std::string name; /* Whether the goal is finished. */ - ExitCode exitCode; + ExitCode exitCode = ecBusy; /* Build result. */ BuildResult buildResult; @@ -65,10 +65,7 @@ struct Goal : public std::enable_shared_from_this Goal(Worker & worker, DerivedPath path) : worker(worker) , buildResult { .path = std::move(path) } - { - nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; - exitCode = ecBusy; - } + { } virtual ~Goal() { From 09796c026376b49a8bc5bb3a2865db015d3dfb06 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:24:10 +0100 Subject: [PATCH 75/83] Random cleanup --- src/libstore/build/goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index d2420b107..58e805f55 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -28,7 +28,7 @@ void Goal::addWaitee(GoalPtr waitee) void Goal::waiteeDone(GoalPtr waitee, ExitCode result) { - assert(waitees.find(waitee) != waitees.end()); + assert(waitees.count(waitee)); waitees.erase(waitee); trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size())); From fe5509df9a58359a67da8c72ed8721cb3a33371d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:24:48 +0100 Subject: [PATCH 76/83] Only return wanted outputs --- src/libstore/build/derivation-goal.cc | 5 +---- src/libstore/build/local-derivation-goal.cc | 3 ++- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 40c445836..7dc5737fb 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -311,14 +311,11 @@ void DerivationGoal::outputsSubstitutionTried() gaveUpOnSubstitution(); } + /* At least one of the output paths could not be produced using a substitute. So we have to build instead. */ void DerivationGoal::gaveUpOnSubstitution() { - /* Make sure checkPathValidity() from now on checks all - outputs. */ - wantedOutputs.clear(); - /* The inputs must be built before we can build this goal. */ if (useDerivation) for (auto & i : dynamic_cast(drv.get())->inputDrvs) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 75e7e6ca3..ed83c4770 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2613,7 +2613,8 @@ DrvOutputs LocalDerivationGoal::registerOutputs() signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - builtOutputs.emplace(thisRealisation.id, thisRealisation); + if (wantedOutputs.empty() || wantedOutputs.count(outputName)) + builtOutputs.emplace(thisRealisation.id, thisRealisation); } return builtOutputs; From 540d7e33d879e21d34de0935292a65bf27f5e312 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:25:12 +0100 Subject: [PATCH 77/83] Retry substitution after an incomplete closure only once This avoids an infinite loop in the final test in tests/binary-cache.sh. I think this was only not triggered previously by accident (because we were clearing wantedOutputs in between). --- src/libstore/build/derivation-goal.cc | 5 ++--- src/libstore/build/derivation-goal.hh | 8 ++++++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 7dc5737fb..3d1c4fbc1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -207,8 +207,6 @@ void DerivationGoal::haveDerivation() if (!drv->type().hasKnownOutputPaths()) settings.requireExperimentalFeature(Xp::CaDerivations); - retrySubstitution = false; - for (auto & i : drv->outputsAndOptPaths(worker.store)) if (i.second.second) worker.store.addTempRoot(*i.second.second); @@ -423,7 +421,8 @@ void DerivationGoal::inputsRealised() return; } - if (retrySubstitution) { + if (retrySubstitution && !retriedSubstitution) { + retriedSubstitution = true; haveDerivation(); return; } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index ea2db89b2..f556b6f25 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -61,8 +61,12 @@ struct DerivationGoal : public Goal bool needRestart = false; /* Whether to retry substituting the outputs after building the - inputs. */ - bool retrySubstitution; + inputs. This is done in case of an incomplete closure. */ + bool retrySubstitution = false; + + /* Whether we've retried substitution, in which case we won't try + again. */ + bool retriedSubstitution = false; /* The derivation stored at drvPath. */ std::unique_ptr drv; From 187dc080a2ab3403c1b1b47b3face205a4a67fb4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 24 Mar 2022 23:36:14 +0100 Subject: [PATCH 78/83] tests/build.sh: Test that 'nix build' only prints wanted outputs --- tests/build.sh | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/build.sh b/tests/build.sh index c77f620f7..13a0f42be 100644 --- a/tests/build.sh +++ b/tests/build.sh @@ -1,15 +1,27 @@ source common.sh -expectedJSONRegex='\[\{"drvPath":".*multiple-outputs-a.drv","outputs":\{"first":".*multiple-outputs-a-first","second":".*multiple-outputs-a-second"}},\{"drvPath":".*multiple-outputs-b.drv","outputs":\{"out":".*multiple-outputs-b"}}]' +clearStore + +# Make sure that 'nix build' only returns the outputs we asked for. +nix build -f multiple-outputs.nix --json a --no-link | jq --exit-status ' + (.[0] | + (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys | length == 1) and + (.outputs.first | match(".*multiple-outputs-a-first"))) +' + nix build -f multiple-outputs.nix --json a.all b.all --no-link | jq --exit-status ' (.[0] | (.drvPath | match(".*multiple-outputs-a.drv")) and + (.outputs | keys | length == 2) and (.outputs.first | match(".*multiple-outputs-a-first")) and (.outputs.second | match(".*multiple-outputs-a-second"))) and (.[1] | (.drvPath | match(".*multiple-outputs-b.drv")) and + (.outputs | keys | length == 1) and (.outputs.out | match(".*multiple-outputs-b"))) ' + testNormalization () { clearStore outPath=$(nix-build ./simple.nix --no-out-link) From cbcb69a39ce7ceb300d5a5588258b7f8c0c0664d Mon Sep 17 00:00:00 2001 From: polykernel <81340136+polykernel@users.noreply.github.com> Date: Sun, 20 Mar 2022 11:39:36 -0400 Subject: [PATCH 79/83] nix: allow whitespace characters before command in repl Before this change, processLine always uses the first character as the start of the line. This cause whitespaces to matter at the beginning of the line whereas it does not matter anywhere else. This commit trims leading white spaces of the string line so that subsequent operations can be performed on the string without explicitly tracking starting and ending indices of the string. --- src/nix/repl.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 916353d8c..1f9d4fb4e 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -396,6 +396,7 @@ StorePath NixRepl::getDerivationPath(Value & v) { bool NixRepl::processLine(std::string line) { + line = trim(line); if (line == "") return true; _isInterrupted = false; From 50c229ad9aeece3b4728fedb741d8ef7adb2b2c1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 08:02:49 +0100 Subject: [PATCH 80/83] Use wantOutput Co-authored-by: John Ericson --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index ed83c4770..b176f318b 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2613,7 +2613,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs() signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } - if (wantedOutputs.empty() || wantedOutputs.count(outputName)) + if (wantOutput(outputName, wantedOutputs)) builtOutputs.emplace(thisRealisation.id, thisRealisation); } From 86b05ccd54f2e98ac2b5cef3bcecb29ed6ec4fd8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 14:04:18 +0100 Subject: [PATCH 81/83] Only provide builtin.{getFlake,fetchClosure} is the corresponding experimental feature is enabled This allows writing fallback code like if builtins ? fetchClosure then builtins.fetchClose { ... } else builtins.storePath ... --- doc/manual/src/release-notes/rl-next.md | 9 ++++++--- src/libexpr/eval.cc | 17 ----------------- src/libexpr/eval.hh | 6 ------ src/libexpr/flake/flake.cc | 9 ++++++--- src/libexpr/primops.cc | 20 ++++++++++++-------- src/libexpr/primops.hh | 2 ++ src/libexpr/primops/fetchClosure.cc | 6 ++++-- src/nix/main.cc | 1 + 8 files changed, 31 insertions(+), 39 deletions(-) diff --git a/doc/manual/src/release-notes/rl-next.md b/doc/manual/src/release-notes/rl-next.md index 33eaa8a2c..2ec864ee4 100644 --- a/doc/manual/src/release-notes/rl-next.md +++ b/doc/manual/src/release-notes/rl-next.md @@ -5,9 +5,12 @@ * `nix store make-content-addressable` has been renamed to `nix store make-content-addressed`. -* New builtin function `builtins.fetchClosure` that copies a closure - from a binary cache at evaluation time and rewrites it to - content-addressed form (if it isn't already). Like +* New experimental builtin function `builtins.fetchClosure` that + copies a closure from a binary cache at evaluation time and rewrites + it to content-addressed form (if it isn't already). Like `builtins.storePath`, this allows importing pre-built store paths; the difference is that it doesn't require the user to configure binary caches and trusted public keys. + + This function is only available if you enable the experimental + feature `fetch-closure`. diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 3ec5ed202..437c7fc53 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -507,23 +507,6 @@ EvalState::~EvalState() } -void EvalState::requireExperimentalFeatureOnEvaluation( - const ExperimentalFeature & feature, - const std::string_view fName, - const Pos & pos) -{ - if (!settings.isExperimentalFeatureEnabled(feature)) { - throw EvalError({ - .msg = hintfmt( - "Cannot call '%2%' because experimental Nix feature '%1%' is disabled. You can enable it via '--extra-experimental-features %1%'.", - feature, - fName - ), - .errPos = pos - }); - } -} - void EvalState::allowPath(const Path & path) { if (allowedPaths) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index 198a62ad2..ab46b448c 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -149,12 +149,6 @@ public: std::shared_ptr buildStore = nullptr); ~EvalState(); - void requireExperimentalFeatureOnEvaluation( - const ExperimentalFeature &, - const std::string_view fName, - const Pos & pos - ); - void addToSearchPath(const std::string & s); SearchPath getSearchPath() { return searchPath; } diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 6a1aca40d..fdcfb18d7 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -706,8 +706,6 @@ void callFlake(EvalState & state, static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Value & v) { - state.requireExperimentalFeatureOnEvaluation(Xp::Flakes, "builtins.getFlake", pos); - std::string flakeRefS(state.forceStringNoCtx(*args[0], pos)); auto flakeRef = parseFlakeRef(flakeRefS, {}, true); if (evalSettings.pureEval && !flakeRef.input.isLocked()) @@ -723,7 +721,12 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va v); } -static RegisterPrimOp r2("__getFlake", 1, prim_getFlake); +static RegisterPrimOp r2({ + .name = "__getFlake", + .args = {"args"}, + .fun = prim_getFlake, + .experimentalFeature = Xp::Flakes, +}); } diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f5d9aa9ae..f3eb5e925 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -3814,7 +3814,7 @@ RegisterPrimOp::RegisterPrimOp(std::string name, size_t arity, PrimOpFun fun) .name = name, .args = {}, .arity = arity, - .fun = fun + .fun = fun, }); } @@ -3886,13 +3886,17 @@ void EvalState::createBaseEnv() if (RegisterPrimOp::primOps) for (auto & primOp : *RegisterPrimOp::primOps) - addPrimOp({ - .fun = primOp.fun, - .arity = std::max(primOp.args.size(), primOp.arity), - .name = symbols.create(primOp.name), - .args = primOp.args, - .doc = primOp.doc, - }); + if (!primOp.experimentalFeature + || settings.isExperimentalFeatureEnabled(*primOp.experimentalFeature)) + { + addPrimOp({ + .fun = primOp.fun, + .arity = std::max(primOp.args.size(), primOp.arity), + .name = symbols.create(primOp.name), + .args = primOp.args, + .doc = primOp.doc, + }); + } /* Add a wrapper around the derivation primop that computes the `drvPath' and `outPath' attributes lazily. */ diff --git a/src/libexpr/primops.hh b/src/libexpr/primops.hh index 5b16e075f..905bd0366 100644 --- a/src/libexpr/primops.hh +++ b/src/libexpr/primops.hh @@ -16,6 +16,7 @@ struct RegisterPrimOp size_t arity = 0; const char * doc; PrimOpFun fun; + std::optional experimentalFeature; }; typedef std::vector PrimOps; @@ -35,6 +36,7 @@ struct RegisterPrimOp /* These primops are disabled without enableNativeCode, but plugins may wish to use them in limited contexts without globally enabling them. */ + /* Load a ValueInitializer from a DSO and return whatever it initializes */ void prim_importNative(EvalState & state, const Pos & pos, Value * * args, Value & v); diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index 47f40ef33..efeb93daf 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -7,8 +7,6 @@ namespace nix { static void prim_fetchClosure(EvalState & state, const Pos & pos, Value * * args, Value & v) { - settings.requireExperimentalFeature(Xp::FetchClosure); - state.forceAttrs(*args[0], pos); std::optional fromStoreUrl; @@ -145,8 +143,12 @@ static RegisterPrimOp primop_fetchClosure({ specifying a binary cache from which the path can be fetched. Also, requiring a content-addressed final store path avoids the need for users to configure binary cache public keys. + + This function is only available if you enable the experimental + feature `fetch-closure`. )", .fun = prim_fetchClosure, + .experimentalFeature = Xp::FetchClosure, }); } diff --git a/src/nix/main.cc b/src/nix/main.cc index 9bc6c15fa..6198681e7 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -289,6 +289,7 @@ void mainWrapped(int argc, char * * argv) } if (argc == 2 && std::string(argv[1]) == "__dump-builtins") { + settings.experimentalFeatures = {Xp::Flakes, Xp::FetchClosure}; evalSettings.pureEval = false; EvalState state({}, openStore("dummy://")); auto res = nlohmann::json::object(); From 8c363eb3eb3db96287a10eb5fcde23986a96164f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 14:19:55 +0100 Subject: [PATCH 82/83] Document getFlake Fixes #5523. --- src/libexpr/flake/flake.cc | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index fdcfb18d7..22257c6b3 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -724,6 +724,24 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va static RegisterPrimOp r2({ .name = "__getFlake", .args = {"args"}, + .doc = R"( + Fetch a flake from a flake reference, and return its output attributes and some metadata. For example: + + ```nix + (builtins.getFlake "nix/55bc52401966fbffa525c574c14f67b00bc4fb3a").packages.x86_64-linux.nix + ``` + + Unless impure evaluation is allowed (`--impure`), the flake reference + must be "locked", e.g. contain a Git revision or content hash. An + example of an unlocked usage is: + + ```nix + (builtins.getFlake "github:edolstra/dwarffs").rev + ``` + + This function is only available if you enable the experimental feature + `flakes`. + )", .fun = prim_getFlake, .experimentalFeature = Xp::Flakes, }); From fc35b11a7cde047cd799a6c83b0da312b713e0fb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Mar 2022 15:22:22 +0100 Subject: [PATCH 83/83] Fix mismatched tag warning on clang --- src/libexpr/eval.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index ab46b448c..e7915dd99 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -17,7 +17,7 @@ namespace nix { -struct Store; +class Store; class EvalState; class StorePath; enum RepairFlag : bool;