Merge pull request #10757 from obsidiansystems/fix-4977

Require `drvPath` attribute to end with `.drv`
This commit is contained in:
John Ericson 2024-05-24 12:14:59 -04:00 committed by GitHub
commit e0c94b91ee
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 71 additions and 34 deletions

View file

@ -753,6 +753,7 @@ StorePath AttrCursor::forceDerivation()
{ {
auto aDrvPath = getAttr(root->state.sDrvPath, true); auto aDrvPath = getAttr(root->state.sDrvPath, true);
auto drvPath = root->state.store->parseStorePath(aDrvPath->getString()); auto drvPath = root->state.store->parseStorePath(aDrvPath->getString());
drvPath.requireDerivation();
if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) { if (!root->state.store->isValidPath(drvPath) && !settings.readOnlyMode) {
/* The eval cache contains 'drvPath', but the actual path has /* The eval cache contains 'drvPath', but the actual path has
been garbage-collected. So force it to be regenerated. */ been garbage-collected. So force it to be regenerated. */

View file

@ -27,8 +27,7 @@ EvalErrorBuilder<T> & EvalErrorBuilder<T>::atPos(Value & value, PosIdx fallback)
template<class T> template<class T>
EvalErrorBuilder<T> & EvalErrorBuilder<T>::withTrace(PosIdx pos, const std::string_view text) EvalErrorBuilder<T> & EvalErrorBuilder<T>::withTrace(PosIdx pos, const std::string_view text)
{ {
error.err.traces.push_front( error.addTrace(error.state.positions[pos], text);
Trace{.pos = error.state.positions[pos], .hint = HintFmt(std::string(text))});
return *this; return *this;
} }

View file

@ -69,13 +69,21 @@ std::string PackageInfo::querySystem() const
std::optional<StorePath> PackageInfo::queryDrvPath() const std::optional<StorePath> PackageInfo::queryDrvPath() const
{ {
if (!drvPath && attrs) { if (!drvPath && attrs) {
if (auto i = attrs->get(state->sDrvPath)) {
NixStringContext context; NixStringContext context;
if (auto i = attrs->get(state->sDrvPath)) auto found = state->coerceToStorePath(i->pos, *i->value, context, "while evaluating the 'drvPath' attribute of a derivation");
drvPath = {state->coerceToStorePath(i->pos, *i->value, context, "while evaluating the 'drvPath' attribute of a derivation")}; try {
else found.requireDerivation();
} catch (Error & e) {
e.addTrace(state->positions[i->pos], "while evaluating the 'drvPath' attribute of a derivation");
throw;
}
drvPath = {std::move(found)};
} else
drvPath = {std::nullopt}; drvPath = {std::nullopt};
} }
return drvPath.value_or(std::nullopt); drvPath.value_or(std::nullopt);
return *drvPath;
} }

View file

@ -271,16 +271,27 @@ private:
void printDerivation(Value & v) void printDerivation(Value & v)
{ {
std::optional<StorePath> storePath;
if (auto i = v.attrs()->get(state.sDrvPath)) {
NixStringContext context; NixStringContext context;
std::string storePath; storePath = state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation");
if (auto i = v.attrs()->get(state.sDrvPath)) }
storePath = state.store->printStorePath(state.coerceToStorePath(i->pos, *i->value, context, "while evaluating the drvPath of a derivation"));
/* This unforutately breaks printing nested values because of
how the pretty printer is used (when pretting printing and warning
to same terminal / std stream). */
#if 0
if (storePath && !storePath->isDerivation())
warn(
"drvPath attribute '%s' is not a valid store path to a derivation, this value not work properly",
state.store->printStorePath(*storePath));
#endif
if (options.ansiColors) if (options.ansiColors)
output << ANSI_GREEN; output << ANSI_GREEN;
output << "«derivation"; output << "«derivation";
if (!storePath.empty()) { if (storePath) {
output << " " << storePath; output << " " << state.store->printStorePath(*storePath);
} }
output << "»"; output << "»";
if (options.ansiColors) if (options.ansiColors)

View file

@ -930,10 +930,9 @@ DerivationOutputsAndOptPaths BasicDerivation::outputsAndOptPaths(const StoreDirC
std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath) std::string_view BasicDerivation::nameFromPath(const StorePath & drvPath)
{ {
drvPath.requireDerivation();
auto nameWithSuffix = drvPath.name(); auto nameWithSuffix = drvPath.name();
constexpr std::string_view extension = ".drv"; nameWithSuffix.remove_suffix(drvExtension.size());
assert(hasSuffix(nameWithSuffix, extension));
nameWithSuffix.remove_suffix(extension.size());
return nameWithSuffix; return nameWithSuffix;
} }

View file

@ -49,11 +49,17 @@ StorePath::StorePath(const Hash & hash, std::string_view _name)
checkName(baseName, name()); checkName(baseName, name());
} }
bool StorePath::isDerivation() const bool StorePath::isDerivation() const noexcept
{ {
return hasSuffix(name(), drvExtension); return hasSuffix(name(), drvExtension);
} }
void StorePath::requireDerivation() const
{
if (!isDerivation())
throw FormatError("store path '%s' is not a valid derivation path", to_string());
}
StorePath StorePath::dummy("ffffffffffffffffffffffffffffffff-x"); StorePath StorePath::dummy("ffffffffffffffffffffffffffffffff-x");
StorePath StorePath::random(std::string_view name) StorePath StorePath::random(std::string_view name)

View file

@ -35,30 +35,23 @@ public:
StorePath(const Hash & hash, std::string_view name); StorePath(const Hash & hash, std::string_view name);
std::string_view to_string() const std::string_view to_string() const noexcept
{ {
return baseName; return baseName;
} }
bool operator < (const StorePath & other) const bool operator == (const StorePath & other) const noexcept = default;
{ auto operator <=> (const StorePath & other) const noexcept = default;
return baseName < other.baseName;
}
bool operator == (const StorePath & other) const
{
return baseName == other.baseName;
}
bool operator != (const StorePath & other) const
{
return baseName != other.baseName;
}
/** /**
* Check whether a file name ends with the extension for derivations. * Check whether a file name ends with the extension for derivations.
*/ */
bool isDerivation() const; bool isDerivation() const noexcept;
/**
* Throw an exception if `isDerivation` is false.
*/
void requireDerivation() const;
std::string_view name() const std::string_view name() const
{ {
@ -82,7 +75,7 @@ typedef std::vector<StorePath> StorePaths;
* The file extension of \ref Derivation derivations when serialized * The file extension of \ref Derivation derivations when serialized
* into store objects. * into store objects.
*/ */
const std::string drvExtension = ".drv"; constexpr std::string_view drvExtension = ".drv";
} }

View file

@ -140,6 +140,7 @@ bool createUserEnv(EvalState & state, PackageInfos & elems,
NixStringContext context; NixStringContext context;
auto & aDrvPath(*topLevel.attrs()->find(state.sDrvPath)); auto & aDrvPath(*topLevel.attrs()->find(state.sDrvPath));
auto topLevelDrv = state.coerceToStorePath(aDrvPath.pos, *aDrvPath.value, context, ""); auto topLevelDrv = state.coerceToStorePath(aDrvPath.pos, *aDrvPath.value, context, "");
topLevelDrv.requireDerivation();
auto & aOutPath(*topLevel.attrs()->find(state.sOutPath)); auto & aOutPath(*topLevel.attrs()->find(state.sOutPath));
auto topLevelOut = state.coerceToStorePath(aOutPath.pos, *aOutPath.value, context, ""); auto topLevelOut = state.coerceToStorePath(aOutPath.pos, *aOutPath.value, context, "");

View file

@ -100,6 +100,8 @@ struct CmdBundle : InstallableValueCommand
NixStringContext context2; NixStringContext context2;
auto drvPath = evalState->coerceToStorePath(attr1->pos, *attr1->value, context2, ""); auto drvPath = evalState->coerceToStorePath(attr1->pos, *attr1->value, context2, "");
drvPath.requireDerivation();
auto attr2 = vRes->attrs()->get(evalState->sOutPath); auto attr2 = vRes->attrs()->get(evalState->sOutPath);
if (!attr2) if (!attr2)
throw Error("the bundler '%s' does not produce a derivation", bundler.what()); throw Error("the bundler '%s' does not produce a derivation", bundler.what());

View file

@ -24,6 +24,9 @@ nix-instantiate --eval -E 'builtins.traceVerbose "Hello" 123' 2>&1 | grepQuietIn
nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 | grepQuietInverse Hello nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" 123' 2>&1 | grepQuietInverse Hello
expectStderr 1 nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw "Foo")' | grepQuiet Hello expectStderr 1 nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello" (throw "Foo")' | grepQuiet Hello
expectStderr 1 nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello %" (throw "Foo")' | grepQuiet 'Hello %' expectStderr 1 nix-instantiate --show-trace --eval -E 'builtins.addErrorContext "Hello %" (throw "Foo")' | grepQuiet 'Hello %'
# Relies on parsing the expression derivation as a derivation, can't use --eval
expectStderr 1 nix-instantiate --show-trace lang/non-eval-fail-bad-drvPath.nix | grepQuiet "store path '8qlfcic10lw5304gqm8q45nr7g7jl62b-cachix-1.7.3-bin' is not a valid derivation path"
nix-instantiate --eval -E 'let x = builtins.trace { x = x; } true; in x' \ nix-instantiate --eval -E 'let x = builtins.trace { x = x; } true; in x' \
2>&1 | grepQuiet -E 'trace: { x = «potential infinite recursion»; }' 2>&1 | grepQuiet -E 'trace: { x = «potential infinite recursion»; }'

View file

@ -0,0 +1,14 @@
let
package = {
type = "derivation";
name = "cachix-1.7.3";
system = builtins.currentSystem;
outputs = [ "out" ];
# Illegal, because does not end in `.drv`
drvPath = "${builtins.storeDir}/8qlfcic10lw5304gqm8q45nr7g7jl62b-cachix-1.7.3-bin";
outputName = "out";
outPath = "${builtins.storeDir}/8qlfcic10lw5304gqm8q45nr7g7jl62b-cachix-1.7.3-bin";
out = package;
};
in
package