Build/auto update#3
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a self-update mechanism for the anfra CLI, including a manual update command, background update checks, and an install.sh script. The review feedback highlights several critical issues: a compilation failure on Windows due to a missing detachProcess implementation, fragile manual command parsing in main.go that should be replaced with Cobra's ExecuteC(), a security risk in install.sh where credentials could leak during redirects, a potential script crash if the SHELL environment variable is empty, and opportunities to improve context propagation for cancellation and semver comparison robustness.
| @@ -0,0 +1,14 @@ | |||
| //go:build !windows | |||
There was a problem hiding this comment.
The file is configured with //go:build !windows, but there is no Windows implementation of detachProcess. This will cause compilation to fail on Windows with undefined: detachProcess.
To support Windows compilation (even if Windows builds are not officially released yet), please add a detach_windows.go file with a stub or Windows-specific implementation:
//go:build windows
package main
import "os/exec"
func detachProcess(cmd *exec.Cmd) {
// Stub or Windows-specific process detaching logic
}| func newUpdateCmd() *cobra.Command { | ||
| var checkOnly bool | ||
| cmd := &cobra.Command{ | ||
| Use: "update", | ||
| Short: "Update anfra to the latest release (use --check to only report)", | ||
| RunE: func(_ *cobra.Command, _ []string) error { | ||
| return runUpdate(checkOnly) | ||
| }, | ||
| } | ||
| cmd.Flags().BoolVar(&checkOnly, "check", false, "only check for an update; do not install") | ||
| return cmd | ||
| } |
There was a problem hiding this comment.
Pass the command's context cmd.Context() to runUpdate to support cancellation (e.g., if the user presses Ctrl+C during a large download).
| func newUpdateCmd() *cobra.Command { | |
| var checkOnly bool | |
| cmd := &cobra.Command{ | |
| Use: "update", | |
| Short: "Update anfra to the latest release (use --check to only report)", | |
| RunE: func(_ *cobra.Command, _ []string) error { | |
| return runUpdate(checkOnly) | |
| }, | |
| } | |
| cmd.Flags().BoolVar(&checkOnly, "check", false, "only check for an update; do not install") | |
| return cmd | |
| } | |
| func newUpdateCmd() *cobra.Command { | |
| var checkOnly bool | |
| cmd := &cobra.Command{ | |
| Use: "update", | |
| Short: "Update anfra to the latest release (use --check to only report)", | |
| RunE: func(cmd *cobra.Command, _ []string) error { | |
| return runUpdate(cmd.Context(), checkOnly) | |
| }, | |
| } | |
| cmd.Flags().BoolVar(&checkOnly, "check", false, "only check for an update; do not install") | |
| return cmd | |
| } |
| func runUpdate(checkOnly bool) error { | ||
| // Root at Background (repo convention, see host.go/runServe); timeouts are | ||
| // bounded per-request inside the update package (a quick lookup, then a large | ||
| // download). | ||
| ctx := context.Background() |
There was a problem hiding this comment.
Update runUpdate to accept and use the passed context instead of context.Background().
| func runUpdate(checkOnly bool) error { | |
| // Root at Background (repo convention, see host.go/runServe); timeouts are | |
| // bounded per-request inside the update package (a quick lookup, then a large | |
| // download). | |
| ctx := context.Background() | |
| func runUpdate(ctx context.Context, checkOnly bool) error { | |
| // Root at Background (repo convention, see host.go/runServe); timeouts are | |
| // bounded per-request inside the update package (a quick lookup, then a large | |
| // download). |
Uh oh!
There was an error while loading. Please reload this page.