cc-wrapper: Always export environment variables for binutils#27415
cc-wrapper: Always export environment variables for binutils#27415Ericson2314 merged 2 commits intoNixOS:stagingfrom
Conversation
Before, this only happened when cross compiling.
|
@Ericson2314, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @copumpkin and @jwiegley to be potential reviewers. |
While this requires newer bash, stdenv's setup.sh now does across the board anyways. This way is more concise.
c41d2a1 to
3739858
Compare
| if | ||
| PATH=$_PATH type -p "@binPrefix@$CMD" > /dev/null | ||
| then | ||
| export "${ENV_PREFIX}${CMD^^}=@binPrefix@${CMD}"; |
There was a problem hiding this comment.
@Dridus Sorry but I changed it back as new we require newish bash anyways.
shlevy
left a comment
There was a problem hiding this comment.
Just glanced through, but LGTM
|
Well, this one is uncharacteristically small an readable :). |
|
IMHO we went in the wrong direction, we should have stopped (and should stop) exporting AR, AS, LD etc. [1] (everything but CC [2]) for cross rather than start exporting them for native, and provide the cross utilities under unprefixed names in the PATH instead. This would have avoided (and will avoid) much debugging and unnecessary workarounds against upstream build scripts, both for native and for cross packaging [3]. [1] This does not apply to BUILD_{var} and {var}_FOR_BUILD. |
|
@orivej my original goal was to end up always prefixing all tools with their target platforms (wrapper script only for multi-platform tools). I think the current design makes more sense with that objective. Yours probably would be epitomized with instead prefixing the binaries with |
Motivation for this change
Before, this only happened when cross compiling. This is less of a hack (and soon the entire
preWrapscript will go), uniformity is good.Things done
Built Linux and Darwin stdenvs.
CC @ali-abrar This narrows down the bugs we've seen to the
infixSalt-ing, basically.