Skip to content

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

Merged
Mic92 merged 2 commits intoNixOS:masterfrom
DeterminateSystems:fix-ignore-local-registries
Apr 25, 2025
Merged

Actually ignore system/user registries during locking (2nd attempt)#13082
Mic92 merged 2 commits intoNixOS:masterfrom
DeterminateSystems:fix-ignore-local-registries

Conversation

@edolstra
Copy link
Member

Motivation

Makes #12068 actually work. All registries are used for resolving top-level flakerefs, but only the global and flag registries are used for resolving flake inputs.

Fixes #13050.

Context


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

@github-actions github-actions bot added with-tests Issues related to testing. PRs with tests have some priority fetching Networking with the outside (non-Nix) world, input locking labels Apr 24, 2025
@edolstra edolstra force-pushed the fix-ignore-local-registries branch from b977dcb to 68de26d Compare April 24, 2025 16:59
@edolstra edolstra added the backport 2.28-maintenance Automatically creates a PR against the branch label Apr 24, 2025
@SharzyL
Copy link
Contributor

SharzyL commented Apr 25, 2025

Should we also add tests to verify user registry not ignored with other operations (to prevent something like #13050)?

# 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

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

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.

@Mic92
Copy link
Member

Mic92 commented Apr 25, 2025

Should we also add tests to verify user registry not ignored with other operations (to prevent something like #13050)?

Yeah. At least in nixpkgs we have now a patch merged and backported, so we can take this pull request a bit slower.

@Mic92
Copy link
Member

Mic92 commented Apr 25, 2025

Looks like the test is only testing "global" registry entries:

\[\][\[\]joerg@turingmachine:/home/joerg/git/nix/tests/functional/flakes]$\[\] nix registry list
global flake:flake2 git+file:///tmp/nix-test/flakes/flakes/flake%202
global flake:flake1 git+file:///tmp/nix-test/flakes/flakes/flake1
global flake:flake3 git+file:///tmp/nix-test/flakes/flakes/flake%2520
global flake:nixpkgs flake:flake1

@Mic92
Copy link
Member

Mic92 commented Apr 25, 2025

I added a regression test and made sure it fails before your fix: 57e3906
@edolstra would you mind pushing branches to the Nix repository directly. This would simplify collaboration because I am not allowed to push DeterminateSystems

@Mic92 Mic92 merged commit 68de26d into NixOS:master Apr 25, 2025
12 checks passed
edolstra added a commit that referenced this pull request Apr 25, 2025
…3082

Actually ignore system/user registries during locking (2nd attempt) (backport #13082)
@cole-h cole-h deleted the fix-ignore-local-registries branch April 25, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport 2.28-maintenance Automatically creates a PR against the branch fetching Networking with the outside (non-Nix) world, input locking with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

User registry ignored on nix eval and nix build

3 participants