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.

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

xen: nuke, move to by-name. #345192

Merged
merged 13 commits into from
Oct 7, 2024
Merged

xen: nuke, move to by-name. #345192

merged 13 commits into from
Oct 7, 2024

Conversation

SigmaSquadron
Copy link
Contributor

@SigmaSquadron SigmaSquadron commented Sep 28, 2024

Description of changes

  • Migrate Xen to by-name, since it can't be migrated automatically.
    • This has the side effect of simplifying some Xen stuff.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review pr 345192". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.11 Release Notes (or backporting 23.11 and 24.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
  • Fits CONTRIBUTING.md.
    • Intermediate commits will be squashed.

Add a 👍 reaction to pull requests you find important.

@SigmaSquadron SigmaSquadron changed the title Xen by name (WIP): xen: move to by-name Sep 28, 2024
@CertainLach
Copy link
Member

Do package sets work with by-name?

@SigmaSquadron
Copy link
Contributor Author

Do package sets work with by-name?

I don't think so. This also sends all xenPackages to the top-level attribute. (xenPackages.xen_4_19 -> xen_4_19).

@SigmaSquadron
Copy link
Contributor Author

I have reached the conclusion I have no idea what I'm doing.

@emilazy
Copy link
Member

emilazy commented Sep 29, 2024

Hate to say it, but this is a violation of the by-name contract. cc @infinisil for advice on the correct way to handle it, but the one being used for the Darwin SDK rework that seemed okay was to have the package take e.g. a xenVersion argument that the all-packages.nix calls override appropriately.

It’s also fine to just not be in by-name. It’s not expected that every package can currently fit into the schema.

@CertainLach
Copy link
Member

CertainLach commented Sep 29, 2024

Ah, yes:

For each package directory, pkgs.lib.isDerivation pkgs.${name} must be true.

@emilazy
Copy link
Member

emilazy commented Sep 29, 2024

The problem is that by-name packages have to look like callPackage ../by-name/ab/abcd args and have to be derivations. With the current pkgs/by-name/xe/xen structure, it’s impossible to satisfy both.

With the xenVersion thing, you can even get fancy and make e.g. pkgs/by-name/xe/xen_4_18-slim/package.nix be { xen }: xen.override { xenVersion = "4.18"; xenSlim = true; } if you really don’t want any all-packages.nix entries.

@SigmaSquadron
Copy link
Contributor Author

SigmaSquadron commented Sep 29, 2024

Ah, yes:

For each package directory, pkgs.lib.isDerivation pkgs.${name} must be true.

I still don't see what exactly is wrong here?

xen

@SigmaSquadron SigmaSquadron changed the title (WIP): xen: move to by-name (WIP): xen: drop 4.17, move to by-name. Sep 29, 2024
@emilazy
Copy link
Member

emilazy commented Sep 29, 2024

But you only satisfy that condition because you violate this one:

  • For each package directory, the pkgs.${name} attribute must be defined as callPackage pkgs/by-name/${shard}/${name}/package.nix args for some args.

@SigmaSquadron SigmaSquadron marked this pull request as ready for review October 5, 2024 23:49
@SigmaSquadron SigmaSquadron force-pushed the xen-by-name branch 11 times, most recently from 469dc95 to 9938013 Compare October 6, 2024 17:16
SigmaSquadron and others added 3 commits October 6, 2024 15:55
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Co-authored-by: Yaroslav Bolyukin <iam@lach.pw>
As the builder is generic, more people may be using it, so we should try
to keep this value as close to the upstream source of truth as possible.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
SigmaSquadron and others added 10 commits October 6, 2024 20:24
- Removes the non-slim build instructions, massively simplifying
everything in the package.

- Removes unecessary patches.

- Inherits functions from lib instead of repeating lib.* everywhere.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
It was incredibly slow and will be replaced by something r-ryantm can
run soon.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Co-authored-by: Yaroslav Bolyukin <iam@lach.pw>
Co-authored-by: Emily <vcs@emily.moe>
…ths.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
…nd the xenPackages set

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
They were removed during the great Xen deletion.

Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
Signed-off-by: Fernando Rodrigues <alpha@sigmasquadron.net>
@emilazy emilazy merged commit 85c36fe into NixOS:master Oct 7, 2024
26 of 27 checks passed
@SigmaSquadron SigmaSquadron deleted the xen-by-name branch October 7, 2024 02:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants