Skip to content

Commit

Permalink
Remove TreeInfo
Browse files Browse the repository at this point in the history 
The attributes previously stored in TreeInfo (narHash, revCount,
lastModified) are now stored in Input. This makes it less arbitrary
what attributes are stored where.

As a result, the lock file format has changed. An entry like

    "info": {
      "lastModified": 1585405475,
      "narHash": "sha256-bESW0n4KgPmZ0luxvwJ+UyATrC6iIltVCsGdLiphVeE="
    },
    "locked": {
      "owner": "NixOS",
      "repo": "nixpkgs",
      "rev": "b88ff468e9850410070d4e0ccd68c7011f15b2be",
      "type": "github"
    },

is now stored as

    "locked": {
      "owner": "NixOS",
      "repo": "nixpkgs",
      "rev": "b88ff468e9850410070d4e0ccd68c7011f15b2be",
      "type": "github",
      "lastModified": 1585405475,
      "narHash": "sha256-bESW0n4KgPmZ0luxvwJ+UyATrC6iIltVCsGdLiphVeE="
    },

The 'Input' class is now a dumb set of attributes. All the fetcher
implementations subclass InputScheme, not Input. This simplifies the
API.

Also, fix substitution of flake inputs. This was broken since lazy
flake fetching started using fetchTree internally.
  • Loading branch information
