Skip to content

nativeCheckInputs#206742

Merged
symphorien merged 8 commits intoNixOS:masterfrom
symphorien:nativeCheckInputs
Jan 21, 2023
Merged

nativeCheckInputs#206742
symphorien merged 8 commits intoNixOS:masterfrom
symphorien:nativeCheckInputs

Conversation

@symphorien
Copy link
Member

@symphorien symphorien commented Dec 18, 2022

Description of changes

Fixes #161570

tl;dr: checkInputs used to be added to nativeBuildInputs, which means they are added to PATH but one cannot link to them when strictDeps is set. As linking to check-only libs is possible (quite common for ocaml for example) we need to distinguish

  • checkInputs, added to buildInputs when doCheck is set. One can link against them.
  • nativeCheckInputs, added to nativeBuildInputs when doCheck is set. They are added to PATH.
    (same for installCheck)

The PR contains manual commits to adapt the builders (mkDerivation, buildPythonPackage notably), a commit to use the new feature in ocamlPackages.batteries, and a treewide search-and-replace from checkInputs to nativeCheckInputs.

as a result hashes are unchanged except for the reverse-dependencies of ocamlPackages.batteries (8).

Supersedes https://github.com/NixOS/nixpkgs/pull/185406/files

This PR will inevitably accumulate merge conflicts. When it obtains enough positive reviews, I will regenerate the treewide commit and merge immediately. Maybe this can be done just after staging-next is merged?

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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: vim Advanced text editor labels Dec 18, 2022
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Dec 18, 2022
@symphorien symphorien mentioned this pull request Dec 19, 2022
Closed
@Mindavi
Copy link
Contributor

Mindavi commented Dec 21, 2022

I agree with the general idea of this.

ncfavier
ncfavier reviewed Dec 21, 2022
View reviewed changes
@rrbutani rrbutani mentioned this pull request Jan 25, 2023
Merged
92 tasks
@ncfavier ncfavier mentioned this pull request Jan 25, 2023
Merged
13 tasks
This was referenced Jan 25, 2023
Closed
Closed
Closed
Closed
rrbutani added a commit to rrbutani/nixpkgs that referenced this pull request Jan 27, 2023
@rrbutani
llvmPackages_15: apply some patches from llvmPackages_14, part 2
9bd9267 
See NixOS#194634 (comment) for
details.

PRs:
  - NixOS#211401
  - NixOS#211161
  - NixOS#206742
  - NixOS#211687
github-actions bot pushed a commit that referenced this pull request Jan 28, 2023
@rrbutani @github-actions
llvmPackages_15: apply some patches from llvmPackages_14, part 2
2bc885c 
See #194634 (comment) for
details.

PRs:
  - #211401
  - #211161
  - #206742
  - #211687
(cherry picked from commit 9bd9267)
K900 pushed a commit that referenced this pull request Jan 28, 2023
@rrbutani @K900
llvmPackages_15: apply some patches from llvmPackages_14, part 2
735a99c 
See #194634 (comment) for
details.

PRs:
  - #211401
  - #211161
  - #206742
  - #211687
@lschuermann
Copy link
Member

lschuermann commented Feb 5, 2023

@symphorien For downstream package sets which aim to target both the nixos-unstable and latest NixOS release channels (nixos-22.11), this means that for the time being, neither checkInputs or nativeCheckInputs works reliably on both (either resulting in an infinite recursion error, or no longer propagating check dependencies as it used to). Is there any reliable way to detect which one to use?

@symphorien
Copy link
Member Author

symphorien commented Feb 5, 2023

A possible solution is:

${if pkgs.lib.versionAtLeast pkgs.lib.version "23.05" then "nativeCheckInputs" else "checkInputs"} = [ thedep ];

