From aeb7148afd56b228604b79373a45793d36d660a3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 18 Sep 2019 23:59:45 +0200 Subject: [PATCH] Some effort to minimize flake dependencies For example, if the top-level flake depends on "nixpkgs/release-19.03", and one of its dependencies depends on "nixpkgs", then the latter will be mapped to "nixpkgs/release-19.03", rather than whatever the default branch of "nixpkgs" is. Thus you get only one "nixpkgs" dependency rather than two. This currently only works in a breadth-first way, so the other way around (i.e. if the top-level flake depends on "nixpkgs", and a dependency depends on "nixpkgs/release-19.03") still results in two "nixpkgs" dependencies. --- src/libexpr/flake/flake.cc | 90 +++++++++++++++++++++++++++-------- src/libexpr/flake/flake.hh | 7 --- src/libexpr/flake/flakeref.cc | 17 +++++++ src/libexpr/flake/flakeref.hh | 6 +++ 4 files changed, 94 insertions(+), 26 deletions(-) diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index accdb4194..d9bbf85c2 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -112,7 +112,9 @@ static FlakeRef lookupFlake(EvalState & state, const FlakeRef & flakeRef, const return flakeRef; } -FlakeRef maybeLookupFlake( +/* If 'allowLookup' is true, then resolve 'flakeRef' using the + registries. */ +static FlakeRef maybeLookupFlake( EvalState & state, const FlakeRef & flakeRef, bool allowLookup) @@ -126,6 +128,23 @@ FlakeRef maybeLookupFlake( return flakeRef; } +typedef std::vector> RefMap; + +static FlakeRef lookupInRefMap( + const RefMap & refMap, + const FlakeRef & flakeRef) +{ + // FIXME: inefficient. + for (auto & i : refMap) { + if (flakeRef.contains(i.first)) { + debug("mapping '%s' to previously seen input '%s' -> '%s", + flakeRef, i.first, i.second); + return i.second; + } + } + + return flakeRef; +} static SourceInfo fetchFlake(EvalState & state, const FlakeRef & resolvedRef) { @@ -205,15 +224,21 @@ static void expectType(EvalState & state, ValueType type, showType(type), showType(value.type), pos); } -Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup) +static Flake getFlake(EvalState & state, const FlakeRef & originalRef, + bool allowLookup, RefMap & refMap) { - auto flakeRef = maybeLookupFlake(state, originalRef, allowLookup); + auto flakeRef = lookupInRefMap(refMap, + maybeLookupFlake(state, + lookupInRefMap(refMap, originalRef), allowLookup)); SourceInfo sourceInfo = fetchFlake(state, flakeRef); debug("got flake source '%s' with flakeref %s", sourceInfo.storePath, sourceInfo.resolvedRef.to_string()); FlakeRef resolvedRef = sourceInfo.resolvedRef; + refMap.push_back({originalRef, resolvedRef}); + refMap.push_back({flakeRef, resolvedRef}); + state.store->assertStorePath(sourceInfo.storePath); if (state.allowedPaths) @@ -314,13 +339,27 @@ Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup return flake; } -static SourceInfo getNonFlake(EvalState & state, const FlakeRef & flakeRef) +Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup) { + RefMap refMap; + return getFlake(state, originalRef, allowLookup, refMap); +} + +static SourceInfo getNonFlake(EvalState & state, const FlakeRef & originalRef, + bool allowLookup, RefMap & refMap) +{ + auto flakeRef = lookupInRefMap(refMap, + maybeLookupFlake(state, + lookupInRefMap(refMap, originalRef), allowLookup)); + auto sourceInfo = fetchFlake(state, flakeRef); debug("got non-flake source '%s' with flakeref %s", sourceInfo.storePath, sourceInfo.resolvedRef.to_string()); FlakeRef resolvedRef = sourceInfo.resolvedRef; + refMap.push_back({originalRef, resolvedRef}); + refMap.push_back({flakeRef, resolvedRef}); + state.store->assertStorePath(sourceInfo.storePath); if (state.allowedPaths) @@ -360,6 +399,7 @@ bool allowedToUseRegistries(HandleLockFile handle, bool isTopRef) Note that this is lazy: we only recursively fetch inputs that are not in the lockfile yet. */ static std::pair updateLocks( + RefMap & refMap, const std::string & inputPath, EvalState & state, const Flake & flake, @@ -372,6 +412,8 @@ static std::pair updateLocks( flake.originalRef, flake.sourceInfo.narHash); + std::vector> postponed; + for (auto & [id, input] : flake.inputs) { auto inputPath2 = (inputPath.empty() ? "" : inputPath + "/") + id; auto i = oldEntry.inputs.find(id); @@ -380,29 +422,36 @@ static std::pair updateLocks( } else { if (handleLockFile == AllPure || handleLockFile == TopRefUsesRegistries) throw Error("cannot update flake input '%s' in pure mode", id); - if (input.isFlake) { - auto actualInput = getFlake(state, input.ref, - allowedToUseRegistries(handleLockFile, false)); + + auto warn = [&](const SourceInfo & sourceInfo) { if (i == oldEntry.inputs.end()) - printMsg(lvlWarn, "mapped flake input '%s' to '%s'", - inputPath2, actualInput.sourceInfo.resolvedRef); + printInfo("mapped flake input '%s' to '%s'", + inputPath2, sourceInfo.resolvedRef); else printMsg(lvlWarn, "updated flake input '%s' from '%s' to '%s'", - inputPath2, i->second.originalRef, actualInput.sourceInfo.resolvedRef); - newEntry.inputs.insert_or_assign(id, - updateLocks(inputPath2, state, actualInput, handleLockFile, {}, false).second); + inputPath2, i->second.originalRef, sourceInfo.resolvedRef); + }; + + if (input.isFlake) { + auto actualInput = getFlake(state, input.ref, + allowedToUseRegistries(handleLockFile, false), refMap); + warn(actualInput.sourceInfo); + postponed.push_back([&, id{id}, inputPath2, actualInput]() { + newEntry.inputs.insert_or_assign(id, + updateLocks(refMap, inputPath2, state, actualInput, handleLockFile, {}, false).second); + }); } else { - auto sourceInfo = getNonFlake(state, - maybeLookupFlake(state, input.ref, - allowedToUseRegistries(handleLockFile, false))); - printMsg(lvlWarn, "mapped flake input '%s' to '%s'", - inputPath2, sourceInfo.resolvedRef); + auto sourceInfo = getNonFlake(state, input.ref, + allowedToUseRegistries(handleLockFile, false), refMap); + warn(sourceInfo); newEntry.inputs.insert_or_assign(id, LockedInput(sourceInfo.resolvedRef, input.ref, sourceInfo.narHash)); } } } + for (auto & f : postponed) f(); + return {flake, newEntry}; } @@ -423,8 +472,10 @@ ResolvedFlake resolveFlake(EvalState & state, const FlakeRef & topRef, HandleLoc + "/" + flake.sourceInfo.resolvedRef.subdir + "/flake.lock"); } + RefMap refMap; + LockFile lockFile(updateLocks( - "", state, flake, handleLockFile, oldLockFile, true).second); + refMap, "", state, flake, handleLockFile, oldLockFile, true).second); if (!(lockFile == oldLockFile)) { if (allowedToWrite(handleLockFile)) { @@ -500,7 +551,8 @@ static void prim_callFlake(EvalState & state, const Pos & pos, Value * * args, V callFlake(state, flake, lazyInput->lockedInput, v); } else { - auto sourceInfo = getNonFlake(state, lazyInput->lockedInput.ref); + RefMap refMap; + auto sourceInfo = getNonFlake(state, lazyInput->lockedInput.ref, false, refMap); if (sourceInfo.narHash != lazyInput->lockedInput.narHash) throw Error("the content hash of repository '%s' doesn't match the hash recorded in the referring lockfile", sourceInfo.resolvedRef); diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh index b366e650b..63d848889 100644 --- a/src/libexpr/flake/flake.hh +++ b/src/libexpr/flake/flake.hh @@ -80,13 +80,6 @@ struct Flake Flake getFlake(EvalState & state, const FlakeRef & flakeRef, bool allowLookup); -/* If 'allowLookup' is true, then resolve 'flakeRef' using the - registries. */ -FlakeRef maybeLookupFlake( - EvalState & state, - const FlakeRef & flakeRef, - bool allowLookup); - /* Fingerprint of a locked flake; used as a cache key. */ typedef Hash Fingerprint; diff --git a/src/libexpr/flake/flakeref.cc b/src/libexpr/flake/flakeref.cc index 4ce326c0b..4930d03ce 100644 --- a/src/libexpr/flake/flakeref.cc +++ b/src/libexpr/flake/flakeref.cc @@ -255,6 +255,23 @@ FlakeRef FlakeRef::baseRef() const // Removes the ref and rev from a FlakeRef. return result; } +bool FlakeRef::contains(const FlakeRef & other) const +{ + if (!(data == other.data)) + return false; + + if (ref && ref != other.ref) + return false; + + if (rev && rev != other.rev) + return false; + + if (subdir != other.subdir) + return false; + + return true; +} + std::optional parseFlakeRef( const std::string & uri, bool allowRelative) { diff --git a/src/libexpr/flake/flakeref.hh b/src/libexpr/flake/flakeref.hh index 6b47330a7..39e019dbd 100644 --- a/src/libexpr/flake/flakeref.hh +++ b/src/libexpr/flake/flakeref.hh @@ -182,6 +182,12 @@ struct FlakeRef return std::get_if(&data) && rev == Hash(rev->type); } + + /* Return true if 'other' is not less specific than 'this'. For + example, 'nixpkgs' contains 'nixpkgs/release-19.03', and both + 'nixpkgs' and 'nixpkgs/release-19.03' contain + 'nixpkgs/release-19.03/'. */ + bool contains(const FlakeRef & other) const; }; std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef);