Skip to content

mk-python-derivation: fix config.doCheckByDefault being ignored#300338

Closed
yshui wants to merge 3 commits intoNixOS:masterfrom
yshui:python-do-check
Closed

mk-python-derivation: fix config.doCheckByDefault being ignored#300338
yshui wants to merge 3 commits intoNixOS:masterfrom
yshui:python-do-check

Conversation

@yshui
Copy link
Contributor

@yshui yshui commented Mar 30, 2024

Description of changes

make buildPythonPackage respect config.doCheckByDefault.

Don't know if you know this: { ... } @ attrs syntax won't include default arguments in attrs (!!). So despite the , doCheck ? config.doCheckByDefault or false in the parameter list, attrs.doCheck will not exist if doCheck is not explicitly passed. And because doCheck is only accessed through attrs, the default argument is meaningless there.

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.05 Release Notes (or backporting 23.05 and 23.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.

Add a 👍 reaction to pull requests you find important.

@yshui yshui requested a review from FRidh as a code owner March 30, 2024 22:50
@github-actions github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Mar 30, 2024
@yshui
Copy link
Contributor Author

yshui commented Mar 30, 2024

Perhaps the intention is indeed to enable check by default unless disabled, in that case the default argument config.doCheckByDefault in the parameter list is confusing and should be removed.

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Mar 30, 2024
yshui added 2 commits April 2, 2024 19:25
runtime deps check fails otherwise.
runtime deps check fails otherwise.
@ofborg ofborg bot requested review from kira-bruneau April 2, 2024 19:31
@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label May 22, 2024
@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jan 2, 2025
@kira-bruneau
Copy link
Contributor

Perhaps the intention is indeed to enable check by default unless disabled, in that case the default argument config.doCheckByDefault in the parameter list is confusing and should be removed.

Oh yep, it looks like that was addressed in #308377.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md 6.topic: python Python is a high-level, general-purpose programming language. 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants