theft: init at 0.4.3 #30568
theft: init at 0.4.3 #30568
Conversation
| install -m644 vendor/greatest.h $out/include/ | ||
| ''; | ||
|
|
||
| meta = { |
There was a problem hiding this comment.
If you change this to meta = with stdenv.lib; { you can get rid of all the stdenv.lib stuff below.
There was a problem hiding this comment.
Understood, but are you indicating this as a helpful hint, or is this a requirement for acceptance of this package? If it's not required, my preference would be for the current explicit form, but I will make this change if required.
There was a problem hiding this comment.
It's pretty subjective, so no, not a requirement. I however, find it easier to read.
| }; | ||
|
|
||
| preConfigure = "patchShebangs ./scripts/mk_bits_lut"; | ||
| # doCheck = true; |
There was a problem hiding this comment.
Any particular reason checks are not enabled? Please document that.
There was a problem hiding this comment.
checks are now enabled
| preConfigure = "patchShebangs ./scripts/mk_bits_lut"; | ||
| # doCheck = true; | ||
|
|
||
| propagatedBuildInputs = [ ]; |
|
|
||
| propagatedBuildInputs = [ ]; | ||
|
|
||
| buildInputs = [ ]; |
|
|
||
| buildInputs = [ ]; | ||
|
|
||
| installFlags = [ "PREFIX=$(out)" ]; |
There was a problem hiding this comment.
This should be passed to the configure phase automatically. Are you sure this is needed?
There was a problem hiding this comment.
This package does not use autoconf or any other configuration option. The --prefix option is supplied during the configurePhase, but no corresponding specification is normally provided during the installPhase. The Makefile for this project assumes the older-style convention of specifying PREFIX when running the install target, which is what the above is intended to accomplish. This is necessary and works correctly in my testing of this derivation.
|
@peterhoeg, thank you for the initial review. I have update the pull request and would appreciate your feedback and re-review. |
| installFlags = [ "PREFIX=$(out)" ]; | ||
| postInstall = '' | ||
| install -m644 vendor/greatest.h $out/include/ | ||
| ''; |
peterhoeg
changed the title
|
Thanks @kquick ! |
Motivation for this change
Package is not currently in nixpkgs.
Depends on pull request #30565
Things done
build-use-sandboxinnix.confon non-NixOS)N/A
nix-shell -p nox --run "nox-review wip"N/A
./result/bin/)N/A