Stability pass: deterministic dependency order, retry backoff, input validation

Found via manual audit plus a staticcheck run:

- collectDependencyClosure iterated a package's dependencies map
  directly, so which package "won" a same-key collision (and the
  order things got downloaded/unpacked in) could vary between runs
  of the exact same download command. Sort the dependency targets
  first, matching what --print-deps-tree's tree-printer already did.
  Verified two consecutive --print-deps-tree runs now produce
  byte-identical output.
- HTTP retry loops (manifest fetch, payload download) retried
  immediately with no backoff, which just hammers a server harder
  during exactly the kind of transient failure retries exist for.
  Added a capped exponential backoff (1s/2s/4s/8s/10s).
- --architecture/--host-arch accepted any string silently; a typo'd
  value matched nothing during package selection and surfaced as a
  confusing downstream failure far from the actual mistake. Now
  rejected up front with a clear error.
- pumpLines' bufio.Scanner silently stops (dropping the rest of a
  tool's output) if a single line ever exceeds its buffer - narrow but
  real for pathological cases like heavily templated C++ diagnostics.
  Now at least reports that truncation happened instead of losing
  output with no trace.
- Removed select.go's unused off() helper (staticcheck U1000).

Re-verified end-to-end after these changes: a real KMDF driver build
and a plain cl/link build both still succeed.
This commit is contained in:
Cheviiot
2026-07-25 04:14:34 +10:00
parent a1743e4435
commit d11b534fa1
5 changed files with 48 additions and 3 deletions
+14
View File
@@ -45,6 +45,17 @@ func runDownload(args []string) int {
} }
packages := fs.Args() packages := fs.Args()
for _, a := range archsFlag {
if !validArchitectures[a] {
fmt.Fprintf(os.Stderr, "vintner download: invalid --architecture %q (expected one of x86, x64, arm, arm64, host)\n", a)
return 2
}
}
if *hostArch != "" && !validHostArchs[*hostArch] {
fmt.Fprintf(os.Stderr, "vintner download: invalid --host-arch %q (expected one of x86, x64, arm64)\n", *hostArch)
return 2
}
opts := &download.Options{ opts := &download.Options{
Package: packages, Package: packages,
Ignore: []string(ignoreFlag), Ignore: []string(ignoreFlag),
@@ -267,6 +278,9 @@ func printPackageList(headerKey string, pkgs []*download.Package, language strin
} }
} }
var validArchitectures = map[string]bool{"x86": true, "x64": true, "arm": true, "arm64": true, "host": true}
var validHostArchs = map[string]bool{"x86": true, "x64": true, "arm64": true}
func detectHostArch() string { func detectHostArch() string {
if runtime.GOARCH == "arm64" { if runtime.GOARCH == "arm64" {
return "arm64" return "arm64"
+14
View File
@@ -84,6 +84,9 @@ func FetchPayloads(selected []*Package, cacheDir string, allowHashMismatch bool)
func fetchOnePayloadWithRetries(payload Payload, dest, fileID string, allowHashMismatch bool) (int64, error) { func fetchOnePayloadWithRetries(payload Payload, dest, fileID string, allowHashMismatch bool) (int64, error) {
var lastErr error var lastErr error
for attempt := 0; attempt < maxDownloadAttempts; attempt++ { for attempt := 0; attempt < maxDownloadAttempts; attempt++ {
if attempt > 0 {
time.Sleep(retryBackoff(attempt))
}
n, err := tryDownloadPayload(payload, dest, fileID, allowHashMismatch) n, err := tryDownloadPayload(payload, dest, fileID, allowHashMismatch)
if err == nil { if err == nil {
return n, nil return n, nil
@@ -94,6 +97,17 @@ func fetchOnePayloadWithRetries(payload Payload, dest, fileID string, allowHashM
return 0, fmt.Errorf("giving up on %s after %d attempts: %w", fileID, maxDownloadAttempts, lastErr) return 0, fmt.Errorf("giving up on %s after %d attempts: %w", fileID, maxDownloadAttempts, lastErr)
} }
// retryBackoff gives a transient failure (network blip, momentary rate
// limiting) a little room to clear before hammering the same URL again:
// 1s, 2s, 4s, 8s, capped at 10s.
func retryBackoff(attempt int) time.Duration {
d := time.Second << uint(attempt-1)
if d > 10*time.Second {
d = 10 * time.Second
}
return d
}
func tryDownloadPayload(payload Payload, dest, fileID string, allowHashMismatch bool) (int64, error) { func tryDownloadPayload(payload Payload, dest, fileID string, allowHashMismatch bool) (int64, error) {
if fi, err := os.Stat(dest); err == nil && fi.Mode().IsRegular() { if fi, err := os.Stat(dest); err == nil && fi.Mode().IsRegular() {
if payload.SHA256 != "" { if payload.SHA256 != "" {
+3
View File
@@ -198,6 +198,9 @@ const maxManifestAttempts = 5
func httpGet(url string) ([]byte, error) { func httpGet(url string) ([]byte, error) {
var lastErr error var lastErr error
for attempt := 0; attempt < maxManifestAttempts; attempt++ { for attempt := 0; attempt < maxManifestAttempts; attempt++ {
if attempt > 0 {
time.Sleep(retryBackoff(attempt))
}
data, err := tryHTTPGet(url) data, err := tryHTTPGet(url)
if err == nil { if err == nil {
return data, nil return data, nil
+10 -3
View File
@@ -15,8 +15,7 @@ var reSDKVersion = regexp.MustCompile(`^\d+\.\d+\.\d+`)
// default) explicitly chose to include/exclude the component. // default) explicitly chose to include/exclude the component.
type TriState = *bool type TriState = *bool
func on() TriState { v := true; return &v } func on() TriState { v := true; return &v }
func off() TriState { v := false; return &v }
// Options holds every flag that feeds package selection and download. // Options holds every flag that feeds package selection and download.
type Options struct { type Options struct {
@@ -303,6 +302,7 @@ func selectSDK(opts *Options, idx Index) error {
} }
} }
if !found { if !found {
sort.Strings(versions)
return fmt.Errorf("WinSDK version %s not found (available: %s)", opts.SDKVersion, strings.Join(versions, ", ")) return fmt.Errorf("WinSDK version %s not found (available: %s)", opts.SDKVersion, strings.Join(versions, ", "))
} }
} }
@@ -347,7 +347,14 @@ func collectDependencyClosure(idx Index, included map[string]bool, target string
included[key] = true included[key] = true
ret := []*Package{p} ret := []*Package{p}
for target, dep := range p.Dependencies() { deps := p.Dependencies()
targets := make([]string, 0, len(deps))
for target := range deps {
targets = append(targets, target)
}
sort.Strings(targets)
for _, target := range targets {
dep := deps[target]
id := target id := target
if dep.TargetID != "" { if dep.TargetID != "" {
id = dep.TargetID id = dep.TargetID
+7
View File
@@ -310,4 +310,11 @@ func pumpLines(r io.Reader, w *os.File, filter lineFilter) {
} }
fmt.Fprintln(w, line) fmt.Fprintln(w, line)
} }
// bufio.Scanner silently stops (dropping the rest of the stream) once a
// single line exceeds its 16MB buffer - surface that rather than letting
// build output vanish without explanation (heavily templated C++ error
// messages are the realistic way to hit this).
if err := scanner.Err(); err != nil {
fmt.Fprintf(w, "vintner: output truncated: %v\n", err)
}
} }