strictDeps breaks libs in checkInputs #161570

symphorien · 2022-02-23T20:06:08Z

Describe the bug

checkInputs are added to nativeBuildInputs but not buildInputs when doCheck is set, so libs added to checkInputs cannot be found by compilers when strictDeps is true.

https://github.com/NixOS/nixpkgs/blob/master/pkgs/stdenv/generic/make-derivation.nix#L150-L151

Steps To Reproduce

with import <nixpkgs> {};
stdenv.mkDerivation {
  name = "whatever";
  checkInputs = [ zlib ];
  nativeBuildInputs = [ pkg-config ];
  doCheck = true;
  strictDeps = true;
  buildCommand = ''
    pkg-config --cflags zlib
    touch $out
  '';
}

When strictDeps is set, pkg-config fails to find zlib, whereas it finds it without strictDeps.

Note: needing libs in checkInputs is very common in ocamlPackages.

today we only add them to nativeBuildInputs, but this is insufficient when strictDeps is true. also affects installCheckInputs. Fixes NixOS#161570

FRidh · 2022-02-25T09:19:21Z

Related #53440 which added checkInputs to nativeBuildInputs.

Mindavi · 2022-02-26T15:39:42Z

Reading this, I'm confused about checkInputs. I thought they were for testing..? In the example given here I'd definitely expect zlib to be in buildInputs, rather than checkInputs

symphorien · 2022-02-26T15:48:45Z

Yes the example is not credible but minimal. Actual use cases are for ocaml derivations: it is common to require additional deps to like oUnit2 build the executables that test the lib. These are needed to build executables which will not be installed, and thus are not runtime dependencies.

Actual affected package: ocamlPackages.batteries https://github.com/NixOS/nixpkgs/pull/161344/files#diff-9b5b534f3d86a94e6b3b2a85dc60f09f76cf592b24850a7f291cc1f700cedba2

Artturin · 2022-02-27T01:18:31Z

this could be also done in https://github.com/NixOS/nixpkgs/blob/master/pkgs/build-support/ocaml/dune.nix

symphorien · 2022-02-27T10:47:47Z

needing extra libs to run tests is not isolated to ocaml. For example presage fails to find cppunit with strictDeps = true (it does not fail the build, but does not run unit tests). If I add cppunit to buildInputs, ./configure finds cppunit. Also not all ocaml packages are using dune, many of them are still using plain mkDerivation.

Ericson2314 · 2022-03-07T21:06:14Z

Per 87651b3 this was originally buildInputs and a1a5ea5 made it for nativeBuildInputs as @FRidh mentions.

TBH this is why I just don't really like checkInput as a concept. It is just too unclear what it should do. In a way, the best thing is to make it only buildInputs, and then extend PATH and other stuff to include those too as if they were nativeBuidInputs.

symphorien · 2022-03-07T21:48:28Z

Are you afraid that checkInputs might run setupHooks ?
Honestly I think that's a feature: you need them for a lot of things, for example to be able to compile ocaml code...

Plus the mental model "checkInputs are inputs added when doCheck is true" should be kept simple. The fact that it is added to both native and non native buildinputs comes from the fact that we never cross compile tests, so nativeCheckInputs makes no sense.

Ericson2314 · 2022-03-07T22:05:29Z

Well consider the "env hooks" setup hooks do. What should those be run against? Also consider splicing, which does still happen in the build == host != target native case (buildPackages what bootstrapping). There is also sorts of subtle stuff.

I think I would either prefer

Full explicit. we had a checkBuildInputs and checkNativeBuildInputs, or maybe deps{Build,Host,Target}{Build,Host,Target}Check.
Go back to checkInputs is just buildInputs, and extend the PATH and whatnot before the testing phase. (Maybe also run all the setup hooks again?!)

symphorien · 2022-03-12T21:41:10Z

Also consider splicing, which does still happen in the build == host != target native case

I had not thought of that. However adding checkInputs to both native and non native still looks correct to me in this case, unless you need target libs for checks, which would be weird as you can't run target executables during checks. And even if it really happens this is probably very rare, whereas we are talking about fixing every ocamlPackage and everything using cppunit and similar things.

Go back to checkInputs is just buildInputs, and extend the PATH and whatnot before the testing phase. (Maybe also run all the setup hooks again?!)

That would be very impractical. Notably, foo.overrideAttrs (_: { doCheck = false; }) would not remove checkInputs from the build closure anymore.

Full explicit. we had a checkBuildInputs and checkNativeBuildInputs, or maybe deps{Build,Host,Target}{Build,Host,Target}Check.

Why not. That would require a treewide change from checkInputs to checkNativeBuildInputs to keep current packages working.

