From b13fd4c58e81b2b2b0d72caa5ce80de861622610 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Wed, 23 Nov 2022 11:39:50 -0500 Subject: [PATCH 1/2] Fix why-depends for CA derivations why-depends assumed that we knew the output path of the second argument. For CA derivations, we might not know until it's built. One way to solve this would be to build the second installable to get the output path. In this case we don't need to, though. If the first installable (A) depends on the second (B), then getting the store path of A will necessitate having the store path B. The contrapositive is, if the store path of B is not known (i.e. it's a CA derivation which hasn't been built), then A does not depend on B. --- src/nix/why-depends.cc | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 1d9ab28ba..285e36722 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -84,19 +84,35 @@ struct CmdWhyDepends : SourceExprCommand auto package = parseInstallable(store, _package); auto packagePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, package); auto dependency = parseInstallable(store, _dependency); - auto dependencyPath = Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency); - auto dependencyPathHash = dependencyPath.hashPart(); + 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()); 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(); From bd8571a5c3724ba5917564a5243af173966515c5 Mon Sep 17 00:00:00 2001 From: Taeer Bar-Yam Date: Wed, 23 Nov 2022 12:06:47 -0500 Subject: [PATCH 2/2] add explanation and test --- src/nix/why-depends.cc | 11 +++++++++++ tests/ca/why-depends.sh | 5 +++++ 2 files changed, 16 insertions(+) create mode 100644 tests/ca/why-depends.sh diff --git a/src/nix/why-depends.cc b/src/nix/why-depends.cc index 285e36722..723017497 100644 --- a/src/nix/why-depends.cc +++ b/src/nix/why-depends.cc @@ -83,6 +83,17 @@ struct CmdWhyDepends : SourceExprCommand { auto package = parseInstallable(store, _package); auto packagePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, package); + + /* We don't need to build `dependency`. We try to get the store + * path if it's already known, and if not, then it's not a dependency. + * + * Why? If `package` does depends on `dependency`, then getting the + * store path of `package` above necessitated having the store path + * of `dependency`. The contrapositive is, if the store path of + * `dependency` is not already known at this point (i.e. it's a CA + * derivation which hasn't been built), then `package` did not need it + * to build. + */ auto dependency = parseInstallable(store, _dependency); auto derivedDependency = dependency->toDerivedPath(); auto optDependencyPath = std::visit(overloaded { diff --git a/tests/ca/why-depends.sh b/tests/ca/why-depends.sh new file mode 100644 index 000000000..0c079f63b --- /dev/null +++ b/tests/ca/why-depends.sh @@ -0,0 +1,5 @@ +source common.sh + +export NIX_TESTS_CA_BY_DEFAULT=1 + +cd .. && source why-depends.sh