149 lines
7.9 KiB
Markdown
149 lines
7.9 KiB
Markdown
# Host-native launcher review findings
|
|
|
|
Reviewed the implementation from `bf07bed` through `0269648` against the
|
|
[feature roadmap](host-native-launchers.md) and
|
|
[implementation plan](implementation.md).
|
|
|
|
The maintained launchers select the Go host before preparing and running the
|
|
tool. API-only recovery prepares its private module on the host; printed Go
|
|
handoffs discover the host again when run. The CLI retains the native fallback
|
|
and saved-record selection. Application build targets still come from the
|
|
profile, and the public Go facades and dependency boundaries are unchanged.
|
|
|
|
## Make the adoption smoke command runnable
|
|
|
|
**Evidence:** The [adoption guide](../consumers/adoption.md#cli-tool-dependency)
|
|
tells readers to save the launcher as `releasekit` and run
|
|
`./releasekit version`. A newly saved shell file ordinarily lacks its execute
|
|
bit. In a disposable directory, the documented command failed with status 126
|
|
and “Permission denied.” Invoking the same snippet through `sh` ran a
|
|
module-registered fixture tool on Linux/amd64 with inherited
|
|
`GOOS=windows GOARCH=arm64`.
|
|
|
|
**Fix:** Use `sh ./releasekit version` in the smoke instructions, or give an
|
|
explicit `chmod +x ./releasekit` step before direct execution. Verify the
|
|
published instructions from a newly created file under a foreign target.
|
|
|
|
## Limit the recovery guarantee to GOOS and GOARCH
|
|
|
|
**Evidence:** The [CLI contract](../cli.md) says Go handoffs select the host
|
|
“independently of inherited cross-compilation settings.” The
|
|
[command formatter](../../cmd/releasekit/main.go) overrides `GOOS` and
|
|
`GOARCH` and retains other build settings. For example, building the actual CLI
|
|
with the native target, `CGO_ENABLED=1`, and an unavailable C compiler named
|
|
by `CC` still fails in `runtime/cgo`. This is consistent with the roadmap's
|
|
scope, but the CLI wording implies broader independence.
|
|
|
|
**Fix:** Say that generated Go handoffs set `GOOS` and `GOARCH` from the
|
|
selected Go toolchain at invocation time. Preserve the other inherited settings
|
|
and the existing workspace and toolchain overrides. The documented handoff
|
|
uses POSIX shell syntax; identify that syntax when telling operators where to
|
|
paste the command.
|
|
|
|
## Exercise a profile target that differs from the launcher host
|
|
|
|
**Evidence:** The binary-delivery case in
|
|
[`TestMultiModuleWoodpeckerDelivery`](../../cmd/releasekit/multi_module_woodpecker_test.go)
|
|
supplies a foreign target to the workflow and checks the uploaded
|
|
`core-v2.1.0-linux-amd64` asset. The profile itself selects `linux/amd64`.
|
|
On the Linux/amd64 review host, the test cannot distinguish using the
|
|
profile target from accidentally using the normalized launcher target. The
|
|
current build code still applies the selected profile target explicitly.
|
|
|
|
**Fix:** In that fixture, select a supported profile target distinct from the
|
|
host and assert the uploaded asset matches it. Where practical, inspect the
|
|
uploaded executable header as well as the name, so the test catches a
|
|
host-built payload with a profile-derived filename. Keep the existing local
|
|
forge fixture and native version check; the cross-built asset need not run on
|
|
the host.
|
|
|
|
## Reconcile the feature roadmap with the completed implementation
|
|
|
|
**Evidence:** The [feature roadmap](host-native-launchers.md) still declares
|
|
“implementation pending” and describes the pre-change launchers as the current
|
|
baseline. All six stages in the [implementation plan](implementation.md) are
|
|
marked complete, and the maintained launchers and recovery code implement the
|
|
host selection described there.
|
|
|
|
**Fix:** Mark the roadmap implemented and label the old baseline as historical,
|
|
or retire the completed planning documents after review. Preserve the planned
|
|
release-note entry for the next selected release version.
|
|
|
|
## Verification
|
|
|
|
`go test ./...`, `go run ./internal/apicheck`, and syntax checks for the
|
|
maintained shell scripts passed. The Go suite exercised real tool preparation,
|
|
Woodpecker command blocks with local fixtures, and later execution of printed
|
|
recovery commands. Disposable smoke probes also verified the maintainer
|
|
launcher forwards the child status and the manual retry preserves the calling
|
|
shell's target variables. The review made no changes to production code or
|
|
tests.
|
|
|
|
## Integration audit
|
|
|
|
The audit below examines how the new bootstrap paths interact with existing
|
|
invocation recipes, product checks, and publication recovery.
|
|
|
|
### Use the external Go executable in printed recovery commands
|
|
|
|
**Evidence:** Before host normalization, the
|
|
[command formatter](../../cmd/releasekit/main.go) emitted
|
|
`env GOWORK=off GOTOOLCHAIN=local go ...`, which resolves Go through `PATH`.
|
|
[Recovery preparation](../../internal/gotool/recovery.go) still executes an
|
|
external Go command. The new printed command uses bare `go` for both host
|
|
queries and the later tool invocation, so a function named `go` in the
|
|
operator's shell can intercept it. In a local rejected-push fixture, the
|
|
actual printed command reconciled the saved record in an ordinary shell.
|
|
With `go() { GOOS=windows GOARCH=arm64 command go "$@"; }` defined, the same
|
|
command failed with an exec format error. The function overrode the final
|
|
assignments even though the host queries reported the correct host. In this
|
|
case both paths exited with status 1, but only one produced a reconciliation
|
|
result.
|
|
|
|
**Fix:** Restore external command lookup for the host queries and tool
|
|
invocation, for example by using `env go -C ...` throughout the subshell.
|
|
Keep selection through the operator's current `PATH`, including executable
|
|
shims, and keep the exact record and exit status. Exercise a generated
|
|
handoff from a POSIX shell that defines a cross-build Go function; assert
|
|
the reconciliation result as well as the status.
|
|
|
|
### Explain how host normalization affects configured product checks
|
|
|
|
**Evidence:** The updated launchers export `GOOS` and `GOARCH` to the
|
|
ReleaseKit process. Existing [configured checks](../config.md) inherit that
|
|
process environment unless their `env` maps override it;
|
|
[`RunChecks`](../../cmd/releasekit/internal/cli/config.go) adds the configured
|
|
and ReleaseKit-owned values to the child. This differs from built-in
|
|
application compilation, which chooses its target from the profile. A
|
|
disposable consumer's check using shell defaults for an application target
|
|
passed with the previous direct `go tool` invocation and no target
|
|
variables, then failed through the updated launcher because it observed
|
|
`linux/amd64`. Adding explicit `GOOS=windows` and `GOARCH=arm64` to that
|
|
check's `env` restored its behavior.
|
|
|
|
**Fix:** In the launcher adoption guidance, explain that host variables
|
|
reach configured child checks and that checks which need an application
|
|
target should set it explicitly in their existing `env` map or command.
|
|
Link to the configuration contract. Keep the current environment overlay
|
|
semantics and the separate profile selection for built-in builds.
|
|
|
|
### Align direct source and standalone CI recipes with the launcher guidance
|
|
|
|
**Evidence:** The [README quick start](../../README.md) and
|
|
[CLI reference](../cli.md) still recommend bare `go run ./cmd/releasekit`
|
|
for source invocation. The [consumer delivery guide](../consumers/delivery.md#woodpecker-tag-workflows)
|
|
also recommends bare `go install ...@PIN` for consumers who install the
|
|
delivery executable separately. These older recipes remain target-sensitive
|
|
alongside the new host-native launchers. On Linux/amd64 with inherited
|
|
`GOOS=windows GOARCH=arm64`, the README's actual `version` command failed
|
|
with an exec format error. A versioned install from a disposable local
|
|
module proxy failed under the same variables with `GOBIN` set; the native
|
|
install succeeded and its binary ran.
|
|
|
|
**Fix:** Link these recipes to the host-selection instructions and make the
|
|
native target requirement explicit at the source invocation and at the
|
|
standalone installation step. Use the existing recipe where a copyable
|
|
command is useful. Keep the selected version, install location, and
|
|
`go run` exit-status caveat. An already installed native binary does not
|
|
need a new bootstrap step.
|