FRidh · 2022-03-29T08:25:27Z

At the time I did not want to separate into a checkNativeBuildInputs (just enabling strictDeps was already quite the effort). By now, I think we should go that way. It's quite some work, updating all Python expressions, but it is fairly simple. Who knows, maybe someone knows how to automate it. It's primarily moving pytestCheckHook from checkInputs to checkNativeInputs, though there are some other packages as well.

symphorien · 2022-03-29T20:29:38Z

It's quite some work, updating all Python expressions, but it is fairly simple

Since all current checkInputs are actually checkNativeBuildInputs, a "simple" sed on all files in nixpkgs probably works.

Ericson2314 · 2022-03-31T22:50:19Z

Sorry for being slow in responding.

That would be very impractical. Notably, foo.overrideAttrs (_: { doCheck = false; }) would not remove checkInputs from the build closure anymore.

Err I mean check* would still be conditional, but there would no longer be a temptation to put the things that are morally build inputs in native build inputs.

Ericson2314 · 2022-03-31T22:58:42Z

At the time I did not want to separate into a checkNativeBuildInputs (just enabling strictDeps was already quite the effort). By now, I think we should go that way. It's quite some work, updating all Python expressions, but it is fairly simple.

Yay! Very glad to hear this.

@symphorien so in conjunction with my previous comment, @FRidh is right. Many things which are checkInputs should indeed remain checkInputs and not become nativeCheckInputs.

#53440 has some more info. The emulators stuff I think at least is a good thought experiment.

Artturin · 2022-04-01T02:36:48Z

At the time I did not want to separate into a checkNativeBuildInputs (just enabling strictDeps was already quite the effort). By now, I think we should go that way. It's quite some work, updating all Python expressions, but it is fairly simple. Who knows, maybe someone knows how to automate it. It's primarily moving pytestCheckHook from checkInputs to checkNativeInputs, though there are some other packages as well.

@siraben has recently made a program that uses treesitter to find these
https://github.com/siraben/nix-lint

Helps with NixOS/nixpkgs#161570

siraben · 2022-04-01T03:04:38Z

@Artturin I added that as one of the options in the tool. Here's some of the cases that it detects, correctly accounting for the list being split across multiple lines.

/Users/siraben/Git/forks/nixpkgs/pkgs/development/python-modules/paramiko/default.nix:32
[
  {
    text: 'checkInputs = [\n' +
      '    invoke\n' +
      '    mock\n' +
      '    pytest-relaxed\n' +
      '    pytestCheckHook\n' +
      '  ];',
    row: 31,
    column: 2
  }
]
/Users/siraben/Git/forks/nixpkgs/pkgs/development/python-modules/parfive/default.nix:38
[
  {
    text: 'checkInputs = [\n' +
      '    aiofiles\n' +
      '    pytest-asyncio\n' +
      '    pytest-localserver\n' +
      '    pytest-socket\n' +
      '    pytestCheckHook\n' +
      '  ];',
    row: 37,
    column: 2
  }
]

Ericson2314 · 2022-04-02T15:12:01Z

So, do we have a plan hear?

Make checkInputs and nativeCheckInputs
Sort python check input's, with the help of nix-lint
Finally do the OCaml cross stuff?

FRidh · 2022-04-02T15:47:15Z

Actually I am confused again whether the Python libs should be native or not. Thinking about the emulator, the emulator would be native but everything it runs would not be native, right?

Ericson2314 · 2022-04-02T17:37:18Z

Yeah, so most/all of the python stuff that is not the setup hook should stay checkInputs as that goes back to being buildInputs.

ulrikstrid · 2022-04-03T07:07:15Z

So, do we have a plan hear?

Make checkInputs and nativeCheckInputs

Sort python check input's, with the help of nix-lint

Finally do the OCaml cross stuff?

@Ericson2314 I have a PR for strictDeps in OCaml, what can I change in that to prepare for these changes?

ulrikstrid · 2022-04-03T07:10:20Z

#162385

It also applies the dune patch that @Artturin suggested, but it's easy to revert

FRidh · 2022-08-06T10:52:05Z

mkDerivation change in #185406. Now what I need is a script for fixing all Python expressions. Should have fixed this right away since there are over 2000 Python expressions using checkInputs now.

ulrikstrid · 2022-08-06T16:47:46Z

Should I rebase my OCaml PR on yours @FRidh ?

symphorien · 2022-12-19T07:22:03Z

#206742 proposes a fix.

When strictDeps is set, only nativeCheckInputs are added to PATH and only checkInputs can be linked against. See NixOS#161570

symphorien added the 0.kind: bug Something is broken label Feb 23, 2022

