buildkite: allow configuration and actual use of agent-specific hooks #32387
Conversation
|
cc @domenkozar |
| @@ -31,6 +31,14 @@ in | |||
| ''; | |||
| }; | |||
|
|
|||
| hooks-path = mkOption { | |||
| type = types.str; | |||
| default = "${pkgs.buildkite-agent}/share/hooks"; | |||
There was a problem hiding this comment.
This needs also defaultText = "${pkgs.buildkite-agent}/share/hooks"; otherwise manual will depend on buildkite-agent package
| build-path="/var/lib/buildkite-agent/builds" | ||
| bootstrap-script="${pkgs.buildkite-agent}/share/bootstrap.sh" | ||
| EOF | ||
|
|
||
| chmod +x ${cfg.hooks-path}/* 2>/dev/null || true # Guard against read-only paths. |
There was a problem hiding this comment.
This makes no sense, since cfg.hooks-path is a read-only nix store path. Whoever provides the path should make sure scripts are executable.
There was a problem hiding this comment.
The default does indeed point to a read-only store path -- but a different value might be used -- one pointing to a non-read-only location.
I've set the default to match the old value, although I agree that the old value doesn't make a lot of sense.
Do you have a better idea for the default?
There was a problem hiding this comment.
@deepfire the default is OK, but whoever provides the path should make sure it has correct permissions. The service should just consume it.
| @@ -31,6 +31,14 @@ in | |||
| ''; | |||
| }; | |||
|
|
|||
| hooks-path = mkOption { | |||
There was a problem hiding this comment.
As you can see, the immediately following option is called meta-data.
I erred on the side of following the particular author's naming convention.
Should it be ignored?
There was a problem hiding this comment.
meta-data should also be renamed, but that's out of scope for this PR.
|
@jtojnar why was this closed? |
|
Sorry, accidentaly deleted |
deepfire
force-pushed
the
buildkite-hook-configuration
branch
from
72ca6d2
to
0f6f501
Compare
deepfire
force-pushed
the
buildkite-hook-configuration
branch
from
0f6f501
to
89d9ff6
Compare
| @@ -31,6 +31,15 @@ in | |||
| ''; | |||
| }; | |||
|
|
|||
| hooksPath = mkOption { | |||
| type = types.str; | |||
… hitting the store)
|
Merging as it doesn't change the default, but just makes it configurable. |
Motivation for this change
Allow actually using the agent hooks. As of now, the directory containing them is hard-configured to point to a fixed location in the Nix store, which is read-only.
Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)