Skip to content

Commit

Permalink
Special-case error message to add extra information
Browse files Browse the repository at this point in the history
The Derivation parser and old ATerm unfortunately leaves few ways to get
nice errors when an old version of Nix encounters a new version of the
format. The most likely scenario for this to occur is with a new client
making a derivation that the old daemon it is communicating with cannot
understand.

The extensions we just created for dynamic derivation deps will add a
version field, solving the problem going forward, but there is still the
issue of what to do about old versions of Nix up to now.

The solution here is to carefully catch the bad error from the daemon
that is likely to indicate this problem, and add some extra context to
it.

There is another "Ugly backwards compatibility hack" in
`remote-store.cc` that also works by transforming an error.

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
  • Loading branch information
Ericson2314 and roberth committed Sep 7, 2023
1 parent 7ad66cb commit 80d7994
Show file tree
Hide file tree
Showing 4 changed files with 51 additions and 2 deletions.
19 changes: 18 additions & 1 deletion src/libstore/remote-store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,24 @@ void RemoteStore::ConnectionHandle::processStderr(Sink * sink, Source * source,
auto ex = handle->processStderr(sink, source, flush);
if (ex) {
daemonException = true;
std::rethrow_exception(ex);
try {
std::rethrow_exception(ex);
} catch (const Error & e) {
// Nix versions before #4628 did not have an adequate behavior for reporting that the derivation format was upgraded.
// To avoid having to add compatibility logic in many places, we expect to catch almost all occurrences of the
// old incomprehensible error here, so that we can explain to users what's going on when their daemon is
// older than #4628 (2023).
if (experimentalFeatureSettings.isEnabled(Xp::DynamicDerivations) &&
GET_PROTOCOL_MINOR(handle->daemonVersion) <= 35)
{
auto m = e.msg();
if (m.find("parsing derivation") != std::string::npos &&
m.find("expected string") != std::string::npos &&
m.find("Derive([") != std::string::npos)
throw Error("%s, this might be because the daemon is too old to understand dependencies on dynamic derivations. Check to see if the raw dervation is in the form '%s'", std::move(m), "DrvWithVersion(..)");
}
throw;
}
}
}

Expand Down
3 changes: 2 additions & 1 deletion tests/dyn-drv/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ dyn-drv-tests := \
$(d)/recursive-mod-json.sh \
$(d)/build-built-drv.sh \
$(d)/eval-outputOf.sh \
$(d)/dep-built-drv.sh
$(d)/dep-built-drv.sh \
$(d)/old-daemon-error-hack.sh

install-tests-groups += dyn-drv

Expand Down
20 changes: 20 additions & 0 deletions tests/dyn-drv/old-daemon-error-hack.nix
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
with import ./config.nix;

# A simple content-addressed derivation.
# The derivation can be arbitrarily modified by passing a different `seed`,
# but the output will always be the same
rec {
stub = mkDerivation {
name = "stub";
buildCommand = ''
echo stub > $out
'';
};
wrapper = mkDerivation {
name = "has-dynamic-drv-dep";
buildCommand = ''
exit 1 # we're not building this derivation
${builtins.outputOf stub.outPath "out"}
'';
};
}
11 changes: 11 additions & 0 deletions tests/dyn-drv/old-daemon-error-hack.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Purposely bypassing our usual common for this subgroup
source ../common.sh

# Need backend to support text-hashing too
isDaemonNewer "2.18.0pre20230906" && skipTest "Daemon is too new"

enableFeatures "ca-derivations dynamic-derivations"

restartDaemon

expectStderr 1 nix-instantiate --read-write-mode ./old-daemon-error-hack.nix | grepQuiet "the daemon is too old to understand dependencies on dynamic derivations"

0 comments on commit 80d7994

Please sign in to comment.