bundlerApp: take buildInputs #45435
bundlerApp: take buildInputs #45435
Conversation
|
cc @manveru |
|
I don't see any problems with it and the code looks good, but haven't used |
| @@ -25,6 +25,7 @@ | |||
| , preferLocalBuild ? false | |||
| , allowSubstitutes ? false | |||
| , meta ? {} | |||
| , buildInputs ? [ makeWrapper ] | |||
There was a problem hiding this comment.
makeWrapper should be passed as a nativeBuildInput for cross-compiling
|
|
Also I don't think makeWrapper should be passed in as default? Especially since when specifying the parameter it wouldn't be merged with the default value. |
|
@globin if |
|
That would be better |
|
@zimbatm It looks like the other |
|
sorry about the extra work. in that case only buildInputs is already an improvement |
alyssais
force-pushed
the
bundler-buildInputs
branch
from
e594ad9
to
3d1530b
Compare
alyssais
force-pushed
the
bundler-buildInputs
branch
from
3d1530b
to
b338e3c
Compare
It would be reasonable to have a Ruby program that depends on some other
program being in the PATH. In this case, the obvious thing to do would
be something like this:
bundlerApp {
# ...
buildInputs = [ makeWrapper ];
postBuild = ''
wrapProgram "$out/bin/foo" \
--prefix PATH : ${lib.makeBinPath [ dep ]}
'';
}
However, this doesn't work, because even though it just forwards most of
its arguments to `runCommand`, `bundlerApp` won't take a `buildInputs`
parameter. It doesn't even specify its own `buildInputs`, which means
that the `scripts` parameter to `bundlerApp` (which depends on
`makeWrapper`) is completely broken, and, as far as I can tell, has been
since its inception. I've added a `makeWrapper` build input if the
scripts parameter is present to fix this.
I've added a `buildInputs` option to `bundlerApp`. It's also passed
through to bundled-common because `postBuild` scripts are run there as
well. This actually means that in this example we'd end up going through
two layers of wrappers (one from `bundlerApp` and one from
bundled-common), but that has always been the case and isn't likely to
break anything. That oddity does suggest that it might be prudent to
not forward `postBuild` to bundled-common (or to at least use a
different option) though...
FWIW, as far as I can tell no package in nixpkgs uses either the
`scripts` or `postBuild` options to `bundlerApp`.
alyssais
force-pushed
the
bundler-buildInputs
branch
from
b338e3c
to
938b575
Compare
GrahamcOfBorg
added
10.rebuild-darwin: 1-10
10.rebuild-linux: 1-10
and removed
10.rebuild-darwin: 11-100
10.rebuild-linux: 11-100
labels
|
Updated to fix a merge conflict. @zimbatm is there anything else needing done here? |
|
all good, thanks! |
It would be reasonable to have a Ruby program that depends on some other program being in the PATH. In this case, the obvious thing to do would be something like this:
However, this doesn't work, because even though it just forwards most of its arguments to
runCommand,bundlerAppwon't take abuildInputsparameter. It doesn't even specify its ownbuildInputs, which means that thescriptsparameter tobundlerApp(which depends onmakeWrapper) is completely broken, and, as far as I can tell, has been since its inception. By making the default[ makeWrapper ], I have fixed this.I've added a
buildInputsoption tobundlerApp. It's also passed through to bundled-common becausepostBuildscripts are run there as well. This actually means that in this example we'd end up going through two layers of wrappers (one frombundlerAppand one from bundled-common), but that has always been the case and isn't likely to break anything. That oddity does suggest that it might be prudent to not forwardpostBuildto bundled-common (or to at least use a different option) though...FWIW, as far as I can tell no package in nixpkgs uses either the
scriptsorpostBuildoptions tobundlerApp.sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)