symphorien mentioned this issue Feb 23, 2022

stdenv.mkDerivation: add checkInputs to buildInputs #161575

Closed

13 tasks

symphorien linked a pull request Feb 23, 2022 that will close this issue

stdenv.mkDerivation: add checkInputs to buildInputs #161575

Closed

13 tasks

symphorien mentioned this issue Feb 26, 2022

ocamlPackages tree-wide: Move buildInputs that should be nativeBuildInputs #161344

Merged

13 tasks

siraben added a commit to nix-community/nixpkgs-lint that referenced this issue Apr 1, 2022

Add pytestCheckHook in checkInputs lint

1baf637

Helps with NixOS/nixpkgs#161570

symphorien mentioned this issue May 5, 2022

ocamlPackages treewide: strictDeps all packages #162385

Merged

13 tasks

Artturin mentioned this issue Jul 24, 2022

upower, power-profiles-daemon: Fix GI_TYPELIB_PATH for tests #182687

Merged

13 tasks

FRidh mentioned this issue Aug 6, 2022

stdenv: fix usage of native check inputs when using strictDeps, fixes… #185406

Closed

13 tasks

symphorien mentioned this issue Dec 18, 2022

nativeCheckInputs #206742

Merged

13 tasks

symphorien added a commit to symphorien/nixpkgs that referenced this issue Jan 21, 2023

mkDerivation: introduce native checkInputs and nativeCheckInputs

f39abbc

When strictDeps is set, only nativeCheckInputs are added to PATH and only checkInputs can be linked against. See NixOS#161570

symphorien closed this as completed in #206742 Jan 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

strictDeps breaks libs in checkInputs #161570

strictDeps breaks libs in checkInputs #161570

symphorien commented Feb 23, 2022

FRidh commented Feb 25, 2022

Mindavi commented Feb 26, 2022

symphorien commented Feb 26, 2022

Artturin commented Feb 27, 2022

symphorien commented Feb 27, 2022

Ericson2314 commented Mar 7, 2022 •

edited

Loading

symphorien commented Mar 7, 2022

Ericson2314 commented Mar 7, 2022

symphorien commented Mar 12, 2022

FRidh commented Mar 29, 2022 •

edited

Loading

symphorien commented Mar 29, 2022

Ericson2314 commented Mar 31, 2022

Ericson2314 commented Mar 31, 2022 •

edited

Loading

Artturin commented Apr 1, 2022 •

edited

Loading

siraben commented Apr 1, 2022 •

edited

Loading

Ericson2314 commented Apr 2, 2022

FRidh commented Apr 2, 2022

Ericson2314 commented Apr 2, 2022

ulrikstrid commented Apr 3, 2022

ulrikstrid commented Apr 3, 2022

FRidh commented Aug 6, 2022 •

edited

Loading

ulrikstrid commented Aug 6, 2022 •

edited

Loading

symphorien commented Dec 19, 2022

strictDeps breaks libs in checkInputs #161570

strictDeps breaks libs in checkInputs #161570

Comments

symphorien commented Feb 23, 2022

Describe the bug

Steps To Reproduce

FRidh commented Feb 25, 2022

Mindavi commented Feb 26, 2022

symphorien commented Feb 26, 2022

Artturin commented Feb 27, 2022

symphorien commented Feb 27, 2022

Ericson2314 commented Mar 7, 2022 • edited Loading

symphorien commented Mar 7, 2022

Ericson2314 commented Mar 7, 2022

symphorien commented Mar 12, 2022

FRidh commented Mar 29, 2022 • edited Loading

symphorien commented Mar 29, 2022

Ericson2314 commented Mar 31, 2022

Ericson2314 commented Mar 31, 2022 • edited Loading

Artturin commented Apr 1, 2022 • edited Loading

siraben commented Apr 1, 2022 • edited Loading

Ericson2314 commented Apr 2, 2022

FRidh commented Apr 2, 2022

Ericson2314 commented Apr 2, 2022

ulrikstrid commented Apr 3, 2022

ulrikstrid commented Apr 3, 2022

FRidh commented Aug 6, 2022 • edited Loading

ulrikstrid commented Aug 6, 2022 • edited Loading

symphorien commented Dec 19, 2022

Ericson2314 commented Mar 7, 2022 •

edited

Loading

FRidh commented Mar 29, 2022 •

edited

Loading

Ericson2314 commented Mar 31, 2022 •

edited

Loading

Artturin commented Apr 1, 2022 •

edited

Loading

siraben commented Apr 1, 2022 •

edited

Loading

FRidh commented Aug 6, 2022 •

edited

Loading

ulrikstrid commented Aug 6, 2022 •

edited

Loading