From 523965697d9a16f0dd89f0d5b795b44ce5d6f712 Mon Sep 17 00:00:00 2001 From: Jade Lovelace Date: Fri, 14 Mar 2025 17:03:43 -0700 Subject: [PATCH] post-build-hook: don't emit config settings that aren't overridden This is a redesign from 9b1f3cbc133eafdaadf89ee9e4f3ce6a11cbbcce where this was introduced. I deleted the AbstractConfig::toKeyValue since it was conspicuously and obviously broken for two years since 450e5ec6185e2e1102e67ec7a348a0dc8955692d. I asked myself if anyone was using it, given that it only emitted settings that were aliases (broken!), and found that nobody used it. The motivation for this change is the same for only emitting overridden settings to the protocol: the nix inside there may not be able to parse our defaults, as is the case of CppNix since the consensual accept-flake-config was added to Lix. Fixes: https://git.lix.systems/lix-project/lix/issues/739 Change-Id: Ib9874a52137f1f22220c25bcfa2425a4802509c7 --- doc/manual/rl-next/post-build-hook-config.md | 10 ++++++++ lix/libstore/build/derivation-goal.cc | 2 +- lix/libutil/config.cc | 13 ++-------- lix/libutil/config.hh | 14 ++++------ tests/functional/post-hook.sh | 27 ++++++++++++++++++++ 5 files changed, 45 insertions(+), 21 deletions(-) create mode 100644 doc/manual/rl-next/post-build-hook-config.md diff --git a/doc/manual/rl-next/post-build-hook-config.md b/doc/manual/rl-next/post-build-hook-config.md new file mode 100644 index 000000000..f4663a67b --- /dev/null +++ b/doc/manual/rl-next/post-build-hook-config.md @@ -0,0 +1,10 @@ +--- +synopsis: "`post-build-hook` only receives settings that are set" +cls: [2800] +issues: [fj#739] +category: Fixes +credits: jade +--- +If one is using `post-build-hook` to upload paths to a cache, it used to be broken if CppNix was used inside the script, since CppNix would fail about unsupported configuration option values in some of Lix's defaults. +This is because `post-build-hook` receives the settings of the nix daemon in the `NIX_CONFIG` environment variable. +Now Lix only emits overridden settings to `post-build-hook` invocations, which fixes this issue in the majority of cases: where the configuration is not explicitly incompatible. diff --git a/lix/libstore/build/derivation-goal.cc b/lix/libstore/build/derivation-goal.cc index 8023281eb..2a1034748 100644 --- a/lix/libstore/build/derivation-goal.cc +++ b/lix/libstore/build/derivation-goal.cc @@ -977,7 +977,7 @@ void runPostBuildHook( auto drvPathPretty = store.printStorePath(drvPath); hookEnvironment.emplace("DRV_PATH", drvPathPretty); hookEnvironment.emplace("OUT_PATHS", chomp(concatStringsSep(" ", store.printStorePathSet(outputPaths)))); - hookEnvironment.emplace("NIX_CONFIG", globalConfig.toKeyValue()); + hookEnvironment.emplace("NIX_CONFIG", globalConfig.toKeyValue(true)); struct LogSink : Sink { Activity & act; diff --git a/lix/libutil/config.cc b/lix/libutil/config.cc index 5cbd81541..4fbfdc8e8 100644 --- a/lix/libutil/config.cc +++ b/lix/libutil/config.cc @@ -187,15 +187,6 @@ nlohmann::json Config::toJSON() return res; } -std::string Config::toKeyValue() -{ - std::string res; - for (const auto & s : _settings) - if (s.second.isAlias) - res += fmt("%s = %s\n", s.first, s.second.setting->to_string()); - return res; -} - void Config::convertToArgs(Args & args, const std::string & category) { for (auto & s : _settings) { @@ -534,11 +525,11 @@ nlohmann::json GlobalConfig::toJSON() return res; } -std::string GlobalConfig::toKeyValue() +std::string GlobalConfig::toKeyValue(bool overriddenOnly) { std::string res; std::map settings; - globalConfig.getSettings(settings); + globalConfig.getSettings(settings, overriddenOnly); for (const auto & s : settings) res += fmt("%s = %s\n", s.first, s.second.value); return res; diff --git a/lix/libutil/config.hh b/lix/libutil/config.hh index 33a60afde..fe14fcb4d 100644 --- a/lix/libutil/config.hh +++ b/lix/libutil/config.hh @@ -95,12 +95,6 @@ public: */ virtual nlohmann::json toJSON() = 0; - /** - * Outputs all settings in a key-value pair format suitable to be used as - * `nix.conf` - */ - virtual std::string toKeyValue() = 0; - /** * Converts settings to `Args` to be used on the command line interface * - args: args to write to @@ -166,8 +160,6 @@ public: nlohmann::json toJSON() override; - std::string toKeyValue() override; - void convertToArgs(Args & args, const std::string & category) override; }; @@ -371,7 +363,11 @@ struct GlobalConfig : public AbstractConfig nlohmann::json toJSON() override; - std::string toKeyValue() override; + /** + * Outputs all settings in a key-value pair format suitable to be used as + * `nix.conf` + */ + std::string toKeyValue(bool overriddenOnly = false); void convertToArgs(Args & args, const std::string & category) override; diff --git a/tests/functional/post-hook.sh b/tests/functional/post-hook.sh index 752f8220c..4be7da417 100644 --- a/tests/functional/post-hook.sh +++ b/tests/functional/post-hook.sh @@ -29,3 +29,30 @@ clearStore nix copy --from "$REMOTE_STORE" --no-require-sigs -f dependencies.nix nix copy --from "$REMOTE_STORE" --no-require-sigs -f dependencies.nix input1_drv nix copy --from "$REMOTE_STORE" --no-require-sigs -f multiple-outputs.nix a^second + +clearStore + +# Should fail if the build hook fails +cat > "$TEST_ROOT/fail.sh" <<-EOF +#!${shell} +false +EOF +chmod +x "$TEST_ROOT/fail.sh" +expect 1 nix-build -o "$TEST_ROOT/result" dependencies.nix --post-build-hook "$TEST_ROOT/fail.sh" +clearStore + +# Ensure that settings are passed into the post-build-hook, but only overridden +# ones. +rm -f "${TEST_ROOT}/nix-config" +cat > "${TEST_ROOT}/settings.sh" <<-EOF +#!${shell} +echo "\$NIX_CONFIG" > "${TEST_ROOT}/nix-config" +EOF +chmod +x "${TEST_ROOT}/settings.sh" +nix-build -o "${TEST_ROOT}/result" dependencies.nix --timeout 1337 --post-build-hook "${TEST_ROOT}/settings.sh" +# Ensure pure-eval cannot become not a setting with the test passing. +nix config show pure-eval +# Defaulted setting does not appear. +expect 1 grepQuiet pure-eval "${TEST_ROOT}/nix-config" +# Overridden setting appears. +grepQuiet "timeout = 1337" "${TEST_ROOT}/nix-config"