From 7f4f86795cc4215d0b8906d2203976768c5f7b21 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Wed, 25 Sep 2024 23:57:46 +0200 Subject: [PATCH] libstore: remove Goal::key this was a debugging aid from day one that should not have any impact on build semantics, and if it *does* have an impact on build semantics then build semantics are seriously broken. keeping the order imposed by these keys will be impossible once we let a real event loop schedule our jobs. Change-Id: I5c313324e1f213ab6453d82f41ae5e59de809a5b --- src/libstore/build/derivation-goal.cc | 10 ---------- src/libstore/build/derivation-goal.hh | 2 -- src/libstore/build/drv-output-substitution-goal.cc | 7 ------- src/libstore/build/drv-output-substitution-goal.hh | 2 -- src/libstore/build/goal.cc | 7 ------- src/libstore/build/goal.hh | 8 +------- src/libstore/build/substitution-goal.hh | 9 --------- tests/functional/build.sh | 7 ++----- 8 files changed, 3 insertions(+), 49 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 037b4fb10..f28285ad8 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -113,16 +113,6 @@ DerivationGoal::~DerivationGoal() noexcept(false) } -std::string DerivationGoal::key() -{ - /* Ensure that derivations get built in order of their name, - i.e. a derivation named "aardvark" always comes before - "baboon". And substitution goals always happen before - derivation goals (due to "b$"). */ - return "b$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); -} - - void DerivationGoal::killChild() { hook.reset(); diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index b1311bd7e..6e8e979d3 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -247,8 +247,6 @@ struct DerivationGoal : public Goal Finished timedOut(Error && ex); - std::string key() override; - kj::Promise> work() noexcept override; /** diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 923cbba58..846268a3a 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -166,13 +166,6 @@ try { return {std::current_exception()}; } -std::string DrvOutputSubstitutionGoal::key() -{ - /* "a$" ensures substitution goals happen before derivation - goals. */ - return "a$" + std::string(id.to_string()); -} - kj::Promise> DrvOutputSubstitutionGoal::work() noexcept { return (this->*state)(slotToken.valid()); diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 805b65bfa..543fa7ed4 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -74,8 +74,6 @@ public: kj::Promise> outPathValid(bool inBuildSlot) noexcept; kj::Promise> finished() noexcept; - std::string key() override; - kj::Promise> work() noexcept override; JobCategory jobCategory() const override { diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 5f0ed485c..957bc2aaf 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -6,13 +6,6 @@ namespace nix { -bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { - std::string s1 = a->key(); - std::string s2 = b->key(); - return s1 < s2; -} - - void Goal::trace(std::string_view s) { debug("%1%: %2%", name, s); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index f808be160..4436e44b1 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -22,14 +22,10 @@ class Worker; */ typedef std::shared_ptr GoalPtr; -struct CompareGoalPtrs { - bool operator() (const GoalPtr & a, const GoalPtr & b) const; -}; - /** * Set of goals. */ -typedef std::set Goals; +typedef std::set Goals; /** * Used as a hint to the worker on how to schedule a particular goal. For example, @@ -167,8 +163,6 @@ public: return name; } - virtual std::string key() = 0; - virtual void cleanup() { } /** diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index cef3a4c5c..5411afa01 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -90,15 +90,6 @@ public: ); ~PathSubstitutionGoal(); - /** - * We prepend "a$" to the key name to ensure substitution goals - * happen before derivation goals. - */ - std::string key() override - { - return "a$" + std::string(storePath.name()) + "$" + worker.store.printStorePath(storePath); - } - kj::Promise> work() noexcept override; /** diff --git a/tests/functional/build.sh b/tests/functional/build.sh index f80711b31..a14f6e3c2 100644 --- a/tests/functional/build.sh +++ b/tests/functional/build.sh @@ -146,11 +146,8 @@ out="$(nix build -f fod-failing.nix -L 2>&1)" && status=0 || status=$? test "$status" = 1 # one "hash mismatch" error, one "build of ... failed" test "$(<<<"$out" grep -E '^error:' | wc -l)" = 2 -<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x1\\.drv'" -<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x3\\.drv'" -<<<"$out" grepQuiet -vE "hash mismatch in fixed-output derivation '.*-x2\\.drv'" -<<<"$out" grepQuiet -E "likely URL: https://meow.puppy.forge/puppy.tar.gz" -<<<"$out" grepQuiet -vE "likely URL: https://kitty.forge/cat.tar.gz" +<<<"$out" grepQuiet -E "hash mismatch in fixed-output derivation '.*-x.\\.drv'" +<<<"$out" grepQuiet -E "likely URL: " <<<"$out" grepQuiet -E "error: build of '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out', '.*-x[1-4]\\.drv\\^out' failed" out="$(nix build -f fod-failing.nix -L x1 x2 x3 --keep-going 2>&1)" && status=0 || status=$?