Idris: fix "addEnvHooks: command not found" (#33820) #33838
Conversation
On the current head 'addEnvHooks' doesn't seem to be available in 'preHook'. This patch tries to stay as close as the original by moving the code to 'setupHook'.
|
Sorry about that. I'll eventually do a mass rebuild hosting up the definition so it's in scope of the prehook. For now though, this workaround isn't quite right. Make the script a nativeBuildInput instead of a setup hook. The former will have it just affect the current build, as the preHook did, while the latter makes it not affect the current build and just downstream builds. |
| @@ -35,6 +36,7 @@ | |||
| ''; | |||
|
|
|||
| installPhase = '' | |||
There was a problem hiding this comment.
Likewise you won't need this.
There was a problem hiding this comment.
Do you mean I won't need the installPhase or just the export line?
There was a problem hiding this comment.
Just the export line. Guess I highlighted the wrong one sorry.
|
@Ericson2314: Before I give it a go, could you give feedback on whether moving the script to |
| @@ -2,26 +2,27 @@ | |||
| # | |||
| # args: Additional arguments to pass to mkDerivation. Generally should include at least | |||
| # name and src. | |||
| { stdenv, idris, gmp }: args: stdenv.mkDerivation ({ | |||
| preHook = '' | |||
There was a problem hiding this comment.
have you tried postHook, that runs after the default setup.sh but before genericBuild
There was a problem hiding this comment.
Please forgive my ignorance but now I'm curious - how would this part https://github.com/NixOS/nixpkgs/pull/33838/files#diff-852bb06e74b1ca7f9271dc4dd42aaca9R20 be implemented when using postHook? (and sorry for the noise - I'm struggling to figure out how the various pieces fit together here).
There was a problem hiding this comment.
I only looked through this very briefly so I might be missing something.
The preHook runs at the very start of $stdenv/setup only a few things like helpers and PATH are defined at this point. On the other had the postHook will run at the very end of $stdenv/setup when all of the phases, etc. are defined. This is still before all of the phases run but at this point addEnvHooks will be available, I think that will work without having to use a setup hook.
There was a problem hiding this comment.
@LnL7 thank you! It seems to me that addEnvHooks would be available in postHook but at the point postHook adds hooks via addEnvHooks said hooks would have been executed already?
What I'm trying to say is that adding a hook via addEnvHooks in postHook seems to be too late for it to execute.
(apologies for derailing the discussion, I will shut up now 😆)
There was a problem hiding this comment.
Oh, you're right, you can ignore the comment then 😄. The env hooks run before that so we need to either move that down or define addEnvHooks earlier.
|
I'm not working with Idris anymore and this PR is not a proper fix, therefore I've decided to close this pull request. |
Motivation for this change
On the current head 'addEnvHooks' doesn't seem to be available in 'preHook'.
This patch tries to stay as close as the original by moving the code to
'setupHook' and changing the ln options.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)