@edolstra
edolstra committed May 29, 2020
1 parent 5633c09 commit 950b468
Show file tree
Hide file tree
Showing 30 changed files with 939 additions and 1,121 deletions.
2 changes: 1 addition & 1 deletion src/libexpr/common-eval-args.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ Path lookupFileArg(EvalState & state, string s)
if (isUri(s)) {
return state.store->toRealPath(
fetchers::downloadTarball(
state.store, resolveUri(s), "source", false).storePath);
state.store, resolveUri(s), "source", false).first.storePath);
} else if (s.size() > 2 && s.at(0) == '<' && s.at(s.size() - 1) == '>') {
Path p = s.substr(1, s.size() - 2);
return state.findFile(p);
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/flake/call-flake.nix
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ let
sourceInfo =
if key == lockFile.root
then rootSrc
else fetchTree ({ inherit (node.info) narHash; } // removeAttrs node.locked ["dir"]);
else fetchTree (node.info or {} // removeAttrs node.locked ["dir"]);
subdir = if key == lockFile.root then rootSubdir else node.locked.dir or "";
flake = import (sourceInfo + (if subdir != "" then "/" else "") + subdir + "/flake.nix");
inputs = builtins.mapAttrs (inputName: key: allNodes.${key}) (node.inputs or {});
Expand Down
52 changes: 24 additions & 28 deletions src/libexpr/flake/flake.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ static FlakeRef maybeLookupFlake(
const FlakeRef & flakeRef,
bool allowLookup)
{
if (!flakeRef.input->isDirect()) {
if (!flakeRef.input.isDirect()) {
if (allowLookup)
return flakeRef.resolve(store);
else
Expand Down Expand Up @@ -49,16 +49,15 @@ static FlakeRef lookupInFlakeCache(
static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
EvalState & state,
const FlakeRef & originalRef,
std::optional<TreeInfo> treeInfo,
bool allowLookup,
FlakeCache & flakeCache)
{
/* 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. */
if (treeInfo && originalRef.input->isDirect() && originalRef.input->isImmutable()) {
if (originalRef.input.isDirect() && originalRef.input.isImmutable() && originalRef.input.hasAllInfo()) {
try {
auto storePath = treeInfo->computeStorePath(*state.store);
auto storePath = originalRef.input.computeStorePath(*state.store);

state.store->ensurePath(storePath);

Expand All @@ -74,7 +73,6 @@ static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
Tree {
.actualPath = actualPath,
.storePath = std::move(storePath),
.info = *treeInfo,
},
originalRef,
originalRef
Expand All @@ -99,8 +97,7 @@ static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree(
if (state.allowedPaths)
state.allowedPaths->insert(tree.actualPath);

if (treeInfo)
assert(tree.storePath == treeInfo->computeStorePath(*state.store));
assert(!originalRef.input.getNarHash() || tree.storePath == originalRef.input.computeStorePath(*state.store));

return {std::move(tree), resolvedRef, lockedRef};
}
Expand Down Expand Up @@ -202,12 +199,11 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
static Flake getFlake(
EvalState & state,
const FlakeRef & originalRef,
std::optional<TreeInfo> treeInfo,
bool allowLookup,
FlakeCache & flakeCache)
{
auto [sourceInfo, resolvedRef, lockedRef] = fetchOrSubstituteTree(
state, originalRef, treeInfo, allowLookup, flakeCache);
state, originalRef, allowLookup, flakeCache);

// Guard against symlink attacks.
auto flakeFile = canonPath(sourceInfo.actualPath + "/" + lockedRef.subdir + "/flake.nix");
Expand Down Expand Up @@ -278,7 +274,7 @@ static Flake getFlake(
Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup)
{
FlakeCache flakeCache;
return getFlake(state, originalRef, {}, allowLookup, flakeCache);
return getFlake(state, originalRef, allowLookup, flakeCache);
}

/* Compute an in-memory lock file for the specified top-level flake,
Expand All @@ -292,7 +288,7 @@ LockedFlake lockFlake(

FlakeCache flakeCache;

auto flake = getFlake(state, topRef, {}, lockFlags.useRegistries, flakeCache);
auto flake = getFlake(state, topRef, lockFlags.useRegistries, flakeCache);

// FIXME: symlink attack
auto oldLockFile = LockFile::read(
Expand Down Expand Up @@ -393,7 +389,7 @@ LockedFlake lockFlake(
didn't change and there is no override from a
higher level flake. */
auto childNode = std::make_shared<LockedNode>(
oldLock->lockedRef, oldLock->originalRef, oldLock->info, oldLock->isFlake);
oldLock->lockedRef, oldLock->originalRef, oldLock->isFlake);

node->inputs.insert_or_assign(id, childNode);

Expand All @@ -409,7 +405,7 @@ LockedFlake lockFlake(

if (hasChildUpdate) {
auto inputFlake = getFlake(
state, oldLock->lockedRef, oldLock->info, false, flakeCache);
state, oldLock->lockedRef, false, flakeCache);
computeLocks(inputFlake.inputs, childNode, inputPath, oldLock);
} else {
/* No need to fetch this flake, we can be
Expand Down Expand Up @@ -440,11 +436,11 @@ LockedFlake lockFlake(
/* We need to create a new lock file entry. So fetch
this input. */

if (!lockFlags.allowMutable && !input.ref.input->isImmutable())
if (!lockFlags.allowMutable && !input.ref.input.isImmutable())
throw Error("cannot update flake input '%s' in pure mode", inputPathS);

if (input.isFlake) {
auto inputFlake = getFlake(state, input.ref, {}, lockFlags.useRegistries, flakeCache);
auto inputFlake = getFlake(state, input.ref, lockFlags.useRegistries, flakeCache);

/* Note: in case of an --override-input, we use
the *original* ref (input2.ref) for the
Expand All @@ -454,7 +450,7 @@ LockedFlake lockFlake(
file. That is, overrides are sticky unless you
use --no-write-lock-file. */
auto childNode = std::make_shared<LockedNode>(
inputFlake.lockedRef, input2.ref, inputFlake.sourceInfo->info);
inputFlake.lockedRef, input2.ref);

node->inputs.insert_or_assign(id, childNode);

Expand All @@ -479,9 +475,9 @@ LockedFlake lockFlake(

else {
auto [sourceInfo, resolvedRef, lockedRef] = fetchOrSubstituteTree(
state, input.ref, {}, lockFlags.useRegistries, flakeCache);
state, input.ref, lockFlags.useRegistries, flakeCache);
node->inputs.insert_or_assign(id,
std::make_shared<LockedNode>(lockedRef, input.ref, sourceInfo.info, false));
std::make_shared<LockedNode>(lockedRef, input.ref, false));
}
}
}
Expand Down Expand Up @@ -534,7 +530,7 @@ LockedFlake lockFlake(
printInfo("inputs of flake '%s' changed:\n%s", topRef, chomp(diff));

if (lockFlags.writeLockFile) {
if (auto sourcePath = topRef.input->getSourcePath()) {
if (auto sourcePath = topRef.input.getSourcePath()) {
if (!newLockFile.isImmutable()) {
if (settings.warnDirty)
warn("will not write lock file of flake '%s' because it has a mutable input", topRef);
Expand All @@ -555,7 +551,7 @@ LockedFlake lockFlake(

newLockFile.write(path);

topRef.input->markChangedFile(
topRef.input.markChangedFile(
(topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock",
lockFlags.commitLockFile
? std::optional<std::string>(fmt("%s: %s\n\nFlake input changes:\n\n%s",
Expand All @@ -567,19 +563,19 @@ LockedFlake lockFlake(
also just clear the 'rev' field... */
auto prevLockedRef = flake.lockedRef;
FlakeCache dummyCache;
flake = getFlake(state, topRef, {}, lockFlags.useRegistries, dummyCache);
flake = getFlake(state, topRef, lockFlags.useRegistries, dummyCache);

if (lockFlags.commitLockFile &&
flake.lockedRef.input->getRev() &&
prevLockedRef.input->getRev() != flake.lockedRef.input->getRev())
warn("committed new revision '%s'", flake.lockedRef.input->getRev()->gitRev());
flake.lockedRef.input.getRev() &&
prevLockedRef.input.getRev() != flake.lockedRef.input.getRev())
warn("committed new revision '%s'", flake.lockedRef.input.getRev()->gitRev());

/* Make sure that we picked up the change,
i.e. the tree should usually be dirty
now. Corner case: we could have reverted from a
dirty to a clean tree! */
if (flake.lockedRef.input == prevLockedRef.input
&& !flake.lockedRef.input->isImmutable())
&& !flake.lockedRef.input.isImmutable())
throw Error("'%s' did not change after I updated its 'flake.lock' file; is 'flake.lock' under version control?", flake.originalRef);
}
} else
Expand Down Expand Up @@ -625,7 +621,7 @@ static void prim_getFlake(EvalState & state, const Pos & pos, Value * * args, Va
{
auto flakeRefS = state.forceStringNoCtx(*args[0], pos);
auto flakeRef = parseFlakeRef(flakeRefS, {}, true);
if (evalSettings.pureEval && !flakeRef.input->isImmutable())
if (evalSettings.pureEval && !flakeRef.input.isImmutable())
throw Error("cannot call 'getFlake' on mutable flake reference '%s', at %s (use --impure to override)", flakeRefS, pos);

callFlake(state,
Expand All @@ -650,8 +646,8 @@ Fingerprint LockedFlake::getFingerprint() const
return hashString(htSHA256,
fmt("%s;%d;%d;%s",
flake.sourceInfo->storePath.to_string(),
flake.sourceInfo->info.revCount.value_or(0),
flake.sourceInfo->info.lastModified.value_or(0),
flake.lockedRef.input.getRevCount().value_or(0),
flake.lockedRef.input.getLastModified().value_or(0),
lockFile));
}

Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/flake/flake.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ void callFlake(
void emitTreeAttrs(
EvalState & state,
const fetchers::Tree & tree,
std::shared_ptr<const fetchers::Input> input,
const fetchers::Input & input,
Value & v);

}
22 changes: 11 additions & 11 deletions src/libexpr/flake/flakeref.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,15 @@ const static std::string subDirRegex = subDirElemRegex + "(?:/" + subDirElemRege

std::string FlakeRef::to_string() const
{
auto url = input->toURL();
auto url = input.toURL();
if (subdir != "")
url.query.insert_or_assign("dir", subdir);
return url.to_string();
}

fetchers::Attrs FlakeRef::toAttrs() const
{
auto attrs = input->toAttrs();
auto attrs = input.toAttrs();
if (subdir != "")
attrs.emplace("dir", subdir);
return attrs;
Expand All @@ -37,13 +37,13 @@ std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef)

bool FlakeRef::operator ==(const FlakeRef & other) const
{
return *input == *other.input && subdir == other.subdir;
return input == other.input && subdir == other.subdir;
}

FlakeRef FlakeRef::resolve(ref<Store> store) const
{
auto [input2, extraAttrs] = lookupInRegistries(store, input);
return FlakeRef(input2, fetchers::maybeGetStrAttr(extraAttrs, "dir").value_or(subdir));
return FlakeRef(std::move(input2), fetchers::maybeGetStrAttr(extraAttrs, "dir").value_or(subdir));
}

FlakeRef parseFlakeRef(
Expand Down Expand Up @@ -98,7 +98,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
};

return std::make_pair(
FlakeRef(inputFromURL(parsedURL), ""),
FlakeRef(Input::fromURL(parsedURL), ""),
percentDecode(std::string(match[6])));
}

Expand Down Expand Up @@ -143,7 +143,7 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
}

return std::make_pair(
FlakeRef(inputFromURL(parsedURL), get(parsedURL.query, "dir").value_or("")),
FlakeRef(Input::fromURL(parsedURL), get(parsedURL.query, "dir").value_or("")),
fragment);
}

Expand All @@ -155,15 +155,15 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
attrs.insert_or_assign("type", "path");
attrs.insert_or_assign("path", path);

return std::make_pair(FlakeRef(inputFromAttrs(attrs), ""), fragment);
return std::make_pair(FlakeRef(Input::fromAttrs(std::move(attrs)), ""), fragment);
}

else {
auto parsedURL = parseURL(url);
std::string fragment;
std::swap(fragment, parsedURL.fragment);
return std::make_pair(
FlakeRef(inputFromURL(parsedURL), get(parsedURL.query, "dir").value_or("")),
FlakeRef(Input::fromURL(parsedURL), get(parsedURL.query, "dir").value_or("")),
fragment);
}
}
Expand All @@ -183,14 +183,14 @@ FlakeRef FlakeRef::fromAttrs(const fetchers::Attrs & attrs)
auto attrs2(attrs);
attrs2.erase("dir");
return FlakeRef(
fetchers::inputFromAttrs(attrs2),
fetchers::Input::fromAttrs(std::move(attrs2)),
fetchers::maybeGetStrAttr(attrs, "dir").value_or(""));
}

std::pair<fetchers::Tree, FlakeRef> FlakeRef::fetchTree(ref<Store> store) const
{
auto [tree, lockedInput] = input->fetchTree(store);
return {std::move(tree), FlakeRef(lockedInput, subdir)};
auto [tree, lockedInput] = input.fetch(store);
return {std::move(tree), FlakeRef(std::move(lockedInput), subdir)};
}

}
10 changes: 4 additions & 6 deletions src/libexpr/flake/flakeref.hh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,15 @@ typedef std::string FlakeId;

struct FlakeRef
{
std::shared_ptr<const fetchers::Input> input;
fetchers::Input input;

Path subdir;

bool operator==(const FlakeRef & other) const;

FlakeRef(const std::shared_ptr<const fetchers::Input> & input, const Path & subdir)
: input(input), subdir(subdir)
{
assert(input);
}
FlakeRef(fetchers::Input && input, const Path & subdir)
: input(std::move(input)), subdir(subdir)
{ }

// FIXME: change to operator <<.
std::string to_string() const;
Expand Down
Loading

0 comments on commit 950b468

Please sign in to comment.