Skip to content

nixos-rebuild-ng: fix repl behaving differently on git flakes than bu…#375493

Merged
thiagokokada merged 1 commit intoNixOS:masterfrom
Mic92:nixos-rebuild-ng
Jan 22, 2025
Merged

nixos-rebuild-ng: fix repl behaving differently on git flakes than bu…#375493
thiagokokada merged 1 commit intoNixOS:masterfrom
Mic92:nixos-rebuild-ng

Conversation

@Mic92
Copy link
Copy Markdown
Member

@Mic92 Mic92 commented Jan 21, 2025

…ild commands

Since we use builtins.getFlake we have behavior differences between normal nix build and the nix repl because builtins.getFlake won't pick up local flakes as git+file but assumes path:// flakes instead. This can have surprising effects such as beeing able to access untracked files that would lead to build failures otherwise or copying large files to the nix store.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@github-actions github-actions bot added the 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS label Jan 21, 2025
@nix-owners nix-owners bot requested a review from thiagokokada January 21, 2025 10:40
@Mic92
Copy link
Copy Markdown
Member Author

Mic92 commented Jan 21, 2025

Similar fix for nixos-option: #371835

@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Jan 21, 2025
Copy link
Copy Markdown
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

Please run nix-build -A nixos-rebuild-ng.tests.linters and fix the issues.

…ild commands

Since we use builtins.getFlake we have behavior differences between
normal nix build and the nix repl because builtins.getFlake won't pick
up local flakes as git+file but assumes path:// flakes instead.
This can have surprising effects such as beeing able to access untracked
files that would lead to build failures otherwise or copying large files
to the nix store.
@Mic92
Copy link
Copy Markdown
Member Author

Mic92 commented Jan 21, 2025

❯ nix-build -A nixos-rebuild-ng.tests.linters
Finished at 22:48:15 after 2s
/nix/store/h94qc3wjzyr3ayhlb19glfqrld22gh5w-nixos-rebuild-ng-linters

Copy link
Copy Markdown
Contributor

@thiagokokada thiagokokada left a comment

Choose a reason for hiding this comment

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

Can't say I completely understand the implications of this change since I don't use things like worktrees, but code-wise it looks good to me and I tested for the basic case and it still works.

@thiagokokada thiagokokada merged commit 524e049 into NixOS:master Jan 22, 2025
2 of 4 checks passed
@Mic92 Mic92 deleted the nixos-rebuild-ng branch January 22, 2025 18:21
thiagokokada added a commit to thiagokokada/nixpkgs that referenced this pull request Jun 16, 2025
PR NixOS#375493 was introduced to fix an issue of different behavior between
`nixos-rebuild repl` and `nixos-rebuild switch` by forcing usage of
`git+file://` protocol when evaluating the Flake. This sadly
reintroduced an older issue from the original `nixos-rebuild` that is
caused by a pretty nasty bug in `nix`:
- NixOS#144811

Let's do the same fix we did for `nixos-rebuild` and just stopping
normalizing the Flake (NixOS#153515).
This will bring back the original issues this code is supposed to fix,
but I argue that a difference between `nixos-rebuild repl` and
`nixos-rebuild switch` is better than having a broken system.
thiagokokada added a commit to thiagokokada/nixpkgs that referenced this pull request Jun 19, 2025
When `path://` or `git+file://` protocol is used in Flake mode (that is
the most common case since we normalize the paths, see PR NixOS#375493) and
the current working directory in a symlink pointing base store path to
the Nix store (e.g., /run/opengl-driver/lib), there is a nasty bug where
Nix resolves the path as the Nix store path of the current derivation
instead of the target derivation.

Since we blindly activate this path, this can corrupt the installation
and break some other activation scripts, like `systemd-boot-builder.py`.
While it is possible to recover this situation using `nix-env -p
/nix/var/nix/profiles/system --delete-generations old`, this is far from
ideal.

This commit solves it by validating that the resolved NixOS
configuration path includes at least `$out/nixos-version`. I am not sure
if this is going to break some cases so there is a escape hatch in the
form of the environment variable
`NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM`,
but in general it looks safe.
nixpkgs-ci bot pushed a commit that referenced this pull request Jun 22, 2025
When `path://` or `git+file://` protocol is used in Flake mode (that is
the most common case since we normalize the paths, see PR #375493) and
the current working directory in a symlink pointing base store path to
the Nix store (e.g., /run/opengl-driver/lib), there is a nasty bug where
Nix resolves the path as the Nix store path of the current derivation
instead of the target derivation.

Since we blindly activate this path, this can corrupt the installation
and break some other activation scripts, like `systemd-boot-builder.py`.
While it is possible to recover this situation using `nix-env -p
/nix/var/nix/profiles/system --delete-generations old`, this is far from
ideal.

This commit solves it by validating that the resolved NixOS
configuration path includes at least `$out/nixos-version`. I am not sure
if this is going to break some cases so there is a escape hatch in the
form of the environment variable
`NIXOS_REBUILD_I_UNDERSTAND_THE_CONSEQUENCES_PLEASE_BREAK_MY_SYSTEM`,
but in general it looks safe.

(cherry picked from commit 0dce56f)
thiagokokada added a commit to thiagokokada/nixpkgs that referenced this pull request Jul 6, 2025
This code seems to be causing yet another issue related to Git
submodules. Considering the amount of issues that this code is causing,
I think it is better to remove it.

Manual revert of NixOS#375493.
nixpkgs-ci bot pushed a commit that referenced this pull request Jul 7, 2025
This code seems to be causing yet another issue related to Git
submodules. Considering the amount of issues that this code is causing,
I think it is better to remove it.

Manual revert of #375493.

(cherry picked from commit b401f9d)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants