Skip to content

Actually ignore system/user registries during locking (2nd attempt) #13082

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.

Sign up for GitHub

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

Merged
Mic92 merged 2 commits into from
Apr 25, 2025
Merged

Actually ignore system/user registries during locking (2nd attempt) #13082

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dda265f
Reapply "Actually ignore system/user registries during locking"
edolstra Apr 24, 2025
68de26d
lockFlake(): Allow registry lookups for the top-level flake
edolstra Apr 24, 2025
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
2 changes: 1 addition & 1 deletion src/libcmd/installables.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ void completeFlakeInputAttrPath(
std::string_view prefix)
{
for (auto & flakeRef : flakeRefs) {
auto flake = flake::getFlake(*evalState, flakeRef, true);
auto flake = flake::getFlake(*evalState, flakeRef, fetchers::UseRegistries::All);
for (auto & input : flake.inputs)
if (hasPrefix(input.first, prefix))
completions.add(input.first);
Expand Down
2 changes: 1 addition & 1 deletion src/libexpr/primops/fetchTree.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ static void fetchTree(
}

if (!state.settings.pureEval && !input.isDirect() && experimentalFeatureSettings.isEnabled(Xp::Flakes))
input = lookupInRegistries(state.store, input).first;
input = lookupInRegistries(state.store, input, fetchers::UseRegistries::Limited).first;

if (state.settings.pureEval && !input.isLocked()) {
if (input.getNarHash())
Expand Down
4 changes: 3 additions & 1 deletion src/libfetchers/include/nix/fetchers/input-cache.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace nix::fetchers {

enum class UseRegistries : int;

struct InputCache
{
struct CachedResult
Expand All @@ -11,7 +13,7 @@ struct InputCache
Input lockedInput;
};

CachedResult getAccessor(ref<Store> store, const Input & originalInput, bool useRegistries);
CachedResult getAccessor(ref<Store> store, const Input & originalInput, UseRegistries useRegistries);

struct CachedInput
{
Expand Down
8 changes: 6 additions & 2 deletions src/libfetchers/include/nix/fetchers/registry.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ void overrideRegistry(
const Input & to,
const Attrs & extraAttrs);

using RegistryFilter = std::function<bool(Registry::RegistryType)>;
enum class UseRegistries : int {
No,
All,
Limited, // global and flag registry only
};

/**
* Rewrite a flakeref using the registries. If `filter` is set, only
Expand All @@ -74,6 +78,6 @@ using RegistryFilter = std::function<bool(Registry::RegistryType)>;
std::pair<Input, Attrs> lookupInRegistries(
ref<Store> store,
const Input & input,
const RegistryFilter & filter = {});
UseRegistries useRegistries);

}
12 changes: 4 additions & 8 deletions src/libfetchers/input-cache.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@

namespace nix::fetchers {

InputCache::CachedResult InputCache::getAccessor(ref<Store> store, const Input & originalInput, bool useRegistries)
InputCache::CachedResult
InputCache::getAccessor(ref<Store> store, const Input & originalInput, UseRegistries useRegistries)
{
auto fetched = lookup(originalInput);
Input resolvedInput = originalInput;
Expand All @@ -15,13 +16,8 @@ InputCache::CachedResult InputCache::getAccessor(ref<Store> store, const Input &
auto [accessor, lockedInput] = originalInput.getAccessor(store);
fetched.emplace(CachedInput{.lockedInput = lockedInput, .accessor = accessor});
} else {
if (useRegistries) {
auto [res, extraAttrs] =
lookupInRegistries(store, originalInput, [](fetchers::Registry::RegistryType type) {
/* Only use the global registry and CLI flags
to resolve indirect flakerefs. */
return type == fetchers::Registry::Flag || type == fetchers::Registry::Global;
});
if (useRegistries != UseRegistries::No) {
auto [res, extraAttrs] = lookupInRegistries(store, originalInput, useRegistries);
resolvedInput = std::move(res);
fetched = lookup(resolvedInput);
if (!fetched) {
Expand Down
13 changes: 11 additions & 2 deletions src/libfetchers/registry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ std::shared_ptr<Registry> Registry::read(
const Settings & settings,
const Path & path, RegistryType type)
{
debug("reading registry '%s'", path);

auto registry = std::make_shared<Registry>(settings, type);

if (!pathExists(path))
Expand Down Expand Up @@ -179,29 +181,36 @@ Registries getRegistries(const Settings & settings, ref<Store> store)
std::pair<Input, Attrs> lookupInRegistries(
ref<Store> store,
const Input & _input,
const RegistryFilter & filter)
UseRegistries useRegistries)
{
Attrs extraAttrs;
int n = 0;
Input input(_input);

if (useRegistries == UseRegistries::No)
return {input, extraAttrs};

restart:

n++;
if (n > 100) throw Error("cycle detected in flake registry for '%s'", input.to_string());

for (auto & registry : getRegistries(*input.settings, store)) {
if (filter && !filter(registry->type)) continue;
if (useRegistries == UseRegistries::Limited
&& !(registry->type == fetchers::Registry::Flag || registry->type == fetchers::Registry::Global))
continue;
// FIXME: O(n)
for (auto & entry : registry->entries) {
if (entry.exact) {
if (entry.from == input) {
debug("resolved flakeref '%s' against registry %d exactly", input.to_string(), registry->type);
input = entry.to;
extraAttrs = entry.extraAttrs;
goto restart;
}
} else {
if (entry.from.contains(input)) {
debug("resolved flakeref '%s' against registry %d", input.to_string(), registry->type);
input = entry.to.applyOverrides(
!entry.from.getRef() && input.getRef() ? input.getRef() : std::optional<std::string>(),
!entry.from.getRev() && input.getRev() ? input.getRev() : std::optional<Hash>());
Expand Down
28 changes: 22 additions & 6 deletions src/libflake/flake.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ static FlakeRef applySelfAttrs(
static Flake getFlake(
EvalState & state,
const FlakeRef & originalRef,
bool useRegistries,
fetchers::UseRegistries useRegistries,
const InputAttrPath & lockRootAttrPath)
{
// Fetch a lazy tree first.
Expand Down Expand Up @@ -368,7 +368,7 @@ static Flake getFlake(
return readFlake(state, originalRef, resolvedRef, lockedRef, state.storePath(storePath), lockRootAttrPath);
}

Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool useRegistries)
Flake getFlake(EvalState & state, const FlakeRef & originalRef, fetchers::UseRegistries useRegistries)
{
return getFlake(state, originalRef, useRegistries, {});
}
Expand All @@ -393,8 +393,14 @@ LockedFlake lockFlake(
experimentalFeatureSettings.require(Xp::Flakes);

auto useRegistries = lockFlags.useRegistries.value_or(settings.useRegistries);
auto useRegistriesTop = useRegistries ? fetchers::UseRegistries::All : fetchers::UseRegistries::No;
auto useRegistriesInputs = useRegistries ? fetchers::UseRegistries::Limited : fetchers::UseRegistries::No;

auto flake = getFlake(state, topRef, useRegistries, {});
auto flake = getFlake(
state,
topRef,
useRegistriesTop,
{});

if (lockFlags.applyNixConfig) {
flake.config.apply(settings);
Expand Down Expand Up @@ -569,7 +575,11 @@ LockedFlake lockFlake(
if (auto resolvedPath = resolveRelativePath()) {
return readFlake(state, ref, ref, ref, *resolvedPath, inputAttrPath);
} else {
return getFlake(state, ref, useRegistries, inputAttrPath);
return getFlake(
state,
ref,
useRegistriesInputs,
inputAttrPath);
}
};

Expand Down Expand Up @@ -717,7 +727,10 @@ LockedFlake lockFlake(
if (auto resolvedPath = resolveRelativePath()) {
return {*resolvedPath, *input.ref};
} else {
auto cachedInput = state.inputCache->getAccessor(state.store, input.ref->input, useRegistries);
auto cachedInput = state.inputCache->getAccessor(
state.store,
input.ref->input,
useRegistriesInputs);

auto lockedRef = FlakeRef(std::move(cachedInput.lockedInput), input.ref->subdir);

Expand Down Expand Up @@ -834,7 +847,10 @@ LockedFlake lockFlake(
repo, so we should re-read it. FIXME: we could
also just clear the 'rev' field... */
auto prevLockedRef = flake.lockedRef;
flake = getFlake(state, topRef, useRegistries);
flake = getFlake(
state,
topRef,
useRegistriesTop);

if (lockFlags.commitLockFile &&
flake.lockedRef.input.getRev() &&
Expand Down
4 changes: 2 additions & 2 deletions src/libflake/flakeref.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ std::ostream & operator << (std::ostream & str, const FlakeRef & flakeRef)

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

Expand Down
2 changes: 1 addition & 1 deletion src/libflake/include/nix/flake/flake.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ struct Flake
}
};

Flake getFlake(EvalState & state, const FlakeRef & flakeRef, bool useRegistries);
Flake getFlake(EvalState & state, const FlakeRef & flakeRef, fetchers::UseRegistries useRegistries);

/**
* Fingerprint of a locked flake; used as a cache key.
Expand Down
2 changes: 1 addition & 1 deletion src/libflake/include/nix/flake/flakeref.hh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ struct FlakeRef

FlakeRef resolve(
ref<Store> store,
const fetchers::RegistryFilter & filter = {}) const;
fetchers::UseRegistries useRegistries = fetchers::UseRegistries::All) const;

static FlakeRef fromAttrs(
const fetchers::Settings & fetchSettings,
Expand Down
7 changes: 7 additions & 0 deletions tests/functional/flakes/flakes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ nix store gc
nix registry list --flake-registry "file://$registry" --refresh | grepQuiet flake3
mv "$registry.tmp" "$registry"

# Ensure that locking ignores the user registry.
mkdir -p "$TEST_HOME/.config/nix"
ln -sfn "$registry" "$TEST_HOME/.config/nix/registry.json"
nix flake metadata --flake-registry '' flake1
Copy link
Member

@Mic92 Mic92 Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edolstra it looks like the test was not able to capture the previous regression. Do you know why this is the case?

Copy link
Member

@Mic92 Mic92 Apr 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, We actually tried to use a flake from the registry. So this usecase is not tested in this pull request.

expectStderr 1 nix flake update --flake-registry '' --flake "$flake3Dir" | grepQuiet "cannot find flake 'flake:flake1' in the flake registries"
rm "$TEST_HOME/.config/nix/registry.json"

# Test whether flakes are registered as GC roots for offline use.
# FIXME: use tarballs rather than git.
rm -rf "$TEST_HOME/.cache"
Expand Down
Loading