Skip to content

Commit 52ded62

Browse files
committed
build: fix Phase 1 lint failures from PR #174 CI
CI's golangci-lint flagged 7 issues. All addressed: - gofmt drift in 4 files (struct-tag alignment): key.go, manifest.go, builder_test.go, validate_test.go. Plus a few pre-existing e2e test files the formatter touched along the way. `make fmt`-clean. - errcheck: `defer r.Close()` on zip.OpenReader return — wrap in a func literal that discards the error (we're past success at that point; nothing to do with a close failure). - deferInLoop: extracted the per-MANIFEST.MF `f.Open()` + scan into a helper (`readManifestMainClass`) so the defer is scoped to one call, not a loop iteration. Behaviour unchanged. - exitAfterDefer in cmd/build.go: previous code called `os.Exit` after a `defer cancel()` — the cancel would never run. Removed the inline os.Exit; structured errors now flow through cobra RunE return value and `cmd/root.go::Execute` renders + sets exit code (the canonical path the rest of trond uses). `make lint` is now clean. `go test ./...` still green.
1 parent 73a248f commit 52ded62

9 files changed

Lines changed: 62 additions & 59 deletions

File tree

cmd/build.go

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package cmd
22

33
import (
4-
"errors"
54
"fmt"
65
"os"
76
"os/signal"
@@ -115,18 +114,15 @@ func runBuild(cmd *cobra.Command, _ []string) error {
115114
ctx, cancel := signal.NotifyContext(cmd.Context(), os.Interrupt, syscall.SIGTERM)
116115
defer cancel()
117116

118-
outputFmt, _ := cmd.Flags().GetString("output")
119-
120117
res, err := build.Run(ctx, req)
121118
if err != nil {
122-
var se *output.StructuredError
123-
if errors.As(err, &se) {
124-
output.WriteError(os.Stderr, se, outputFmt)
125-
os.Exit(se.ExitCode)
126-
}
119+
// StructuredError propagates through cobra RunE; cmd.Execute
120+
// renders it and sets the exit code (see cmd/root.go::Execute).
121+
// Returning here lets `defer cancel()` run normally.
127122
return err
128123
}
129124

125+
outputFmt, _ := cmd.Flags().GetString("output")
130126
if outputFmt == "json" {
131127
return output.WriteJSON(os.Stdout, res)
132128
}

cmd/recipe_matrix_e2e_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ func TestE2E_Recipe_DryRunMatrix(t *testing.T) {
3838
expectSteps []string
3939
}{
4040
{
41-
recipe: "nile-test-fullnode",
42-
params: []string{"intent_path=" + intentPath},
41+
recipe: "nile-test-fullnode",
42+
params: []string{"intent_path=" + intentPath},
4343
expectSteps: []string{"validate", "preflight", "apply", "verify"},
4444
},
4545
{
@@ -52,13 +52,13 @@ func TestE2E_Recipe_DryRunMatrix(t *testing.T) {
5252
expectSteps: []string{"validate", "preflight"},
5353
},
5454
{
55-
recipe: "destroy-private-network-cleanly",
56-
params: []string{"network=private-dev"},
55+
recipe: "destroy-private-network-cleanly",
56+
params: []string{"network=private-dev"},
5757
expectSteps: []string{"status-check", "destroy"},
5858
},
5959
{
60-
recipe: "recover-failed-upgrade",
61-
params: []string{"node=my-fullnode"},
60+
recipe: "recover-failed-upgrade",
61+
params: []string{"node=my-fullnode"},
6262
expectSteps: []string{"diagnose", "rollback"},
6363
},
6464
{

cmd/schema_manifest_e2e_test.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@ func TestE2E_SchemaManifestReverse(t *testing.T) {
3333
out := runTrondCtx(ctx, t, env, "schema", "--output", "json")
3434

3535
var manifest struct {
36-
SchemaVersion string `json:"schema_version"`
37-
Tool string `json:"tool"`
36+
SchemaVersion string `json:"schema_version"`
37+
Tool string `json:"tool"`
3838
Commands []manifestCommand `json:"commands"`
3939
}
4040
if err := json.Unmarshal(out, &manifest); err != nil {
@@ -59,12 +59,12 @@ func TestE2E_SchemaManifestReverse(t *testing.T) {
5959
}
6060

6161
type manifestCommand struct {
62-
Name string `json:"name"`
63-
FullName string `json:"full_name"`
64-
Use string `json:"use"`
65-
Aliases []string `json:"aliases"`
66-
Flags []manifestFlag `json:"flags"`
67-
Subcommands []manifestCommand `json:"subcommands"`
62+
Name string `json:"name"`
63+
FullName string `json:"full_name"`
64+
Use string `json:"use"`
65+
Aliases []string `json:"aliases"`
66+
Flags []manifestFlag `json:"flags"`
67+
Subcommands []manifestCommand `json:"subcommands"`
6868
}
6969

7070
type manifestFlag struct {

cmd/statedir_e2e_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,8 @@ func TestE2E_StateDirPriority(t *testing.T) {
3636
wantNot []string
3737
}{
3838
{
39-
name: "flag-beats-env",
40-
args: []string{"--state-dir", flagDir},
39+
name: "flag-beats-env",
40+
args: []string{"--state-dir", flagDir},
4141
extraEnv: []string{
4242
"TROND_STATE_DIR=" + envDir,
4343
},

internal/build/builder_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@ type recordingRunner struct {
2020
resolved *resolved
2121
outTmp string
2222
// behavior knobs:
23-
plantArtifact string // if non-empty, write this content as the .tmp on Run
24-
returnErr error
25-
delayBeforeRun time.Duration
26-
respectCancel bool
23+
plantArtifact string // if non-empty, write this content as the .tmp on Run
24+
returnErr error
25+
delayBeforeRun time.Duration
26+
respectCancel bool
2727
}
2828

2929
func (r *recordingRunner) RunDockerBuild(ctx context.Context, res *resolved, outDir, outTmp string) error {

internal/build/key.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import (
1818
// GradleArgs (also pass 2) means `--offline` builds don't collide
1919
// with networked builds.
2020
type CacheKey struct {
21-
GitRevision string // full sha
22-
PatchHash string // sha256 hex if dirty, else ""
23-
BuilderImageDigest string // "sha256:abc..." or override ref
21+
GitRevision string // full sha
22+
PatchHash string // sha256 hex if dirty, else ""
23+
BuilderImageDigest string // "sha256:abc..." or override ref
2424
JDKVersion string
2525
ArtifactKind string // "jar" | "image"
2626
GradleTask string

internal/build/manifest.go

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -12,24 +12,24 @@ import (
1212
//
1313
// Output schema: schemas/output/build.schema.json.
1414
type Manifest struct {
15-
CacheKey string `json:"cache_key"`
16-
SourcePath string `json:"source_path"`
17-
SourceRevision string `json:"source_revision"`
18-
PatchHash string `json:"patch_hash,omitempty"`
19-
Dirty bool `json:"dirty"`
20-
BuilderImage string `json:"builder_image"`
21-
BuilderImageDigest string `json:"builder_image_digest"`
22-
JDKVersion string `json:"jdk_version"`
23-
ArtifactKind string `json:"artifact_kind"` // "jar" | "image"
24-
ArtifactPath string `json:"artifact_path,omitempty"` // for jar
25-
ImageTag string `json:"image_tag,omitempty"` // for image
26-
ImageID string `json:"image_id,omitempty"` // for image
27-
SHA256 string `json:"sha256,omitempty"` // for jar
28-
GradleTask string `json:"gradle_task"`
29-
GradleArgs []string `json:"gradle_args,omitempty"`
30-
Builder string `json:"builder"` // "docker" | "host"
31-
DurationMs int64 `json:"duration_ms"`
32-
CreatedAt time.Time `json:"created_at"`
15+
CacheKey string `json:"cache_key"`
16+
SourcePath string `json:"source_path"`
17+
SourceRevision string `json:"source_revision"`
18+
PatchHash string `json:"patch_hash,omitempty"`
19+
Dirty bool `json:"dirty"`
20+
BuilderImage string `json:"builder_image"`
21+
BuilderImageDigest string `json:"builder_image_digest"`
22+
JDKVersion string `json:"jdk_version"`
23+
ArtifactKind string `json:"artifact_kind"` // "jar" | "image"
24+
ArtifactPath string `json:"artifact_path,omitempty"` // for jar
25+
ImageTag string `json:"image_tag,omitempty"` // for image
26+
ImageID string `json:"image_id,omitempty"` // for image
27+
SHA256 string `json:"sha256,omitempty"` // for jar
28+
GradleTask string `json:"gradle_task"`
29+
GradleArgs []string `json:"gradle_args,omitempty"`
30+
Builder string `json:"builder"` // "docker" | "host"
31+
DurationMs int64 `json:"duration_ms"`
32+
CreatedAt time.Time `json:"created_at"`
3333
}
3434

3535
// CacheHit is the body returned to callers when a previous build

internal/build/validate.go

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -145,17 +145,12 @@ func ValidateJARMainClass(path, expected string) error {
145145
if err != nil {
146146
return fmt.Errorf("open jar: %w", err)
147147
}
148-
defer r.Close()
148+
defer func() { _ = r.Close() }()
149149
for _, f := range r.File {
150150
if f.Name != "META-INF/MANIFEST.MF" {
151151
continue
152152
}
153-
rc, err := f.Open()
154-
if err != nil {
155-
return fmt.Errorf("read manifest: %w", err)
156-
}
157-
defer rc.Close()
158-
got, err := scanManifestMainClass(rc)
153+
got, err := readManifestMainClass(f)
159154
if err != nil {
160155
return err
161156
}
@@ -167,6 +162,18 @@ func ValidateJARMainClass(path, expected string) error {
167162
return fmt.Errorf("jar has no META-INF/MANIFEST.MF")
168163
}
169164

165+
// readManifestMainClass scopes the open+close to one function call so
166+
// the linter sees a clean defer rather than `defer` inside the for
167+
// loop (deferInLoop) of the caller.
168+
func readManifestMainClass(f *zip.File) (string, error) {
169+
rc, err := f.Open()
170+
if err != nil {
171+
return "", fmt.Errorf("read manifest: %w", err)
172+
}
173+
defer func() { _ = rc.Close() }()
174+
return scanManifestMainClass(rc)
175+
}
176+
170177
// scanManifestMainClass extracts the Main-Class value from a JAR
171178
// manifest. Honors the JAR-manifest spec's line-continuation rule:
172179
// values longer than 72 bytes wrap to the next line with a leading

internal/build/validate_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,9 +117,9 @@ func TestValidateImageTag(t *testing.T) {
117117
{"UPPER:case", false},
118118
{"foo bar:baz", false},
119119
{"foo:bar baz", false},
120-
{"foo", false}, // missing tag
121-
{"foo:", false}, // empty tag
122-
{":bar", false}, // empty repo
120+
{"foo", false}, // missing tag
121+
{"foo:", false}, // empty tag
122+
{":bar", false}, // empty repo
123123
}
124124
for _, tc := range tests {
125125
t.Run(tc.tag, func(t *testing.T) {

0 commit comments

Comments
 (0)