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.

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

nativeCheckInputs #206742

Merged
merged 8 commits into from
Jan 21, 2023
Merged

nativeCheckInputs #206742

merged 8 commits into from
Jan 21, 2023

Conversation

symphorien
Copy link
Member

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.

@Mindavi
Copy link
Contributor

Mindavi commented Dec 21, 2022

I agree with the general idea of this.

@rrbutani rrbutani mentioned this pull request Jan 25, 2023
92 tasks
rrbutani added a commit to rrbutani/nixpkgs that referenced this pull request Jan 27, 2023
github-actions bot pushed a commit that referenced this pull request Jan 28, 2023
@lschuermann
Copy link
Member

@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

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
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 <leon@is.currently.online>
@nixos-discourse
Copy link

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

jtojnar pushed a commit to jtojnar/nixpkgs-hammering that referenced this pull request Feb 11, 2023
xanderio pushed a commit to xanderio/nixpkgs that referenced this pull request Feb 13, 2023
strumtrar pushed a commit to strumtrar/nix-litex that referenced this pull request Mar 13, 2023
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 <twatson52@icloud.com>
Signed-off-by: Leon Schuermann <leon@is.currently.online>
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 20, 2023
imincik added a commit to imincik/geospatial-nix that referenced this pull request Mar 21, 2023
lschuermann added a commit to lschuermann/nix-litex that referenced this pull request Mar 22, 2023
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 <leon@is.currently.online>
thiagokokada added a commit to thiagokokada/nix-alien that referenced this pull request Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

strictDeps breaks libs in checkInputs
9 participants