aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorregnat <rg@regnat.ovh>2022-03-08 06:02:25 +0100
committerTaeer Bar-Yam <taeer.bar-yam@tweag.io>2022-12-08 16:03:20 -0500
commit04b113f6cb5356d44992f98928a595cdf93d561a (patch)
treee2bfb930f02057394be2714b6968a625677501ad
parent1dd7779c7cdca7061e80ebc3b7886a2aa97b0d8a (diff)
Fix `nix log` with CA derivations
Fix #6209 When trying to run `nix log <installable>`, try first to resolve the derivation pointed to by `<installable>` as it is the resolved one that holds the build log. This has a couple of shortcomings: 1. It’s expensive as it requires re-reading the derivation 2. It’s brittle because if the derivation doesn’t exist anymore or can’t be resolved (which is the case if any one of its build inputs is missing), then we can’t access the log anymore However, I don’t think we can do better (at least not right now). The alternatives I see are: 1. Copy the build log for the un-resolved derivation. But that means a lot of duplication 2. Store the results of the resolving in the db. Which might be the best long-term solution, but leads to a whole new class of potential issues.
-rw-r--r--src/libstore/binary-cache-store.cc16
-rw-r--r--src/libstore/local-fs-store.cc15
-rw-r--r--src/libstore/store-api.cc28
-rw-r--r--src/libstore/store-api.hh7
-rw-r--r--tests/build-hook-ca-floating.nix55
-rw-r--r--tests/build-hook.nix11
-rw-r--r--tests/build-remote.sh9
7 files changed, 59 insertions, 82 deletions
diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc
index 12d0c32fb..14584f0a2 100644
--- a/src/libstore/binary-cache-store.cc
+++ b/src/libstore/binary-cache-store.cc
@@ -504,18 +504,10 @@ void BinaryCacheStore::addSignatures(const StorePath & storePath, const StringSe
std::optional<std::string> BinaryCacheStore::getBuildLog(const StorePath & path)
{
- auto drvPath = path;
-
- if (!path.isDerivation()) {
- try {
- auto info = queryPathInfo(path);
- // FIXME: add a "Log" field to .narinfo
- if (!info->deriver) return std::nullopt;
- drvPath = *info->deriver;
- } catch (InvalidPath &) {
- return std::nullopt;
- }
- }
+ auto maybePath = getBuildDerivationPath(path);
+ if (!maybePath)
+ return std::nullopt;
+ auto drvPath = maybePath.value();
auto logPath = "log/" + std::string(baseNameOf(printStorePath(drvPath)));
diff --git a/src/libstore/local-fs-store.cc b/src/libstore/local-fs-store.cc
index c5ae7536f..3a0585b15 100644
--- a/src/libstore/local-fs-store.cc
+++ b/src/libstore/local-fs-store.cc
@@ -89,17 +89,10 @@ const std::string LocalFSStore::drvsLogDir = "drvs";
std::optional<std::string> LocalFSStore::getBuildLog(const StorePath & path_)
{
- auto path = path_;
-
- if (!path.isDerivation()) {
- try {
- auto info = queryPathInfo(path);
- if (!info->deriver) return std::nullopt;
- path = *info->deriver;
- } catch (InvalidPath &) {
- return std::nullopt;
- }
- }
+ auto maybePath = getBuildDerivationPath(path_);
+ if (!maybePath)
+ return std::nullopt;
+ auto path = maybePath.value();
auto baseName = path.to_string();
diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc
index 8811ab578..ebd641aa4 100644
--- a/src/libstore/store-api.cc
+++ b/src/libstore/store-api.cc
@@ -1300,6 +1300,34 @@ Derivation readDerivationCommon(Store& store, const StorePath& drvPath, bool req
}
}
+std::optional<StorePath> Store::getBuildDerivationPath(const StorePath & path)
+{
+
+ if (!path.isDerivation()) {
+ try {
+ auto info = queryPathInfo(path);
+ if (!info->deriver) return std::nullopt;
+ return *info->deriver;
+ } catch (InvalidPath &) {
+ return std::nullopt;
+ }
+ }
+
+ if (!settings.isExperimentalFeatureEnabled(Xp::CaDerivations) || !isValidPath(path))
+ return path;
+
+ auto drv = readDerivation(path);
+ if (!derivationHasKnownOutputPaths(drv.type())) {
+ // The build log is actually attached to the corresponding
+ // resolved derivation, so we need to get it first
+ auto resolvedDrv = drv.tryResolve(*this);
+ if (resolvedDrv)
+ return writeDerivation(*this, *resolvedDrv, NoRepair, true);
+ }
+
+ return path;
+}
+
Derivation Store::readDerivation(const StorePath & drvPath)
{ return readDerivationCommon(*this, drvPath, true); }
diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh
index 151ec10d6..88a11d953 100644
--- a/src/libstore/store-api.hh
+++ b/src/libstore/store-api.hh
@@ -618,6 +618,13 @@ public:
*/
StorePathSet exportReferences(const StorePathSet & storePaths, const StorePathSet & inputPaths);
+ /**
+ * Given a store path, return the realisation actually used in the realisation of this path:
+ * - If the path is a content-addressed derivation, try to resolve it
+ * - Otherwise, find one of its derivers
+ */
+ std::optional<StorePath> getBuildDerivationPath(const StorePath &);
+
/* Hack to allow long-running processes like hydra-queue-runner to
occasionally flush their path info cache. */
void clearPathInfoCache()
diff --git a/tests/build-hook-ca-floating.nix b/tests/build-hook-ca-floating.nix
index 67295985f..dfdbb82da 100644
--- a/tests/build-hook-ca-floating.nix
+++ b/tests/build-hook-ca-floating.nix
@@ -1,53 +1,6 @@
{ busybox }:
-with import ./config.nix;
-
-let
-
- mkDerivation = args:
- derivation ({
- inherit system;
- builder = busybox;
- args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
- outputHashMode = "recursive";
- outputHashAlgo = "sha256";
- __contentAddressed = true;
- } // removeAttrs args ["builder" "meta"])
- // { meta = args.meta or {}; };
-
- input1 = mkDerivation {
- shell = busybox;
- name = "build-remote-input-1";
- buildCommand = "echo FOO > $out";
- requiredSystemFeatures = ["foo"];
- };
-
- input2 = mkDerivation {
- shell = busybox;
- name = "build-remote-input-2";
- buildCommand = "echo BAR > $out";
- requiredSystemFeatures = ["bar"];
- };
-
- input3 = mkDerivation {
- shell = busybox;
- name = "build-remote-input-3";
- buildCommand = ''
- read x < ${input2}
- echo $x BAZ > $out
- '';
- requiredSystemFeatures = ["baz"];
- };
-
-in
-
- mkDerivation {
- shell = busybox;
- name = "build-remote";
- buildCommand =
- ''
- read x < ${input1}
- read y < ${input3}
- echo "$x $y" > $out
- '';
- }
+import ./build-hook.nix {
+ inherit busybox;
+ contentAddressed = true;
+}
diff --git a/tests/build-hook.nix b/tests/build-hook.nix
index 643334caa..7effd7903 100644
--- a/tests/build-hook.nix
+++ b/tests/build-hook.nix
@@ -1,15 +1,22 @@
-{ busybox }:
+{ busybox, contentAddressed ? false }:
with import ./config.nix;
let
+ caArgs = if contentAddressed then {
+ outputHashMode = "recursive";
+ outputHashAlgo = "sha256";
+ __contentAddressed = true;
+ } else {};
+
mkDerivation = args:
derivation ({
inherit system;
builder = busybox;
args = ["sh" "-e" args.builder or (builtins.toFile "builder-${args.name}.sh" "if [ -e .attrs.sh ]; then source .attrs.sh; fi; eval \"$buildCommand\"")];
- } // removeAttrs args ["builder" "meta" "passthru"])
+ } // removeAttrs args ["builder" "meta" "passthru"]
+ // caArgs)
// { meta = args.meta or {}; passthru = args.passthru or {}; };
input1 = mkDerivation {
diff --git a/tests/build-remote.sh b/tests/build-remote.sh
index e73c37ea4..25a482003 100644
--- a/tests/build-remote.sh
+++ b/tests/build-remote.sh
@@ -63,12 +63,9 @@ nix path-info --store $TEST_ROOT/machine3 --all \
| grep builder-build-remote-input-3.sh
-# Temporarily disabled because of https://github.com/NixOS/nix/issues/6209
-if [[ -z "$CONTENT_ADDRESSED" ]]; then
- for i in input1 input3; do
- nix log --store $TEST_ROOT/machine0 --file "$file" --arg busybox $busybox passthru."$i" | grep hi-$i
- done
-fi
+for i in input1 input3; do
+nix log --store $TEST_ROOT/machine0 --file "$file" --arg busybox $busybox passthru."$i" | grep hi-$i
+done
# Behavior of keep-failed
out="$(nix-build 2>&1 failing.nix \