Skip to content
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

image/images: Adapt remaining images to system.build.image & normalized filenames, #359345

Merged
merged 9 commits into from
Jan 5, 2025
Merged

image/images: Adapt remaining images to system.build.image & normalized filenames, #359345

merged 9 commits into from
Jan 5, 2025

Conversation

phaer
Copy link
Member

@phaer phaer commented Nov 26, 2024

This builds on changes in #359339 and adapts remaining images to declare system.build.image and normalize file names.
It also adds a generic disk-image.nix in order to be able to build plain qcow/raw images, similar to how nixos generators does.

It was split off #347275 in order to upstream that one in smaller chunks, easier to review, test and (hopefully not) revert.

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 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 8.has: module (update) This PR changes an existing module in `nixos/` labels Nov 26, 2024
This was referenced Nov 26, 2024
Merged
Merged
@ofborg ofborg bot added the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 27, 2024
@phaer phaer force-pushed the virtualisation-images-phase-three branch from 4e802fe to 4c1cdf8 Compare November 29, 2024 22:24
@phaer phaer marked this pull request as ready for review November 29, 2024 22:24
@phaer phaer removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Nov 29, 2024
@phaer phaer requested a review from adamcstephens November 29, 2024 22:25
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 labels Nov 29, 2024
@infinisil
Copy link
Member

You're the lucky first customer of #359704 :D

@infinisil infinisil mentioned this pull request Nov 29, 2024
Merged
1 task
@phaer phaer force-pushed the virtualisation-images-phase-three branch from 4c1cdf8 to bc627af Compare November 30, 2024 09:38
Lassulus
Lassulus approved these changes Dec 13, 2024
View reviewed changes
Copy link
Member

@Lassulus Lassulus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, this makes the whole image story much more cleaned up in nixpkgs.

@misuzu misuzu added the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 14, 2024
@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 15, 2024
phaer added 9 commits December 16, 2024 15:34
@phaer
virtualisation/lxc: use system.build.image
891c3be
@phaer
installer/sd-image: use system.build.image...
22255dd 
and rename:
- sdImage.imageBaseName -> image.BaseName
- sdImage.imageName -> image.fileName
@phaer
installer/iso-image: use system.build.image
a582fba 
and rename:
- isoImage.isoBaseName -> image.baseName
- isoImage.isoName -> image.fileName
@phaer
ec2/amazon-image: use system.build.image
fdb144f 
and rename:
- amazonImage.name -> image.baseName
@phaer
openstack-image: use system.build.image
e04d4cc
@phaer
openstack-image-zfs: use system.build.image
53e68e0
@phaer
modules/netboot: add system.build.image for new kexecTarball
63d0a4e
@phaer
virtualisation/disk-image: init
48ec455 
New module to support qemu, qemu-efi, raw, raw-efi outputs as known
from nixos-generators in system.build.images.
@phaer
image/images: Add remaining image modules
e3347c1 
to achieve feature parity with nixos-generators
@phaer phaer force-pushed the virtualisation-images-phase-three branch from bc627af to e3347c1 Compare December 16, 2024 14:35
@phaer
Copy link
Member Author

phaer commented Dec 16, 2024

Needed to rebase the changes to sd-image. Now that #347275 has been merged, it would be nice to get this reviewed/merged soon-ish as it would make nixos-rebuild build-image much more useful.

@misuzu misuzu removed the 2.status: merge conflict This PR has merge conflicts with the target branch label Dec 16, 2024
@wegank wegank removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 16, 2024
@phaer phaer mentioned this pull request Jan 5, 2025
Merged
13 tasks
@thiagokokada thiagokokada merged commit 6e6188a into NixOS:master Jan 5, 2025
38 checks passed
@phaer phaer deleted the virtualisation-images-phase-three branch January 5, 2025 21:26
@vcunat
Copy link
Member

vcunat commented Jan 8, 2025

I believe that the normalization of file names broke the nixos-unstable channel.

At a quick glance we copy the files while keeping their names. So now they clash in the output and the script detects something is wrong and channel update fails. Here's a previous release directory example: https://releases.nixos.org/nixos/unstable/nixos-25.05beta733374.8f3e1f807051

@vcunat
Copy link
Member

vcunat commented Jan 8, 2025 •
edited
Loading

So either we revert this rename or someone needs to rethink the impacts around release workflows and adapt the scripts. (and of course keeping compatibility with 24.11 where this rename doesn't happen)

@phaer
Copy link
Member Author

phaer commented Jan 8, 2025

At a quick glance we copy the files while keeping their names. So now they clash in the output and the script detects something is wrong and channel update fails.

@vcunat Ah sorry, I am still not familiar enough the release process to know what to test here.
What's a good attribute to test on a (somewhat beefy) dev machine in order to reproduce this?

Most filenames should be unique as via the use of tags above. So while there could definitely be collisions they might be relatively easy to fix once found

@vcunat
Copy link
Member

vcunat commented Jan 8, 2025

The file is called nixos-minimal.iso now.

@vcunat
Copy link
Member

vcunat commented Jan 8, 2025 •
edited
Loading

You can build the thing e.g. by

nix-build nixos/release.nix -A iso_minimal.x86_64-linux

@vcunat
Copy link
Member

vcunat commented Jan 8, 2025

And of course you can browse what the build farm produces:
https://hydra.nixos.org/job/nixos/trunk-combined/nixos.iso_minimal.x86_64-linux

@phaer
Copy link
Member Author

phaer commented Jan 8, 2025

Thank you! I meant to ask for an attribute or command to reproduce the breakage, if possible.
I can confirm that some iso image names changed in unintended ways here (looking into that now), that alone doesn't break the iso build?

So now they clash in the output and the script detects something is wrong

Output of which script?

@vcunat
Copy link
Member

vcunat commented Jan 8, 2025 •
edited
Loading

https://github.com/NixOS/nixos-channel-scripts/blob/6192f53a652f9d8711a9ad5da80c07d8650b0b54/mirror-nixos-branch.pl#L189

@phaer phaer mentioned this pull request Jan 8, 2025
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Lassulus Lassulus Lassulus approved these changes

@thiagokokada thiagokokada thiagokokada approved these changes

@adamcstephens adamcstephens Awaiting requested review from adamcstephens

@arianvp arianvp Awaiting requested review from arianvp

@ElvishJerricco ElvishJerricco Awaiting requested review from ElvishJerricco

@jtojnar jtojnar Awaiting requested review from jtojnar

Assignees
No one assigned
Labels
6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 8.has: module (update) This PR changes an existing module in `nixos/` 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@phaer @infinisil @vcunat @Lassulus @thiagokokada @misuzu @wegank