sha256convert: keep converted repo on post-conversion failure; redact URL parse errors · Entire
sha256convert: keep converted repo on post-conversion failure; redact URL parse errors
d755e2c→main·
nodo·1mo ago·3 files·+116 added/-17 removed
Two fixes from PR review:
Cleanup lifecycle: disarm the target-dir cleanup once the conversion is complete (refs + HEAD written). Previously a failure in an optional post-step -- --write-mapping, origin notes, or --sign-mode tips -- left cleanupTarget armed and os.RemoveAll'd the whole converted repo, silently discarding a (possibly multi-hour) conversion over a path typo or a signing misconfig. This also contradicted the docs, which promise the repo survives a signing failure. Those steps now surface the error but leave the valid converted repo on disk, matching the --check path.
Credential redaction: openSource no longer propagates url.Parse's error verbatim (it embeds the raw URL, leaking https://user:token@host into output/logs/CI). It now surfaces only the underlying *url.Error.Err.
Add a gated regression test asserting the target survives an unwritable --write-mapping path and stays fsck-clean; verified it fails without the cleanup fix.
Docs: correct the stale determinism note (the notes ref uses a pinned SOURCE_DATE_EPOCH / Unix-epoch timestamp, not time.Now(), so the whole conversion is reproducible); document the real HEAD-selection fallback; note signed attestation tags are also excluded side outputs under --check.
Code Changes
- cmd/git-sync/internal/sha256convert
- Msha256convert.go+28/-8
- Msha256convert_test.go+73
- docs
- Mconvert-sha256.md+15/-9
// The converted repo is now complete: every reachable object is
// written, all refs point at translated tips, and HEAD resolves.
// Everything past here — origin notes, the mapping file, signing, and
// --check — is optional enrichment or post-hoc verification. A failure
// in any of those must surface the error but must NOT delete a
// successful conversion: a multi-hour kernel-scale run, a --write-mapping
// path typo, or a signing-key misconfig should never silently discard
// the repo the user just built. Disarm the target cleanup here so those
// steps leave the converted repo on disk for inspection or re-run.
cleanupTarget = false
res.Protocol = refService.Protocol
res.RefsConverted = refsWritten
res.Counts = tr.snapshotCounts()
// more code here...
TestRun_GitHTTPBackend_KeepsTargetOnPostConversionFailure
// Test that a failure in an optional post-conversion step leaves the converted repo on disk
func TestRun_GitHTTPBackend_KeepsTargetOnPostConversionFailure(t *testing.T) {
// Setup for the test...
}
Side Outputs
One-off, not incremental. Each run produces a fresh SHA256 repo from scratch — there is no "fetch the new SHA1 commits and append to the existing SHA256 repo" mode. Realistic use: convert once, then make the converted repo the new canonical store. Branch and tag hashes are deterministic across runs against the same source state; only refs/notes/sha1-origin differs because its wrapper commit carries time.Now() as the committer timestamp. The conversion is fully deterministic: branch hashes, tag hashes, and the refs/notes/sha1-origin ref are all identical across runs against the same source state. The notes wrapper commit's timestamp is pinned to the Unix epoch — or to SOURCE_DATE_EPOCH when that environment variable is set — rather than time.Now(), so even the notes ref reproduces byte-for-byte.
Loose-object storage. Every translated object is written as a loose file under objects/<aa>/<rest> — no pack file is produced.
2. **HEAD** — resolves to a non-zero hash and that object exists in the store.
3. **refs** — every written ref resolves to an object in the store. Side outputs this run created — `refs/notes/sha1-origin` and any `--sign-mode tips` attestation tags — are counted separately, so the reported total matches `RefsConverted`.
4. **git fsck --full** — the external `git` binary runs a full integrity check. Skipped (and reported as such) when `git` isn't on `PATH`; the conversion still succeeds.