Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Derivations can output "text-hashed" data #3959

Merged
merged 54 commits into from
May 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
a4e5de1
Derivations can output "text-hashed" data
Ericson2314 Oct 12, 2020
d8d7f50
Merge remote-tracking branch 'obsidian/path-info' into ca-drv-exotic
Ericson2314 Oct 13, 2020
00c607b
Work around clang destructing + capturing bug again
Ericson2314 Oct 13, 2020
b6b383d
Work around clang destructing + capturing bug yet again
Ericson2314 Oct 13, 2020
9c5de06
Merge remote-tracking branch 'obsidian/path-info' into ca-drv-exotic
Ericson2314 Oct 13, 2020
47f0d7b
Cleanup tabs
Ericson2314 Oct 13, 2020
4636cc9
Merge remote-tracking branch 'obsidian/path-info' into ca-drv-exotic
Ericson2314 Oct 15, 2020
90d76fa
Merge remote-tracking branch 'obsidian/path-info' into ca-drv-exotic
Ericson2314 Feb 25, 2021
7863036
Merge remote-tracking branch 'obsidian/path-info' into ca-drv-exotic
Ericson2314 Feb 27, 2021
cdc9f34
Merge commit 'e12308dd63f0ad27b22dcdb3da89c411eebcad2b' into ca-drv-e…
Ericson2314 Apr 5, 2021
386765e
Merge commit 'd5cef6c33a051dfc672cb1e5f4739948b167315b' into ca-drv-e…
Ericson2314 Apr 5, 2021
d0ed11c
Merge commit '1b6cf0d5f56e166a1cbbf38142375b7a92fc88f2' into ca-drv-e…
Ericson2314 Apr 5, 2021
9af9ab4
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Sep 30, 2021
d6e0c51
Fix texted hash output test to work when testing daemon
Ericson2314 Sep 30, 2021
edf67e1
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Oct 1, 2021
195daa8
Merge remote-tracking branch 'upstream/master' into ca-drv-exotic
Ericson2314 Oct 8, 2021
9386507
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Mar 10, 2022
ff2a8cc
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Mar 25, 2022
8abb627
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Apr 19, 2022
8f9990a
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Apr 19, 2022
08b8657
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Apr 19, 2022
989b806
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 6, 2023
85ceaad
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 6, 2023
848b083
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 6, 2023
7e1cfa9
Make derivation primop code for fixed output more concise
Ericson2314 Jan 6, 2023
81727f8
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 6, 2023
7c82213
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 14, 2023
30610f2
Use `builtins.unsafeDiscardOutputDependency` in the ca/text-hash-out …
Ericson2314 Jan 14, 2023
e68e8e3
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Jan 23, 2023
5abd643
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Feb 28, 2023
f7f44f7
Merge commit 'aa99005004bccc9be506a2a2f162f78bad4bcb41' into ca-drv-e…
Ericson2314 Apr 1, 2023
1f8e1ed
Merge commit 'a6d00a7bfb18e7ec461ac1d54203cc628aca5c66' into ca-drv-e…
Ericson2314 Apr 1, 2023
eeecfac
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Apr 1, 2023
1fcd49d
Merge branch 'path-info' into ca-drv-exotic
Ericson2314 Apr 1, 2023
e12efa3
Merge remote-tracking branch 'upstream/master' into ca-drv-exotic
Ericson2314 Apr 17, 2023
2eb493c
Fix `DerivationOutput::fromJSON`
Ericson2314 Apr 17, 2023
f56c4a5
Merge remote-tracking branch 'upstream/master' into ca-drv-exotic
Ericson2314 Apr 17, 2023
668377f
`TextHashMethod` -> `TextIngestionMethod`, gate with XP feature
Ericson2314 Apr 17, 2023
76baaeb
Merge remote-tracking branch 'upstream/master' into ca-drv-exotic
Ericson2314 Apr 19, 2023
20decfd
Gate `dynamic-derivations` with drv `fromJSON` too
Ericson2314 Apr 19, 2023
aba8a8a
Add a few more content addressing methods
Ericson2314 Apr 19, 2023
7103c6d
Remove references from fixed output derivation ab syntax
Ericson2314 Apr 19, 2023
61d3e64
Require daemon version for text hashing test
Ericson2314 Apr 19, 2023
f3a31b1
Make `tests/dyn-drv` test dir
Ericson2314 Apr 19, 2023
e266627
Add a more interesting test
Ericson2314 Apr 20, 2023
969def6
Fix typo in tests
Ericson2314 Apr 20, 2023
8eeaf59
Add more docs to `TextIngestionMethod`
Ericson2314 Apr 21, 2023
278c94d
Rename a few things in new tests
Ericson2314 May 5, 2023
35dcbe1
Fix spurious change
Ericson2314 May 9, 2023
6513f4f
Fix bug, `newInfo` -> `newInfo0`
Ericson2314 May 9, 2023
d3c125e
Apply suggestions from code review
Ericson2314 May 9, 2023
753fc16
Cleanups to content address types
Ericson2314 May 9, 2023
e514b39
Add name to some error messages
Ericson2314 May 9, 2023
6a3a87a
Improve error message for self reference with text hashing
Ericson2314 May 9, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ perl/Makefile.config
/tests/shell.drv
/tests/config.nix
/tests/ca/config.nix
/tests/dyn-drv/config.nix
/tests/repl-result-out

