From 3b5429aec110a22f8fe0d92b72faf1921b8ede2d Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 2 May 2021 17:24:14 +0200 Subject: [PATCH 1/9] Source complete env in `nix-shell` with `__structuredAttrs = true;` This is needed to push the adoption of structured attrs[1] forward. It's now checked if a `__json` exists in the environment-map of the derivation to be openend in a `nix-shell`. Derivations with structured attributes enabled also make use of a file named `.attrs.json` containing every environment variable represented as JSON which is useful for e.g. `exportReferencesGraph`[2]. To provide an environment similar to the build sandbox, `nix-shell` now adds a `.attrs.json` to `cwd` (which is mostly equal to the one in the build sandbox) and removes it using an exit hook when closing the shell. To avoid leaking internals of the build-process to the `nix-shell`, the entire logic to generate JSON and shell code for structured attrs was moved into the `ParsedDerivation` class. [1] https://nixos.mayflower.consulting/blog/2020/01/20/structured-attrs/ [2] https://nixos.org/manual/nix/unstable/expressions/advanced-attributes.html#advanced-attributes --- src/libstore/build/derivation-goal.cc | 37 ------- src/libstore/build/local-derivation-goal.cc | 113 ++------------------ src/libstore/parsed-derivations.cc | 110 +++++++++++++++++++ src/libstore/parsed-derivations.hh | 4 + src/libstore/store-api.cc | 36 +++++++ src/libstore/store-api.hh | 2 + src/nix-build/nix-build.cc | 45 +++++++- tests/structured-attrs-shell.nix | 19 ++++ tests/structured-attrs.sh | 6 ++ 9 files changed, 227 insertions(+), 145 deletions(-) create mode 100644 tests/structured-attrs-shell.nix diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 9100d3333..4c49f3cab 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -143,7 +143,6 @@ void DerivationGoal::work() (this->*state)(); } - void DerivationGoal::addWantedOutputs(const StringSet & outputs) { /* If we already want all outputs, there is nothing to do. */ @@ -1048,42 +1047,6 @@ HookReply DerivationGoal::tryBuildHook() } -StorePathSet DerivationGoal::exportReferences(const StorePathSet & storePaths) -{ - StorePathSet paths; - - for (auto & storePath : storePaths) { - if (!inputPaths.count(storePath)) - throw BuildError("cannot export references of path '%s' because it is not in the input closure of the derivation", worker.store.printStorePath(storePath)); - - worker.store.computeFSClosure({storePath}, paths); - } - - /* If there are derivations in the graph, then include their - outputs as well. This is useful if you want to do things - like passing all build-time dependencies of some path to a - derivation that builds a NixOS DVD image. */ - auto paths2 = paths; - - for (auto & j : paths2) { - if (j.isDerivation()) { - Derivation drv = worker.store.derivationFromPath(j); - for (auto & k : drv.outputsAndOptPaths(worker.store)) { - if (!k.second.second) - /* FIXME: I am confused why we are calling - `computeFSClosure` on the output path, rather than - derivation itself. That doesn't seem right to me, so I - won't try to implemented this for CA derivations. */ - throw UnimplementedError("exportReferences on CA derivations is not yet implemented"); - worker.store.computeFSClosure(*k.second.second, paths); - } - } - } - - return paths; -} - - void DerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 279139020..9bb6f276c 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -518,7 +518,7 @@ void LocalDerivationGoal::startBuilder() /* Write closure info to . */ writeFile(tmpDir + "/" + fileName, worker.store.makeValidityRegistration( - exportReferences({storePath}), false, false)); + worker.store.exportReferences({storePath}, inputPaths), false, false)); } } @@ -1084,113 +1084,18 @@ void LocalDerivationGoal::initEnv() } -static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*"); - - void LocalDerivationGoal::writeStructuredAttrs() { - auto structuredAttrs = parsedDrv->getStructuredAttrs(); - if (!structuredAttrs) return; + if (auto structAttrs = parsedDrv->generateStructuredAttrs(inputRewrites, worker.store, inputPaths)) { + auto value = structAttrs.value(); + auto jsonSh = value.first; + auto json = value.second; - auto json = *structuredAttrs; - - /* Add an "outputs" object containing the output paths. */ - nlohmann::json outputs; - for (auto & i : drv->outputs) { - /* The placeholder must have a rewrite, so we use it to cover both the - cases where we know or don't know the output path ahead of time. */ - outputs[i.first] = rewriteStrings(hashPlaceholder(i.first), inputRewrites); + writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); + chownToBuilder(tmpDir + "/.attrs.sh"); + writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); + chownToBuilder(tmpDir + "/.attrs.json"); } - json["outputs"] = outputs; - - /* Handle exportReferencesGraph. */ - auto e = json.find("exportReferencesGraph"); - if (e != json.end() && e->is_object()) { - for (auto i = e->begin(); i != e->end(); ++i) { - std::ostringstream str; - { - JSONPlaceholder jsonRoot(str, true); - StorePathSet storePaths; - for (auto & p : *i) - storePaths.insert(worker.store.parseStorePath(p.get())); - worker.store.pathInfoToJSON(jsonRoot, - exportReferences(storePaths), false, true); - } - json[i.key()] = nlohmann::json::parse(str.str()); // urgh - } - } - - writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); - chownToBuilder(tmpDir + "/.attrs.json"); - - /* As a convenience to bash scripts, write a shell file that - maps all attributes that are representable in bash - - namely, strings, integers, nulls, Booleans, and arrays and - objects consisting entirely of those values. (So nested - arrays or objects are not supported.) */ - - auto handleSimpleType = [](const nlohmann::json & value) -> std::optional { - if (value.is_string()) - return shellEscape(value); - - if (value.is_number()) { - auto f = value.get(); - if (std::ceil(f) == f) - return std::to_string(value.get()); - } - - if (value.is_null()) - return std::string("''"); - - if (value.is_boolean()) - return value.get() ? std::string("1") : std::string(""); - - return {}; - }; - - std::string jsonSh; - - for (auto i = json.begin(); i != json.end(); ++i) { - - if (!std::regex_match(i.key(), shVarName)) continue; - - auto & value = i.value(); - - auto s = handleSimpleType(value); - if (s) - jsonSh += fmt("declare %s=%s\n", i.key(), *s); - - else if (value.is_array()) { - std::string s2; - bool good = true; - - for (auto i = value.begin(); i != value.end(); ++i) { - auto s3 = handleSimpleType(i.value()); - if (!s3) { good = false; break; } - s2 += *s3; s2 += ' '; - } - - if (good) - jsonSh += fmt("declare -a %s=(%s)\n", i.key(), s2); - } - - else if (value.is_object()) { - std::string s2; - bool good = true; - - for (auto i = value.begin(); i != value.end(); ++i) { - auto s3 = handleSimpleType(i.value()); - if (!s3) { good = false; break; } - s2 += fmt("[%s]=%s ", shellEscape(i.key()), *s3); - } - - if (good) - jsonSh += fmt("declare -A %s=(%s)\n", i.key(), s2); - } - } - - writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); - chownToBuilder(tmpDir + "/.attrs.sh"); } diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index c5c3ae3dc..029da8bd3 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -1,6 +1,8 @@ #include "parsed-derivations.hh" #include +#include +#include "json.hh" namespace nix { @@ -121,4 +123,112 @@ bool ParsedDerivation::substitutesAllowed() const return getBoolAttr("allowSubstitutes", true); } +static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*"); +std::optional ParsedDerivation::generateStructuredAttrs( + std::optional inputRewrites, Store & store, const StorePathSet & inputPaths) +{ + auto structuredAttrs = getStructuredAttrs(); + if (!structuredAttrs) return std::nullopt; + + auto json = *structuredAttrs; + + /* Add an "outputs" object containing the output paths. */ + nlohmann::json outputs; + for (auto & i : drv.outputs) { + if (inputRewrites) { + /* The placeholder must have a rewrite, so we use it to cover both the + cases where we know or don't know the output path ahead of time. */ + outputs[i.first] = rewriteStrings(hashPlaceholder(i.first), inputRewrites.value()); + } else { + /* This case is only relevant for the nix-shell */ + outputs[i.first] = hashPlaceholder(i.first); + } + } + json["outputs"] = outputs; + + /* Handle exportReferencesGraph. */ + auto e = json.find("exportReferencesGraph"); + if (e != json.end() && e->is_object()) { + for (auto i = e->begin(); i != e->end(); ++i) { + std::ostringstream str; + { + JSONPlaceholder jsonRoot(str, true); + StorePathSet storePaths; + for (auto & p : *i) + storePaths.insert(store.parseStorePath(p.get())); + store.pathInfoToJSON(jsonRoot, + store.exportReferences(storePaths, inputPaths), false, true); + } + json[i.key()] = nlohmann::json::parse(str.str()); // urgh + } + } + + /* As a convenience to bash scripts, write a shell file that + maps all attributes that are representable in bash - + namely, strings, integers, nulls, Booleans, and arrays and + objects consisting entirely of those values. (So nested + arrays or objects are not supported.) */ + + auto handleSimpleType = [](const nlohmann::json & value) -> std::optional { + if (value.is_string()) + return shellEscape(value); + + if (value.is_number()) { + auto f = value.get(); + if (std::ceil(f) == f) + return std::to_string(value.get()); + } + + if (value.is_null()) + return std::string("''"); + + if (value.is_boolean()) + return value.get() ? std::string("1") : std::string(""); + + return {}; + }; + + std::string jsonSh; + + for (auto i = json.begin(); i != json.end(); ++i) { + + if (!std::regex_match(i.key(), shVarName)) continue; + + auto & value = i.value(); + + auto s = handleSimpleType(value); + if (s) + jsonSh += fmt("declare %s=%s\n", i.key(), *s); + + else if (value.is_array()) { + std::string s2; + bool good = true; + + for (auto i = value.begin(); i != value.end(); ++i) { + auto s3 = handleSimpleType(i.value()); + if (!s3) { good = false; break; } + s2 += *s3; s2 += ' '; + } + + if (good) + jsonSh += fmt("declare -a %s=(%s)\n", i.key(), s2); + } + + else if (value.is_object()) { + std::string s2; + bool good = true; + + for (auto i = value.begin(); i != value.end(); ++i) { + auto s3 = handleSimpleType(i.value()); + if (!s3) { good = false; break; } + s2 += fmt("[%s]=%s ", shellEscape(i.key()), *s3); + } + + if (good) + jsonSh += fmt("declare -A %s=(%s)\n", i.key(), s2); + } + } + + return std::make_pair(jsonSh, json); +} } diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index c9fbe68c4..4b8b8c8ff 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -6,6 +6,8 @@ namespace nix { +typedef std::pair StructuredAttrsWithShellRC; + class ParsedDerivation { StorePath drvPath; @@ -36,6 +38,8 @@ public: bool willBuildLocally(Store & localStore) const; bool substitutesAllowed() const; + + std::optional generateStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths); }; } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 93fcb068f..cfdfb5269 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -627,6 +627,42 @@ string Store::makeValidityRegistration(const StorePathSet & paths, } +StorePathSet Store::exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths) +{ + StorePathSet paths; + + for (auto & storePath : storePaths) { + if (!inputPaths.count(storePath)) + throw BuildError("cannot export references of path '%s' because it is not in the input closure of the derivation", printStorePath(storePath)); + + computeFSClosure({storePath}, paths); + } + + /* If there are derivations in the graph, then include their + outputs as well. This is useful if you want to do things + like passing all build-time dependencies of some path to a + derivation that builds a NixOS DVD image. */ + auto paths2 = paths; + + for (auto & j : paths2) { + if (j.isDerivation()) { + Derivation drv = derivationFromPath(j); + for (auto & k : drv.outputsAndOptPaths(*this)) { + if (!k.second.second) + /* FIXME: I am confused why we are calling + `computeFSClosure` on the output path, rather than + derivation itself. That doesn't seem right to me, so I + won't try to implemented this for CA derivations. */ + throw UnimplementedError("exportReferences on CA derivations is not yet implemented"); + computeFSClosure(*k.second.second, paths); + } + } + } + + return paths; +} + + void Store::pathInfoToJSON(JSONPlaceholder & jsonOut, const StorePathSet & storePaths, bool includeImpureInfo, bool showClosureSize, Base hashBase, diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index f66298991..b742fed6d 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -695,6 +695,8 @@ public: const Stats & getStats(); + StorePathSet exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths); + /* Return the build log of the specified store path, if available, or null otherwise. */ virtual std::shared_ptr getBuildLog(const StorePath & path) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 9acbedda2..f91a56d6a 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -1,10 +1,15 @@ #include #include #include +#include #include #include #include +#include +#include + +#include "parsed-derivations.hh" #include "store-api.hh" #include "local-fs-store.hh" #include "globals.hh" @@ -422,12 +427,41 @@ static void main_nix_build(int argc, char * * argv) } else env[var.first] = var.second; + std::string structuredAttrsRC; + std::string exitCmd; + + if (env.count("__json")) { + StorePathSet inputs; + for (auto & [depDrvPath, wantedDepOutputs] : drv.inputDrvs) { + auto outputs = store->queryPartialDerivationOutputMap(depDrvPath); + for (auto & i : wantedDepOutputs) { + auto o = outputs.at(i); + store->computeFSClosure(*o, inputs); + } + } + + ParsedDerivation parsedDrv( + StorePath(store->parseStorePath(drvInfo.queryDrvPath())), + drv + ); + + if (auto structAttrs = parsedDrv.generateStructuredAttrs(std::nullopt, *store, inputs)) { + auto val = structAttrs.value(); + structuredAttrsRC = val.first; + auto attrsJSON = std::filesystem::current_path().string() + "/.attrs.json"; + writeFile(attrsJSON, val.second.dump()); + exitCmd = "\n_rm_attrs_json() { rm -f " + attrsJSON + "; }" + + "\nexitHooks+=(_rm_attrs_json)" + + "\nfailureHooks+=(_rm_attrs_json)\n"; + } + } + /* Run a shell using the derivation's environment. For convenience, source $stdenv/setup to setup additional environment variables and shell functions. Also don't lose the current $PATH directories. */ auto rcfile = (Path) tmpDir + "/rc"; - writeFile(rcfile, fmt( + std::string rc = fmt( R"(_nix_shell_clean_tmpdir() { rm -rf %1%; }; )"s + (keepTmp ? "trap _nix_shell_clean_tmpdir EXIT; " @@ -436,8 +470,9 @@ static void main_nix_build(int argc, char * * argv) "_nix_shell_clean_tmpdir; ") + (pure ? "" : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;") + "%2%" - "dontAddDisableDepTrack=1; " - "[ -e $stdenv/setup ] && source $stdenv/setup; " + "dontAddDisableDepTrack=1;\n" + + structuredAttrsRC + exitCmd + + "\n[ -e $stdenv/setup ] && source $stdenv/setup; " "%3%" "PATH=%4%:\"$PATH\"; " "SHELL=%5%; " @@ -455,7 +490,9 @@ static void main_nix_build(int argc, char * * argv) shellEscape(dirOf(*shell)), shellEscape(*shell), (getenv("TZ") ? (string("export TZ=") + shellEscape(getenv("TZ")) + "; ") : ""), - envCommand)); + envCommand); + vomit("Sourcing nix-shell with file %s and contents:\n%s", rcfile, rc); + writeFile(rcfile, rc); Strings envStrs; for (auto & i : env) diff --git a/tests/structured-attrs-shell.nix b/tests/structured-attrs-shell.nix new file mode 100644 index 000000000..0c01c2568 --- /dev/null +++ b/tests/structured-attrs-shell.nix @@ -0,0 +1,19 @@ +with import ./config.nix; +let + dep = mkDerivation { + name = "dep"; + buildCommand = '' + mkdir $out; echo bla > $out/bla + ''; + }; +in +mkDerivation { + name = "structured2"; + __structuredAttrs = true; + outputs = [ "out" "dev" ]; + my.list = [ "a" "b" "c" ]; + exportReferencesGraph.refs = [ dep ]; + buildCommand = '' + touch ''${outputs[out]}; touch ''${outputs[dev]} + ''; +} diff --git a/tests/structured-attrs.sh b/tests/structured-attrs.sh index dcfe6d580..c0fcb021a 100644 --- a/tests/structured-attrs.sh +++ b/tests/structured-attrs.sh @@ -8,3 +8,9 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result [[ $(cat $TEST_ROOT/result/foo) = bar ]] [[ $(cat $TEST_ROOT/result-dev/foo) = foo ]] + +export NIX_BUILD_SHELL=$SHELL +[[ ! -e '.attrs.json' ]] +env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ + --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < .attrs.json)"' +[[ ! -e '.attrs.json' ]] From 3944a120ec6986c723bf36bfade9b331dd4af68a Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 9 May 2021 17:46:16 +0200 Subject: [PATCH 2/9] Set environment variables for .attrs.json & .attrs.sh This way no derivation has to expect that these files are in the `cwd` during the build. This is problematic for `nix-shell` where these files would have to be inserted into the nix-shell's `cwd` which can become problematic with e.g. recursive `nix-shell`. To remain backwards-compatible, the location inside the build sandbox will be kept, however using these files directly should be deprecated from now on. --- src/libstore/build/local-derivation-goal.cc | 2 ++ src/nix-build/nix-build.cc | 13 +++++++------ tests/structured-attrs.nix | 2 +- tests/structured-attrs.sh | 4 +--- 4 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 9bb6f276c..e6b552b94 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1093,8 +1093,10 @@ void LocalDerivationGoal::writeStructuredAttrs() writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); + env["ATTRS_SH_FILE"] = tmpDir + "/.attrs.sh"; writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); chownToBuilder(tmpDir + "/.attrs.json"); + env["ATTRS_JSON_FILE"] = tmpDir + "/.attrs.json"; } } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index f91a56d6a..9bd7869f9 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -428,7 +428,6 @@ static void main_nix_build(int argc, char * * argv) env[var.first] = var.second; std::string structuredAttrsRC; - std::string exitCmd; if (env.count("__json")) { StorePathSet inputs; @@ -448,11 +447,13 @@ static void main_nix_build(int argc, char * * argv) if (auto structAttrs = parsedDrv.generateStructuredAttrs(std::nullopt, *store, inputs)) { auto val = structAttrs.value(); structuredAttrsRC = val.first; - auto attrsJSON = std::filesystem::current_path().string() + "/.attrs.json"; + auto attrsJSON = (Path) tmpDir + "/.attrs.json"; writeFile(attrsJSON, val.second.dump()); - exitCmd = "\n_rm_attrs_json() { rm -f " + attrsJSON + "; }" - + "\nexitHooks+=(_rm_attrs_json)" - + "\nfailureHooks+=(_rm_attrs_json)\n"; + auto attrsSH = (Path) tmpDir + "/.attrs.sh"; + writeFile(attrsSH, val.first); + env["ATTRS_SH_FILE"] = attrsSH; + env["ATTRS_JSON_FILE"] = attrsJSON; + keepTmp = true; } } @@ -471,7 +472,7 @@ static void main_nix_build(int argc, char * * argv) (pure ? "" : "[ -n \"$PS1\" ] && [ -e ~/.bashrc ] && source ~/.bashrc;") + "%2%" "dontAddDisableDepTrack=1;\n" - + structuredAttrsRC + exitCmd + + + structuredAttrsRC + "\n[ -e $stdenv/setup ] && source $stdenv/setup; " "%3%" "PATH=%4%:\"$PATH\"; " diff --git a/tests/structured-attrs.nix b/tests/structured-attrs.nix index c39c3a346..f69ee45e9 100644 --- a/tests/structured-attrs.nix +++ b/tests/structured-attrs.nix @@ -36,7 +36,7 @@ mkDerivation { echo bar > $dest echo foo > $dest2 - json=$(cat .attrs.json) + json=$(cat $ATTRS_JSON_FILE) [[ $json =~ '"narHash":"sha256:1r7yc43zqnzl5b0als5vnyp649gk17i37s7mj00xr8kc47rjcybk"' ]] [[ $json =~ '"narSize":288' ]] [[ $json =~ '"closureSize":288' ]] diff --git a/tests/structured-attrs.sh b/tests/structured-attrs.sh index c0fcb021a..241835539 100644 --- a/tests/structured-attrs.sh +++ b/tests/structured-attrs.sh @@ -10,7 +10,5 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result [[ $(cat $TEST_ROOT/result-dev/foo) = foo ]] export NIX_BUILD_SHELL=$SHELL -[[ ! -e '.attrs.json' ]] env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ - --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < .attrs.json)"' -[[ ! -e '.attrs.json' ]] + --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < $ATTRS_JSON_FILE)"' From 447928bdb55d160617387e7ac5954f9a8f36004b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Thu, 13 May 2021 01:20:49 +0200 Subject: [PATCH 3/9] Fix usage of structured attrs for `nix develop` --- src/nix/develop.cc | 5 +++++ src/nix/get-env.sh | 8 +++++++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index e51de2de8..3443fab73 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -256,6 +256,11 @@ struct Common : InstallableCommand, MixProfile // FIXME: properly unquote 'outputs'. StringMap rewrites; for (auto & outputName : tokenizeString>(replaceStrings(outputs->second.quoted, "'", ""))) { + std::regex ptrn(R"re(\[([A-z0-9]+)\]=.*)re"); + std::smatch match; + if (std::regex_match(outputName, match, ptrn)) { + outputName = match[1]; + } auto from = buildEnvironment.env.find(outputName); assert(from != buildEnvironment.env.end()); // FIXME: unquote diff --git a/src/nix/get-env.sh b/src/nix/get-env.sh index 091c0f573..b6b8310a9 100644 --- a/src/nix/get-env.sh +++ b/src/nix/get-env.sh @@ -8,7 +8,13 @@ if [[ -n $stdenv ]]; then source $stdenv/setup fi -for __output in $outputs; do +if [ -e .attrs.sh ]; then + __olist="${!outputs[@]}" +else + __olist=$outputs +fi + +for __output in $__olist; do if [[ -z $__done ]]; then export > ${!__output} set >> ${!__output} From f1e281c4fe1263a0c848bc8aaf57a0e61a99fa93 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Thu, 13 May 2021 16:11:56 +0200 Subject: [PATCH 4/9] Split shell & json creation for build environments with structured attrs --- src/libstore/build/local-derivation-goal.cc | 7 +++---- src/libstore/parsed-derivations.cc | 10 +++++++--- src/libstore/parsed-derivations.hh | 3 ++- src/nix-build/nix-build.cc | 13 ++++++++----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index e6b552b94..60495ae3e 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1086,10 +1086,9 @@ void LocalDerivationGoal::initEnv() void LocalDerivationGoal::writeStructuredAttrs() { - if (auto structAttrs = parsedDrv->generateStructuredAttrs(inputRewrites, worker.store, inputPaths)) { - auto value = structAttrs.value(); - auto jsonSh = value.first; - auto json = value.second; + if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(inputRewrites, worker.store, inputPaths)) { + auto json = structAttrsJson.value(); + auto jsonSh = parsedDrv->writeStructuredAttrsShell(json); writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 029da8bd3..5675600c4 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -124,8 +124,7 @@ bool ParsedDerivation::substitutesAllowed() const } static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*"); -std::optional ParsedDerivation::generateStructuredAttrs( - std::optional inputRewrites, Store & store, const StorePathSet & inputPaths) +std::optional ParsedDerivation::prepareStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths) { auto structuredAttrs = getStructuredAttrs(); if (!structuredAttrs) return std::nullopt; @@ -163,6 +162,11 @@ std::optional ParsedDerivation::generateStructuredAt } } + return json; +} + +std::string ParsedDerivation::writeStructuredAttrsShell(nlohmann::json & json) +{ /* As a convenience to bash scripts, write a shell file that maps all attributes that are representable in bash - namely, strings, integers, nulls, Booleans, and arrays and @@ -229,6 +233,6 @@ std::optional ParsedDerivation::generateStructuredAt } } - return std::make_pair(jsonSh, json); + return jsonSh; } } diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index 4b8b8c8ff..37af36630 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -39,7 +39,8 @@ public: bool substitutesAllowed() const; - std::optional generateStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths); + std::optional prepareStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths); + std::string writeStructuredAttrsShell(nlohmann::json & json); }; } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 9bd7869f9..4e4f27458 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -444,13 +444,16 @@ static void main_nix_build(int argc, char * * argv) drv ); - if (auto structAttrs = parsedDrv.generateStructuredAttrs(std::nullopt, *store, inputs)) { - auto val = structAttrs.value(); - structuredAttrsRC = val.first; + if (auto structAttrs = parsedDrv.prepareStructuredAttrs(std::nullopt, *store, inputs)) { + auto json = structAttrs.value(); + structuredAttrsRC = parsedDrv.writeStructuredAttrsShell(json); + auto attrsJSON = (Path) tmpDir + "/.attrs.json"; - writeFile(attrsJSON, val.second.dump()); + writeFile(attrsJSON, json.dump()); + auto attrsSH = (Path) tmpDir + "/.attrs.sh"; - writeFile(attrsSH, val.first); + writeFile(attrsSH, structuredAttrsRC); + env["ATTRS_SH_FILE"] = attrsSH; env["ATTRS_JSON_FILE"] = attrsJSON; keepTmp = true; From 3504c811a55ecd58e0712cf24829c67c192f5e80 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 18 May 2021 15:07:30 +0200 Subject: [PATCH 5/9] Add testcase for `nix develop` with `__structuredAttrs` --- src/nix/develop.cc | 3 +++ src/nix/get-env.sh | 9 ++++++--- tests/shell.nix | 8 ++++++++ tests/structured-attrs-shell.nix | 2 ++ tests/structured-attrs.sh | 5 +++++ 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/nix/develop.cc b/src/nix/develop.cc index 3443fab73..ee782c4ec 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -256,6 +256,9 @@ struct Common : InstallableCommand, MixProfile // FIXME: properly unquote 'outputs'. StringMap rewrites; for (auto & outputName : tokenizeString>(replaceStrings(outputs->second.quoted, "'", ""))) { + // Hacky way to obtain the key of an associate array. This is needed for strctured attrs where + // `outputs` is an associative array. If the regex isn't matched, the non-structured-attrs behavior will + // be used. std::regex ptrn(R"re(\[([A-z0-9]+)\]=.*)re"); std::smatch match; if (std::regex_match(outputName, match, ptrn)) { diff --git a/src/nix/get-env.sh b/src/nix/get-env.sh index b6b8310a9..431440516 100644 --- a/src/nix/get-env.sh +++ b/src/nix/get-env.sh @@ -8,6 +8,9 @@ if [[ -n $stdenv ]]; then source $stdenv/setup fi +# In case of `__structuredAttrs = true;` the list of outputs is an associative +# array with a format like `outname => /nix/store/hash-drvname-outname`, so `__olist` +# must contain the array's keys (hence `${!...[@]}`) in this case. if [ -e .attrs.sh ]; then __olist="${!outputs[@]}" else @@ -16,10 +19,10 @@ fi for __output in $__olist; do if [[ -z $__done ]]; then - export > ${!__output} - set >> ${!__output} + export > "${!__output}" + set >> "${!__output}" __done=1 else - echo -n >> ${!__output} + echo -n >> "${!__output}" fi done diff --git a/tests/shell.nix b/tests/shell.nix index 24ebcc04c..53a32059b 100644 --- a/tests/shell.nix +++ b/tests/shell.nix @@ -8,6 +8,14 @@ let pkgs = rec { for pkg in $buildInputs; do export PATH=$PATH:$pkg/bin done + + # mimic behavior of stdenv for `$out` etc. for structured attrs. + if [ -n "''${ATTRS_SH_FILE}" ]; then + for o in "''${!outputs[@]}"; do + eval "''${o}=''${outputs[$o]}" + export "''${o}" + done + fi ''; stdenv = mkDerivation { diff --git a/tests/structured-attrs-shell.nix b/tests/structured-attrs-shell.nix index 0c01c2568..57c1e6bd2 100644 --- a/tests/structured-attrs-shell.nix +++ b/tests/structured-attrs-shell.nix @@ -6,10 +6,12 @@ let mkdir $out; echo bla > $out/bla ''; }; + inherit (import ./shell.nix { inNixShell = true; }) stdenv; in mkDerivation { name = "structured2"; __structuredAttrs = true; + inherit stdenv; outputs = [ "out" "dev" ]; my.list = [ "a" "b" "c" ]; exportReferencesGraph.refs = [ dep ]; diff --git a/tests/structured-attrs.sh b/tests/structured-attrs.sh index 241835539..f851b3cbb 100644 --- a/tests/structured-attrs.sh +++ b/tests/structured-attrs.sh @@ -12,3 +12,8 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result export NIX_BUILD_SHELL=$SHELL env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < $ATTRS_JSON_FILE)"' + +# `nix develop` is a slightly special way of dealing with environment vars, it parses +# these from a shell-file exported from a derivation. This is to test especially `outputs` +# (which is an associative array in thsi case) being fine. +nix develop -f structured-attrs-shell.nix -c bash -c 'test -n "$out"' From a92245b11026c884e76a1903abc0d47f84d79f5c Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 18 May 2021 15:17:02 +0200 Subject: [PATCH 6/9] Remove now-obsolete typedef --- src/libstore/parsed-derivations.hh | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index 37af36630..f02d5868e 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -6,8 +6,6 @@ namespace nix { -typedef std::pair StructuredAttrsWithShellRC; - class ParsedDerivation { StorePath drvPath; From 27ce722638eeabb987bc9b4a1234c2818c5bf401 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 22 Jun 2021 19:45:08 +0200 Subject: [PATCH 7/9] Prefix env vars for attrs.* files with NIX_ --- src/libstore/build/local-derivation-goal.cc | 4 ++-- src/nix-build/nix-build.cc | 4 ++-- tests/shell.nix | 2 +- tests/structured-attrs.nix | 2 +- tests/structured-attrs.sh | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 60495ae3e..de64737f4 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1092,10 +1092,10 @@ void LocalDerivationGoal::writeStructuredAttrs() writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); - env["ATTRS_SH_FILE"] = tmpDir + "/.attrs.sh"; + env["NIX_ATTRS_SH_FILE"] = tmpDir + "/.attrs.sh"; writeFile(tmpDir + "/.attrs.json", rewriteStrings(json.dump(), inputRewrites)); chownToBuilder(tmpDir + "/.attrs.json"); - env["ATTRS_JSON_FILE"] = tmpDir + "/.attrs.json"; + env["NIX_ATTRS_JSON_FILE"] = tmpDir + "/.attrs.json"; } } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 4e4f27458..f67776083 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -454,8 +454,8 @@ static void main_nix_build(int argc, char * * argv) auto attrsSH = (Path) tmpDir + "/.attrs.sh"; writeFile(attrsSH, structuredAttrsRC); - env["ATTRS_SH_FILE"] = attrsSH; - env["ATTRS_JSON_FILE"] = attrsJSON; + env["NIX_ATTRS_SH_FILE"] = attrsSH; + env["NIX_ATTRS_JSON_FILE"] = attrsJSON; keepTmp = true; } } diff --git a/tests/shell.nix b/tests/shell.nix index 53a32059b..4581fa40a 100644 --- a/tests/shell.nix +++ b/tests/shell.nix @@ -10,7 +10,7 @@ let pkgs = rec { done # mimic behavior of stdenv for `$out` etc. for structured attrs. - if [ -n "''${ATTRS_SH_FILE}" ]; then + if [ -n "''${NIX_ATTRS_SH_FILE}" ]; then for o in "''${!outputs[@]}"; do eval "''${o}=''${outputs[$o]}" export "''${o}" diff --git a/tests/structured-attrs.nix b/tests/structured-attrs.nix index f69ee45e9..e93139a44 100644 --- a/tests/structured-attrs.nix +++ b/tests/structured-attrs.nix @@ -36,7 +36,7 @@ mkDerivation { echo bar > $dest echo foo > $dest2 - json=$(cat $ATTRS_JSON_FILE) + json=$(cat $NIX_ATTRS_JSON_FILE) [[ $json =~ '"narHash":"sha256:1r7yc43zqnzl5b0als5vnyp649gk17i37s7mj00xr8kc47rjcybk"' ]] [[ $json =~ '"narSize":288' ]] [[ $json =~ '"closureSize":288' ]] diff --git a/tests/structured-attrs.sh b/tests/structured-attrs.sh index f851b3cbb..9612020b8 100644 --- a/tests/structured-attrs.sh +++ b/tests/structured-attrs.sh @@ -11,7 +11,7 @@ nix-build structured-attrs.nix -A all -o $TEST_ROOT/result export NIX_BUILD_SHELL=$SHELL env NIX_PATH=nixpkgs=shell.nix nix-shell structured-attrs-shell.nix \ - --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < $ATTRS_JSON_FILE)"' + --run 'test -e .attrs.json; test "3" = "$(jq ".my.list|length" < $NIX_ATTRS_JSON_FILE)"' # `nix develop` is a slightly special way of dealing with environment vars, it parses # these from a shell-file exported from a derivation. This is to test especially `outputs` From 6f206549ba02c6f9bdbf9707bba9193a1d82e822 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Tue, 22 Jun 2021 20:37:25 +0200 Subject: [PATCH 8/9] Move `writeStructuredAttrsShell` out of `ParsedDerivation` class --- src/libstore/build/local-derivation-goal.cc | 2 +- src/libstore/parsed-derivations.cc | 12 ++++++------ src/libstore/parsed-derivations.hh | 3 ++- src/libstore/store-api.hh | 3 +++ src/nix-build/nix-build.cc | 2 +- 5 files changed, 13 insertions(+), 9 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index de64737f4..15a7cb4ac 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1088,7 +1088,7 @@ void LocalDerivationGoal::writeStructuredAttrs() { if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(inputRewrites, worker.store, inputPaths)) { auto json = structAttrsJson.value(); - auto jsonSh = parsedDrv->writeStructuredAttrsShell(json); + auto jsonSh = writeStructuredAttrsShell(json); writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); chownToBuilder(tmpDir + "/.attrs.sh"); diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 5675600c4..f021e4de3 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -165,13 +165,13 @@ std::optional ParsedDerivation::prepareStructuredAttrs(std::opti return json; } -std::string ParsedDerivation::writeStructuredAttrsShell(nlohmann::json & json) +/* As a convenience to bash scripts, write a shell file that + maps all attributes that are representable in bash - + namely, strings, integers, nulls, Booleans, and arrays and + objects consisting entirely of those values. (So nested + arrays or objects are not supported.) */ +std::string writeStructuredAttrsShell(nlohmann::json & json) { - /* As a convenience to bash scripts, write a shell file that - maps all attributes that are representable in bash - - namely, strings, integers, nulls, Booleans, and arrays and - objects consisting entirely of those values. (So nested - arrays or objects are not supported.) */ auto handleSimpleType = [](const nlohmann::json & value) -> std::optional { if (value.is_string()) diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index f02d5868e..8061e3a8b 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -38,7 +38,8 @@ public: bool substitutesAllowed() const; std::optional prepareStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths); - std::string writeStructuredAttrsShell(nlohmann::json & json); }; +std::string writeStructuredAttrsShell(nlohmann::json & json); + } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index b742fed6d..7415cb54c 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -695,6 +695,9 @@ public: const Stats & getStats(); + /* Computes the full closure of of a set of store-paths for e.g. + derivations that need this information for `exportReferencesGraph`. + */ StorePathSet exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths); /* Return the build log of the specified store path, if available, diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index f67776083..459876465 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -446,7 +446,7 @@ static void main_nix_build(int argc, char * * argv) if (auto structAttrs = parsedDrv.prepareStructuredAttrs(std::nullopt, *store, inputs)) { auto json = structAttrs.value(); - structuredAttrsRC = parsedDrv.writeStructuredAttrsShell(json); + structuredAttrsRC = writeStructuredAttrsShell(json); auto attrsJSON = (Path) tmpDir + "/.attrs.json"; writeFile(attrsJSON, json.dump()); From 644415d3912633773d2c8f219572fbfa452f4b56 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Thu, 24 Jun 2021 15:06:07 +0200 Subject: [PATCH 9/9] Perform input rewrites only in LocalDerivationGoal --- src/libstore/build/local-derivation-goal.cc | 11 ++++++++++- src/libstore/parsed-derivations.cc | 11 ++--------- src/libstore/parsed-derivations.hh | 2 +- src/nix-build/nix-build.cc | 2 +- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 15a7cb4ac..259134eba 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1086,8 +1086,17 @@ void LocalDerivationGoal::initEnv() void LocalDerivationGoal::writeStructuredAttrs() { - if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(inputRewrites, worker.store, inputPaths)) { + if (auto structAttrsJson = parsedDrv->prepareStructuredAttrs(worker.store, inputPaths)) { auto json = structAttrsJson.value(); + nlohmann::json rewritten; + for (auto & [i, v] : json["outputs"].get()) { + /* The placeholder must have a rewrite, so we use it to cover both the + cases where we know or don't know the output path ahead of time. */ + rewritten[i] = rewriteStrings(v, inputRewrites); + } + + json["outputs"] = rewritten; + auto jsonSh = writeStructuredAttrsShell(json); writeFile(tmpDir + "/.attrs.sh", rewriteStrings(jsonSh, inputRewrites)); diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index f021e4de3..ee6bbb045 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -124,7 +124,7 @@ bool ParsedDerivation::substitutesAllowed() const } static std::regex shVarName("[A-Za-z_][A-Za-z0-9_]*"); -std::optional ParsedDerivation::prepareStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths) +std::optional ParsedDerivation::prepareStructuredAttrs(Store & store, const StorePathSet & inputPaths) { auto structuredAttrs = getStructuredAttrs(); if (!structuredAttrs) return std::nullopt; @@ -134,14 +134,7 @@ std::optional ParsedDerivation::prepareStructuredAttrs(std::opti /* Add an "outputs" object containing the output paths. */ nlohmann::json outputs; for (auto & i : drv.outputs) { - if (inputRewrites) { - /* The placeholder must have a rewrite, so we use it to cover both the - cases where we know or don't know the output path ahead of time. */ - outputs[i.first] = rewriteStrings(hashPlaceholder(i.first), inputRewrites.value()); - } else { - /* This case is only relevant for the nix-shell */ - outputs[i.first] = hashPlaceholder(i.first); - } + outputs[i.first] = hashPlaceholder(i.first); } json["outputs"] = outputs; diff --git a/src/libstore/parsed-derivations.hh b/src/libstore/parsed-derivations.hh index 8061e3a8b..5e3fb93ca 100644 --- a/src/libstore/parsed-derivations.hh +++ b/src/libstore/parsed-derivations.hh @@ -37,7 +37,7 @@ public: bool substitutesAllowed() const; - std::optional prepareStructuredAttrs(std::optional inputRewrites, Store & store, const StorePathSet & inputPaths); + std::optional prepareStructuredAttrs(Store & store, const StorePathSet & inputPaths); }; std::string writeStructuredAttrsShell(nlohmann::json & json); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 459876465..519cbd5bb 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -444,7 +444,7 @@ static void main_nix_build(int argc, char * * argv) drv ); - if (auto structAttrs = parsedDrv.prepareStructuredAttrs(std::nullopt, *store, inputs)) { + if (auto structAttrs = parsedDrv.prepareStructuredAttrs(*store, inputs)) { auto json = structAttrs.value(); structuredAttrsRC = writeStructuredAttrsShell(json);