Record host launcher algorithm audit findings
This commit is contained in:
@@ -146,3 +146,63 @@ standalone installation step. Use the existing recipe where a copyable
|
|||||||
command is useful. Keep the selected version, install location, and
|
command is useful. Keep the selected version, install location, and
|
||||||
`go run` exit-status caveat. An already installed native binary does not
|
`go run` exit-status caveat. An already installed native binary does not
|
||||||
need a new bootstrap step.
|
need a new bootstrap step.
|
||||||
|
|
||||||
|
## Algorithmic audit
|
||||||
|
|
||||||
|
The host normalization itself adds a fixed number of Go invocations per
|
||||||
|
launch. The CLI renders a fixed argument list, so quoting and formatting take
|
||||||
|
time and space linear in the lengths of the directory and record paths. Go
|
||||||
|
dependency inspection and compilation retain their existing roles. The
|
||||||
|
findings below concern a pre-existing algorithm on the audited recovery path
|
||||||
|
and a new module-selection defect in the smoke recipe.
|
||||||
|
|
||||||
|
### Relocate recovery replacements in one pass without merging selectors
|
||||||
|
|
||||||
|
**Evidence:** The API-only path in
|
||||||
|
[`RecoveryTool`](../../internal/gotool/recovery.go) walks the parsed
|
||||||
|
`go.mod` replacements. It calls `modfile.AddReplace` for each relative
|
||||||
|
local replacement. In the pinned `golang.org/x/mod` implementation,
|
||||||
|
each call scans the entire replacement slice. With `R` total directives
|
||||||
|
and `L` relative local directives, relocation visits `O(R × L)` entries,
|
||||||
|
and takes quadratic time when all or most entries require relocation.
|
||||||
|
This loop predates host normalization and still runs before the new host
|
||||||
|
query.
|
||||||
|
|
||||||
|
The same loop can panic on a valid module that declares both an unversioned
|
||||||
|
and a version-specific replacement for the same dependency. When the
|
||||||
|
unversioned directive appears first and uses a relative path, calling
|
||||||
|
`AddReplace` with an empty old version treats both directives as matches
|
||||||
|
and clears the version-specific entry. The outer loop then visits that
|
||||||
|
cleared entry as if it were another relative replacement. In a disposable
|
||||||
|
API-only consumer with both forms for ReleaseKit, Go accepted the module
|
||||||
|
and validation passed. After a local hook rejected the tag push, ReleaseKit
|
||||||
|
saved the publication record, then panicked in `modfile.addReplace`. It
|
||||||
|
exited with status 2 instead of reporting the rejected push and a recovery
|
||||||
|
handoff.
|
||||||
|
|
||||||
|
**Fix:** Rewrite each relative right-hand path on its existing parsed
|
||||||
|
directive in one pass, preserving the exact left-hand path and version
|
||||||
|
selector. For example, update the typed replacement and its corresponding
|
||||||
|
syntax token before formatting. This takes `O(R)` directive visits plus
|
||||||
|
the bytes processed for the paths and avoids selector merging. Keep the
|
||||||
|
existing relocation policy, the separate synthetic replacement for the
|
||||||
|
original module, and the private module lifetime. Add a recovery fixture
|
||||||
|
with distinct local destinations for a wildcard and a version-specific
|
||||||
|
replacement. Check that the handoff preserves both directives, selects the
|
||||||
|
version-specific destination, and reports the original publication status.
|
||||||
|
|
||||||
|
### Anchor smoke-check module paths to the intended checkout
|
||||||
|
|
||||||
|
**Evidence:** The [multi-module smoke recipe](../../examples/multi-module/README.md)
|
||||||
|
replaced `go -C integrations/...` with subshells that run
|
||||||
|
`cd integrations/...` before host discovery. A shell with `CDPATH` set can
|
||||||
|
resolve these relative names in another checkout. In a disposable probe
|
||||||
|
with matching module paths under two directories, the published snippet
|
||||||
|
sent both nested checks to the checkout named by `CDPATH` and returned
|
||||||
|
success. If the other checkout has valid profiles and tools, the smoke
|
||||||
|
result can appear valid for the wrong candidate.
|
||||||
|
|
||||||
|
**Fix:** Use `cd ./integrations/ckconfig` and `cd ./integrations/core/v2`,
|
||||||
|
or clear `CDPATH` for those two directory changes. The number of lookups is
|
||||||
|
already fixed; the change corrects which directory is selected. Verify the
|
||||||
|
copyable recipe with a conflicting `CDPATH` entry.
|
||||||
|
|||||||
Reference in New Issue
Block a user