Skip to content

Commit d8aa28b

Browse files
committed
feat: lefthook env support, refactor commit-msg hook
Adds lefthook `env` support. This is needed for `generate` and `docs` make targets in some repos, as the `USERNAME` must be set to `siderolabs` (the GH org name). On Zsh, `USERNAME` will always evaluate to the Linux username, even with an export/override. Without setting `"USERNAME": "siderolabs"` for these make targets, lefthook will generate artifacts with mismatched repo/image identifiers (containing the local username). This diff adds a "USERNAME" env override to all make targets invoked by lefthook. Refactors commit-msg hook to be pre-push instead. Using commit-msg caused an edge case issue, where the user was unable to amend the commit message with `make conformance` running at that stage. post-commit is more suitable for this scenario. Signed-off-by: Maja Bojarska <maja.bojarska@siderolabs.com>
1 parent 3c738c6 commit d8aa28b

16 files changed

Lines changed: 313 additions & 39 deletions

internal/output/lefthook/command.go

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@ package lefthook
66

77
// Command represents a single named command under a hook's commands: map.
88
type Command struct { //nolint:govet
9-
Run string `yaml:"run"`
10-
Tags []string `yaml:"tags,omitempty"`
11-
Glob string `yaml:"glob,omitempty"`
12-
Files string `yaml:"files,omitempty"`
13-
Skip []string `yaml:"skip,omitempty"`
14-
Only []string `yaml:"only,omitempty"`
15-
Interactive bool `yaml:"interactive,omitempty"`
16-
StageFixed bool `yaml:"stage_fixed,omitempty"`
17-
Priority int `yaml:"priority,omitempty"`
9+
Run string `yaml:"run"`
10+
Tags []string `yaml:"tags,omitempty"`
11+
Glob string `yaml:"glob,omitempty"`
12+
Files string `yaml:"files,omitempty"`
13+
Env map[string]string `yaml:"env,omitempty"`
14+
Skip []string `yaml:"skip,omitempty"`
15+
Only []string `yaml:"only,omitempty"`
16+
Interactive bool `yaml:"interactive,omitempty"`
17+
StageFixed bool `yaml:"stage_fixed,omitempty"`
18+
Priority int `yaml:"priority,omitempty"`
1819
}
1920

2021
// WithRun sets the shell command lefthook executes for this command.
@@ -45,6 +46,17 @@ func (c *Command) WithFiles(files string) *Command {
4546
return c
4647
}
4748

