Skip to content

musl: tighten platforms#228712

Merged
alyssais merged 2 commits intoNixOS:masterfrom
alyssais:musl-platforms
May 1, 2023
Merged

musl: tighten platforms#228712
alyssais merged 2 commits intoNixOS:masterfrom
alyssais:musl-platforms

Conversation

@alyssais
Copy link
Member

Description of changes

This will make it possible to check whether we can use pkgsStatic opportunistically, in places like busybox-sandbox-shell, which currently decides not to use pkgsStatic based on a hard-coded set of platforms.

I haven't actually changed busybox-sandbox-shell to use it yet, because that would just cause conflicts with #227560, so we can do it later.

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.11 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.

This will make it possible to check whether we can use pkgsStatic
opportunistically, in places like busybox-sandbox-shell, which
currently decides not to use pkgsStatic based on a hard-coded set of
platforms.
@alyssais alyssais requested a review from wegank April 28, 2023 10:14
musl now supports RISC-V.  Let's centralise musl availability checks
in musl.meta.platforms, so we don't have to keep cleaning up ad-hoc
checks like this all over the tree.
@github-actions github-actions bot added the 6.topic: stdenv Standard environment label Apr 28, 2023
@wegank
Copy link
Member

wegank commented Apr 28, 2023

And I think busybox-sandbox-shell can be changed now?

@alyssais
Copy link
Member Author

And I think busybox-sandbox-shell can be changed now?

In theory, but the check we'd want to use (lib.meta.availableOn stdenv.hostPlatform pkgsStatic.cc.libc) currently produces an error because it fails the meta check before getting to our availableOn call. #213096 would fix it, so I think it's worth waiting for that.

(We don't want to just check for musl there, because pkgsStatic only means musl on Linux.)

@alyssais alyssais added 6.topic: musl Running or building packages with musl libc 6.topic: portability General portability concerns, not specific to cross-compilation or a specific platform labels Apr 28, 2023
@ofborg ofborg 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 Apr 28, 2023
@wegank wegank requested a review from NickCao April 28, 2023 14:15
@alyssais
Copy link
Member Author

freshBootstrapTools successfully built on riscv64-linux by 0x4a6f (#exotic:nixos.org)

@NickCao
Copy link
Member

NickCao commented Apr 29, 2023

The change looks reasonable to me, but can we somehow subtract the bad platforms instead of listing the good ones?

@alyssais
Copy link
Member Author

The change looks reasonable to me, but can we somehow subtract the bad platforms instead of listing the good ones?

We can, but I don't think we should. badPlatforms is good when you have a package that is generally portable, but for one reason or another is incompatible with a certain platform. But for stuff like this, where the package needs architecture-specific code for every architecture it supports, it's better to list them explicitly, or we'll end up with false positives any time a new architecture is added to Nixpkgs, and false negatives (when a package is incorrectly marked as being unsupported on a platform) are a lot easier to spot than false positives.

@alyssais alyssais merged commit 5130c4f into NixOS:master May 1, 2023
@alyssais alyssais deleted the musl-platforms branch May 1, 2023 11:25
@alyssais alyssais mentioned this pull request May 2, 2023
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: musl Running or building packages with musl libc 6.topic: portability General portability concerns, not specific to cross-compilation or a specific platform 6.topic: stdenv Standard environment 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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants