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

splice.nix: make pkgs splicedPackages #349316

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

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Oct 17, 2024

This will make pkgs used in callPackage, and pkgsCross.X.pkgs have packages with __spliced.

https://www.github.com/NixOS/nixpkgs/blob/3029741718f4c765fbc5ebf76bea3d6c8ff15fe5/pkgs/development/interpreters/python/passthrufun.nix#L37

https://www.github.com/NixOS/nixpkgs/blob/d2bd9a39dec88eddd5c192abee69939e67f43d12/pkgs/top-level/python-packages.nix#L10720

nix-repl> pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
error:
       … while evaluating the attribute 'aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced'
         at /home/artturin/nixgits/my-nixpkgs/.worktree/1/pkgs/development/python-modules/protobuf/4.nix:119:13:
          118|   passthru = {
          119|     inherit protobuf;
             |             ^
          120|   };

       error: attribute '__spliced' missing
       at «string»:1:1:
            1| pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
             | ^

to

nix-repl> pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
{
  buildBuild = «derivation /nix/store/s7da5mfvx4h1n86j78knaj9cprglxqz6-protobuf-25.4.drv»;
  buildHost = «derivation /nix/store/s7da5mfvx4h1n86j78knaj9cprglxqz6-protobuf-25.4.drv»;
  buildTarget = «repeated»;
  hostHost = «derivation /nix/store/mszvybzs4zxh43awyrjnybsfcb265n9r-protobuf-aarch64-unknown-linux-gnu-25.4.drv»;
  hostTarget = «repeated»;
}

Will allow getting rid of many of these https://github.com/search?q=lang%3Anix+NOT+is%3Afork+%2Fpkgs.%2B__splicedPackages%2F&type=code
#251005 is required to get rid of all of them

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/)
  • 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
    • (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.

@Atemu
Copy link
Member

Atemu commented Oct 21, 2024

This sounds like an obvious win to me, is there anything left to test or verify?

@kjeremy
Copy link
Contributor

kjeremy commented Oct 23, 2024

Would #263082 have caught this? Maybe it would be worth reviving that.

@Aleksanaa
Copy link
Member

I got the feeling of this may be doable a couple of weeks ago. But now there are not many people who really dare to change the cross logic, come on 🫣

@Artturin Artturin marked this pull request as ready for review October 31, 2024 16:46
@nix-owners nix-owners bot requested a review from Ericson2314 October 31, 2024 16:47
@Artturin Artturin force-pushed the pkgsisspliced branch 2 times, most recently from 4bb1955 to 47c47cc Compare October 31, 2024 17:28
@@ -151,6 +151,8 @@ in

newScope = extra: lib.callPackageWith (pkgsForCall // extra);

pkgs = if actuallySplice then splicedPackages // { recurseForDerivations = false; } else pkgs;
Copy link
Member Author

Choose a reason for hiding this comment

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

@Ericson2314
I wonder why you didn't do this instead of adding __splicedPackages? f27f491

This will make `pkgs` used in `callPackage`, and `pkgsCross.X.pkgs` have
packages with `__spliced`.

https://www.github.com/NixOS/nixpkgs/blob/3029741718f4c765fbc5ebf76bea3d6c8ff15fe5/pkgs/development/interpreters/python/passthrufun.nix#L37

https://www.github.com/NixOS/nixpkgs/blob/d2bd9a39dec88eddd5c192abee69939e67f43d12/pkgs/top-level/python-packages.nix#L10720

```
nix-repl> pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
error:
       … while evaluating the attribute 'aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced'
         at /home/artturin/nixgits/my-nixpkgs/.worktree/1/pkgs/development/python-modules/protobuf/4.nix:119:13:
          118|   passthru = {
          119|     inherit protobuf;
             |             ^
          120|   };

       error: attribute '__spliced' missing
       at «string»:1:1:
            1| pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
             | ^
```

to

```
nix-repl> pkgsCross.aarch64-multiplatform.python3Packages.protobuf4.protobuf.__spliced
{
  buildBuild = «derivation /nix/store/s7da5mfvx4h1n86j78knaj9cprglxqz6-protobuf-25.4.drv»;
  buildHost = «derivation /nix/store/s7da5mfvx4h1n86j78knaj9cprglxqz6-protobuf-25.4.drv»;
  buildTarget = «repeated»;
  hostHost = «derivation /nix/store/mszvybzs4zxh43awyrjnybsfcb265n9r-protobuf-aarch64-unknown-linux-gnu-25.4.drv»;
  hostTarget = «repeated»;
}
```
Now that `pkgs` is `__splicedPackages` on cross we can remove these
variables I set.
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