Skip to content

check-meta: add allowBrokenPredicate #340081

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

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

Conversation

amarshall
Copy link
Member

Similar to allowUnfreePredicate, sometimes users may want to only allow specific broken packages to avoid unexpectedly building others.

Some packages may be marked broken for policy reasons (lack of upstream support) or due to broken or unsupported functionality that the user may not care about. An example might be forcing ZFS to build on a newer, unsupported Kernel where compilation succeeds and the user is willing to take the risk of being unsupported.

I think the larger topic for discussion would be: should broken be used in this way? I.e., should broken only be used for packages that actually fail to build or fundamentally work at all, or is it okay to use it for things that might work but are explicitly unsupported? The manual for broken doesn’t say much about when to use it.

If the answer is that broken should only be used for things that fail to build or fundamentally work, then perhaps this PR doesn’t make sense as there’s little use-case for bypassing broken on a per-package basis in one’s config. It does bring up the question, though: how should we then mark packages whose config is unsupported? Back to the ZFS example, running a filesystem on Kernels not explicitly tested and supported by upstream seems like a bad idea by default, and users should have to opt-in to that risk in some way.

Description of changes

Things done

Tested with

$ nix-build -A linuxKernel.packages.linux_testing.zfs_unstable
…error: Package ‘zfs-kernel-2.2.5-6.11-rc6’ … is marked as broken, refusing to evaluate.…

$ < config.nix echo '{ pkgs }: { allowBrokenPredicate = pkg: pkgs.lib.hasInfix "zfs" pkg.pname; }'
$ NIXPKGS_CONFIG=$(pwd)/config.nix nix-build -A linuxKernel.packages.linux_testing.zfs_unstable
…building…
  • 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.

@github-actions github-actions bot added 8.has: documentation This PR adds or changes documentation 6.topic: stdenv Standard environment labels Sep 6, 2024
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Sep 6, 2024
Copy link
Contributor

@adamcstephens adamcstephens left a comment

Choose a reason for hiding this comment

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

Sounds good to me, but this should get much wider comment before merging.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 6, 2024
@FliegendeWurst FliegendeWurst added the 9.needs: community feedback This needs feedback from more community members. label Dec 4, 2024
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 2, 2025
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

LGTM, but has a merge conflict

@wegank wegank added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels May 15, 2025
@amarshall amarshall force-pushed the allowbrokenpredicate branch from 61a110e to 3432881 Compare May 16, 2025 12:42
@ofborg ofborg bot removed the 2.status: merge conflict This PR has merge conflicts with the target branch label May 16, 2025
@github-actions github-actions bot added 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. labels May 16, 2025
Copy link
Member

@Pandapip1 Pandapip1 left a comment

Choose a reason for hiding this comment

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

nixfmt wants the following changes

@github-project-automation github-project-automation bot moved this to In Progress in Stdenv May 16, 2025
Similar to allowUnfreePredicate, sometimes users may want to only allow
specific broken packages to avoid unexpectedly building others.

Some packages may be marked broken for policy reasons (lack of upstream
support) or due to broken or unsupported functionality that the user may
not care about. An example might be forcing ZFS to build on a newer,
unsupported Kernel where compilation succeeds and the user is willing to
take the risk of being unsupported.
@amarshall amarshall force-pushed the allowbrokenpredicate branch from 3432881 to 89e5b47 Compare May 18, 2025 12:11
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/2447

@Pandapip1
Copy link
Member

Bump!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
6.topic: stdenv Standard environment 8.has: documentation This PR adds or changes documentation 9.needs: community feedback This needs feedback from more community members. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

6 participants