Skip to content

make-{iso9660-image,squashfs,system-tarball}: drop references to build closure #425700

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

Jump to bottom
Merged
merged 4 commits into from
Aug 21, 2025
Merged

make-{iso9660-image,squashfs,system-tarball}: drop references to build closure #425700

merged 4 commits into from
Aug 21, 2025

Conversation

Enzime
Copy link
Member

@Enzime Enzime commented Jul 16, 2025

This PR sets unsafeDiscardReferences.out = true; on make-iso9660-image, make-squashfs and make-system-tarball which means that the outputs don't depend on the build closure as they are all self-contained.

I've tested this PR works for make-iso9660-image by running:

nix flake archive --to ssh-ng://example.com && ssh example.com nix build --print-build-logs --print-out-paths $(nix flake metadata --json | jq -r '.path')#nixosConfigurations.default.config.system.build.images.iso && nix copy --from ssh-ng://example.com $(nix flake metadata --json | jq -r '.path')#nixosConfigurations.default.config.system.build.images.iso

Prior to this PR, nix copy would copy the entire build closure, however after this PR, nix copy only copies the final store path.

To test make-system-tarball, I used the attribute path system.build.images.kexec instead.

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/)
  • Nixpkgs 25.11 Release Notes (or backporting 25.05 Nixpkgs Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • NixOS 25.11 Release Notes (or backporting 25.05 NixOS Release notes)
    • (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, pkgs/README.md, maintainers/README.md and other contributing documentation in corresponding paths.

Add a 👍 reaction to pull requests you find important.

@Enzime Enzime requested review from ncfavier and phaer July 16, 2025 06:41
@nixpkgs-ci nixpkgs-ci bot added 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS labels Jul 16, 2025
@ncfavier
Copy link
Member

ncfavier commented Jul 16, 2025 •
edited
Loading

I believe this would require bumping the minimum supported Nix version to 2.18, when unsafeDiscardReferences was stabilised.

@Enzime
Copy link
Member Author

Enzime commented Jul 16, 2025

unsafeDiscardReferences.out = true; has been used in repart-image.nix since Sep 2024 (#343252) so I'm not sure if the minimum supported version needs to be bumped to be able to use it

@ncfavier
Copy link
Member

That may be fine because it's a new feature. make-iso9660-image and friends are more critical pieces of infrastructure, so I think they should support the minimum required Nix version. See also #398444.

Pinging @Mic92 @infinisil

@Mic92
Copy link
Member

Mic92 commented Jul 16, 2025

Afaik the presents of this attribute doesn't prevent nix 2.3 from building these images - it might not able to substitute from nixos.org but I think that's an acceptable breakage because the substitution of still maintained nix versions will be vastly improved. Regarding the version bump, I don't know if requiring a certain nix version is meaningful at all anymore with the presents of forks. We might rather need feature checks... but I think this discussion shouldn't be done here and is out of scope for this pull request.

@ncfavier
Copy link
Member

ncfavier commented Jul 16, 2025 •
edited
Loading

Afaik the presents of this attribute doesn't prevent nix 2.3 from building these images

We should make sure that's true before merging this then: can Nix 2.3 (without the discard-references experimental feature) build the NixOS ISO image from this PR? If so, the version bump discussion is indeed out of scope.

@Mic92 Mic92 mentioned this pull request Aug 4, 2025
Merged
2 tasks
@wolfgangwalther
Copy link
Contributor

I believe this would require bumping the minimum supported Nix version to 2.18, when unsafeDiscardReferences was stabilised.

We have bumped minver to 2.18, so this should not be blocking anymore.

@Enzime
Copy link
Member Author

Enzime commented Aug 21, 2025

Could you take a look at this again now that minver has been bumped to 2.18? @ncfavier @Mic92

@nixpkgs-ci nixpkgs-ci bot added the 12.approvals: 1 This PR was reviewed and approved by one person. label Aug 21, 2025
@ncfavier ncfavier merged commit 494a23f into NixOS:master Aug 21, 2025
31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ncfavier ncfavier ncfavier approved these changes

@phaer phaer Awaiting requested review from phaer

Assignees
No one assigned
Labels
6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 12.approvals: 1 This PR was reviewed and approved by one person.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Enzime @ncfavier @Mic92 @wolfgangwalther