execline: wrap unconditionally; strip #78372
Conversation
I don't think there's any situation in which an unwrapped execlineb is useful -- if you want to use different versions of the execlineb tool it'll still prefer ones in PATH. At the same time, implementing the wrapper in this way, as a series of two derivations, meant that we didn't get stdenv goodness for the wrapper. This meant that, for example, the wrapper was not stripped, and so execline ended up with runtime dependencies on gcc and the Linux headers. I don't want to have to reimplement this sort of stuff when it's already in stdenv, and so it makes much more sense to create the wrapper in the mkDerivation call, where all of stdenv's normal magic will find it.
| cc \ | ||
| -O \ | ||
| -Wall -Wpedantic \ | ||
| -D 'EXECLINEB_PATH()="${execline}/bin/execlineb"' \ | ||
| -D 'EXECLINE_BIN_PATH()="${execline}/bin"' \ |
There was a problem hiding this comment.
@Profpatsch why are these functions, btw?
There was a problem hiding this comment.
I don’t remember, but I remember that there was a semantic difference that makes it work/not work
There was a problem hiding this comment.
Consult the gcc manual of your choice I guess :)
ofborg
bot
added
8.has: package (new)
10.rebuild-darwin: 1-10
10.rebuild-linux: 1-10
labels
I think there is, if you don’t want to have an overhead of two We don’t have to remove the wrapper, we can use |
|
> I don't think there's any situation in which an unwrapped execlineb is
> useful
I think there is, if you don’t want to have an overhead of two `exec`
calls per call to execlineb (which is about 2–10ms).
I really don't think this is an important use case. If you were worried
about exec overhead, surely you wouldn't be using execline, where
literally every operation adds another exec?
|
|
>> I don't think there's any situation in which an unwrapped execlineb is
>> useful
>
> I think there is, if you don’t want to have an overhead of two `exec`
> calls per call to execlineb (which is about 2–10ms).
I really don't think this is an important use case. If you were worried
about exec overhead, surely you wouldn't be using execline, where
literally every operation adds another exec?
If it really is important, you can always use .execlineb-wrapped. I
certainly don't think it's an important enough use case to warrant its
own option in the execline package. If you're supplying your own
execline tools you can even just add one more symlink to point
"execlineb" in your environment to "${execline}/bin/.execlineb-wrapped".
|
That’s a good counterargument. |
Motivation for this change
I don't think there's any situation in which an unwrapped execlineb is
useful -- if you want to use different versions of the execlineb tool
it'll still prefer ones in PATH. At the same time, implementing the
wrapper in this way, as a series of two derivations, meant that we
didn't get stdenv goodness for the wrapper. This meant that, for
example, the wrapper was not stripped, and so execline ended up with
runtime dependencies on gcc and the Linux headers. I don't want to
have to reimplement this sort of stuff when it's already in stdenv,
and so it makes much more sense to create the wrapper in the
mkDerivation call, where all of stdenv's normal magic will find it.
Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)nix path-info -Sbefore and after)