cc-wrapper, bintools-wrapper: Improve hooks that run before invocation #366960
Conversation
The existing post-link-hook is in an unfortunate position since it does not enable interposing the arguments of the compiler. This is an RFC to add a hook which enables that. Signed-off-by: Peter Waller <[email protected]>
I observe that the old position meant that if the wrapper were to interfere with the arguments going into the compiler, it would not be reflected in the debug output. Signed-off-by: Peter Waller <[email protected]>
github-actions
bot
added
the
8.has: documentation
|
/cc @Ericson2314 in case you have any opinions. I have not yet had a chance to use these, but I have use cases for them for experimentation and development. I hope they are obvious source modifications. Being able to directly influence extraBefore and extraAfter in particular after all other processing is a useful capability. |
|
Test failure is that the manual failed to build because it had to build everything from scratch and looks like it timed out after 6 hours. |
ofborg
bot
added
10.rebuild-darwin-stdenv
|
/cc @RossComputerGuy @philiptaron @emilazy (hope you don't mind the ping). I think this change is an obvious small source transformation, though it incurs a stdenv rebuild so has not received much testing. Slightly unfortunate to end up with two hooks for the linker wrapper; I opted not to remove the old one to retain backwards compatibility for now. |
There was a problem hiding this comment.
The change to run before the logging makes a lot of sense to me.
The additional hook also makes sense to me, since it allows for mutation of the linker arguments. Doing the search on post-link-hook and seeing that it's used for codesigning on Darwin also clarifies its usecase and lets me feel assured that supporting two hooks is a worthwhile idea.
| @@ -1411,6 +1411,7 @@ these in the [Hooks Reference](#chap-hooks). | |||
| ### Compiler and Linker wrapper hooks {#compiler-linker-wrapper-hooks} | |||
|
|
|||
| If the file `${cc}/nix-support/cc-wrapper-hook` exists, it will be run at the end of the [compiler wrapper](#cc-wrapper). | |||
| If the file `${binutils}/nix-support/ld-wrapper-hook` exists, it will be run at the end of the linker wrapper, before the linker runs. | |||
| If the file `${binutils}/nix-support/post-link-hook` exists, it will be run at the end of the linker wrapper. | |||
There was a problem hiding this comment.
Within nixpkgs, this appears to be used by pkgs/os-specific/darwin/signing-utils/post-link-sign-hook.nix only.
There was a problem hiding this comment.
This hook is nearly vestigial. Darwin only uses this hook early in the bootstrap. Once it builds a version of ld64 supporting code-sigining, it’s no longer used. It can be dropped once Darwin’s bootstrap tools are updated with that version of ld64, which should hopefully be doable after this staging-next merge. Getting clang 19 into the bootstrap is also important for other reasons (such as guaranteeing C23 support in the bootstrap).
Edit: Looks like staging-next was merged, so now just the bootstrap tools need updated.
|
I'm going to run the stdenv test suite then I plan on merging this. Please countermand me, @NixOS/stdenv, if you disagree! |
RFC: Improve these hooks. bintools only had a hook which could run after the linker, which meant it could not be used to modify arguments. cc-wrapper-hook could run before the compiler invocation, but it ran before the NIX_DEBUG, which implied that if the wrapper were to modify arguments, it would not be reflected in the debug output.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.