49+
// WithEnv sets an environment variable on the command; safe to call multiple times.
50+
func (c *Command) WithEnv(name, value string) *Command {
51+
if c.Env == nil {
52+
c.Env = map[string]string{}
53+
}
54+
55+
c.Env[name] = value
56+
57+
return c
58+
}
59+
4860
// WithSkip lists git states or refs where the command should be skipped (e.g. "merge", "rebase").
4961
func (c *Command) WithSkip(skip ...string) *Command {
5062
c.Skip = skip

internal/output/lefthook/hook.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,8 @@ const (
1515

1616
// HookGroupPreCommit is the lefthook "pre-commit" hook name.
1717
HookGroupPreCommit = "pre-commit"
18-
// HookGroupCommitMsg is the lefthook "commit-msg" hook name.
19-
HookGroupCommitMsg = "commit-msg"
18+
// HookGroupPostCommit is the lefthook "post-commit" hook name.
19+
HookGroupPostCommit = "post-commit"
2020
)
2121

2222
// Hook represents the configuration for a single git hook (e.g. pre-commit)

internal/output/lefthook/lefthook.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ func (o *Output) Enable() {
4747
}
4848

4949
// Hook returns the configuration for the named git hook (e.g. "pre-commit",
50-
// "commit-msg"), creating it on first access. New hooks are blank — set the
50+
// "post-commit"), creating it on first access. New hooks are blank — set the
5151
// execution model via WithParallel/WithPiped, and populate either Commands
5252
// or Jobs depending on which lefthook style you want.
5353
func (o *Output) Hook(name string) *Hook {

internal/output/lefthook/lefthook_test.go

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ func TestCommandsRoundTrip(t *testing.T) {
2929
preCommit.Command("fmt").WithRun("make fmt")
3030
preCommit.Command("lint").WithRun("make lint")
3131

32-
o.Hook("commit-msg").Command("conformance").WithRun("make conformance")
32+
o.Hook("post-commit").Command("conformance").WithRun("make conformance")
3333

3434
require.Equal(t, []string{"lefthook.yml"}, o.Filenames())
3535

@@ -46,11 +46,11 @@ func TestCommandsRoundTrip(t *testing.T) {
4646
require.NoError(t, yaml.Unmarshal(buf.Bytes(), &decoded))
4747

4848
require.Contains(t, decoded, "pre-commit")
49-
require.Contains(t, decoded, "commit-msg")
49+
require.Contains(t, decoded, "post-commit")
5050

5151
assert.Equal(t, "make fmt", decoded["pre-commit"].Commands["fmt"].Run)
5252
assert.Equal(t, "make lint", decoded["pre-commit"].Commands["lint"].Run)
53-
assert.Equal(t, "make conformance", decoded["commit-msg"].Commands["conformance"].Run)
53+
assert.Equal(t, "make conformance", decoded["post-commit"].Commands["conformance"].Run)
5454
}
5555

5656
func TestParallelDefaultIsOmitted(t *testing.T) {
@@ -70,7 +70,7 @@ func TestParallelFalseIsEmitted(t *testing.T) {
7070
o := lefthook.NewOutput()
7171
o.Enable()
7272

73-
o.Hook("commit-msg").
73+
o.Hook("post-commit").
7474
WithParallel(false).
7575
Command("conformance").WithRun("make conformance")
7676

@@ -252,6 +252,8 @@ func TestCommandBuilders(t *testing.T) {
252252
WithTags("format", "go").
253253
WithGlob("*.go").
254254
WithFiles("git diff --name-only").
255+
WithEnv("GOFMT_OPTS", "-s").
256+
WithEnv("DEBUG", "1").
255257
WithSkip("merge", "rebase").
256258
WithOnly("ref: main").
257259
WithInteractive().
@@ -265,15 +267,16 @@ func TestCommandBuilders(t *testing.T) {
265267
var decoded map[string]struct { //nolint:govet
266268
Parallel *bool `yaml:"parallel"`
267269
Commands map[string]struct { //nolint:govet
268-
Run string `yaml:"run"`
269-
Tags []string `yaml:"tags"`
270-
Glob string `yaml:"glob"`
271-
Files string `yaml:"files"`
272-
Skip []string `yaml:"skip"`
273-
Only []string `yaml:"only"`
274-
Interactive bool `yaml:"interactive"`
275-
StageFixed bool `yaml:"stage_fixed"`
276-
Priority int `yaml:"priority"`
270+
Run string `yaml:"run"`
271+
Tags []string `yaml:"tags"`
272+
Glob string `yaml:"glob"`
273+
Files string `yaml:"files"`
274+
Env map[string]string `yaml:"env"`
275+
Skip []string `yaml:"skip"`
276+
Only []string `yaml:"only"`
277+
Interactive bool `yaml:"interactive"`
278+
StageFixed bool `yaml:"stage_fixed"`
279+
Priority int `yaml:"priority"`
277280
} `yaml:"commands"`
278281
}
279282

@@ -288,6 +291,7 @@ func TestCommandBuilders(t *testing.T) {
288291
assert.Equal(t, []string{"format", "go"}, cmd.Tags)
289292
assert.Equal(t, "*.go", cmd.Glob)
290293
assert.Equal(t, "git diff --name-only", cmd.Files)
294+
assert.Equal(t, map[string]string{"GOFMT_OPTS": "-s", "DEBUG": "1"}, cmd.Env)
291295
assert.Equal(t, []string{"merge", "rebase"}, cmd.Skip)
292296
assert.Equal(t, []string{"ref: main"}, cmd.Only)
293297
assert.True(t, cmd.Interactive)

internal/project/common/conformance.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ func (conformance *Conformance) CompileMakefile(output *makefile.Output) error {
4848

4949
// CompileLefthook implements lefthook.Compiler.
5050
func (conformance *Conformance) CompileLefthook(output *lefthook.Output) error {
51-
output.Hook(lefthook.HookGroupCommitMsg).WithParallel(false).
52-
Command("conformance").WithRun("make conformance")
51+
output.Hook(lefthook.HookGroupPostCommit).WithParallel(false).
52+
Command("conformance").WithRun("make conformance").WithEnv("USERNAME", conformance.meta.GitHubOrganization)
5353

5454
return nil
5555
}

internal/project/common/conformance_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,50 @@
55
package common_test
66

77
import (
8+
"bytes"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
"go.yaml.in/yaml/v4"
1114

1215
"github.com/siderolabs/kres/internal/output/lefthook"
1316
"github.com/siderolabs/kres/internal/output/makefile"
1417
"github.com/siderolabs/kres/internal/project/common"
18+
"github.com/siderolabs/kres/internal/project/meta"
1519
)
1620

1721
func TestConformanceInterfaces(t *testing.T) {
1822
assert.Implements(t, (*makefile.Compiler)(nil), new(common.Conformance))
1923
assert.Implements(t, (*lefthook.Compiler)(nil), new(common.Conformance))
2024
}
25+
26+
func TestConformanceLefthook(t *testing.T) {
27+
conformance := common.NewConformance(&meta.Options{
28+
GitHubOrganization: "testorg",
29+
})
30+
31+
output := lefthook.NewOutput()
32+
output.Enable()
33+
34+
require.NoError(t, conformance.CompileLefthook(output))
35+
36+
var buf bytes.Buffer
37+
38+
require.NoError(t, output.GenerateFile("lefthook.yml", &buf))
39+
40+
var decoded map[string]struct {
41+
Commands map[string]struct {
42+
Env map[string]string `yaml:"env"`
43+
} `yaml:"commands"`
44+
}
45+
46+
require.NoError(t, yaml.Unmarshal(buf.Bytes(), &decoded))
47+
48+
hook := decoded["post-commit"]
49+
require.NotEmpty(t, hook.Commands)
50+
51+
conformanceCmd, exists := hook.Commands["conformance"]
52+
require.True(t, exists, "conformance command not found")
53+
assert.Equal(t, "testorg", conformanceCmd.Env["USERNAME"])
54+
}

internal/project/common/lint.go

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,12 +91,19 @@ func (lint *Lint) CompileLefthook(output *lefthook.Output) error {
9191
// stage 1: mutating formatters (shared group — generate/fmt blocks append here too).
9292
hookPreCommit.Group(lefthook.PreCommitFixStage).
9393
WithParallel(false).
94-
Job().WithName("lint-fmt").WithRun("make lint-fmt").WithStageFixed()
94+
Job().
95+
WithName("lint-fmt").
96+
WithRun("make lint-fmt").
97+
WithEnv("USERNAME", lint.meta.GitHubOrganization).
98+
WithStageFixed()
9599

96100
// stage 2: lint runs after, against the formatted tree.
97101
hookPreCommit.Group(lefthook.PreCommitLintStage).
98102
WithParallel(false).
99-
Job().WithName("lint").WithRun("make lint")
103+
Job().
104+
WithName("lint").
105+
WithRun("make lint").
106+
WithEnv("USERNAME", lint.meta.GitHubOrganization)
100107

101108
return nil
102109
}

internal/project/common/lint_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,92 @@
55
package common_test
66

77
import (
8+
"bytes"
89
"testing"
910

1011
"github.com/stretchr/testify/assert"
12+
"github.com/stretchr/testify/require"
13+
"go.yaml.in/yaml/v4"
1114

1215
"github.com/siderolabs/kres/internal/output/ghworkflow"
1316
"github.com/siderolabs/kres/internal/output/lefthook"
1417
"github.com/siderolabs/kres/internal/output/makefile"
1518
"github.com/siderolabs/kres/internal/project/common"
19+
"github.com/siderolabs/kres/internal/project/meta"
1620
)
1721

1822
func TestLintInterfaces(t *testing.T) {
1923
assert.Implements(t, (*makefile.Compiler)(nil), new(common.Lint))
2024
assert.Implements(t, (*ghworkflow.Compiler)(nil), new(common.Lint))
2125
assert.Implements(t, (*lefthook.Compiler)(nil), new(common.Lint))
2226
}
27+
28+
func TestLintLefthook(t *testing.T) {
29+
lint := common.NewLint(&meta.Options{
30+
GitHubOrganization: "testorg",
31+
})
32+
33+
output := lefthook.NewOutput()
34+
output.Enable()
35+
36+
require.NoError(t, lint.CompileLefthook(output))
37+
38+
var buf bytes.Buffer
39+
40+
require.NoError(t, output.GenerateFile("lefthook.yml", &buf))
41+
42+
var decoded map[string]struct {
43+
Jobs []struct {
44+
Group *struct {
45+
Jobs []struct {
46+
Env map[string]string `yaml:"env"`
47+
Name string `yaml:"name"`
48+
} `yaml:"jobs"`
49+
} `yaml:"group"`
50+
} `yaml:"jobs"`
51+
}
52+
53+
require.NoError(t, yaml.Unmarshal(buf.Bytes(), &decoded))
54+
55+
hook := decoded["pre-commit"]
56+
require.NotEmpty(t, hook.Jobs, "should have at least one job group")
57+
58+
// Check lint-fmt job in PreCommitFixStage
59+
require.NotNil(t, hook.Jobs[0].Group)
60+
61+
var lintFmtJob *struct {
62+
Env map[string]string `yaml:"env"`
63+
Name string `yaml:"name"`
64+
}
65+
66+
for i := range hook.Jobs[0].Group.Jobs {
67+
if hook.Jobs[0].Group.Jobs[i].Name == "lint-fmt" {
68+
lintFmtJob = &hook.Jobs[0].Group.Jobs[i]
69+
70+
break
71+
}
72+
}
73+
74+
require.NotNil(t, lintFmtJob, "lint-fmt job not found")
75+
assert.Equal(t, "testorg", lintFmtJob.Env["USERNAME"])
76+
77+
// Check lint job in PreCommitLintStage
78+
require.Len(t, hook.Jobs, 2, "should have two job groups (fix and lint stages)")
79+
require.NotNil(t, hook.Jobs[1].Group)
80+
81+
var lintJob *struct {
82+
Env map[string]string `yaml:"env"`
83+
Name string `yaml:"name"`
84+
}
85+
86+
for i := range hook.Jobs[1].Group.Jobs {
87+
if hook.Jobs[1].Group.Jobs[i].Name == "lint" {
88+
lintJob = &hook.Jobs[1].Group.Jobs[i]
89+
90+
break
91+
}
92+
}
93+
94+
require.NotNil(t, lintJob, "lint job not found")
95+
assert.Equal(t, "testorg", lintJob.Env["USERNAME"])
96+
}

internal/project/custom/custom_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,15 @@ func TestCompileLefthookCommands(t *testing.T) {
135135
step := custom.NewStep(&meta.Options{}, "custom")
136136
step.Lefthook.Enabled = true
137137
step.Lefthook.Hooks = map[string]*lefthook.Hook{
138-
"commit-msg": {
138+
"post-commit": {
139139
Parallel: new(false),
140140
Commands: map[string]*lefthook.Command{
141141
"conformance": {Run: "make conformance"},
142142
},
143143
},
144144
}
145145

146-
hook := decodeLefthook(t, step)["commit-msg"]
146+
hook := decodeLefthook(t, step)["post-commit"]
147147

148148
require.NotNil(t, hook.Parallel)
149149
assert.False(t, *hook.Parallel)

internal/project/golang/generate.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -437,7 +437,10 @@ func (generate *Generate) CompileLefthook(output *lefthook.Output) error {
437437
output.Hook(lefthook.HookGroupPreCommit).
438438
Group(lefthook.PreCommitFixStage).
439439
WithParallel(false).
440-
Job().WithName("generate").WithRun("make generate").WithStageFixed()
440+
Job().WithName("generate").
441+
WithRun("make generate").
442+
WithEnv("USERNAME", generate.meta.GitHubOrganization).
443+
WithStageFixed()
441444

442445
return nil
443446
}

0 commit comments

Comments
 (0)