From 0123640009c1340db7a36e2da1301ce7a4fc530d Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 3 Feb 2025 10:28:39 -0500 Subject: [PATCH] `ParsedDerivation`: don't take `drvPath` It is just use for adding context to errors, but we have `addTrace` to do that. Let the callers do that instead. The callers doing so is a bit duplicated, yes, but this will get better once `DerivationOptions` is included in `Derivation`. --- src/libstore-tests/derivation-advanced-attrs.cc | 8 ++++---- src/libstore/build/derivation-goal.cc | 9 +++++++-- .../include/nix/store/parsed-derivations.hh | 3 +-- src/libstore/misc.cc | 10 ++++++++-- src/libstore/parsed-derivations.cc | 14 +++++++------- src/nix-build/nix-build.cc | 10 ++++++++-- 6 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/libstore-tests/derivation-advanced-attrs.cc b/src/libstore-tests/derivation-advanced-attrs.cc index 9a901ebff..b8cfa2498 100644 --- a/src/libstore-tests/derivation-advanced-attrs.cc +++ b/src/libstore-tests/derivation-advanced-attrs.cc @@ -81,7 +81,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_defaults) auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(drvPath, got); + ParsedDerivation parsedDrv(got); DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv); EXPECT_TRUE(!parsedDrv.hasStructuredAttrs()); @@ -116,7 +116,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes) auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(drvPath, got); + ParsedDerivation parsedDrv(got); DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv); StringSet systemFeatures{"rainbow", "uid-range"}; @@ -157,7 +157,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(drvPath, got); + ParsedDerivation parsedDrv(got); DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv); EXPECT_TRUE(parsedDrv.hasStructuredAttrs()); @@ -191,7 +191,7 @@ TEST_F(DerivationAdvancedAttrsTest, Derivation_advancedAttributes_structuredAttr auto drvPath = writeDerivation(*store, got, NoRepair, true); - ParsedDerivation parsedDrv(drvPath, got); + ParsedDerivation parsedDrv(got); DerivationOptions options = DerivationOptions::fromParsedDerivation(parsedDrv); StringSet systemFeatures{"rainbow", "uid-range"}; diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 76456dac5..8e43ab2ae 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -180,8 +180,13 @@ Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); - parsedDrv = std::make_unique(drvPath, *drv); - drvOptions = std::make_unique(DerivationOptions::fromParsedDerivation(*parsedDrv)); + parsedDrv = std::make_unique(*drv); + try { + drvOptions = std::make_unique(DerivationOptions::fromParsedDerivation(*parsedDrv)); + } catch (Error & e) { + e.addTrace({}, "while parsing derivation '%s'", worker.store.printStorePath(drvPath)); + throw; + } if (!drv->type().hasKnownOutputPaths()) experimentalFeatureSettings.require(Xp::CaDerivations); diff --git a/src/libstore/include/nix/store/parsed-derivations.hh b/src/libstore/include/nix/store/parsed-derivations.hh index d65db6133..2b1ab97d5 100644 --- a/src/libstore/include/nix/store/parsed-derivations.hh +++ b/src/libstore/include/nix/store/parsed-derivations.hh @@ -12,7 +12,6 @@ struct DerivationOptions; class ParsedDerivation { - StorePath drvPath; BasicDerivation & drv; std::unique_ptr structuredAttrs; @@ -34,7 +33,7 @@ class ParsedDerivation public: - ParsedDerivation(const StorePath & drvPath, BasicDerivation & drv); + ParsedDerivation(BasicDerivation & drv); ~ParsedDerivation(); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 0e2b62db5..b9e729092 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -222,8 +222,14 @@ void Store::queryMissing(const std::vector & targets, if (knownOutputPaths && invalid.empty()) return; auto drv = make_ref(derivationFromPath(drvPath)); - ParsedDerivation parsedDrv(StorePath(drvPath), *drv); - DerivationOptions drvOptions = DerivationOptions::fromParsedDerivation(parsedDrv); + ParsedDerivation parsedDrv(*drv); + DerivationOptions drvOptions; + try { + drvOptions = DerivationOptions::fromParsedDerivation(parsedDrv); + } catch (Error & e) { + e.addTrace({}, "while parsing derivation '%s'", printStorePath(drvPath)); + throw; + } if (!knownOutputPaths && settings.useSubstitutes && drvOptions.substitutesAllowed()) { experimentalFeatureSettings.require(Xp::CaDerivations); diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index cc7203c6b..66bf76cac 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -5,8 +5,8 @@ namespace nix { -ParsedDerivation::ParsedDerivation(const StorePath & drvPath, BasicDerivation & drv) - : drvPath(drvPath), drv(drv) +ParsedDerivation::ParsedDerivation(BasicDerivation & drv) + : drv(drv) { /* Parse the __json attribute, if any. */ auto jsonAttr = drv.env.find("__json"); @@ -14,7 +14,7 @@ ParsedDerivation::ParsedDerivation(const StorePath & drvPath, BasicDerivation & try { structuredAttrs = std::make_unique(nlohmann::json::parse(jsonAttr->second)); } catch (std::exception & e) { - throw Error("cannot process __json attribute of '%s': %s", drvPath.to_string(), e.what()); + throw Error("cannot process __json attribute: %s", e.what()); } } } @@ -29,7 +29,7 @@ std::optional ParsedDerivation::getStringAttr(const std::string & n return {}; else { if (!i->is_string()) - throw Error("attribute '%s' of derivation '%s' must be a string", name, drvPath.to_string()); + throw Error("attribute '%s' of must be a string", name); return i->get(); } } else { @@ -49,7 +49,7 @@ bool ParsedDerivation::getBoolAttr(const std::string & name, bool def) const return def; else { if (!i->is_boolean()) - throw Error("attribute '%s' of derivation '%s' must be a Boolean", name, drvPath.to_string()); + throw Error("attribute '%s' must be a Boolean", name); return i->get(); } } else { @@ -69,11 +69,11 @@ std::optional ParsedDerivation::getStringsAttr(const std::string & name return {}; else { if (!i->is_array()) - throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath.to_string()); + throw Error("attribute '%s' must be a list of strings", name); Strings res; for (auto j = i->begin(); j != i->end(); ++j) { if (!j->is_string()) - throw Error("attribute '%s' of derivation '%s' must be a list of strings", name, drvPath.to_string()); + throw Error("attribute '%s' must be a list of strings", name); res.push_back(j->get()); } return res; diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 45f891808..d90630438 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -544,8 +544,14 @@ static void main_nix_build(int argc, char * * argv) env["NIX_STORE"] = store->storeDir; env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); - ParsedDerivation parsedDrv(packageInfo.requireDrvPath(), drv); - DerivationOptions drvOptions = DerivationOptions::fromParsedDerivation(parsedDrv); + ParsedDerivation parsedDrv(drv); + DerivationOptions drvOptions; + try { + drvOptions = DerivationOptions::fromParsedDerivation(parsedDrv); + } catch (Error & e) { + e.addTrace({}, "while parsing derivation '%s'", store->printStorePath(packageInfo.requireDrvPath())); + throw; + } int fileNr = 0;