# /tests/lang/
Expand Down
44 changes: 23 additions & 21 deletions src/libexpr/primops.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1092,7 +1092,7 @@ drvName, Bindings * attrs, Value & v)
bool isImpure = false;
std::optional<std::string> outputHash;
std::string outputHashAlgo;
std::optional<FileIngestionMethod> ingestionMethod;
std::optional<ContentAddressMethod> ingestionMethod;

StringSet outputs;
outputs.insert("out");
Expand All @@ -1105,7 +1105,10 @@ drvName, Bindings * attrs, Value & v)
auto handleHashMode = [&](const std::string_view s) {
if (s == "recursive") ingestionMethod = FileIngestionMethod::Recursive;
else if (s == "flat") ingestionMethod = FileIngestionMethod::Flat;
else
else if (s == "text") {
experimentalFeatureSettings.require(Xp::DynamicDerivations);
ingestionMethod = TextIngestionMethod {};
} else
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("invalid value '%s' for 'outputHashMode' attribute", s),
.errPos = state.positions[noPos]
Expand Down Expand Up @@ -1273,11 +1276,16 @@ drvName, Bindings * attrs, Value & v)
}));

/* Check whether the derivation name is valid. */
if (isDerivation(drvName))
if (isDerivation(drvName) &&
!(ingestionMethod == ContentAddressMethod { TextIngestionMethod { } } &&
outputs.size() == 1 &&
*(outputs.begin()) == "out"))
{
state.debugThrowLastTrace(EvalError({
.msg = hintfmt("derivation names are not allowed to end in '%s'", drvExtension),
.msg = hintfmt("derivation names are allowed to end in '%s' only if they produce a single derivation file", drvExtension),
.errPos = state.positions[noPos]
}));
}

