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

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

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 changed the title xen: drop 4.17, move to by-name. xen: nuke, move to by-name. Oct 5, 2024
@SigmaSquadron SigmaSquadron force-pushed the xen-by-name branch 2 times, most recently from 709f2d6 to daf2778 Compare October 5, 2024 21:39
@SigmaSquadron SigmaSquadron marked this pull request as draft October 5, 2024 22:20
@SigmaSquadron SigmaSquadron marked this pull request as ready for review October 5, 2024 23:49
@SigmaSquadron SigmaSquadron force-pushed the xen-by-name branch 6 times, most recently from 5c2bbbc to f21e1b1 Compare October 6, 2024 02:14
SigmaSquadron and others added 11 commits October 6, 2024 03:00
- 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 <[email protected]>
It was incredibly slow and will be replaced by something r-ryantm can
run soon.

Signed-off-by: Fernando Rodrigues <[email protected]>
Signed-off-by: Fernando Rodrigues <[email protected]>
Co-authored-by: Yaroslav Bolyukin <[email protected]>
Co-authored-by: Emily <[email protected]>
…nd the xenPackages set

Signed-off-by: Fernando Rodrigues <[email protected]>
Signed-off-by: Fernando Rodrigues <[email protected]>
Signed-off-by: Fernando Rodrigues <[email protected]>
QEMU could already build a pretty okay qemu_xen, but it's
best to use Xen's own sources and explicitly enable Xen support.

Signed-off-by: Fernando Rodrigues <[email protected]>
Signed-off-by: Fernando Rodrigues <[email protected]>
They were removed during the great Xen deletion.

Signed-off-by: Fernando Rodrigues <[email protected]>
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