From 105d74eb8177016a1056b6642875c318a148a776 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 2 Jan 2023 15:44:04 +0100 Subject: [PATCH 1/3] Revert "Fix why-depends for CA derivations" This reverts commit b13fd4c58e81b2b2b0d72caa5ce80de861622610. --- src/nix/why-depends.cc | 28 ++++++---------------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 723017497..6512aee03 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -95,35 +95,19 @@ struct CmdWhyDepends : SourceExprCommand * to build. */ auto dependency = parseInstallable(store, _dependency); - auto derivedDependency = dependency->toDerivedPath(); - auto optDependencyPath = std::visit(overloaded { - [](const DerivedPath::Opaque & nodrv) -> std::optional { - return { nodrv.path }; - }, - [&](const DerivedPath::Built & hasdrv) -> std::optional { - if (hasdrv.outputs.size() != 1) { - throw Error("argument '%s' should evaluate to one store path", dependency->what()); - } - auto outputMap = store->queryPartialDerivationOutputMap(hasdrv.drvPath); - auto maybePath = outputMap.find(*hasdrv.outputs.begin()); - if (maybePath == outputMap.end()) { - throw Error("unexpected end of iterator"); - } - return maybePath->second; - }, - }, derivedDependency.raw()); + auto dependencyPath = Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency); + auto dependencyPathHash = dependencyPath.hashPart(); StorePathSet closure; store->computeFSClosure({packagePath}, closure, false, false); - if (!optDependencyPath.has_value() || !closure.count(*optDependencyPath)) { - printError("'%s' does not depend on '%s'", package->what(), dependency->what()); + if (!closure.count(dependencyPath)) { + printError("'%s' does not depend on '%s'", + store->printStorePath(packagePath), + store->printStorePath(dependencyPath)); return; } - auto dependencyPath = *optDependencyPath; - auto dependencyPathHash = dependencyPath.hashPart(); - stopProgressBar(); // FIXME auto accessor = store->getFSAccessor(); From 6a90ef072c2a5fcb7aada94763c7ccdb5ae2bae5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 2 Jan 2023 16:09:03 +0100 Subject: [PATCH 2/3] Increase the test coverage of `why-depends` - Test with `--derivation` - Actually test with ca-derivations (was suuposedly done, but not activated because of a missing line in `local.mk`) --- tests/local.mk | 1 + tests/why-depends.sh | 3 +++ 2 files changed, 4 insertions(+) diff --git a/tests/local.mk b/tests/local.mk index bba6ad9c9..2489baecf 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -92,6 +92,7 @@ nix_tests = \ fmt.sh \ eval-store.sh \ why-depends.sh \ + ca/why-depends.sh \ import-derivation.sh \ ca/import-derivation.sh \ nix_path.sh \ diff --git a/tests/why-depends.sh b/tests/why-depends.sh index c12941e76..a04d529b5 100644 --- a/tests/why-depends.sh +++ b/tests/why-depends.sh @@ -6,6 +6,9 @@ cp ./dependencies.nix ./dependencies.builder0.sh ./config.nix $TEST_HOME cd $TEST_HOME +nix why-depends --derivation --file ./dependencies.nix input2_drv input1_drv +nix why-depends --file ./dependencies.nix input2_drv input1_drv + nix-build ./dependencies.nix -A input0_drv -o dep nix-build ./dependencies.nix -o toplevel From 8cac451fce990151046996a13130bb1b91c6ba19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 2 Jan 2023 17:35:48 +0100 Subject: [PATCH 3/3] Fix why-depends for CA derivations (again) This has the same goal as b13fd4c58e81b2b2b0d72caa5ce80de861622610,but achieves it in a different way in order to not break `nix why-depends --derivation`. --- src/libcmd/installables.cc | 5 +---- src/libstore/realisation.hh | 10 ++++++++++ src/libstore/remote-store.cc | 5 +---- src/nix/why-depends.cc | 18 ++++++++++++------ 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index d2600ca91..b5675d5bd 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -931,10 +931,7 @@ std::vector, BuiltPathWithResult>> Instal DrvOutput outputId { *outputHash, output }; auto realisation = store->queryRealisation(outputId); if (!realisation) - throw Error( - "cannot operate on an output of the " - "unbuilt derivation '%s'", - outputId.to_string()); + throw MissingRealisation(outputId); outputs.insert_or_assign(output, realisation->outPath); } else { // If ca-derivations isn't enabled, assume that diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 9070a6ee2..911c61909 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -93,4 +93,14 @@ struct RealisedPath { GENERATE_CMP(RealisedPath, me->raw); }; +class MissingRealisation : public Error +{ +public: + MissingRealisation(DrvOutput & outputId) + : Error( "cannot operate on an output of the " + "unbuilt derivation '%s'", + outputId.to_string()) + {} +}; + } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 48cf731a8..ccf7d7e8b 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -879,10 +879,7 @@ std::vector RemoteStore::buildPathsWithResults( auto realisation = queryRealisation(outputId); if (!realisation) - throw Error( - "cannot operate on an output of unbuilt " - "content-addressed derivation '%s'", - outputId.to_string()); + throw MissingRealisation(outputId); res.builtOutputs.emplace(realisation->id, *realisation); } else { // If ca-derivations isn't enabled, assume that diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 6512aee03..76125e5e4 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -95,19 +95,25 @@ struct CmdWhyDepends : SourceExprCommand * to build. */ auto dependency = parseInstallable(store, _dependency); - auto dependencyPath = Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency); - auto dependencyPathHash = dependencyPath.hashPart(); + auto optDependencyPath = [&]() -> std::optional { + try { + return {Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency)}; + } catch (MissingRealisation &) { + return std::nullopt; + } + }(); StorePathSet closure; store->computeFSClosure({packagePath}, closure, false, false); - if (!closure.count(dependencyPath)) { - printError("'%s' does not depend on '%s'", - store->printStorePath(packagePath), - store->printStorePath(dependencyPath)); + if (!optDependencyPath.has_value() || !closure.count(*optDependencyPath)) { + printError("'%s' does not depend on '%s'", package->what(), dependency->what()); return; } + auto dependencyPath = *optDependencyPath; + auto dependencyPathHash = dependencyPath.hashPart(); + stopProgressBar(); // FIXME auto accessor = store->getFSAccessor();