-
-
Notifications
You must be signed in to change notification settings - Fork 15.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: experiment with a hook based alternative to buildGoModule #353526
base: master
Are you sure you want to change the base?
Conversation
5381b6c
to
1c1769a
Compare
if [ -z "${dontGoBuild-}" ] && [ -z "${buildPhase-}" ]; then | ||
buildPhase=goBuildHook | ||
fi | ||
|
||
|
||
if [ -z "${dontGoCheck-}" ] && [ -z "${checkPhase-}" ]; then | ||
checkPhase=goCheckHook | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the motivation of this PR is to increase composability, meaning constructing one call to stdenv.mkDerivation
with functionalities provided by more than one build helpers, such as buildGoModule
and rustPlatform.buildRustPackage
.
Would it be more composable if we also add goBuildHook
(with runHook
removed, of course) to preBuildHooks
or postBuildHooks
?
Use case: one package, with its own buildPhase
, wants to build some go stuff.
Here is my implementation of this idea for Rust: #334476.
cd1a821
to
4d6cb5e
Compare
4d6cb5e
to
083a109
Compare
083a109
to
9273a74
Compare
I like the idea of moving to a hook based alternative, but I think it will need some discussion on how exactly such a future builder would look like. Maybe you can make your experiment consumable via an out-of-tree flake for now, so people can evaluate and test it? This is what was done for gomod2nix (which didn't end up in nixpkgs in the end). |
fi | ||
exclude+='\)' | ||
|
||
buildGoDir() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, Bash doesn't support nested function declaration. Let's flatten the definition of these Bash functions.
@@ -0,0 +1,140 @@ | |||
|
|||
goBuildHook() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
goBuildHook() { | |
goBuildPhase() { |
AFAICT, fooBarHook
is meant to be run when doing runHook foo
.
Also, I would propose also defining goBuild
, which runs all the goBuildPhase
commands except runHook preBuild
and runHook postBuild
. This way, goBuild
could be inserted into the pre
- and post
-phases of other build helpers/workflows. For example, apptainer
uses the stdenv.mkDerivation
's Make-based workflow during configuration and build but still requires the Go modules realization from goConfigurePhase
. If we have goConfigure
, we could plug it into preConfigure
and switch on dontGoConfigure
and dontGoBuild
.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-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.