From ad337c8697099ac9deb6e0ac16ea91d8acc51e4f Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 12 Feb 2021 17:33:28 +0000 Subject: [PATCH 01/25] Deeper `Command` hierarchy to remove redundancy Simply put, we now have `StorePathCommand : public StorePathsCommand` so `StorePathCommand` doesn't reimplement work. --- src/libcmd/command.cc | 4 +--- src/libcmd/command.hh | 6 +++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/libcmd/command.cc b/src/libcmd/command.cc index efdc98d5a..d29954f67 100644 --- a/src/libcmd/command.cc +++ b/src/libcmd/command.cc @@ -118,10 +118,8 @@ void StorePathsCommand::run(ref store, std::vector paths) run(store, std::move(storePaths)); } -void StorePathCommand::run(ref store) +void StorePathCommand::run(ref store, std::vector storePaths) { - auto storePaths = toStorePaths(store, Realise::Nothing, operateOn, installables); - if (storePaths.size() != 1) throw UsageError("this command requires exactly one store path"); diff --git a/src/libcmd/command.hh b/src/libcmd/command.hh index 8c0b3a94a..c02193924 100644 --- a/src/libcmd/command.hh +++ b/src/libcmd/command.hh @@ -177,13 +177,13 @@ struct StorePathsCommand : public RealisedPathsCommand }; /* A command that operates on exactly one store path. */ -struct StorePathCommand : public InstallablesCommand +struct StorePathCommand : public StorePathsCommand { - using StoreCommand::run; + using StorePathsCommand::run; virtual void run(ref store, const StorePath & storePath) = 0; - void run(ref store) override; + void run(ref store, std::vector storePaths) override; }; /* A helper class for registering commands globally. */ From 35129884f9348f068d538e67bb559cc6104f714e Mon Sep 17 00:00:00 2001 From: Mauricio Scheffer Date: Tue, 16 Feb 2021 23:19:42 +0000 Subject: [PATCH 02/25] Fix Haskell example http://nixos.org redirects to https://nixos.org and apparently the HTTP library doesn't follow the redirect, so the output is empty. When defining https in the request it crashes because the library doesn't seem to support https. So this switches the example to a different http library. --- doc/manual/src/command-ref/nix-shell.md | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/doc/manual/src/command-ref/nix-shell.md b/doc/manual/src/command-ref/nix-shell.md index 88b675e71..938d56e6e 100644 --- a/doc/manual/src/command-ref/nix-shell.md +++ b/doc/manual/src/command-ref/nix-shell.md @@ -232,22 +232,23 @@ terraform apply > in a nix-shell shebang. Finally, using the merging of multiple nix-shell shebangs the following -Haskell script uses a specific branch of Nixpkgs/NixOS (the 18.03 stable +Haskell script uses a specific branch of Nixpkgs/NixOS (the 20.03 stable branch): ```haskell #! /usr/bin/env nix-shell -#! nix-shell -i runghc -p "haskellPackages.ghcWithPackages (ps: [ps.HTTP ps.tagsoup])" -#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/nixos-18.03.tar.gz +#! nix-shell -i runghc -p "haskellPackages.ghcWithPackages (ps: [ps.download-curl ps.tagsoup])" +#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-20.03.tar.gz -import Network.HTTP +import Network.Curl.Download import Text.HTML.TagSoup +import Data.Either +import Data.ByteString.Char8 (unpack) -- Fetch nixos.org and print all hrefs. main = do - resp <- Network.HTTP.simpleHTTP (getRequest "http://nixos.org/") - body <- getResponseBody resp - let tags = filter (isTagOpenName "a") $ parseTags body + resp <- openURI "https://nixos.org/" + let tags = filter (isTagOpenName "a") $ parseTags $ unpack $ fromRight undefined resp let tags' = map (fromAttrib "href") tags mapM_ putStrLn $ filter (/= "") tags' ``` From 5f4701e70d35bb9ea2fb659caf387a30001e28ce Mon Sep 17 00:00:00 2001 From: Mauricio Scheffer Date: Tue, 16 Feb 2021 23:27:04 +0000 Subject: [PATCH 03/25] Update doc/manual/src/command-ref/nix-shell.md Co-authored-by: Cole Helbling --- doc/manual/src/command-ref/nix-shell.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/manual/src/command-ref/nix-shell.md b/doc/manual/src/command-ref/nix-shell.md index 938d56e6e..54812a49f 100644 --- a/doc/manual/src/command-ref/nix-shell.md +++ b/doc/manual/src/command-ref/nix-shell.md @@ -238,7 +238,7 @@ branch): ```haskell #! /usr/bin/env nix-shell #! nix-shell -i runghc -p "haskellPackages.ghcWithPackages (ps: [ps.download-curl ps.tagsoup])" -#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs-channels/archive/nixos-20.03.tar.gz +#! nix-shell -I nixpkgs=https://github.com/NixOS/nixpkgs/archive/nixos-20.03.tar.gz import Network.Curl.Download import Text.HTML.TagSoup From 6042febfce3011aaa5e3c369ea14a0d93ad2880e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 15:30:49 +0100 Subject: [PATCH 04/25] Restore warning about 'nix' being experimental Fixes #4552. --- doc/manual/generate-manpage.nix | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/manual/generate-manpage.nix b/doc/manual/generate-manpage.nix index a563c31f8..964b57086 100644 --- a/doc/manual/generate-manpage.nix +++ b/doc/manual/generate-manpage.nix @@ -7,7 +7,10 @@ let showCommand = { command, def, filename }: - "# Name\n\n" + '' + **Warning**: This program is **experimental** and its interface is subject to change. + '' + + "# Name\n\n" + "`${command}` - ${def.description}\n\n" + "# Synopsis\n\n" + showSynopsis { inherit command; args = def.args; } From 063de66909ff1b20394cdebdca1ef62bb6ca1d51 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 16:42:03 +0100 Subject: [PATCH 05/25] nix develop: Fix quoted string handling Fixes #4540. --- src/nix/develop.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 3c44fdb0e..0938cbe5b 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -59,7 +59,7 @@ BuildEnvironment readEnvironment(const Path & path) R"re((?:\$?"(?:[^"\\]|\\[$`"\\\n])*"))re"; static std::string squotedStringRegex = - R"re((?:\$?'(?:[^'\\]|\\[abeEfnrtv\\'"?])*'))re"; + R"re((?:\$?(?:'(?:[^'\\]|\\[abeEfnrtv\\'"?])*'|\\')+))re"; static std::string indexedArrayRegex = R"re((?:\(( *\[[0-9]+\]="(?:[^"\\]|\\.)*")*\)))re"; From cced73496b835b545be91cbebc4f89f61a7b106f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 16:53:19 +0100 Subject: [PATCH 06/25] nix flake show: Handle 'overlays' output Fixes #4542. --- src/nix/flake.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 4cd7d77a0..091af8084 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -880,7 +880,8 @@ struct CmdFlakeShow : FlakeCommand || attrPath[0] == "nixosConfigurations" || attrPath[0] == "nixosModules" || attrPath[0] == "defaultApp" - || attrPath[0] == "templates")) + || attrPath[0] == "templates" + || attrPath[0] == "overlays")) || ((attrPath.size() == 1 || attrPath.size() == 2) && (attrPath[0] == "checks" || attrPath[0] == "packages" @@ -943,7 +944,8 @@ struct CmdFlakeShow : FlakeCommand else { logger->cout("%s: %s", headerPrefix, - attrPath.size() == 1 && attrPath[0] == "overlay" ? "Nixpkgs overlay" : + (attrPath.size() == 1 && attrPath[0] == "overlay") + || (attrPath.size() == 2 && attrPath[0] == "overlays") ? "Nixpkgs overlay" : attrPath.size() == 2 && attrPath[0] == "nixosConfigurations" ? "NixOS configuration" : attrPath.size() == 2 && attrPath[0] == "nixosModules" ? "NixOS module" : ANSI_YELLOW "unknown" ANSI_NORMAL); From f33878b6562c746d5865a86e64f02c75feaf5b3e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 17:11:14 +0100 Subject: [PATCH 07/25] Make 'nix --version -vv' work Fixes #3743. --- src/nix/main.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/nix/main.cc b/src/nix/main.cc index ef5e41a55..5f4eb8918 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -61,6 +61,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs bool printBuildLogs = false; bool useNet = true; bool refresh = false; + bool showVersion = false; NixArgs() : MultiCommand(RegisterCommand::getCommandsFor({})), MixCommonArgs("nix") { @@ -87,7 +88,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs addFlag({ .longName = "version", .description = "Show version information.", - .handler = {[&]() { if (!completions) printVersion(programName); }}, + .handler = {[&]() { showVersion = true; }}, }); addFlag({ @@ -280,6 +281,11 @@ void mainWrapped(int argc, char * * argv) initPlugins(); + if (args.showVersion) { + printVersion(programName); + return; + } + if (!args.command) throw UsageError("no subcommand specified"); From 13897afbe6cf7ef8013c0c94109696bb7b13d0c0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 17:32:10 +0100 Subject: [PATCH 08/25] Throw an error if --arg / --argstr is used with a flake Fixes #3949. --- src/libcmd/installables.cc | 24 ++++++++++++++++++++++-- src/libcmd/installables.hh | 12 +++++++----- src/nix/bundle.cc | 2 +- src/nix/develop.cc | 1 + src/nix/flake.cc | 2 +- src/nix/profile.cc | 8 +++++++- 6 files changed, 39 insertions(+), 10 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 9ad02b5f0..4739dc974 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -496,6 +496,23 @@ static std::string showAttrPaths(const std::vector & paths) return s; } +InstallableFlake::InstallableFlake( + SourceExprCommand * cmd, + ref state, + FlakeRef && flakeRef, + Strings && attrPaths, + Strings && prefixes, + const flake::LockFlags & lockFlags) + : InstallableValue(state), + flakeRef(flakeRef), + attrPaths(attrPaths), + prefixes(prefixes), + lockFlags(lockFlags) +{ + if (cmd && cmd->getAutoArgs(*state)->size()) + throw UsageError("'--arg' and '--argstr' are incompatible with flakes"); +} + std::tuple InstallableFlake::toDerivation() { auto lockedFlake = getLockedFlake(); @@ -628,9 +645,12 @@ std::vector> SourceExprCommand::parseInstallables( try { auto [flakeRef, fragment] = parseFlakeRefWithFragment(s, absPath(".")); result.push_back(std::make_shared( - getEvalState(), std::move(flakeRef), + this, + getEvalState(), + std::move(flakeRef), fragment == "" ? getDefaultFlakeAttrPaths() : Strings{fragment}, - getDefaultFlakeAttrPathPrefixes(), lockFlags)); + getDefaultFlakeAttrPathPrefixes(), + lockFlags)); continue; } catch (...) { ex = std::current_exception(); diff --git a/src/libcmd/installables.hh b/src/libcmd/installables.hh index f37b3f829..b714f097b 100644 --- a/src/libcmd/installables.hh +++ b/src/libcmd/installables.hh @@ -104,11 +104,13 @@ struct InstallableFlake : InstallableValue const flake::LockFlags & lockFlags; mutable std::shared_ptr _lockedFlake; - InstallableFlake(ref state, FlakeRef && flakeRef, - Strings && attrPaths, Strings && prefixes, const flake::LockFlags & lockFlags) - : InstallableValue(state), flakeRef(flakeRef), attrPaths(attrPaths), - prefixes(prefixes), lockFlags(lockFlags) - { } + InstallableFlake( + SourceExprCommand * cmd, + ref state, + FlakeRef && flakeRef, + Strings && attrPaths, + Strings && prefixes, + const flake::LockFlags & lockFlags); std::string what() override { return flakeRef.to_string() + "#" + *attrPaths.begin(); } diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 1789e4598..48f4eb6e3 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -74,7 +74,7 @@ struct CmdBundle : InstallableCommand auto [bundlerFlakeRef, bundlerName] = parseFlakeRefWithFragment(bundler, absPath(".")); const flake::LockFlags lockFlags{ .writeLockFile = false }; - auto bundler = InstallableFlake( + auto bundler = InstallableFlake(this, evalState, std::move(bundlerFlakeRef), Strings{bundlerName == "" ? "defaultBundler" : bundlerName}, Strings({"bundlers."}), lockFlags); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 0938cbe5b..d0b140570 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -443,6 +443,7 @@ struct CmdDevelop : Common, MixEnvironment auto state = getEvalState(); auto bashInstallable = std::make_shared( + this, state, installable->nixpkgsFlakeRef(), Strings{"bashInteractive"}, diff --git a/src/nix/flake.cc b/src/nix/flake.cc index 091af8084..b9cde5d6d 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -595,7 +595,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto [templateFlakeRef, templateName] = parseFlakeRefWithFragment(templateUrl, absPath(".")); - auto installable = InstallableFlake( + auto installable = InstallableFlake(nullptr, evalState, std::move(templateFlakeRef), Strings{templateName == "" ? "defaultTemplate" : templateName}, Strings(attrsPathPrefixes), lockFlags); diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 827f8be5a..4d275f577 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -399,7 +399,13 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf Activity act(*logger, lvlChatty, actUnknown, fmt("checking '%s' for updates", element.source->attrPath)); - InstallableFlake installable(getEvalState(), FlakeRef(element.source->originalRef), {element.source->attrPath}, {}, lockFlags); + InstallableFlake installable( + this, + getEvalState(), + FlakeRef(element.source->originalRef), + {element.source->attrPath}, + {}, + lockFlags); auto [attrPath, resolvedRef, drv] = installable.toDerivation(); From 7bd9898d5ca72ed136032590745c56826317a328 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 17:54:13 +0100 Subject: [PATCH 09/25] nix run: Allow program name to be set in meta.mainProgram This is useful when the program name doesn't match the package name (e.g. ripgrep vs rg). Fixes #4498. --- src/nix/app.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/src/nix/app.cc b/src/nix/app.cc index 80acbf658..cf147c631 100644 --- a/src/nix/app.cc +++ b/src/nix/app.cc @@ -12,11 +12,16 @@ App Installable::toApp(EvalState & state) auto type = cursor->getAttr("type")->getString(); + auto checkProgram = [&](const Path & program) + { + if (!state.store->isInStore(program)) + throw Error("app program '%s' is not in the Nix store", program); + }; + if (type == "app") { auto [program, context] = cursor->getAttr("program")->getStringWithContext(); - if (!state.store->isInStore(program)) - throw Error("app program '%s' is not in the Nix store", program); + checkProgram(program); std::vector context2; for (auto & [path, name] : context) @@ -33,9 +38,17 @@ App Installable::toApp(EvalState & state) auto outPath = cursor->getAttr(state.sOutPath)->getString(); auto outputName = cursor->getAttr(state.sOutputName)->getString(); auto name = cursor->getAttr(state.sName)->getString(); + auto aMeta = cursor->maybeGetAttr("meta"); + auto aMainProgram = aMeta ? aMeta->maybeGetAttr("mainProgram") : nullptr; + auto mainProgram = + aMainProgram + ? aMainProgram->getString() + : DrvName(name).name; + auto program = outPath + "/bin/" + mainProgram; + checkProgram(program); return App { .context = { { drvPath, {outputName} } }, - .program = outPath + "/bin/" + DrvName(name).name, + .program = program, }; } From 1b578255245e2e1347059ad7d9171cf822c723a8 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 17 Feb 2021 17:58:40 +0100 Subject: [PATCH 10/25] Document meta.mainProgram Issue #4498. --- src/nix/run.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/nix/run.md b/src/nix/run.md index c178e8b13..a76750376 100644 --- a/src/nix/run.md +++ b/src/nix/run.md @@ -43,9 +43,10 @@ program specified by the app definition. If *installable* evaluates to a derivation, it will try to execute the program `/bin/`, where *out* is the primary output store -path of the derivation and *name* is the name part of the value of the -`name` attribute of the derivation (e.g. if `name` is set to -`hello-1.10`, it will run `$out/bin/hello`). +path of the derivation and *name* is the `meta.mainProgram` attribute +of the derivation if it exists, and otherwise the name part of the +value of the `name` attribute of the derivation (e.g. if `name` is set +to `hello-1.10`, it will run `$out/bin/hello`). # Flake output attributes From cd44c0af71ace2eb8056c2b26b9249a5aa102b41 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 18 Feb 2021 19:22:37 +0100 Subject: [PATCH 11/25] Increase default stack size on Linux Workaround for #4550. --- src/nix/main.cc | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/nix/main.cc b/src/nix/main.cc index 5f4eb8918..1b68cf15b 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -17,6 +17,10 @@ #include #include +#if __linux__ +#include +#endif + #include extern std::string chrootHelperName; @@ -325,6 +329,17 @@ void mainWrapped(int argc, char * * argv) int main(int argc, char * * argv) { + // Increase the default stack size for the evaluator and for + // libstdc++'s std::regex. + #if __linux__ + rlim_t stackSize = 64 * 1024 * 1024; + struct rlimit limit; + if (getrlimit(RLIMIT_STACK, &limit) == 0 && limit.rlim_cur < stackSize) { + limit.rlim_cur = stackSize; + setrlimit(RLIMIT_STACK, &limit); + } + #endif + return nix::handleExceptions(argv[0], [&]() { nix::mainWrapped(argc, argv); }); From 263f6dbd1cef6eb9560737f6daf963f8968a65d8 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Dec 2020 20:38:37 +0100 Subject: [PATCH 12/25] Don't crash nix-build when not all outputs are realised Change the `nix-build` logic for linking/printing the output paths to allow for some outputs to be missing. This might happen when the toplevel derivation didn't have to be built, either because all the required outputs were already there, or because they have all been substituted. --- src/nix-build/nix-build.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 361f9730d..d975cd16d 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -518,9 +518,11 @@ static void main_nix_build(int argc, char * * argv) if (counter) drvPrefix += fmt("-%d", counter + 1); - auto builtOutputs = store->queryDerivationOutputMap(drvPath); + auto builtOutputs = store->queryPartialDerivationOutputMap(drvPath); - auto outputPath = builtOutputs.at(outputName); + auto maybeOutputPath = builtOutputs.at(outputName); + assert(maybeOutputPath); + auto outputPath = *maybeOutputPath; if (auto store2 = store.dynamic_pointer_cast()) { std::string symlink = drvPrefix; From be1b5c4e59ca1c3504a44e2058807f7207432846 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 8 Dec 2020 18:11:33 +0100 Subject: [PATCH 13/25] Test the garbage collection of CA derivations Simple test to ensure that `nix-build && nix-collect-garbage && nix-build -j0` works as it should --- tests/content-addressed.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/content-addressed.sh b/tests/content-addressed.sh index e8ac88609..7e32e1f28 100644 --- a/tests/content-addressed.sh +++ b/tests/content-addressed.sh @@ -48,6 +48,10 @@ testCutoff () { testGC () { nix-instantiate --experimental-features ca-derivations ./content-addressed.nix -A rootCA --arg seed 5 nix-collect-garbage --experimental-features ca-derivations --option keep-derivations true + clearStore + buildAttr rootCA 1 --out-link $TEST_ROOT/rootCA + nix-collect-garbage --experimental-features ca-derivations + buildAttr rootCA 1 -j0 } testNixCommand () { From 87c8d3d702123528ac068bb703232e24431c535e Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Jan 2021 10:03:05 +0100 Subject: [PATCH 14/25] Register the realisations for unresolved drvs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a build is done, get back to the original derivation, and register all the newly built outputs for this derivation. This allows Nix to work properly with derivations that don't have all their build inputs available − thus allowing garbage collection and (once it's implemented) binary substitution --- src/libstore/build/derivation-goal.cc | 54 ++++++++++++++++++++++++++- src/libstore/build/derivation-goal.hh | 3 ++ src/libstore/derivations.cc | 9 ++++- src/libstore/local-store.cc | 2 +- src/libstore/local-store.hh | 2 +- src/libstore/store-api.cc | 15 +------- src/libstore/store-api.hh | 6 --- 7 files changed, 67 insertions(+), 24 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index eeaec4f2c..315cf3f0a 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -506,6 +506,7 @@ void DerivationGoal::inputsRealised() Derivation drvResolved { *std::move(attempt) }; auto pathResolved = writeDerivation(worker.store, drvResolved); + resolvedDrv = drvResolved; auto msg = fmt("Resolved derivation: '%s' -> '%s'", worker.store.printStorePath(drvPath), @@ -1019,7 +1020,45 @@ void DerivationGoal::buildDone() } void DerivationGoal::resolvedFinished() { - done(BuildResult::Built); + assert(resolvedDrv); + + // If the derivation was originally a full `Derivation` (and not just + // a `BasicDerivation`, we must retrieve it because the `staticOutputHashes` + // will be wrong otherwise + Derivation fullDrv = *drv; + if (auto upcasted = dynamic_cast(drv.get())) + fullDrv = *upcasted; + + auto originalHashes = staticOutputHashes(worker.store, fullDrv); + auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv); + + // `wantedOutputs` might be empty, which means “all the outputs” + auto realWantedOutputs = wantedOutputs; + if (realWantedOutputs.empty()) + realWantedOutputs = resolvedDrv->outputNames(); + + for (auto & wantedOutput : realWantedOutputs) { + assert(originalHashes.count(wantedOutput) != 0); + assert(resolvedHashes.count(wantedOutput) != 0); + auto realisation = worker.store.queryRealisation( + DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} + ); + // We've just built it, but maybe the build failed, in which case the + // realisation won't be there + if (realisation) { + auto newRealisation = *realisation; + newRealisation.id = DrvOutput{originalHashes.at(wantedOutput), wantedOutput}; + worker.store.registerDrvOutput(newRealisation); + } else { + // If we don't have a realisation, then it must mean that something + // failed when building the resolved drv + assert(!result.success()); + } + } + + // This is potentially a bit fishy in terms of error reporting. Not sure + // how to do it in a cleaner way + amDone(nrFailed == 0 ? ecSuccess : ecFailed, ex); } HookReply DerivationGoal::tryBuildHook() @@ -3804,6 +3843,19 @@ void DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } + if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + Derivation fullDrv = *drv; + if (auto upcasted = dynamic_cast(drv.get())) + fullDrv = *upcasted; + auto outputHashes = staticOutputHashes(worker.store, fullDrv); + if (auto real = worker.store.queryRealisation( + DrvOutput{outputHashes.at(i.first), i.first})) { + info.known = { + .path = real->outPath, + .status = PathStatus::Valid, + }; + } + } initialOutputs.insert_or_assign(i.first, info); } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 8ee0be9e1..b7b85c85d 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -48,6 +48,9 @@ struct DerivationGoal : public Goal /* The path of the derivation. */ StorePath drvPath; + /* The path of the corresponding resolved derivation */ + std::optional resolvedDrv; + /* The specific outputs that we need to build. Empty means all of them. */ StringSet wantedOutputs; diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7466c7d41..4b774c42a 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -756,8 +756,13 @@ std::optional Derivation::tryResolveUncached(Store & store) { StringSet newOutputNames; for (auto & outputName : input.second) { auto actualPathOpt = inputDrvOutputs.at(outputName); - if (!actualPathOpt) + if (!actualPathOpt) { + warn("Input %s!%s missing, aborting the resolving", + store.printStorePath(input.first), + outputName + ); return std::nullopt; + } auto actualPath = *actualPathOpt; inputRewrites.emplace( downstreamPlaceholder(store, input.first, outputName), @@ -782,6 +787,8 @@ std::optional Derivation::tryResolve(Store& store, const StoreP // This is quite dirty and leaky, but will disappear once #4340 is merged static Sync>> resolutionsCache; + debug("Trying to resolve %s", store.printStorePath(drvPath)); + { auto resolutions = resolutionsCache.lock(); auto resolvedDrvIter = resolutions->find(drvPath); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index f45af2bac..e06c47cde 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -883,7 +883,7 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) std::map> -LocalStore::queryDerivationOutputMapNoResolve(const StorePath& path_) +LocalStore::queryPartialDerivationOutputMap(const StorePath& path_) { auto path = path_; auto outputs = retrySQLite>>([&]() { diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 9d235ba0a..780cc0f07 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -127,7 +127,7 @@ public: StorePathSet queryValidDerivers(const StorePath & path) override; - std::map> queryDerivationOutputMapNoResolve(const StorePath & path) override; + std::map> queryPartialDerivationOutputMap(const StorePath & path) override; std::optional queryPathFromHashPart(const std::string & hashPart) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 37c11fe86..2658f7617 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -366,7 +366,7 @@ bool Store::PathInfoCacheValue::isKnownNow() return std::chrono::steady_clock::now() < time_point + ttl; } -std::map> Store::queryDerivationOutputMapNoResolve(const StorePath & path) +std::map> Store::queryPartialDerivationOutputMap(const StorePath & path) { std::map> outputs; auto drv = readInvalidDerivation(path); @@ -376,19 +376,6 @@ std::map> Store::queryDerivationOutputMapN return outputs; } -std::map> Store::queryPartialDerivationOutputMap(const StorePath & path) -{ - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - auto resolvedDrv = Derivation::tryResolve(*this, path); - if (resolvedDrv) { - auto resolvedDrvPath = writeDerivation(*this, *resolvedDrv, NoRepair, true); - if (isValidPath(resolvedDrvPath)) - return queryDerivationOutputMapNoResolve(resolvedDrvPath); - } - } - return queryDerivationOutputMapNoResolve(path); -} - OutputPathMap Store::queryDerivationOutputMap(const StorePath & path) { auto resp = queryPartialDerivationOutputMap(path); OutputPathMap result; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 9e98eb8f9..6dcd43ed1 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -415,12 +415,6 @@ public: `std::nullopt`. */ virtual std::map> queryPartialDerivationOutputMap(const StorePath & path); - /* - * Similar to `queryPartialDerivationOutputMap`, but doesn't try to resolve - * the derivation - */ - virtual std::map> queryDerivationOutputMapNoResolve(const StorePath & path); - /* Query the mapping outputName=>outputPath for the given derivation. Assume every output has a mapping and throw an exception otherwise. */ OutputPathMap queryDerivationOutputMap(const StorePath & path); From 93d9eb78a0733c5adcbc6ee7b8a257605ae4a32f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 4 Feb 2021 11:12:24 +0100 Subject: [PATCH 15/25] Syntactic fixes Co-authored-by: Eelco Dolstra --- src/libstore/derivations.cc | 2 +- src/libstore/local-store.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 4b774c42a..36993ffc2 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -787,7 +787,7 @@ std::optional Derivation::tryResolve(Store& store, const StoreP // This is quite dirty and leaky, but will disappear once #4340 is merged static Sync>> resolutionsCache; - debug("Trying to resolve %s", store.printStorePath(drvPath)); + debug("trying to resolve %s", store.printStorePath(drvPath)); { auto resolutions = resolutionsCache.lock(); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index e06c47cde..0962418dd 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -883,7 +883,7 @@ StorePathSet LocalStore::queryValidDerivers(const StorePath & path) std::map> -LocalStore::queryPartialDerivationOutputMap(const StorePath& path_) +LocalStore::queryPartialDerivationOutputMap(const StorePath & path_) { auto path = path_; auto outputs = retrySQLite>>([&]() { From 0bfbd043699908bcaff1493c733ab4798b642b82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Thu, 4 Feb 2021 11:13:38 +0100 Subject: [PATCH 16/25] Don't expose the "bang" drvoutput syntax It's not fixed nor useful atm, so better keep it hidden Co-authored-by: Eelco Dolstra --- src/libstore/derivations.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 36993ffc2..7807089ca 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -757,9 +757,9 @@ std::optional Derivation::tryResolveUncached(Store & store) { for (auto & outputName : input.second) { auto actualPathOpt = inputDrvOutputs.at(outputName); if (!actualPathOpt) { - warn("Input %s!%s missing, aborting the resolving", - store.printStorePath(input.first), - outputName + warn("output %s of input %s missing, aborting the resolving", + outputName, + store.printStorePath(input.first) ); return std::nullopt; } From 4bc28c44f258f4f8c8a3935d1acf746f6abe3d8f Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 4 Feb 2021 14:41:49 +0100 Subject: [PATCH 17/25] Store the output hashes in the initialOutputs of the drv goal That way we 1. Don't have to recompute them several times 2. Can compute them in a place where we know the type of the parent derivation, meaning that we don't need the casting dance we had before --- src/libstore/build/derivation-goal.cc | 49 ++++++++++++++++----------- src/libstore/build/derivation-goal.hh | 1 + 2 files changed, 30 insertions(+), 20 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 315cf3f0a..d8a89a2d0 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -124,6 +124,17 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation , buildMode(buildMode) { this->drv = std::make_unique(BasicDerivation(drv)); + + auto outputHashes = staticOutputHashes(worker.store, drv); + for (auto &[outputName, outputHash] : outputHashes) + initialOutputs.insert({ + outputName, + InitialOutput{ + .wanted = true, // Will be refined later + .outputHash = outputHash + } + }); + state = &DerivationGoal::haveDerivation; name = fmt( "building of '%s' from in-memory derivation", @@ -258,8 +269,20 @@ void DerivationGoal::loadDerivation() assert(worker.store.isValidPath(drvPath)); + auto fullDrv = new Derivation(worker.store.derivationFromPath(drvPath)); + + auto outputHashes = staticOutputHashes(worker.store, *fullDrv); + for (auto &[outputName, outputHash] : outputHashes) + initialOutputs.insert({ + outputName, + InitialOutput{ + .wanted = true, // Will be refined later + .outputHash = outputHash + } + }); + /* Get the derivation. */ - drv = std::unique_ptr(new Derivation(worker.store.derivationFromPath(drvPath))); + drv = std::unique_ptr(fullDrv); haveDerivation(); } @@ -1022,14 +1045,6 @@ void DerivationGoal::buildDone() void DerivationGoal::resolvedFinished() { assert(resolvedDrv); - // If the derivation was originally a full `Derivation` (and not just - // a `BasicDerivation`, we must retrieve it because the `staticOutputHashes` - // will be wrong otherwise - Derivation fullDrv = *drv; - if (auto upcasted = dynamic_cast(drv.get())) - fullDrv = *upcasted; - - auto originalHashes = staticOutputHashes(worker.store, fullDrv); auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv); // `wantedOutputs` might be empty, which means “all the outputs” @@ -1038,7 +1053,7 @@ void DerivationGoal::resolvedFinished() { realWantedOutputs = resolvedDrv->outputNames(); for (auto & wantedOutput : realWantedOutputs) { - assert(originalHashes.count(wantedOutput) != 0); + assert(initialOutputs.count(wantedOutput) != 0); assert(resolvedHashes.count(wantedOutput) != 0); auto realisation = worker.store.queryRealisation( DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} @@ -1047,7 +1062,7 @@ void DerivationGoal::resolvedFinished() { // realisation won't be there if (realisation) { auto newRealisation = *realisation; - newRealisation.id = DrvOutput{originalHashes.at(wantedOutput), wantedOutput}; + newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput}; worker.store.registerDrvOutput(newRealisation); } else { // If we don't have a realisation, then it must mean that something @@ -3829,9 +3844,8 @@ void DerivationGoal::checkPathValidity() { bool checkHash = buildMode == bmRepair; for (auto & i : queryPartialDerivationOutputMap()) { - InitialOutput info { - .wanted = wantOutput(i.first, wantedOutputs), - }; + InitialOutput & info = initialOutputs.at(i.first); + info.wanted = wantOutput(i.first, wantedOutputs); if (i.second) { auto outputPath = *i.second; info.known = { @@ -3844,19 +3858,14 @@ void DerivationGoal::checkPathValidity() }; } if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - Derivation fullDrv = *drv; - if (auto upcasted = dynamic_cast(drv.get())) - fullDrv = *upcasted; - auto outputHashes = staticOutputHashes(worker.store, fullDrv); if (auto real = worker.store.queryRealisation( - DrvOutput{outputHashes.at(i.first), i.first})) { + DrvOutput{initialOutputs.at(i.first).outputHash, i.first})) { info.known = { .path = real->outPath, .status = PathStatus::Valid, }; } } - initialOutputs.insert_or_assign(i.first, info); } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index b7b85c85d..761100d3a 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -37,6 +37,7 @@ struct InitialOutputStatus { struct InitialOutput { bool wanted; + Hash outputHash; std::optional known; }; From f483b623e98a0feb2568e5be076b533c5838ba32 Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 16 Feb 2021 08:16:12 +0100 Subject: [PATCH 18/25] Remove the drv resolution caching mechanism It isn't needed anymore now that don't need to eagerly resolve everything like we used to do. So we can safely get rid of it --- src/libstore/derivations.cc | 34 +--------------------------------- src/libstore/derivations.hh | 4 ---- 2 files changed, 1 insertion(+), 37 deletions(-) diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7807089ca..6d0742b4f 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -745,7 +745,7 @@ static void rewriteDerivation(Store & store, BasicDerivation & drv, const String } -std::optional Derivation::tryResolveUncached(Store & store) { +std::optional Derivation::tryResolve(Store & store) { BasicDerivation resolved { *this }; // Input paths that we'll want to rewrite in the derivation @@ -776,36 +776,4 @@ std::optional Derivation::tryResolveUncached(Store & store) { return resolved; } -std::optional Derivation::tryResolve(Store& store) -{ - auto drvPath = writeDerivation(store, *this, NoRepair, false); - return Derivation::tryResolve(store, drvPath); -} - -std::optional Derivation::tryResolve(Store& store, const StorePath& drvPath) -{ - // This is quite dirty and leaky, but will disappear once #4340 is merged - static Sync>> resolutionsCache; - - debug("trying to resolve %s", store.printStorePath(drvPath)); - - { - auto resolutions = resolutionsCache.lock(); - auto resolvedDrvIter = resolutions->find(drvPath); - if (resolvedDrvIter != resolutions->end()) { - auto & [_, resolvedDrv] = *resolvedDrvIter; - return *resolvedDrv; - } - } - - /* Try resolve drv and use that path instead. */ - auto drv = store.readDerivation(drvPath); - auto attempt = drv.tryResolveUncached(store); - if (!attempt) - return std::nullopt; - /* Store in memo table. */ - resolutionsCache.lock()->insert_or_assign(drvPath, *attempt); - return *attempt; -} - } diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh index 3d8f19aef..4e5985fab 100644 --- a/src/libstore/derivations.hh +++ b/src/libstore/derivations.hh @@ -138,14 +138,10 @@ struct Derivation : BasicDerivation 2. Input placeholders are replaced with realized input store paths. */ std::optional tryResolve(Store & store); - static std::optional tryResolve(Store & store, const StorePath & drvPath); Derivation() = default; Derivation(const BasicDerivation & bd) : BasicDerivation(bd) { } Derivation(BasicDerivation && bd) : BasicDerivation(std::move(bd)) { } - -private: - std::optional tryResolveUncached(Store & store); }; From 2de232d2b301b2f0854b9fa715ab085612c85e00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20de=20Kok?= Date: Tue, 16 Feb 2021 14:32:12 +0100 Subject: [PATCH 19/25] Add x86_64 compute levels as additional system types When performing distributed builds of machine learning packages, it would be nice if builders without the required SIMD instructions can be excluded as build nodes. Since x86_64 has accumulated a large number of different instruction set extensions, listing all possible extensions would be unwieldy. AMD, Intel, Red Hat, and SUSE have recently defined four different microarchitecture levels that are now part of the x86-64 psABI supplement and will be used in glibc 2.33: https://gitlab.com/x86-psABIs/x86-64-ABI https://lwn.net/Articles/844831/ This change uses libcpuid to detect CPU features and then uses them to add the supported x86_64 levels to the additional system types. For example on a Ryzen 3700X: $ ~/aps/bin/nix -vv --version | grep "Additional system" Additional system types: i686-linux, x86_64-v1-linux, x86_64-v2-linux, x86_64-v3-linux --- Makefile.config.in | 1 + configure.ac | 8 ++++ flake.nix | 3 +- src/libstore/globals.cc | 24 +++++++---- src/libutil/compute-levels.cc | 80 +++++++++++++++++++++++++++++++++++ src/libutil/compute-levels.hh | 7 +++ src/libutil/local.mk | 4 ++ tests/compute-levels.sh | 7 +++ tests/local.mk | 3 +- 9 files changed, 126 insertions(+), 11 deletions(-) create mode 100644 src/libutil/compute-levels.cc create mode 100644 src/libutil/compute-levels.hh create mode 100644 tests/compute-levels.sh diff --git a/Makefile.config.in b/Makefile.config.in index d1e59e4e7..9d0500e48 100644 --- a/Makefile.config.in +++ b/Makefile.config.in @@ -9,6 +9,7 @@ CXXFLAGS = @CXXFLAGS@ EDITLINE_LIBS = @EDITLINE_LIBS@ ENABLE_S3 = @ENABLE_S3@ GTEST_LIBS = @GTEST_LIBS@ +HAVE_LIBCPUID = @HAVE_LIBCPUID@ HAVE_SECCOMP = @HAVE_SECCOMP@ LDFLAGS = @LDFLAGS@ LIBARCHIVE_LIBS = @LIBARCHIVE_LIBS@ diff --git a/configure.ac b/configure.ac index 2047ed8d2..a24287ff6 100644 --- a/configure.ac +++ b/configure.ac @@ -218,6 +218,14 @@ LDFLAGS="-lz $LDFLAGS" # Look for libbrotli{enc,dec}. PKG_CHECK_MODULES([LIBBROTLI], [libbrotlienc libbrotlidec], [CXXFLAGS="$LIBBROTLI_CFLAGS $CXXFLAGS"]) +# Look for libcpuid. +if test "$machine_name" = "x86_64"; then + PKG_CHECK_MODULES([LIBCPUID], [libcpuid], [CXXFLAGS="$LIBCPUID_CFLAGS $CXXFLAGS"]) + have_libcpuid=1 + AC_DEFINE([HAVE_LIBCPUID], [1], [Use libcpuid]) +fi +AC_SUBST(HAVE_LIBCPUID, [$have_libcpuid]) + # Look for libseccomp, required for Linux sandboxing. if test "$sys_name" = linux; then diff --git a/flake.nix b/flake.nix index 8c60934e6..3ad7cca97 100644 --- a/flake.nix +++ b/flake.nix @@ -91,7 +91,8 @@ gmock ] ++ lib.optionals stdenv.isLinux [libseccomp utillinuxMinimal] - ++ lib.optional (stdenv.isLinux || stdenv.isDarwin) libsodium; + ++ lib.optional (stdenv.isLinux || stdenv.isDarwin) libsodium + ++ lib.optional stdenv.isx86_64 libcpuid; awsDeps = lib.optional (stdenv.isLinux || stdenv.isDarwin) (aws-sdk-cpp.override { diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 0531aad9f..df07aee9b 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -3,6 +3,7 @@ #include "archive.hh" #include "args.hh" #include "abstract-setting-to-json.hh" +#include "compute-levels.hh" #include #include @@ -133,24 +134,29 @@ StringSet Settings::getDefaultSystemFeatures() StringSet Settings::getDefaultExtraPlatforms() { + StringSet extraPlatforms; + if (std::string{SYSTEM} == "x86_64-linux" && !isWSL1()) - return StringSet{"i686-linux"}; -#if __APPLE__ + extraPlatforms.insert("i686-linux"); + +#if __linux__ + StringSet levels = computeLevels(); + for (auto iter = levels.begin(); iter != levels.end(); ++iter) + extraPlatforms.insert(*iter + "-linux"); +#elif __APPLE__ // Rosetta 2 emulation layer can run x86_64 binaries on aarch64 // machines. Note that we can’t force processes from executing // x86_64 in aarch64 environments or vice versa since they can // always exec with their own binary preferences. - else if (pathExists("/Library/Apple/System/Library/LaunchDaemons/com.apple.oahd.plist")) { + if (pathExists("/Library/Apple/System/Library/LaunchDaemons/com.apple.oahd.plist")) { if (std::string{SYSTEM} == "x86_64-darwin") - return StringSet{"aarch64-darwin"}; + extraPlatforms.insert("aarch64-darwin"); else if (std::string{SYSTEM} == "aarch64-darwin") - return StringSet{"x86_64-darwin"}; - else - return StringSet{}; + extraPlatforms.insert("x86_64-darwin"); } #endif - else - return StringSet{}; + + return extraPlatforms; } bool Settings::isExperimentalFeatureEnabled(const std::string & name) diff --git a/src/libutil/compute-levels.cc b/src/libutil/compute-levels.cc new file mode 100644 index 000000000..19eaedfa8 --- /dev/null +++ b/src/libutil/compute-levels.cc @@ -0,0 +1,80 @@ +#include "types.hh" + +#if HAVE_LIBCPUID +#include +#endif + +namespace nix { + +#if HAVE_LIBCPUID + +StringSet computeLevels() { + StringSet levels; + + if (!cpuid_present()) + return levels; + + cpu_raw_data_t raw; + cpu_id_t data; + + if (cpuid_get_raw_data(&raw) < 0) + return levels; + + if (cpu_identify(&raw, &data) < 0) + return levels; + + if (!(data.flags[CPU_FEATURE_CMOV] && + data.flags[CPU_FEATURE_CX8] && + data.flags[CPU_FEATURE_FPU] && + data.flags[CPU_FEATURE_FXSR] && + data.flags[CPU_FEATURE_MMX] && + data.flags[CPU_FEATURE_SSE] && + data.flags[CPU_FEATURE_SSE2])) + return levels; + + levels.insert("x86_64-v1"); + + if (!(data.flags[CPU_FEATURE_CX16] && + data.flags[CPU_FEATURE_LAHF_LM] && + data.flags[CPU_FEATURE_POPCNT] && + // SSE3 + data.flags[CPU_FEATURE_PNI] && + data.flags[CPU_FEATURE_SSSE3] && + data.flags[CPU_FEATURE_SSE4_1] && + data.flags[CPU_FEATURE_SSE4_2])) + return levels; + + levels.insert("x86_64-v2"); + + if (!(data.flags[CPU_FEATURE_AVX] && + data.flags[CPU_FEATURE_AVX2] && + data.flags[CPU_FEATURE_F16C] && + data.flags[CPU_FEATURE_FMA3] && + // LZCNT + data.flags[CPU_FEATURE_ABM] && + data.flags[CPU_FEATURE_MOVBE])) + return levels; + + levels.insert("x86_64-v3"); + + if (!(data.flags[CPU_FEATURE_AVX512F] && + data.flags[CPU_FEATURE_AVX512BW] && + data.flags[CPU_FEATURE_AVX512CD] && + data.flags[CPU_FEATURE_AVX512DQ] && + data.flags[CPU_FEATURE_AVX512VL])) + return levels; + + levels.insert("x86_64-v4"); + + return levels; +} + +#else + +StringSet computeLevels() { + return StringSet{}; +} + +#endif // HAVE_LIBCPUID + +} diff --git a/src/libutil/compute-levels.hh b/src/libutil/compute-levels.hh new file mode 100644 index 000000000..8ded295f9 --- /dev/null +++ b/src/libutil/compute-levels.hh @@ -0,0 +1,7 @@ +#include "types.hh" + +namespace nix { + +StringSet computeLevels(); + +} diff --git a/src/libutil/local.mk b/src/libutil/local.mk index ae7eb67ad..5341c58e6 100644 --- a/src/libutil/local.mk +++ b/src/libutil/local.mk @@ -7,3 +7,7 @@ libutil_DIR := $(d) libutil_SOURCES := $(wildcard $(d)/*.cc) libutil_LDFLAGS = $(LIBLZMA_LIBS) -lbz2 -pthread $(OPENSSL_LIBS) $(LIBBROTLI_LIBS) $(LIBARCHIVE_LIBS) $(BOOST_LDFLAGS) -lboost_context + +ifeq ($(HAVE_LIBCPUID), 1) + libutil_LDFLAGS += -lcpuid +endif diff --git a/tests/compute-levels.sh b/tests/compute-levels.sh new file mode 100644 index 000000000..e4322dfa1 --- /dev/null +++ b/tests/compute-levels.sh @@ -0,0 +1,7 @@ +source common.sh + +if [[ $(uname -ms) = "Linux x86_64" ]]; then + # x86_64 CPUs must always support the baseline + # microarchitecture level. + nix -vv --version | grep -q "x86_64-v1-linux" +fi diff --git a/tests/local.mk b/tests/local.mk index aa8b4f9bf..06be8cec1 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -38,7 +38,8 @@ nix_tests = \ describe-stores.sh \ flakes.sh \ content-addressed.sh \ - build.sh + build.sh \ + compute-levels.sh # parallel.sh # build-remote-content-addressed-fixed.sh \ From 574eb2be81cc599162722659dcb95f19173c98d1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 22 Feb 2021 15:24:14 +0100 Subject: [PATCH 20/25] Tweak error message --- src/libexpr/eval.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 7271776eb..e2f2308aa 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1381,10 +1381,10 @@ void EvalState::autoCallFunction(Bindings & args, Value & fun, Value & res) } else if (!i.def) { throwMissingArgumentError(i.pos, R"(cannot evaluate a function that has an argument without a value ('%1%') -nix attempted to evaluate a function as a top level expression; in this case it must have its -arguments supplied either by default values, or passed explicitly with --arg or --argstr. - -https://nixos.org/manual/nix/stable/#ss-functions)", i.name); +Nix attempted to evaluate a function as a top level expression; in +this case it must have its arguments supplied either by default +values, or passed explicitly with '--arg' or '--argstr'. See +https://nixos.org/manual/nix/stable/#ss-functions.)", i.name); } } From e2f3b2eb42a0ceca36ce00973bd2d49b1a3e6a2c Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 22 Feb 2021 16:13:09 +0100 Subject: [PATCH 21/25] Make missing auto-call arguments throw an eval error The PR #4240 changed messag of the error that was thrown when an auto-called function was missing an argument. However this change also changed the type of the error, from `EvalError` to a new `MissingArgumentError`. This broke hydra which was relying on an `EvalError` being thrown. Make `MissingArgumentError` a subclass of `EvalError` to un-break hydra. --- src/libexpr/nixexpr.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index cbe9a45bf..8df8055b3 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -17,7 +17,7 @@ MakeError(ThrownError, AssertionError); MakeError(Abort, EvalError); MakeError(TypeError, EvalError); MakeError(UndefinedVarError, Error); -MakeError(MissingArgumentError, Error); +MakeError(MissingArgumentError, EvalError); MakeError(RestrictedPathError, Error); From 35205e2e922952fc0654260a07fc3191c5afc2cc Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Mon, 22 Feb 2021 17:10:55 -0500 Subject: [PATCH 22/25] Warn about instability of plugin API --- src/libstore/globals.hh | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 1d968ef3e..1254698ca 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -831,6 +831,9 @@ public: command, and RegisterSetting to add new nix config settings. See the constructors for those types for more details. + Warning! These APIs are inherently unstable and may change from + release to release. + Since these files are loaded into the same address space as Nix itself, they must be DSOs compatible with the instance of Nix running at the time (i.e. compiled against the same headers, not From ec3497c1d63f4c0547d0402d92015f846f56aac7 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Thu, 28 Jan 2021 07:37:04 -0500 Subject: [PATCH 23/25] Bail if plugin-files is set after plugins have been loaded. We know the flag will be ignored but the user wants it to take effect. --- src/libstore/globals.cc | 11 +++++++++++ src/libstore/globals.hh | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index df07aee9b..03294b7fe 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -243,6 +243,14 @@ void MaxBuildJobsSetting::set(const std::string & str, bool append) } +void PluginFilesSetting::set(const std::string & str, bool append) +{ + if (pluginsLoaded) + throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand"); + BaseSetting::set(str, append); +} + + void initPlugins() { for (const auto & pluginFile : settings.pluginFiles.get()) { @@ -270,6 +278,9 @@ void initPlugins() unknown settings. */ globalConfig.reapplyUnknownSettings(); globalConfig.warnUnknownSettings(); + + /* Tell the user if they try to set plugin-files after we've already loaded */ + settings.pluginFiles.pluginsLoaded = true; } } diff --git a/src/libstore/globals.hh b/src/libstore/globals.hh index 1254698ca..df61d6417 100644 --- a/src/libstore/globals.hh +++ b/src/libstore/globals.hh @@ -28,6 +28,23 @@ struct MaxBuildJobsSetting : public BaseSetting void set(const std::string & str, bool append = false) override; }; +struct PluginFilesSetting : public BaseSetting +{ + bool pluginsLoaded = false; + + PluginFilesSetting(Config * options, + const Paths & def, + const std::string & name, + const std::string & description, + const std::set & aliases = {}) + : BaseSetting(def, name, description, aliases) + { + options->addSetting(this); + } + + void set(const std::string & str, bool append = false) override; +}; + class Settings : public Config { unsigned int getDefaultCores(); @@ -819,7 +836,7 @@ public: Setting minFreeCheckInterval{this, 5, "min-free-check-interval", "Number of seconds between checking free disk space."}; - Setting pluginFiles{ + PluginFilesSetting pluginFiles{ this, {}, "plugin-files", R"( A list of plugin files to be loaded by Nix. Each of these files will From 98d1b64400cc7b75216fc885859883c707c18bef Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Thu, 28 Jan 2021 09:37:43 -0500 Subject: [PATCH 24/25] Initialize plugins after handling initial command line flags This is technically a breaking change, since attempting to set plugin files after the first non-flag argument will now throw an error. This is acceptable given the relative lack of stability in a plugin interface and the need to tie the knot somewhere once plugins can actually define new subcommands. --- doc/manual/src/release-notes/rl-2.4.md | 7 +++++++ src/build-remote/build-remote.cc | 3 +++ src/libmain/common-args.cc | 7 +++++++ src/libmain/common-args.hh | 6 +++++- src/libstore/globals.cc | 1 + src/libutil/args.cc | 8 ++++++++ src/libutil/args.hh | 4 ++++ src/nix-build/nix-build.cc | 2 -- src/nix-channel/nix-channel.cc | 2 -- src/nix-collect-garbage/nix-collect-garbage.cc | 2 -- src/nix-copy-closure/nix-copy-closure.cc | 2 -- src/nix-env/nix-env.cc | 2 -- src/nix-instantiate/nix-instantiate.cc | 2 -- src/nix-store/nix-store.cc | 2 -- src/nix/daemon.cc | 2 -- src/nix/main.cc | 2 -- src/nix/prefetch.cc | 2 -- tests/plugins.sh | 2 +- 18 files changed, 36 insertions(+), 22 deletions(-) create mode 100644 doc/manual/src/release-notes/rl-2.4.md diff --git a/doc/manual/src/release-notes/rl-2.4.md b/doc/manual/src/release-notes/rl-2.4.md new file mode 100644 index 000000000..26ba70904 --- /dev/null +++ b/doc/manual/src/release-notes/rl-2.4.md @@ -0,0 +1,7 @@ +# Release 2.4 (202X-XX-XX) + + - It is now an error to modify the `plugin-files` setting via a + command-line flag that appears after the first non-flag argument + to any command, including a subcommand to `nix`. For example, + `nix-instantiate default.nix --plugin-files ""` must now become + `nix-instantiate --plugin-files "" default.nix`. diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 5b8ab3387..f784b5160 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -53,6 +53,9 @@ static int main_build_remote(int argc, char * * argv) unsetenv("DISPLAY"); unsetenv("SSH_ASKPASS"); + /* If we ever use the common args framework, make sure to + remove initPlugins below and initialize settings first. + */ if (argc != 2) throw UsageError("called without required arguments"); diff --git a/src/libmain/common-args.cc b/src/libmain/common-args.cc index ff96ee7d5..c43e9ebd2 100644 --- a/src/libmain/common-args.cc +++ b/src/libmain/common-args.cc @@ -79,4 +79,11 @@ MixCommonArgs::MixCommonArgs(const string & programName) hiddenCategories.insert(cat); } +void MixCommonArgs::initialFlagsProcessed() +{ + initPlugins(); + pluginsInited(); +} + + } diff --git a/src/libmain/common-args.hh b/src/libmain/common-args.hh index 8e53a7361..31bdf527a 100644 --- a/src/libmain/common-args.hh +++ b/src/libmain/common-args.hh @@ -7,10 +7,14 @@ namespace nix { //static constexpr auto commonArgsCategory = "Miscellaneous common options"; static constexpr auto loggingCategory = "Logging-related options"; -struct MixCommonArgs : virtual Args +class MixCommonArgs : public virtual Args { + void initialFlagsProcessed() override; +public: string programName; MixCommonArgs(const string & programName); +protected: + virtual void pluginsInited() {} }; struct MixDryRun : virtual Args diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 03294b7fe..2780e0bf5 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -253,6 +253,7 @@ void PluginFilesSetting::set(const std::string & str, bool append) void initPlugins() { + assert(!settings.pluginFiles.pluginsLoaded); for (const auto & pluginFile : settings.pluginFiles.get()) { Paths pluginFiles; try { diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 9377fe4c0..eb11fd64b 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -60,6 +60,7 @@ void Args::parseCmdline(const Strings & _cmdline) verbosity = lvlError; } + bool argsSeen = false; for (auto pos = cmdline.begin(); pos != cmdline.end(); ) { auto arg = *pos; @@ -88,6 +89,10 @@ void Args::parseCmdline(const Strings & _cmdline) throw UsageError("unrecognised flag '%1%'", arg); } else { + if (!argsSeen) { + argsSeen = true; + initialFlagsProcessed(); + } pos = rewriteArgs(cmdline, pos); pendingArgs.push_back(*pos++); if (processArgs(pendingArgs, false)) @@ -96,6 +101,9 @@ void Args::parseCmdline(const Strings & _cmdline) } processArgs(pendingArgs, true); + + if (!argsSeen) + initialFlagsProcessed(); } bool Args::processFlag(Strings::iterator & pos, Strings::iterator end) diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 88f068087..4721c21df 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -132,6 +132,10 @@ protected: std::set hiddenCategories; + /* Called after all command line flags before the first non-flag + argument (if any) have been processed. */ + virtual void initialFlagsProcessed() {} + public: void addFlag(Flag && flag); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index d975cd16d..7b4a53919 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -240,8 +240,6 @@ static void main_nix_build(int argc, char * * argv) myArgs.parseCmdline(args); - initPlugins(); - if (packages && fromArgs) throw UsageError("'-p' and '-E' are mutually exclusive"); diff --git a/src/nix-channel/nix-channel.cc b/src/nix-channel/nix-channel.cc index 57189d557..3272c6125 100755 --- a/src/nix-channel/nix-channel.cc +++ b/src/nix-channel/nix-channel.cc @@ -196,8 +196,6 @@ static int main_nix_channel(int argc, char ** argv) return true; }); - initPlugins(); - switch (cmd) { case cNone: throw UsageError("no command specified"); diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc index c1769790a..4f953fab4 100644 --- a/src/nix-collect-garbage/nix-collect-garbage.cc +++ b/src/nix-collect-garbage/nix-collect-garbage.cc @@ -74,8 +74,6 @@ static int main_nix_collect_garbage(int argc, char * * argv) return true; }); - initPlugins(); - auto profilesDir = settings.nixStateDir + "/profiles"; if (removeOld) removeOldGenerations(profilesDir); diff --git a/src/nix-copy-closure/nix-copy-closure.cc b/src/nix-copy-closure/nix-copy-closure.cc index ad2e06067..5e8cc515b 100755 --- a/src/nix-copy-closure/nix-copy-closure.cc +++ b/src/nix-copy-closure/nix-copy-closure.cc @@ -43,8 +43,6 @@ static int main_nix_copy_closure(int argc, char ** argv) return true; }); - initPlugins(); - if (sshHost.empty()) throw UsageError("no host name specified"); diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 106a78fc4..0f10a4cbb 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -1420,8 +1420,6 @@ static int main_nix_env(int argc, char * * argv) myArgs.parseCmdline(argvToStrings(argc, argv)); - initPlugins(); - if (!op) throw UsageError("no operation specified"); auto store = openStore(); diff --git a/src/nix-instantiate/nix-instantiate.cc b/src/nix-instantiate/nix-instantiate.cc index ea2e85eb0..95903d882 100644 --- a/src/nix-instantiate/nix-instantiate.cc +++ b/src/nix-instantiate/nix-instantiate.cc @@ -149,8 +149,6 @@ static int main_nix_instantiate(int argc, char * * argv) myArgs.parseCmdline(argvToStrings(argc, argv)); - initPlugins(); - if (evalOnly && !wantsReadWrite) settings.readOnlyMode = true; diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 37191b9e6..e17b38c3c 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -1067,8 +1067,6 @@ static int main_nix_store(int argc, char * * argv) return true; }); - initPlugins(); - if (!op) throw UsageError("no operation specified"); if (op != opDump && op != opRestore) /* !!! hack */ diff --git a/src/nix/daemon.cc b/src/nix/daemon.cc index 26006167d..2cf2a04c9 100644 --- a/src/nix/daemon.cc +++ b/src/nix/daemon.cc @@ -326,8 +326,6 @@ static int main_nix_daemon(int argc, char * * argv) return true; }); - initPlugins(); - runDaemon(stdio); return 0; diff --git a/src/nix/main.cc b/src/nix/main.cc index 1b68cf15b..b078366fa 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -283,8 +283,6 @@ void mainWrapped(int argc, char * * argv) if (completions) return; - initPlugins(); - if (args.showVersion) { printVersion(programName); return; diff --git a/src/nix/prefetch.cc b/src/nix/prefetch.cc index a831dcd15..b7da3ea5a 100644 --- a/src/nix/prefetch.cc +++ b/src/nix/prefetch.cc @@ -171,8 +171,6 @@ static int main_nix_prefetch_url(int argc, char * * argv) myArgs.parseCmdline(argvToStrings(argc, argv)); - initPlugins(); - if (args.size() > 2) throw UsageError("too many arguments"); diff --git a/tests/plugins.sh b/tests/plugins.sh index 50bfaf7e9..e22bf4408 100644 --- a/tests/plugins.sh +++ b/tests/plugins.sh @@ -2,6 +2,6 @@ source common.sh set -o pipefail -res=$(nix eval --expr builtins.anotherNull --option setting-set true --option plugin-files $PWD/plugins/libplugintest*) +res=$(nix --option setting-set true --option plugin-files $PWD/plugins/libplugintest* eval --expr builtins.anotherNull) [ "$res"x = "nullx" ] From f6c5b05488c588964f51ce97ad2c297fbca7ce96 Mon Sep 17 00:00:00 2001 From: Shea Levy Date: Thu, 28 Jan 2021 10:04:47 -0500 Subject: [PATCH 25/25] Respect command registrations in plugins. --- doc/manual/src/release-notes/rl-2.4.md | 1 + src/libutil/args.cc | 4 ++-- src/nix/main.cc | 6 ++++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/manual/src/release-notes/rl-2.4.md b/doc/manual/src/release-notes/rl-2.4.md index 26ba70904..f7ab9f6ad 100644 --- a/doc/manual/src/release-notes/rl-2.4.md +++ b/doc/manual/src/release-notes/rl-2.4.md @@ -5,3 +5,4 @@ to any command, including a subcommand to `nix`. For example, `nix-instantiate default.nix --plugin-files ""` must now become `nix-instantiate --plugin-files "" default.nix`. + - Plugins that add new `nix` subcommands are now actually respected. diff --git a/src/libutil/args.cc b/src/libutil/args.cc index eb11fd64b..75eb19d28 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -306,8 +306,8 @@ Strings argvToStrings(int argc, char * * argv) return args; } -MultiCommand::MultiCommand(const Commands & commands) - : commands(commands) +MultiCommand::MultiCommand(const Commands & commands_) + : commands(commands_) { expectArgs({ .label = "subcommand", diff --git a/src/nix/main.cc b/src/nix/main.cc index b078366fa..06e221682 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -159,6 +159,12 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs #include "nix.md" ; } + + // Plugins may add new subcommands. + void pluginsInited() override + { + commands = RegisterCommand::getCommandsFor({}); + } }; static void showHelp(std::vector subcommand)