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

splice.nix: Handle maybe throw sets #360602

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

splice.nix: Handle maybe throw sets #360602

wants to merge 1 commit into from

Conversation

Artturin
Copy link
Member

@Artturin Artturin commented Nov 30, 2024 •
edited
Loading

The mash will trigger the throw in pkgsBuildX in the recursive spliceReal of pkgsi686Linux.

$ nix repl --file . --system aarch64-linux
nix-repl> :p pkgsCross.gnu64.__splicedPackages.pkgsi686Linux.bash.__spliced
{
  hostHost = «derivation /nix/store/0w1mahsn6biqmasm5vg85a9lh5rvyr9j-bash-i686-unknown-linux-gnu-5.2p37.drv»;
  hostTarget = «repeated»;
}

Fixes

$ nix eval --impure --expr '(import ./. { localSystem.system = "aarch64-linux"; crossSystem.system = "x86_64-linux"; config.allowUnfree = true; }).steam'
«error: i686 Linux package set can only be used with the x86 family.»

Alternatively pkgs/build-support/build-fhsenv-bubblewrap/buildFHSEnv.nix could change all pkgsi686Linux to pkgsHostTarget.pkgsi686Linux.

Concerned about performance

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.

@Artturin Artturin force-pushed the splicenixwhatifthrow branch from 0278d4d to e895f90 Compare November 30, 2024 23:41
@nix-owners nix-owners bot requested a review from Ericson2314 November 30, 2024 23:41
@Artturin
splice.nix: Handle maybe throw sets
2c81e53 
The [mash](https://www.github.com/NixOS/nixpkgs/blob/86fcbea3f26cf17e4d6560ec722f3ca893053a89/pkgs/top-level/splice.nix#L32)
will trigger the throw in `pkgsBuildX` in the [recursive `spliceReal`](https://www.github.com/NixOS/nixpkgs/blob/a4b5e0bb8f309fb1623d8e619c1610df75761c12/pkgs/top-level/splice.nix#L82) of `pkgsi686Linux`.

```
$ nix repl --file . --system aarch64-linux
nix-repl> :p pkgsCross.gnu64.__splicedPackages.pkgsi686Linux.bash.__spliced
{
  hostHost = «derivation /nix/store/0w1mahsn6biqmasm5vg85a9lh5rvyr9j-bash-i686-unknown-linux-gnu-5.2p37.drv»;
  hostTarget = «repeated»;
}
```

Fixes

```sh
$ nix eval --impure --expr '(import ./. { localSystem.system = "aarch64-linux"; crossSystem.system = "x86_64-linux"; config.allowUnfree = true; }).steam'
«error: i686 Linux package set can only be used with the x86 family.»
```

Alternatively `pkgs/build-support/build-fhsenv-bubblewrap/buildFHSEnv.nix` could change all `pkgsi686Linux` to `pkgsHostTarget.pkgsi686Linux`.
@Artturin Artturin force-pushed the splicenixwhatifthrow branch from e895f90 to 2c81e53 Compare November 30, 2024 23:42
@Artturin Artturin requested a review from K900 November 30, 2024 23:43
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Nov 30, 2024
@kuruczgy kuruczgy mentioned this pull request Dec 2, 2024
Merged
13 tasks
philiptaron
philiptaron approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@philiptaron philiptaron left a comment

Choose a reason for hiding this comment

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

Maybe extract the use of lib to the topmost place that it could occur?

lib:

let
  inherit (lib) ...;
in

pkgs: actuallySplice:

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Dec 3, 2024
@Artturin
Copy link
Member Author

Artturin commented Dec 4, 2024

Maybe extract the use of lib to the topmost place that it could occur?

lib:

let
  inherit (lib) ...;
in

pkgs: actuallySplice:

That's already in #341067 put it's unknown if it improves performance

@philiptaron
Copy link
Contributor

concerned about performance

LGTM https://github.com/NixOS/nixpkgs/pull/360602/checks?check_run_id=33753296675

@Artturin
Copy link
Member Author

Artturin commented Dec 6, 2024

I want to get #341067 evalling first and then apply #341067 and 6761b8e (#341067) here to see the actual perf diff

@Artturin Artturin marked this pull request as draft December 6, 2024 16:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philiptaron philiptaron philiptaron approved these changes

@Ericson2314 Ericson2314 Awaiting requested review from Ericson2314

@K900 K900 Awaiting requested review from K900

Assignees
No one assigned
Labels
10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Artturin @philiptaron @wegank