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

Restore input substitution #11701

Merged
merged 17 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
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
54 changes: 51 additions & 3 deletions 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 @@ -179,6 +185,14 @@ std::pair<StorePath, Input> Input::fetchToStore(ref<Store> store) const
auto narHash = store->queryPathInfo(storePath)->narHash;
final.attrs.insert_or_assign("narHash", narHash.to_string(HashFormat::SRI, true));

// FIXME: we would like to mark inputs as final in
// getAccessorUnchecked(), but then we can't add
// narHash. Or maybe narHash should be excluded from the
// concept of "final" inputs?
final.attrs.insert_or_assign("final", Explicit<bool>(true));

assert(final.isFinal());

scheme->checkLocks(*this, final);

return {storePath, final};
Expand Down Expand Up @@ -206,8 +220,8 @@ void InputScheme::checkLocks(const Input & specified, const Input & final) const

if (auto prevLastModified = specified.getLastModified()) {
if (final.getLastModified() != prevLastModified)
throw Error("'lastModified' attribute mismatch in input '%s', expected %d",
final.to_string(), *prevLastModified);
throw Error("'lastModified' attribute mismatch in input '%s', expected %d, got %d",
final.to_string(), *prevLastModified, final.getLastModified().value_or(-1));
}

if (auto prevRev = specified.getRev()) {
Expand All @@ -221,6 +235,10 @@ void InputScheme::checkLocks(const Input & specified, const Input & final) const
throw Error("'revCount' attribute mismatch in input '%s', expected %d",
final.to_string(), *prevRevCount);
}

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,6 +262,36 @@ 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.
edolstra marked this conversation as resolved.
Show resolved Hide resolved
FIXME: substituting may be slower than fetching normally,
e.g. for fetchers like 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);
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
edolstra marked this conversation as resolved.
Show resolved Hide resolved
* 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()`.
edolstra marked this conversation as resolved.
Show resolved Hide resolved
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
8 changes: 7 additions & 1 deletion src/libfetchers/tarball.cc
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,13 @@ struct TarballInputScheme : CurlInputScheme
input = immutableInput;
}

if (result.lastModified && !input.attrs.contains("lastModified"))
/* If we got a lastModified, then return it. But for
compatibility with old lock files that didn't include
lastModified, don't do this if the original input was final
and didn't contain a lastModified. */
if (result.lastModified
&& !input.attrs.contains("lastModified")
&& (!_input.isFinal() || _input.attrs.contains("lastModified")))
input.attrs.insert_or_assign("lastModified", uint64_t(result.lastModified));

input.attrs.insert_or_assign("narHash",
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.
edolstra marked this conversation as resolved.
Show resolved Hide resolved
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. */
edolstra marked this conversation as resolved.
Show resolved Hide resolved
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
Loading