From 5bb6e3bfafbcd101962b39ce579ee56d670cd83a Mon Sep 17 00:00:00 2001 From: Yorick van Pelt Date: Thu, 2 Mar 2023 17:24:58 +0100 Subject: [PATCH 01/18] NixRepl::mainLoop: restore old curRepl on function exit This fixes completion callbacks after entering and leaving a nested debugger. --- src/libcmd/repl.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index e3afb1531..29c838ec0 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -252,7 +252,9 @@ void NixRepl::mainLoop() el_hist_size = 1000; #endif read_history(historyFile.c_str()); + auto oldRepl = curRepl; curRepl = this; + Finally restoreRepl([&] { curRepl = oldRepl; }); #ifndef READLINE rl_set_complete_func(completionCallback); rl_set_list_possib_func(listPossibleCallback); From e91596eb6922157aaba17a858dc52244dd0e5688 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Mon, 13 Mar 2023 21:08:52 +0100 Subject: [PATCH 02/18] Allow specifying alternative paths for reading/writing flake locks This allows having multiple separate lockfiles for a single project, which can be useful for testing against different versions of nixpkgs; it also allows tracking custom input overrides for remote flakes without requiring local clones of these flakes. For example, if I want to build Nix against my locally pinned nixpkgs, and have a lock file tracking this override independently of future updates to said nixpkgs: nix flake lock --output-lock-file /tmp/nix-flake.lock --override-input nixpkgs flake:nixpkgs nix build --reference-lock-file /tmp/nix-flake.lock Co-Authored-By: Will Fancher --- src/libcmd/installables.cc | 22 ++++++++++++++++++++++ src/libexpr/flake/flake.cc | 31 +++++++++++++++++++------------ src/libexpr/flake/flake.hh | 6 ++++++ 3 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 5ecf6293f..ce42e7770 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -102,6 +102,28 @@ MixFlakeOptions::MixFlakeOptions() }} }); + addFlag({ + .longName = "reference-lock-file", + .description = "Read the given lock file instead of `flake.lock` within the top-level flake", + .category = category, + .labels = {"flake-lock-path"}, + .handler = {[&](std::string lockFilePath) { + lockFlags.referenceLockFilePath = lockFilePath; + }}, + .completer = completePath + }); + + addFlag({ + .longName = "output-lock-file", + .description = "Write the given lock file instead of `flake.lock` within the top-level flake", + .category = category, + .labels = {"flake-lock-path"}, + .handler = {[&](std::string lockFilePath) { + lockFlags.outputLockFilePath = lockFilePath; + }}, + .completer = completePath + }); + addFlag({ .longName = "inputs-from", .description = "Use the inputs of the specified flake as registry entries.", diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 336eb274d..64ec4bdd2 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -337,7 +337,8 @@ LockedFlake lockFlake( // FIXME: symlink attack auto oldLockFile = LockFile::read( - flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock"); + lockFlags.referenceLockFilePath.value_or( + flake.sourceInfo->actualPath + "/" + flake.lockedRef.subdir + "/flake.lock")); debug("old lock file: %s", oldLockFile); @@ -619,13 +620,20 @@ LockedFlake lockFlake( debug("new lock file: %s", newLockFile); + auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock"; + auto sourcePath = topRef.input.getSourcePath(); + auto outputLockFilePath = sourcePath ? std::optional{*sourcePath + "/" + relPath} : std::nullopt; + if (lockFlags.outputLockFilePath) { + outputLockFilePath = lockFlags.outputLockFilePath; + } + /* Check whether we need to / can write the new lock file. */ - if (!(newLockFile == oldLockFile)) { + if (newLockFile != oldLockFile || lockFlags.outputLockFilePath) { auto diff = LockFile::diff(oldLockFile, newLockFile); if (lockFlags.writeLockFile) { - if (auto sourcePath = topRef.input.getSourcePath()) { + if (outputLockFilePath) { if (auto unlockedInput = newLockFile.isUnlocked()) { if (fetchSettings.warnDirty) warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput); @@ -633,25 +641,24 @@ LockedFlake lockFlake( if (!lockFlags.updateLockFile) throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef); - auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock"; - - auto path = *sourcePath + "/" + relPath; - - bool lockFileExists = pathExists(path); + bool lockFileExists = pathExists(*outputLockFilePath); if (lockFileExists) { auto s = chomp(diff); if (s.empty()) - warn("updating lock file '%s'", path); + warn("updating lock file '%s'", *outputLockFilePath); else - warn("updating lock file '%s':\n%s", path, s); + warn("updating lock file '%s':\n%s", *outputLockFilePath, s); } else - warn("creating lock file '%s'", path); + warn("creating lock file '%s'", *outputLockFilePath); - newLockFile.write(path); + newLockFile.write(*outputLockFilePath); std::optional commitMessage = std::nullopt; if (lockFlags.commitLockFile) { + if (lockFlags.outputLockFilePath) { + throw Error("--commit-lock-file and --output-lock-file are currently incompatible"); + } std::string cm; cm = fetchSettings.commitLockFileSummary.get(); diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index 10301d8aa..b5db56312 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -117,6 +117,12 @@ struct LockFlags /* Whether to commit changes to flake.lock. */ bool commitLockFile = false; + /* The path to a lock file to read instead of the `flake.lock` file in the top-level flake */ + std::optional referenceLockFilePath = std::nullopt; + + /* The path to a lock file to write to instead of the `flake.lock` file in the top-level flake */ + std::optional outputLockFilePath = std::nullopt; + /* Flake inputs to be overridden. */ std::map inputOverrides; From 3a1de4c3fe176256514455e1ca951bf28f53bd71 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Tue, 14 Mar 2023 12:02:03 +0100 Subject: [PATCH 03/18] Apply review suggestions Co-authored-by: Eelco Dolstra --- src/libcmd/installables.cc | 4 ++-- src/libexpr/flake/flake.hh | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index ce42e7770..c2fda7c19 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -104,7 +104,7 @@ MixFlakeOptions::MixFlakeOptions() addFlag({ .longName = "reference-lock-file", - .description = "Read the given lock file instead of `flake.lock` within the top-level flake", + .description = "Read the given lock file instead of `flake.lock` within the top-level flake.", .category = category, .labels = {"flake-lock-path"}, .handler = {[&](std::string lockFilePath) { @@ -115,7 +115,7 @@ MixFlakeOptions::MixFlakeOptions() addFlag({ .longName = "output-lock-file", - .description = "Write the given lock file instead of `flake.lock` within the top-level flake", + .description = "Write the given lock file instead of `flake.lock` within the top-level flake.", .category = category, .labels = {"flake-lock-path"}, .handler = {[&](std::string lockFilePath) { diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index b5db56312..e7e201395 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -118,10 +118,10 @@ struct LockFlags bool commitLockFile = false; /* The path to a lock file to read instead of the `flake.lock` file in the top-level flake */ - std::optional referenceLockFilePath = std::nullopt; + std::optional referenceLockFilePath; /* The path to a lock file to write to instead of the `flake.lock` file in the top-level flake */ - std::optional outputLockFilePath = std::nullopt; + std::optional outputLockFilePath; /* Flake inputs to be overridden. */ std::map inputOverrides; From 95dabbadd8a4e37da6739c93d65c70941097a09c Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 13 Mar 2023 08:15:23 -0400 Subject: [PATCH 04/18] Prioritize testing in the maintainers process docs PRs that don't increase our ongoing obligations (i.e. by adding new features) but do increase test coverage of existing features are good things to merge for the health of the project, and thus good to prioritize. --- maintainers/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/maintainers/README.md b/maintainers/README.md index 476a5f51e..36d40a0ab 100644 --- a/maintainers/README.md +++ b/maintainers/README.md @@ -69,6 +69,7 @@ Issues on the board progress through the following states: 2. [security](https://github.com/NixOS/nix/labels/security) 3. [regression](https://github.com/NixOS/nix/labels/regression) 4. [bug](https://github.com/NixOS/nix/issues?q=is%3Aopen+label%3Abug+sort%3Areactions-%2B1-desc) + 5. [tests of existing functionality](https://github.com/NixOS/nix/issues?q=is%3Aopen+label%3Atests+-label%3Afeature+sort%3Areactions-%2B1-desc) - [oldest pull requests](https://github.com/NixOS/nix/pulls?q=is%3Apr+is%3Aopen+sort%3Acreated-asc) - [most popular pull requests](https://github.com/NixOS/nix/pulls?q=is%3Apr+is%3Aopen+sort%3Areactions-%2B1-desc) @@ -91,7 +92,7 @@ Issues on the board progress through the following states: Contributors who took the time to implement concrete change proposals should not wait indefinitely. - - Prioritise fixing bugs over documentation, improvements or new features + - Prioritise fixing bugs and testing over documentation, improvements or new features The team values stability and accessibility higher than raw functionality. From ea207a2eedbcb6c17055f7d92a26d9f7209c84b6 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Sun, 19 Mar 2023 14:11:19 +0100 Subject: [PATCH 05/18] Add tests for alternate lockfile path functionality --- tests/flakes/flakes.sh | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/flakes/flakes.sh b/tests/flakes/flakes.sh index 5c922d7c5..fcbb01b05 100644 --- a/tests/flakes/flakes.sh +++ b/tests/flakes/flakes.sh @@ -96,7 +96,9 @@ json=$(nix flake metadata flake1 --json | jq .) hash1=$(echo "$json" | jq -r .revision) echo -n '# foo' >> $flake1Dir/flake.nix +flake1OriginalCommit=$(git -C $flake1Dir rev-parse HEAD) git -C $flake1Dir commit -a -m 'Foo' +flake1NewCommit=$(git -C $flake1Dir rev-parse HEAD) hash2=$(nix flake metadata flake1 --json --refresh | jq -r .revision) [[ $hash1 != $hash2 ]] @@ -491,3 +493,14 @@ nix store delete $(nix store add-path $badFlakeDir) [[ $(nix-instantiate --eval flake:git+file://$flake3Dir -A x) = 123 ]] [[ $(nix-instantiate -I flake3=flake:flake3 --eval '' -A x) = 123 ]] [[ $(NIX_PATH=flake3=flake:flake3 nix-instantiate --eval '' -A x) = 123 ]] + +# Test alternate lockfile paths. +nix flake lock $flake2Dir --output-lock-file flake2.lock +cmp $flake2Dir/flake.lock flake2.lock >/dev/null # lockfiles should be identical, since we're referencing flake2's original one + +nix flake lock $flake2Dir --output-lock-file flake2-overridden.lock --override-input flake1 git+file://$flake1Dir?rev=$flake1OriginalCommit +expectStderr 1 cmp $flake2Dir/flake.lock flake2-overridden.lock +nix flake metadata $flake2Dir --reference-lock-file flake2-overridden.lock | grepQuiet $flake1OriginalCommit + +# reference-lock-file can only be used if allow-dirty is set. +expectStderr 1 nix flake metadata $flake2Dir --no-allow-dirty --reference-lock-file flake2-overridden.lock From f1c9d83697e074c32f4efdcb2845bc25edc48f13 Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Sun, 19 Mar 2023 14:12:49 +0100 Subject: [PATCH 06/18] Only allow reference lock files when allow-dirty is set --- src/libexpr/flake/flake.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 64ec4bdd2..0d55ce23e 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -334,6 +334,9 @@ LockedFlake lockFlake( } try { + if (!fetchSettings.allowDirty && lockFlags.referenceLockFilePath) { + throw Error("reference lock file was provided, but the `allow-dirty` setting is set to false"); + } // FIXME: symlink attack auto oldLockFile = LockFile::read( From 3c3bd0767f70309417999ebfa34cb89cc78d4a3e Mon Sep 17 00:00:00 2001 From: Linus Heckemann Date: Sun, 19 Mar 2023 14:14:30 +0100 Subject: [PATCH 07/18] Create test lockfiles in TEST_ROOT --- tests/flakes/flakes.sh | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/flakes/flakes.sh b/tests/flakes/flakes.sh index fcbb01b05..f2e216435 100644 --- a/tests/flakes/flakes.sh +++ b/tests/flakes/flakes.sh @@ -495,12 +495,12 @@ nix store delete $(nix store add-path $badFlakeDir) [[ $(NIX_PATH=flake3=flake:flake3 nix-instantiate --eval '' -A x) = 123 ]] # Test alternate lockfile paths. -nix flake lock $flake2Dir --output-lock-file flake2.lock -cmp $flake2Dir/flake.lock flake2.lock >/dev/null # lockfiles should be identical, since we're referencing flake2's original one +nix flake lock $flake2Dir --output-lock-file $TEST_ROOT/flake2.lock +cmp $flake2Dir/flake.lock $TEST_ROOT/flake2.lock >/dev/null # lockfiles should be identical, since we're referencing flake2's original one -nix flake lock $flake2Dir --output-lock-file flake2-overridden.lock --override-input flake1 git+file://$flake1Dir?rev=$flake1OriginalCommit -expectStderr 1 cmp $flake2Dir/flake.lock flake2-overridden.lock -nix flake metadata $flake2Dir --reference-lock-file flake2-overridden.lock | grepQuiet $flake1OriginalCommit +nix flake lock $flake2Dir --output-lock-file $TEST_ROOT/flake2-overridden.lock --override-input flake1 git+file://$flake1Dir?rev=$flake1OriginalCommit +expectStderr 1 cmp $flake2Dir/flake.lock $TEST_ROOT/flake2-overridden.lock +nix flake metadata $flake2Dir --reference-lock-file $TEST_ROOT/flake2-overridden.lock | grepQuiet $flake1OriginalCommit # reference-lock-file can only be used if allow-dirty is set. -expectStderr 1 nix flake metadata $flake2Dir --no-allow-dirty --reference-lock-file flake2-overridden.lock +expectStderr 1 nix flake metadata $flake2Dir --no-allow-dirty --reference-lock-file $TEST_ROOT/flake2-overridden.lock From 7c4dea3cf3ef8cc0185163e69d9f3cb3a0fc95ac Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Jan 2023 19:48:39 -0500 Subject: [PATCH 08/18] Punt on improper global flags for now See the note in the test. We don't want these flags showing up for commands where they are irrelevant. Eventually, this needs a proper fix, but it need not be a blocker for stabilize: for a quick-n-dirty punt, just put these flags behind the `nix-command` unstable feature. This is fine because they are only relevant for commands which we don't need to stabilize for a while. --- src/nix/main.cc | 3 +++ tests/experimental-features.sh | 12 ++++++++++++ 2 files changed, 15 insertions(+) diff --git a/src/nix/main.cc b/src/nix/main.cc index c79d39459..378e53a9e 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -82,6 +82,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs .description = "Print full build logs on standard error.", .category = loggingCategory, .handler = {[&]() { logger->setPrintBuildLogs(true); }}, + .experimentalFeature = Xp::NixCommand, }); addFlag({ @@ -97,6 +98,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs .description = "Disable substituters and consider all previously downloaded files up-to-date.", .category = miscCategory, .handler = {[&]() { useNet = false; }}, + .experimentalFeature = Xp::NixCommand, }); addFlag({ @@ -104,6 +106,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs .description = "Consider all previously downloaded files out-of-date.", .category = miscCategory, .handler = {[&]() { refresh = true; }}, + .experimentalFeature = Xp::NixCommand, }); } diff --git a/tests/experimental-features.sh b/tests/experimental-features.sh index 3be77d5cc..f718585f3 100644 --- a/tests/experimental-features.sh +++ b/tests/experimental-features.sh @@ -21,3 +21,15 @@ both_ways store gc --help expect 1 nix --experimental-features 'nix-command' show-config --flake-registry 'https://no' nix --experimental-features 'nix-command flakes' show-config --flake-registry 'https://no' + +# Double check this is stable +nix --experimental-features '' --help + +# These 3 arguments are currently given to all commands, which is wrong (as not +# all care). To deal with fixing later, we simply make them require the +# nix-command experimental features --- it so happens that the commands we wish +# stabilizing to do not need them anyways. +for arg in '--print-build-logs' '--offline' '--refresh'; do + nix --experimental-features 'nix-command' "$arg" --help + ! nix --experimental-features '' "$arg" --help +done From ca3937fb82574c765f625b2877cca06c5aeba2c6 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 2 Apr 2023 19:44:44 -0400 Subject: [PATCH 09/18] Mention internal API docs in PR template I think we want to ensure that all new items in headers are documented, and the documentation on modified items is kept up to date. It will take a while to document the backlog of undocumented things, but we can at least ensure that new items don't extend that backlog. --- .github/PULL_REQUEST_TEMPLATE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 60742cf6a..4488c7b7d 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -23,6 +23,7 @@ Maintainers: tick if completed or explain if not relevant - unit tests - `src/*/tests` - integration tests - `tests/nixos/*` - [ ] documentation in the manual + - [ ] documentation in the internal API docs - [ ] code and comments are self-explanatory - [ ] commit message explains why the change was made - [ ] new feature or incompatible change: updated release notes From 9383520b755af3f496f7ecb562d10575367e274d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Sun, 2 Apr 2023 20:28:10 -0400 Subject: [PATCH 10/18] Move `querySubstitutablePathInfos` from `LocalStore` to `Store` The code is not local-store-specific, so we should share it with all stores. More uniform behavior is better, and a less store-specific functionality is more maintainable. This fixes a FIXME added in f73d911628 by @edolstra himself. --- src/libstore/local-store.cc | 48 ------------------------------------- src/libstore/local-store.hh | 3 --- src/libstore/store-api.cc | 48 +++++++++++++++++++++++++++++++++++++ src/libstore/store-api.hh | 2 +- 4 files changed, 49 insertions(+), 52 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index dbba0c91f..f58d90895 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -1134,54 +1134,6 @@ StorePathSet LocalStore::querySubstitutablePaths(const StorePathSet & paths) } -// FIXME: move this, it's not specific to LocalStore. -void LocalStore::querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) -{ - if (!settings.useSubstitutes) return; - for (auto & sub : getDefaultSubstituters()) { - for (auto & path : paths) { - if (infos.count(path.first)) - // Choose first succeeding substituter. - continue; - - auto subPath(path.first); - - // Recompute store path so that we can use a different store root. - if (path.second) { - subPath = makeFixedOutputPathFromCA(path.first.name(), *path.second); - if (sub->storeDir == storeDir) - assert(subPath == path.first); - if (subPath != path.first) - debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path.first), sub->printStorePath(subPath), sub->getUri()); - } else if (sub->storeDir != storeDir) continue; - - debug("checking substituter '%s' for path '%s'", sub->getUri(), sub->printStorePath(subPath)); - try { - auto info = sub->queryPathInfo(subPath); - - if (sub->storeDir != storeDir && !(info->isContentAddressed(*sub) && info->references.empty())) - continue; - - auto narInfo = std::dynamic_pointer_cast( - std::shared_ptr(info)); - infos.insert_or_assign(path.first, SubstitutablePathInfo{ - info->deriver, - info->references, - narInfo ? narInfo->fileSize : 0, - info->narSize}); - } catch (InvalidPath &) { - } catch (SubstituterDisabled &) { - } catch (Error & e) { - if (settings.tryFallback) - logError(e.info()); - else - throw; - } - } - } -} - - void LocalStore::registerValidPath(const ValidPathInfo & info) { registerValidPaths({{info.path, info}}); diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 2eaf451bf..1b668b6fd 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -134,9 +134,6 @@ public: StorePathSet querySubstitutablePaths(const StorePathSet & paths) override; - void querySubstitutablePathInfos(const StorePathCAMap & paths, - SubstitutablePathInfos & infos) override; - bool pathInfoIsUntrusted(const ValidPathInfo &) override; bool realisationIsUntrusted(const Realisation & ) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 60e87918a..8fc3ed46a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -507,6 +507,54 @@ StorePathSet Store::queryDerivationOutputs(const StorePath & path) return outputPaths; } + +void Store::querySubstitutablePathInfos(const StorePathCAMap & paths, SubstitutablePathInfos & infos) +{ + if (!settings.useSubstitutes) return; + for (auto & sub : getDefaultSubstituters()) { + for (auto & path : paths) { + if (infos.count(path.first)) + // Choose first succeeding substituter. + continue; + + auto subPath(path.first); + + // Recompute store path so that we can use a different store root. + if (path.second) { + subPath = makeFixedOutputPathFromCA(path.first.name(), *path.second); + if (sub->storeDir == storeDir) + assert(subPath == path.first); + if (subPath != path.first) + debug("replaced path '%s' with '%s' for substituter '%s'", printStorePath(path.first), sub->printStorePath(subPath), sub->getUri()); + } else if (sub->storeDir != storeDir) continue; + + debug("checking substituter '%s' for path '%s'", sub->getUri(), sub->printStorePath(subPath)); + try { + auto info = sub->queryPathInfo(subPath); + + if (sub->storeDir != storeDir && !(info->isContentAddressed(*sub) && info->references.empty())) + continue; + + auto narInfo = std::dynamic_pointer_cast( + std::shared_ptr(info)); + infos.insert_or_assign(path.first, SubstitutablePathInfo{ + info->deriver, + info->references, + narInfo ? narInfo->fileSize : 0, + info->narSize}); + } catch (InvalidPath &) { + } catch (SubstituterDisabled &) { + } catch (Error & e) { + if (settings.tryFallback) + logError(e.info()); + else + throw; + } + } + } +} + + bool Store::isValidPath(const StorePath & storePath) { { diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 58d7b848e..4d1047380 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -456,7 +456,7 @@ public: * resulting ‘infos’ map. */ virtual void querySubstitutablePathInfos(const StorePathCAMap & paths, - SubstitutablePathInfos & infos) { return; }; + SubstitutablePathInfos & infos); /** * Import a path into the store. From 1c55544a428c61448dcaa4b431f20bde52c3991e Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Sat, 1 Apr 2023 22:05:06 +0200 Subject: [PATCH 11/18] eval: Fix crash on missing printValue tBlackhole case Fixes #8119 --- src/libexpr/eval.cc | 10 ++++++++++ tests/lang.sh | 7 +++++++ 2 files changed, 17 insertions(+) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 584bbc879..22337a3ff 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -173,7 +173,17 @@ void Value::print(const SymbolTable & symbols, std::ostream & str, case tFloat: str << fpoint; break; + case tBlackhole: + // Although we know for sure that it's going to be an infinite recursion + // when this value is accessed _in the current context_, it's likely + // that the user will misinterpret a simpler «infinite recursion» output + // as a definitive statement about the value, while in fact it may be + // a valid value after `builtins.trace` and perhaps some other steps + // have completed. + str << "«potential infinite recursion»"; + break; default: + printError("Nix evaluator internal error: Value::print(): invalid value type %1%", internalType); abort(); } } diff --git a/tests/lang.sh b/tests/lang.sh index cdb4174eb..8170cb39d 100644 --- a/tests/lang.sh +++ b/tests/lang.sh @@ -5,12 +5,19 @@ export NIX_REMOTE=dummy:// export NIX_STORE_DIR=/nix/store nix-instantiate --eval -E 'builtins.trace "Hello" 123' 2>&1 | grepQuiet Hello +nix-instantiate --eval -E 'builtins.trace "Hello" 123' 2>/dev/null | grepQuiet 123 nix-instantiate --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 nix-instantiate --trace-verbose --eval -E 'builtins.traceVerbose "Hello" 123' 2>&1 | grepQuiet Hello nix-instantiate --eval -E 'builtins.traceVerbose "Hello" 123' 2>&1 | grepQuietInverse Hello nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 | grepQuietInverse Hello expectStderr 1 nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw "Foo")' | grepQuiet Hello +nix-instantiate --eval -E 'let x = builtins.trace { x = x; } true; in x' \ + 2>&1 | grepQuiet -E 'trace: { x = «potential infinite recursion»; }' + +nix-instantiate --eval -E 'let x = { repeating = x; tracing = builtins.trace x true; }; in x.tracing'\ + 2>&1 | grepQuiet -F 'trace: { repeating = «repeated»; tracing = «potential infinite recursion»; }' + set +x fail=0 From 4a0b893d5e5d5cbf2f39e52d1651bcff8b4943a9 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 16 Jan 2023 23:01:18 -0500 Subject: [PATCH 12/18] Stuctured command stability Prior to this, there was an ad-hoc whitelist in `main.cc`. Now, every command states its stability. In a future PR, we will adjust the manual to take advantage of this new information in the JSON. (It will be easier to do that once we have some experimental feature docs to link too; see #5930 and #7798.) --- src/libutil/args.cc | 17 +++++++++++++++-- src/libutil/args.hh | 2 ++ src/nix/doctor.cc | 8 ++++++++ src/nix/main.cc | 6 ++---- src/nix/repl.cc | 8 ++++++++ src/nix/upgrade-nix.cc | 8 ++++++++ tests/experimental-features.sh | 11 ++++++++--- 7 files changed, 51 insertions(+), 9 deletions(-) diff --git a/src/libutil/args.cc b/src/libutil/args.cc index fc009592c..081dbeb28 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -236,8 +236,6 @@ nlohmann::json Args::toJSON() auto flags = nlohmann::json::object(); for (auto & [name, flag] : longFlags) { - /* Skip experimental flags when listing flags. */ - if (!experimentalFeatureSettings.isEnabled(flag->experimentalFeature)) continue; auto j = nlohmann::json::object(); if (flag->aliases.count(name)) continue; if (flag->shortName) @@ -249,6 +247,11 @@ nlohmann::json Args::toJSON() j["arity"] = flag->handler.arity; if (!flag->labels.empty()) j["labels"] = flag->labels; + // TODO With C++23 use `std::optional::tranform` + if (auto & xp = flag->experimentalFeature) + j["experimental-feature"] = showExperimentalFeature(*xp); + else + j["experimental-feature"] = nullptr; flags[name] = std::move(j); } @@ -345,6 +348,11 @@ Strings argvToStrings(int argc, char * * argv) return args; } +std::optional Command::experimentalFeature () +{ + return { Xp::NixCommand }; +} + MultiCommand::MultiCommand(const Commands & commands_) : commands(commands_) { @@ -408,6 +416,11 @@ nlohmann::json MultiCommand::toJSON() cat["id"] = command->category(); cat["description"] = trim(categories[command->category()]); j["category"] = std::move(cat); + // TODO With C++23 use `std::optional::tranform` + if (auto xp = command->experimentalFeature()) + cat["experimental-feature"] = showExperimentalFeature(*xp); + else + cat["experimental-feature"] = nullptr; cmds[name] = std::move(j); } diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 07d0d8eae..d90129796 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -236,6 +236,8 @@ struct Command : virtual public Args static constexpr Category catDefault = 0; + virtual std::optional experimentalFeature (); + virtual Category category() { return catDefault; } }; diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc index 7da4549a1..354b03cf6 100644 --- a/src/nix/doctor.cc +++ b/src/nix/doctor.cc @@ -39,6 +39,14 @@ struct CmdDoctor : StoreCommand { bool success = true; + /** + * This command is stable before the others + */ + std::optional experimentalFeature() override + { + return std::nullopt; + } + std::string description() override { return "check your system for potential problems and print a PASS or FAIL for each check"; diff --git a/src/nix/main.cc b/src/nix/main.cc index c2e1dda74..4d4164333 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -423,10 +423,8 @@ void mainWrapped(int argc, char * * argv) if (!args.command) throw UsageError("no subcommand specified"); - if (args.command->first != "repl" - && args.command->first != "doctor" - && args.command->first != "upgrade-nix") - experimentalFeatureSettings.require(Xp::NixCommand); + experimentalFeatureSettings.require( + args.command->second->experimentalFeature()); if (args.useNet && !haveInternet()) { warn("you don't have Internet access; disabling some network-dependent features"); diff --git a/src/nix/repl.cc b/src/nix/repl.cc index 7aa8774e9..bb14f3f99 100644 --- a/src/nix/repl.cc +++ b/src/nix/repl.cc @@ -12,6 +12,14 @@ struct CmdRepl : RawInstallablesCommand evalSettings.pureEval = false; } + /** + * This command is stable before the others + */ + std::optional experimentalFeature() override + { + return std::nullopt; + } + std::vector files; Strings getDefaultFlakeAttrPaths() override diff --git a/src/nix/upgrade-nix.cc b/src/nix/upgrade-nix.cc index 17796d6b8..2295d86d0 100644 --- a/src/nix/upgrade-nix.cc +++ b/src/nix/upgrade-nix.cc @@ -32,6 +32,14 @@ struct CmdUpgradeNix : MixDryRun, StoreCommand }); } + /** + * This command is stable before the others + */ + std::optional experimentalFeature() override + { + return std::nullopt; + } + std::string description() override { return "upgrade Nix to the stable version declared in Nixpkgs"; diff --git a/tests/experimental-features.sh b/tests/experimental-features.sh index f718585f3..a4d55f5f4 100644 --- a/tests/experimental-features.sh +++ b/tests/experimental-features.sh @@ -15,15 +15,20 @@ function both_ways { # Simple case, the configuration effects the running command both_ways show-config -# Complicated case, earlier args effect later args +# Skipping for now, because we actually *do* want these to show up in +# the manual, just be marked experimental. Will reenable once the manual +# generation takes advantage of the JSON metadata on this. -both_ways store gc --help +# both_ways store gc --help expect 1 nix --experimental-features 'nix-command' show-config --flake-registry 'https://no' nix --experimental-features 'nix-command flakes' show-config --flake-registry 'https://no' -# Double check this is stable +# Double check these are stable nix --experimental-features '' --help +nix --experimental-features '' doctor --help +nix --experimental-features '' repl --help +nix --experimental-features '' upgrade-nix --help # These 3 arguments are currently given to all commands, which is wrong (as not # all care). To deal with fixing later, we simply make them require the From 62cacc371f7e0d5745c28a8e1a77ddbc834df31c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 17:59:41 +0200 Subject: [PATCH 13/18] Fix BuildResult.toString() for NoSubstituters --- src/libstore/build-result.hh | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index e50ddbb8c..a12c599d9 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -53,6 +53,7 @@ struct BuildResult case LogLimitExceeded: return "LogLimitExceeded"; case NotDeterministic: return "NotDeterministic"; case ResolvesToAlreadyValid: return "ResolvesToAlreadyValid"; + case NoSubstituters: return "NoSubstituters"; default: return "Unknown"; }; }(); From ed7885017c25940f571dc58e94ca47fa84780f9d Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 18:01:30 +0200 Subject: [PATCH 14/18] Fix systemd logging for lvlNotice: eqv to lvlInfo, not lvlVomit --- src/libutil/logging.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 7cac75ce1..28b385461 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -65,7 +65,7 @@ public: switch (lvl) { case lvlError: c = '3'; break; case lvlWarn: c = '4'; break; - case lvlInfo: c = '5'; break; + case lvlNotice: case lvlInfo: c = '5'; break; case lvlTalkative: case lvlChatty: c = '6'; break; default: c = '7'; } From 3dac4c7874b876dc28d522aa4eddd8b4deb64378 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 18:03:20 +0200 Subject: [PATCH 15/18] Add explicit case statements where -Wswitch-enum would report them --- src/libcmd/repl.cc | 2 ++ src/libexpr/eval.cc | 1 + src/libstore/nar-accessor.cc | 1 + src/libutil/logging.cc | 3 ++- src/nix-store/nix-store.cc | 2 +- 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index e3afb1531..e75ac2cf4 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -1024,6 +1024,8 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m str << v.fpoint; break; + case nThunk: + case nExternal: default: str << ANSI_RED "«unknown»" ANSI_NORMAL; break; diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 22337a3ff..a5e9636ae 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -2337,6 +2337,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v case nFloat: return v1.fpoint == v2.fpoint; + case nThunk: // Must not be left by forceValue default: error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow(); } diff --git a/src/libstore/nar-accessor.cc b/src/libstore/nar-accessor.cc index 9a0003588..f0dfcb19b 100644 --- a/src/libstore/nar-accessor.cc +++ b/src/libstore/nar-accessor.cc @@ -275,6 +275,7 @@ json listNar(ref accessor, const Path & path, bool recurse) obj["type"] = "symlink"; obj["target"] = accessor->readLink(path); break; + case FSAccessor::Type::tMissing: default: throw Error("path '%s' does not exist in NAR", path); } diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 28b385461..741e1d1f5 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -67,7 +67,8 @@ public: case lvlWarn: c = '4'; break; case lvlNotice: case lvlInfo: c = '5'; break; case lvlTalkative: case lvlChatty: c = '6'; break; - default: c = '7'; + case lvlDebug: case lvlVomit: c = '7'; + default: c = '7'; break; } prefix = std::string("<") + c + ">"; } diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index a62cb874f..2d784376d 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -443,7 +443,7 @@ static void opQuery(Strings opFlags, Strings opArgs) break; } - default: + default: case qDefault: abort(); } } From 9470ee877dabcf133469467a23649066e2bcce5c Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 18:15:12 +0200 Subject: [PATCH 16/18] Allow open switch-enum in 5 places --- src/libexpr/eval.cc | 8 ++++++++ src/libexpr/flake/flake.cc | 4 ++++ src/libexpr/primops.cc | 4 ++++ src/libstore/filetransfer.cc | 5 +++++ 4 files changed, 21 insertions(+) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index a5e9636ae..7f2065656 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -239,6 +239,9 @@ std::string_view showType(ValueType type) std::string showType(const Value & v) { + // Allow selecting a subset of enum values + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wswitch-enum" switch (v.internalType) { case tString: return v.string.context ? "a string with context" : "a string"; case tPrimOp: @@ -252,16 +255,21 @@ std::string showType(const Value & v) default: return std::string(showType(v.type())); } + #pragma GCC diagnostic pop } PosIdx Value::determinePos(const PosIdx pos) const { + // Allow selecting a subset of enum values + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wswitch-enum" switch (internalType) { case tAttrs: return attrs->pos; case tLambda: return lambda.fun->pos; case tApp: return app.left->determinePos(pos); default: return pos; } + #pragma GCC diagnostic pop } bool Value::isTrivial() const diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 81e94848a..7c6cd091d 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -125,6 +125,9 @@ static FlakeInput parseFlakeInput(EvalState & state, follows.insert(follows.begin(), lockRootPath.begin(), lockRootPath.end()); input.follows = follows; } else { + // Allow selecting a subset of enum values + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wswitch-enum" switch (attr.value->type()) { case nString: attrs.emplace(state.symbols[attr.name], attr.value->string.s); @@ -139,6 +142,7 @@ static FlakeInput parseFlakeInput(EvalState & state, throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected", state.symbols[attr.name], showType(*attr.value)); } + #pragma GCC diagnostic pop } } catch (Error & e) { e.addTrace( diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 72faeada8..f1bce2fb6 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -577,6 +577,9 @@ struct CompareValues return v1->integer < v2->fpoint; if (v1->type() != v2->type()) state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow(); + // Allow selecting a subset of enum values + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wswitch-enum" switch (v1->type()) { case nInt: return v1->integer < v2->integer; @@ -599,6 +602,7 @@ struct CompareValues } default: state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow(); + #pragma GCC diagnostic pop } } catch (Error & e) { if (!errorCtx.empty()) diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 1ba399a29..2346accbe 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -407,6 +407,10 @@ struct curlFileTransfer : public FileTransfer err = Misc; } else { // Don't bother retrying on certain cURL errors either + + // Allow selecting a subset of enum values + #pragma GCC diagnostic push + #pragma GCC diagnostic ignored "-Wswitch-enum" switch (code) { case CURLE_FAILED_INIT: case CURLE_URL_MALFORMAT: @@ -427,6 +431,7 @@ struct curlFileTransfer : public FileTransfer default: // Shut up warnings break; } + #pragma GCC diagnostic pop } attempt++; From fba7be80eb0fab54b570623d3991739cf3da0759 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 18:11:42 +0200 Subject: [PATCH 17/18] Enable -Werror=switch-enum switch statements must now match all enum values or disable the warning. Explicit is good. This has helped us find two bugs, after solving another one by debugging. From now on, adding to an enum will raise errors where they are not explicitly handled, which is good for productivity, and helps us decide the correct behavior in all usages. Notably still excluded from this though are the cases where the warning is disabled by local pragmas. fromTOML.cc did not build despite a top-level pragma, so I've had to resort to a makefile solution for that. --- local.mk | 4 +++- mk/patterns.mk | 4 ++-- src/libexpr/local.mk | 2 ++ src/libutil/logging.cc | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/local.mk b/local.mk index 6a7074e8e..6951c179e 100644 --- a/local.mk +++ b/local.mk @@ -1,6 +1,8 @@ clean-files += Makefile.config -GLOBAL_CXXFLAGS += -Wno-deprecated-declarations +GLOBAL_CXXFLAGS += -Wno-deprecated-declarations -Werror=switch +# Allow switch-enum to be overridden for files that do not support it, usually because of dependency headers. +ERROR_SWITCH_ENUM = -Werror=switch-enum $(foreach i, config.h $(wildcard src/lib*/*.hh), \ $(eval $(call install-file-in, $(i), $(includedir)/nix, 0644))) diff --git a/mk/patterns.mk b/mk/patterns.mk index 86a724806..c81150260 100644 --- a/mk/patterns.mk +++ b/mk/patterns.mk @@ -1,10 +1,10 @@ $(buildprefix)%.o: %.cc @mkdir -p "$(dir $@)" - $(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) -MMD -MF $(call filename-to-dep, $@) -MP + $(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) $(ERROR_SWITCH_ENUM) -MMD -MF $(call filename-to-dep, $@) -MP $(buildprefix)%.o: %.cpp @mkdir -p "$(dir $@)" - $(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) -MMD -MF $(call filename-to-dep, $@) -MP + $(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) $(ERROR_SWITCH_ENUM) -MMD -MF $(call filename-to-dep, $@) -MP $(buildprefix)%.o: %.c @mkdir -p "$(dir $@)" diff --git a/src/libexpr/local.mk b/src/libexpr/local.mk index 2171e769b..d243b9cec 100644 --- a/src/libexpr/local.mk +++ b/src/libexpr/local.mk @@ -46,3 +46,5 @@ $(foreach i, $(wildcard src/libexpr/flake/*.hh), \ $(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh $(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh + +src/libexpr/primops/fromTOML.o: ERROR_SWITCH_ENUM = diff --git a/src/libutil/logging.cc b/src/libutil/logging.cc index 741e1d1f5..5a2dd99af 100644 --- a/src/libutil/logging.cc +++ b/src/libutil/logging.cc @@ -68,7 +68,7 @@ public: case lvlNotice: case lvlInfo: c = '5'; break; case lvlTalkative: case lvlChatty: c = '6'; break; case lvlDebug: case lvlVomit: c = '7'; - default: c = '7'; break; + default: c = '7'; break; // should not happen, and missing enum case is reported by -Werror=switch-enum } prefix = std::string("<") + c + ">"; } From bf2c5c3958c258584ac66519124d05ccf446aaeb Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 3 Apr 2023 18:38:11 +0200 Subject: [PATCH 18/18] nix-store.cc: Refactor, remove qDefault --- src/nix-store/nix-store.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 2d784376d..7035e6a7b 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -277,17 +277,17 @@ static void printTree(const StorePath & path, static void opQuery(Strings opFlags, Strings opArgs) { enum QueryType - { qDefault, qOutputs, qRequisites, qReferences, qReferrers + { qOutputs, qRequisites, qReferences, qReferrers , qReferrersClosure, qDeriver, qBinding, qHash, qSize , qTree, qGraph, qGraphML, qResolve, qRoots }; - QueryType query = qDefault; + std::optional query; bool useOutput = false; bool includeOutputs = false; bool forceRealise = false; std::string bindingName; for (auto & i : opFlags) { - QueryType prev = query; + std::optional prev = query; if (i == "--outputs") query = qOutputs; else if (i == "--requisites" || i == "-R") query = qRequisites; else if (i == "--references") query = qReferences; @@ -312,15 +312,15 @@ static void opQuery(Strings opFlags, Strings opArgs) else if (i == "--force-realise" || i == "--force-realize" || i == "-f") forceRealise = true; else if (i == "--include-outputs") includeOutputs = true; else throw UsageError("unknown flag '%1%'", i); - if (prev != qDefault && prev != query) + if (prev && prev != query) throw UsageError("query type '%1%' conflicts with earlier flag", i); } - if (query == qDefault) query = qOutputs; + if (!query) query = qOutputs; RunPager pager; - switch (query) { + switch (*query) { case qOutputs: { for (auto & i : opArgs) { @@ -443,7 +443,7 @@ static void opQuery(Strings opFlags, Strings opArgs) break; } - default: case qDefault: + default: abort(); } }