Skip to content

Commit

Permalink
Restore input substitution
Browse files Browse the repository at this point in the history
The ability to substitute inputs was removed in NixOS#10612 because it was
broken: with user-specified inputs containing a `narHash` attribute,
substitution resulted in an input that lacked the attributes returned
by the real fetcher (such as `lastModified`).

To fix this, we introduce a new input attribute `final`. If `final =
true`, fetching the input cannot add or change any attributes.

We only attempt to substitute inputs that have `final = true`. This is
implied by lock file entries; we only write a lock file if all its
entries are "final".

The user can specified `final = true` in `fetchTree`, in which case it
is their responsibility to ensure that all attributes returned by the
fetcher are included in the `fetchTree` call. For example,

  nix eval --impure --expr 'builtins.fetchTree { type = "github"; owner = "NixOS"; repo = "patchelf"; final = true; narHash = "sha256-FSoxTcRZMGHNJh8dNtKOkcUtjhmhU6yQXcZZfUPLhQM="; }'

succeeds in a store path with the specified NAR hash exists or is
substitutable, but fails with

  error: fetching final input '{"final":true,"narHash":"sha256-FSoxTcRZMGHNJh8dNtKOkcUtjhmhU6yQXcZZfUPLhQM=","owner":"NixOS","repo":"patchelf","type":"github"}' resulted in different input '{"final":true,"lastModified":1718457448,"narHash":"sha256-FSoxTcRZMGHNJh8dNtKOkcUtjhmhU6yQXcZZfUPLhQM=","owner":"NixOS","repo":"patchelf","rev":"a0f54334df36770b335c051e540ba40afcbf8378","type":"github"}'
  • Loading branch information
edolstra committed Oct 15, 2024
1 parent 806a91f commit 188d97e
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 21 deletions.
3 changes: 2 additions & 1 deletion src/libexpr/call-flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ let
overrides.${key}.sourceInfo
else
# FIXME: remove obsolete node.info.
fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);
# Note: lock file entries are always final.
fetchTree (node.info or {} // removeAttrs node.locked ["dir"] // { final = true; });

subdir = overrides.${key}.dir or node.locked.dir or "";

Expand Down
46 changes: 45 additions & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "source-path.hh"
#include "fetch-to-store.hh"
#include "json-utils.hh"
#include "store-path-accessor.hh"

#include <nlohmann/json.hpp>

Expand Down Expand Up @@ -100,7 +101,7 @@ Input Input::fromAttrs(const Settings & settings, Attrs && attrs)
auto allowedAttrs = inputScheme->allowedAttrs();

for (auto & [name, _] : attrs)
if (name != "type" && allowedAttrs.count(name) == 0)
if (name != "type" && name != "final" && allowedAttrs.count(name) == 0)
throw Error("input attribute '%s' not supported by scheme '%s'", name, schemeName);

auto res = inputScheme->inputFromAttrs(settings, attrs);
Expand Down Expand Up @@ -145,6 +146,11 @@ bool Input::isLocked() const
return scheme && scheme->isLocked(*this);
}

bool Input::isFinal() const
{
return maybeGetBoolAttr(attrs, "final").value_or(false);
}

Attrs Input::toAttrs() const
{
return attrs;
Expand Down Expand Up @@ -221,6 +227,12 @@ void InputScheme::checkLocks(const Input & specified, const Input & final) const
throw Error("'revCount' attribute mismatch in input '%s', expected %d",
final.to_string(), *prevRevCount);
}

assert(final.isFinal());

if (specified.isFinal() && specified.attrs != final.attrs)
throw Error("fetching final input '%s' resulted in different input '%s'",
attrsToJSON(specified.attrs), attrsToJSON(final.attrs));
}

std::pair<ref<SourceAccessor>, Input> Input::getAccessor(ref<Store> store) const
Expand All @@ -244,11 +256,43 @@ std::pair<ref<SourceAccessor>, Input> Input::getAccessorUnchecked(ref<Store> sto
if (!scheme)
throw Error("cannot fetch unsupported input '%s'", attrsToJSON(toAttrs()));

/* The tree may already be in the Nix store, or it could be
substituted (which is often faster than fetching from the
original source). So check that. We only do this for final
inputs, otherwise there is a risk that we don't return the
same attributes (like `lastModified`) that the "real" fetcher
would return.
FIXME: add a setting to disable this.
FIXME: substituting may be slower than fetching normally,
e.g. for fetchers like that Git that are incremental!
*/
if (isFinal() && getNarHash()) {
try {
auto storePath = computeStorePath(*store);

store->ensurePath(storePath);

debug("using substituted/cached input '%s' in '%s'",
to_string(), store->printStorePath(storePath));

auto accessor = makeStorePathAccessor(store, storePath);

accessor->fingerprint = scheme->getFingerprint(store, *this);

return {accessor, *this};
} catch (Error & e) {
debug("substitution of input '%s' failed: %s", to_string(), e.what());
}
}

auto [accessor, final] = scheme->getAccessor(store, *this);

assert(!accessor->fingerprint);
accessor->fingerprint = scheme->getFingerprint(store, final);

final.attrs.insert_or_assign("final", Explicit<bool>(true));

return {accessor, std::move(final)};
}

