lib/strings: Add makeIncludePath

[mrdev023]

Description of changes

Add makeIncludePath, it can be usefull to add wrapProgram with C_INCLUDE_PATH

libraryPath = pkgs.lib.makeLibraryPath (libs);
makeIncludePath = pkgs.lib.makeSearchPathOutput "dev" "include";
includePath = makeIncludePath (libs);

wrappedRuby = (pkgs.symlinkJoin {
  name = "ruby";
  paths = [ ruby ];
  nativeBuildInputs = [ pkgs.makeWrapper ];
  postBuild = ''
    wrapProgram $out/bin/ruby \
      --set LD_LIBRARY_PATH ${libraryPath}
    wrapProgram $out/bin/bundle \
      --set LD_LIBRARY_PATH ${libraryPath} \
      --set LIBRARY_PATH ${libraryPath} \
      --set C_INCLUDE_PATH ${includePath}
  '';
});

• 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:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• 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.

[mrdev023]

Result of nixpkgs-review pr 296237 run on x86_64-linux 1

1 package blacklisted:

• nixos-install-tools

[lolbinarycat]

i think C_INCLUDE_PATH should be automatically setup by stdenv, but i suppose it could be useful to have more precise control, especially when working with programs that parse c headers at runtime for ffi.

[mrdev023]

i think C_INCLUDE_PATH should be automatically setup by stdenv, but i suppose it could be useful to have more precise control, especially when working with programs that parse c headers at runtime for ffi.

the ffi gem seems to manage headers and libs loading itself

[lolbinarycat]

parsing C headers is fairly common for ffi in dynamic languages.

[lolbinarycat]

seems like a trivial function with adequate usecases, i don't see a reason to oppose this.

there are a few tweaks to the documentation i would suggest, however.

[infinisil]

It looks good to me, but it's unfortunate that all makeSearchPath functions are entirely untested, and I'd rather not merge new untested code (also in the new lib contribution guidelines).

So could you write some tests for these functions? lib/tests/misc.nix should work here

[mrdev023]

It looks good to me, but it's unfortunate that all makeSearchPath functions are entirely untested, and I'd rather not merge new untested code (also in the new lib contribution guidelines).

So could you write some tests for these functions? lib/tests/misc.nix should work here

I add it soon as possible, i didn't know where to put the tests ^^

[mrdev023]

It looks good to me, but it's unfortunate that all makeSearchPath functions are entirely untested, and I'd rather not merge new untested code (also in the new lib contribution guidelines).

So could you write some tests for these functions? lib/tests/misc.nix should work here

I add unit test for strings but i don't know how to test with pkgs arguments

[lolbinarycat]

maybe you could add pkgs = import ../.. {}; to the let expression at the top of the file? that seems to be how other testing files do it. then you can just interpolate the package outPath into the expected string, like "${pkgs.openssl.dev.outPath}/include or whatever.

[mrdev023]

maybe you could add pkgs = import ../.. {}; to the let expression at the top of the file? that seems to be how other testing files do it. then you can just interpolate the package outPath into the expected string, like "${pkgs.openssl.dev.outPath}/include or whatever.

I made requested changes. Would you like me to test makeLibraryPath and makeBinPath too?

[mrdev023]

Strange ofborg-eval not like my commit fb1850c

but nix-instantiate --eval --strict lib/tests/misc.nix return empty list

[lolbinarycat]

hmm, the CI failure is strange, maybe it would be best to go back to using fake derivations instead of real ones.

[mrdev023]

hmm, the CI failure is strange, maybe it would be best to go back to using fake derivations instead of real ones.

done.

[infinisil]

hmm, the CI failure is strange, maybe it would be best to go back to using fake derivations instead of real ones.

The CI failure was caused by master being broken, it was not related, see #274481 (comment). But yeah using fake derivations here sounds fine.

It would be great to test it a bit more thoroughly though, getOutput is non-trivial:

nixpkgs/lib/attrsets.nix

Lines 1762 to 1765 in 2d96747

	getOutput = output: pkg:
	if ! pkg ? outputSpecified || ! pkg.outputSpecified
	then pkg.${output} or pkg.out or pkg
	else pkg;

[mrdev023]

hmm, the CI failure is strange, maybe it would be best to go back to using fake derivations instead of real ones.

The CI failure was caused by master being broken, it was not related, see #274481 (comment). But yeah using fake derivations here sounds fine.

It would be great to test it a bit more thoroughly though, getOutput is non-trivial:

nixpkgs/lib/attrsets.nix

Lines 1762 to 1765 in 2d96747

	getOutput = output: pkg:
	if ! pkg ? outputSpecified || ! pkg.outputSpecified
	then pkg.${output} or pkg.out or pkg
	else pkg;

Should I also test getOutput?

[infinisil]

Looking good to me now!

Feel free to make further PRs to add more tests, I definitely won't complain about that :P

[mrdev023]

Looking good to me now!

Feel free to make further PRs to add more tests, I definitely won't complain about that :P

Yes, as soon as I have time, I'll add some unit tests ^^