if (outputHash) {
/* Handle fixed-output derivations.
Expand All @@ -1293,21 +1301,15 @@ drvName, Bindings * attrs, Value & v)
auto h = newHashAllowEmpty(*outputHash, parseHashTypeOpt(outputHashAlgo));

auto method = ingestionMethod.value_or(FileIngestionMethod::Flat);
auto outPath = state.store->makeFixedOutputPath(drvName, FixedOutputInfo {
.hash = {
.method = method,
.hash = h,
},
.references = {},
});
drv.env["out"] = state.store->printStorePath(outPath);
drv.outputs.insert_or_assign("out",
DerivationOutput::CAFixed {
.hash = FixedOutputHash {
.method = method,
.hash = std::move(h),
},
});

DerivationOutput::CAFixed dof {
.ca = ContentAddress::fromParts(
std::move(method),
std::move(h)),
};

drv.env["out"] = state.store->printStorePath(dof.path(*state.store, drvName, "out"));
drv.outputs.insert_or_assign("out", std::move(dof));
}

else if (contentAddressed || isImpure) {
Expand All @@ -1325,13 +1327,13 @@ drvName, Bindings * attrs, Value & v)
if (isImpure)
drv.outputs.insert_or_assign(i,
DerivationOutput::Impure {
.method = method,
.method = method.raw,
.hashType = ht,
});
else
drv.outputs.insert_or_assign(i,
DerivationOutput::CAFloating {
.method = method,
.method = method.raw,
.hashType = ht,
});
}
Expand Down
6 changes: 4 additions & 2 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,13 @@ void DerivationGoal::haveDerivation()
)
)
);
else
else {
auto * cap = getDerivationCA(*drv);
addWaitee(upcast_goal(worker.makePathSubstitutionGoal(
status.known->path,
buildMode == bmRepair ? Repair : NoRepair,
getDerivationCA(*drv))));
cap ? std::optional { *cap } : std::nullopt)));
}
}

if (waitees.empty()) /* to prevent hang (no wake-up event) */
Expand Down
60 changes: 40 additions & 20 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2426,37 +2426,51 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
throw BuildError(
"output path %1% without valid stats info",
actualPath);
if (outputHash.method == FileIngestionMethod::Flat) {
if (outputHash.method == ContentAddressMethod { FileIngestionMethod::Flat } ||
outputHash.method == ContentAddressMethod { TextIngestionMethod {} })
{
/* The output path should be a regular file without execute permission. */
if (!S_ISREG(st->st_mode) || (st->st_mode & S_IXUSR) != 0)
throw BuildError(
"output path '%1%' should be a non-executable regular file "
"since recursive hashing is not enabled (outputHashMode=flat)",
"since recursive hashing is not enabled (one of outputHashMode={flat,text} is true)",
actualPath);
}
rewriteOutput();
/* FIXME optimize and deduplicate with addToStore */
std::string oldHashPart { scratchPath->hashPart() };
HashModuloSink caSink { outputHash.hashType, oldHashPart };
switch (outputHash.method) {
case FileIngestionMethod::Recursive:
dumpPath(actualPath, caSink);
break;
case FileIngestionMethod::Flat:
readFile(actualPath, caSink);
break;
}
std::visit(overloaded {
[&](const TextIngestionMethod &) {
readFile(actualPath, caSink);
},
[&](const FileIngestionMethod & m2) {
switch (m2) {
case FileIngestionMethod::Recursive:
dumpPath(actualPath, caSink);
break;
case FileIngestionMethod::Flat:
readFile(actualPath, caSink);
break;
}
},
}, outputHash.method.raw);
Ericson2314 marked this conversation as resolved.
Show resolved Hide resolved
auto got = caSink.finish().first;

auto optCA = ContentAddressWithReferences::fromPartsOpt(
outputHash.method,
std::move(got),
rewriteRefs());
if (!optCA) {
// TODO track distinct failure modes separately (at the time of
// writing there is just one but `nullopt` is unclear) so this
// message can't get out of sync.
throw BuildError("output path '%s' has illegal content address, probably a spurious self-reference with text hashing");
}
ValidPathInfo newInfo0 {
worker.store,
outputPathName(drv->name, outputName),
FixedOutputInfo {
.hash = {
.method = outputHash.method,
.hash = got,
},
.references = rewriteRefs(),
},
*std::move(optCA),
Hash::dummy,
};
if (*scratchPath != newInfo0.path) {
Expand Down Expand Up @@ -2503,13 +2517,14 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
},

[&](const DerivationOutput::CAFixed & dof) {
auto wanted = dof.ca.getHash();

auto newInfo0 = newInfoFromCA(DerivationOutput::CAFloating {
.method = dof.hash.method,
.hashType = dof.hash.hash.type,
.method = dof.ca.getMethod(),
.hashType = wanted.type,
});

/* Check wanted hash */
const Hash & wanted = dof.hash.hash;
assert(newInfo0.ca);
auto got = newInfo0.ca->getHash();
if (wanted != got) {
Expand All @@ -2522,6 +2537,11 @@ SingleDrvOutputs LocalDerivationGoal::registerOutputs()
wanted.to_string(SRI, true),
got.to_string(SRI, true)));
}
if (!newInfo0.references.empty())
delayedException = std::make_exception_ptr(
BuildError("illegal path references in fixed-output derivation '%s'",
worker.store.printStorePath(drvPath)));

return newInfo0;
},

Expand Down
Loading