(this will fail on 23.05 older than this PR, and because the version string is different on channels and on git repositories I don't think you can make it work reliably).
Another possiblity is to manually add them to nativeBuildInputs if doCheck is true.

lschuermann added a commit to lschuermann/nix-litex that referenced this pull request Feb 6, 2023
@lschuermann
Switch from checkInputs to nativeCheckInputs on >= NixOS 23.05
66cbacd 
This unbreaks package tests on the current nixpkgs-unstable branch, as
`buildPythonPackage` has been adjusted to rename `checkInputs` to
`nativeCheckInputs`, breaking existing checkInputs specifications
because of `strictDeps = 1;`. For more information, check out the
corresponding PR at [1]. This implements the proposed workaround[2]
for supporting both the current nixos-unstable and the last release
branch (NixOS 22.11).

[1]: NixOS/nixpkgs#206742
[2]: NixOS/nixpkgs#206742 (comment)

Signed-off-by: Leon Schuermann <[email protected]>
@nixos-discourse
Copy link

nixos-discourse commented Feb 8, 2023

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

https://discourse.nixos.org/t/what-is-nativecheckinputs-when-should-it-be-used-instead-of-checkinputs/25329/3

@figsoda figsoda mentioned this pull request Feb 11, 2023
Merged
jtojnar pushed a commit to jtojnar/nixpkgs-hammering that referenced this pull request Feb 11, 2023
@figsoda
attribute-ordering: add nativeCheckInputs (#135)
3fd4dda 
it was added in NixOS/nixpkgs#206742
xanderio pushed a commit to xanderio/nixpkgs that referenced this pull request Feb 13, 2023
@rrbutani @xanderio
llvmPackages_15: apply some patches from llvmPackages_14, part 2
335515c 
See NixOS#194634 (comment) for
details.

PRs:
  - NixOS#211401
  - NixOS#211161
  - NixOS#206742
  - NixOS#211687
strumtrar pushed a commit to strumtrar/nix-litex that referenced this pull request Mar 13, 2023
@lschuermann @tpwrules @strumtrar
Switch from checkInputs to nativeCheckInputs on >= NixOS 23.05
fc485c3 
This unbreaks package tests on the current nixpkgs-unstable branch, as
`buildPythonPackage` has been adjusted to rename `checkInputs` to
`nativeCheckInputs`, breaking existing checkInputs specifications
because of `strictDeps = 1;`. For more information, check out the
corresponding PR at [1]. This implements the proposed workaround[2]
for supporting both the current nixos-unstable and the last release
branch (NixOS 22.11).

[1]: NixOS/nixpkgs#206742
[2]: NixOS/nixpkgs#206742 (comment)

Co-authored-by: Thomas Watson <[email protected]>
Signed-off-by: Leon Schuermann <[email protected]>
@imincik imincik mentioned this pull request Mar 20, 2023
Merged
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
@imincik
pkgs(gdal): switch to nativeCheckInputs
d007a35 
Nixpkgs PR: NixOS/nixpkgs#206742
@imincik imincik mentioned this pull request Mar 20, 2023
Merged
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
@imincik
pkgs(rasterio): switch to nativeCheckInputs
af10afb 
Nixpkgs PR: NixOS/nixpkgs#206742
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
@imincik
pkgs(rasterio): switch to nativeCheckInputs
9cc74f6 
Nixpkgs PR: NixOS/nixpkgs#206742
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
@imincik
pkgs(gdal): switch to nativeCheckInputs
faa456f 
Nixpkgs PR: NixOS/nixpkgs#206742
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 21, 2023
@imincik
pkgs(gdal): switch to nativeCheckInputs
3b82b75 
Nixpkgs PR: NixOS/nixpkgs#206742
lschuermann added a commit to lschuermann/nix-litex that referenced this pull request Mar 22, 2023
@lschuermann
Switch from checkInputs to nativeCheckInputs on >= NixOS 23.05
b8eec7f 
This unbreaks package tests on the current nixpkgs-unstable branch, as
`buildPythonPackage` has been adjusted to rename `checkInputs` to
`nativeCheckInputs`, breaking existing checkInputs specifications
because of `strictDeps = 1;`. For more information, check out the
corresponding PR at [1]. This implements the proposed workaround[2]
for supporting both the current nixos-unstable and the last release
branch (NixOS 22.11).

[1]: NixOS/nixpkgs#206742
[2]: NixOS/nixpkgs#206742 (comment)

Signed-off-by: Leon Schuermann <[email protected]>
@figsoda figsoda mentioned this pull request Mar 26, 2023
Merged
thiagokokada added a commit to thiagokokada/nix-alien that referenced this pull request Apr 7, 2023
@thiagokokada
Use native{Check,Build}Inputs
58597a3 
See NixOS/nixpkgs#206742 for details.
@natsukium natsukium mentioned this pull request Jun 28, 2023
Merged
12 tasks
@Artturin Artturin mentioned this pull request Jun 29, 2023
Closed
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SuperSandro2000 SuperSandro2000 SuperSandro2000 left review comments

@ncfavier ncfavier ncfavier approved these changes

@wegank wegank wegank approved these changes

@Mindavi Mindavi Mindavi approved these changes

@jtojnar jtojnar Awaiting requested review from jtojnar

@mmahut mmahut Awaiting requested review from mmahut

@RaghavSood RaghavSood Awaiting requested review from RaghavSood

@aanderse aanderse Awaiting requested review from aanderse

@etu etu Awaiting requested review from etu

@globin globin Awaiting requested review from globin

@Ma27 Ma27 Awaiting requested review from Ma27

@talyz talyz Awaiting requested review from talyz

@figsoda figsoda Awaiting requested review from figsoda

@jonringer jonringer Awaiting requested review from jonringer

@teto teto Awaiting requested review from teto

@ajs124 ajs124 Awaiting requested review from ajs124

@lukegb lukegb Awaiting requested review from lukegb

@mweinelt mweinelt Awaiting requested review from mweinelt

@matthewbauer matthewbauer Awaiting requested review from matthewbauer

+1 more reviewer

@panicgh panicgh panicgh approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: lua Lua is a powerful, efficient, lightweight, embeddable scripting language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: vim Advanced text editor 8.has: changelog This PR adds or changes release notes 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

strictDeps breaks libs in checkInputs

9 participants

@symphorien @Mindavi @nixos-discourse @fricklerhandwerk @panicgh @lschuermann @ncfavier @SuperSandro2000 @wegank