Expand Down
33 changes: 19 additions & 14 deletions src/libfetchers/fetchers.hh
Original file line number Diff line number Diff line change
Expand Up @@ -84,11 +84,21 @@ public:
bool isDirect() const;

/**
* Check whether this is a "locked" input, that is,
* one that contains a commit hash or content hash.
* Check whether this is a "locked" input, that is, it has
* attributes like a Git revision or NAR hash that uniquely
* identify its contents.
*/
bool isLocked() const;

/**
* Check whether this is a "final" input, meaning that fetching it
* will not add or change any attributes. For instance, a Git
* input with a `rev` attribute but without a `lastModified`
* attribute is considered locked but not final. Only "final"
* inputs can be substituted from a binary cache.
*/
bool isFinal() const;

bool operator ==(const Input & other) const noexcept;

bool contains(const Input & other) const;
Expand Down Expand Up @@ -144,6 +154,10 @@ public:
/**
* For locked inputs, return a string that uniquely specifies the
* content of the input (typically a commit hash or content hash).
*
* Only known-equivalent inputs should return the same fingerprint.
*
* This is not a stable identifier between Nix versions, but not guaranteed to change either.
*/
std::optional<std::string> getFingerprint(ref<Store> store) const;
};
Expand Down Expand Up @@ -212,24 +226,15 @@ struct InputScheme
*/
virtual std::optional<ExperimentalFeature> experimentalFeature() const;

/// See `Input::isDirect()`.
virtual bool isDirect(const Input & input) const
{ return true; }

/**
* A sufficiently unique string that can be used as a cache key to identify the `input`.
*
* Only known-equivalent inputs should return the same fingerprint.
*
* This is not a stable identifier between Nix versions, but not guaranteed to change either.
*/
/// See `Input::getFingerprint()`.
virtual std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const
{ return std::nullopt; }

/**
* Return `true` if this input is considered "locked", i.e. it has
* attributes like a Git revision or NAR hash that uniquely
* identify its contents.
*/
/// See `Input::isLocked()`.
virtual bool isLocked(const Input & input) const
{ return false; }

Expand Down
1 change: 1 addition & 0 deletions src/libfetchers/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ struct PathInputScheme : InputScheme
auto query = attrsToQuery(input.attrs);
query.erase("path");
query.erase("type");
query.erase("final");
return ParsedURL {
.scheme = "path",
.path = getStrAttr(input.attrs, "path"),
Expand Down
1 change: 0 additions & 1 deletion src/libflake/flake/flake.cc
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,6 @@ static void forceTrivialValue(EvalState & state, Value & value, const PosIdx pos
state.forceValue(value, pos);
}


static void expectType(EvalState & state, ValueType type,
Value & value, const PosIdx pos)
{
Expand Down
12 changes: 10 additions & 2 deletions src/libflake/flake/lockfile.cc
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,17 @@ LockedNode::LockedNode(
if (!lockedRef.input.isLocked())
throw Error("lock file contains unlocked input '%s'",
fetchers::attrsToJSON(lockedRef.input.toAttrs()));

// For backward compatibility, lock file entries are implicitly final.
assert(!lockedRef.input.attrs.contains("final"));
lockedRef.input.attrs.insert_or_assign("final", Explicit<bool>(true));
}

StorePath LockedNode::computeStorePath(Store & store) const
{
return lockedRef.input.computeStorePath(store);
}


static std::shared_ptr<Node> doFind(const ref<Node> & root, const InputPath & path, std::vector<InputPath> & visited)
{
auto pos = root;
Expand Down Expand Up @@ -191,6 +194,11 @@ std::pair<nlohmann::json, LockFile::KeyMap> LockFile::toJSON() const
if (auto lockedNode = node.dynamic_pointer_cast<const LockedNode>()) {
n["original"] = fetchers::attrsToJSON(lockedNode->originalRef.toAttrs());
n["locked"] = fetchers::attrsToJSON(lockedNode->lockedRef.toAttrs());
/* For backward compatibility, omit the "final"
attribute. We never allow non-final inputs in lock files
anyway. */
assert(lockedNode->lockedRef.input.isFinal());
n["locked"].erase("final");
if (!lockedNode->isFlake)
n["flake"] = false;
}
Expand Down Expand Up @@ -239,7 +247,7 @@ std::optional<FlakeRef> LockFile::isUnlocked() const
for (auto & i : nodes) {
if (i == ref<const Node>(root)) continue;
auto node = i.dynamic_pointer_cast<const LockedNode>();
if (node && !node->lockedRef.input.isLocked())
if (node && (!node->lockedRef.input.isLocked() || !node->lockedRef.input.isFinal()))
return node->lockedRef;
}

Expand Down
4 changes: 2 additions & 2 deletions src/libflake/flake/lockfile.hh
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ struct LockFile
std::pair<std::string, KeyMap> to_string() const;

/**
* Check whether this lock file has any unlocked inputs. If so,
* return one.
* Check whether this lock file has any unlocked or non-final
* inputs. If so, return one.
*/
std::optional<FlakeRef> isUnlocked() const;

Expand Down

0 comments on commit 188d97e

Please sign in to comment.