Repository navigation
feat(sources): Reproducible Golang Vendor Packaging - #424
Open
Antonio Salinas (Tonisal-byte) wants to merge 5 commits into
Open
Antonio Salinas (Tonisal-byte) wants to merge 5 commits into
Antonio Salinas (Tonisal-byte) wants to merge 5 commits into
Conversation
Preserve upstream packaging while converting supported Go application specs and generating vendor archives from prepared sources. Wire configuration, history, documentation, and regression coverage across preparation, builds, rendering, and source diffs. Validation: ./magefile.go check all passed, including Python lint and type checks. Build and unit phases passed. Full ./magefile.go all was attempted via Podman, but Docker-oriented scenario file-mount and network-isolation expectations fail in this environment; Docker WSL integration is unavailable. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Preserve upstream Source entries, tool configuration, and reviewed licenses when regenerating already-vendored packages. Use a common spec plan and reusable Go-only generation backend. Keep command construction in Go, automatically clone component configuration while retaining shared back-references, and exclude payloads from rendered specs. Add regression coverage and document both workflows. Validation: unit tests, build, and all quality checks passed. All ten prepared containerd files are byte-for-byte identical to the pre-refactor baseline. Full Mage scenarios remain blocked by Docker provider initialization in this WSL environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the inline tool-config option with config-file, resolved relative to its declaring component configuration. Explicit files replace upstream settings, including empty files, and missing or invalid files fail without fallback. Hash file contents rather than checkout paths for lock fingerprints and lock-free change detection. Preserve upstream settings when no file is selected, and update the schema, documentation, emitted guidance, and regression coverage. Validation: unit tests and all quality checks passed. The changed-code mutation audit killed all 16 mutations. Containerd source generation with a separate vendor config reproduced the prior outputs, and a file-only edit invalidated its isolated lock fingerprint. Docker scenario initialization remains unavailable in this environment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical issues allow host-file disclosure through symlinks and can produce incorrect archive replacements, mismatched source filenames and checksums, and duplicate patch application.
8 open issues
Review effort: Balanced
Address the eight unique findings in PR #424: reject source-file collisions, pin expanded archive names, snapshot regular chroot outputs without following symlinks, preserve module-aware checks, extract generated metadata after prep edits, reject ambiguous extraction layouts, honor native module subdirectories, and run archive hooks from staged dist-git. Add regression coverage and update user documentation and emitted agent guidance. Unit and quality checks pass; changed-code mutation audit kills all 37 mutations. The containerd update/prepare/render/build flow and isolated RPM daemon smoke test pass, including staged-sidecar hook checks. Ran ./magefile.go scenarioUpdate before pushing; the schema snapshot is current. Full Docker-dependent scenarios remain blocked by the unavailable Docker provider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical defects can break history-dependent rpmautospec processing, misplace module-specific vendor and license paths, and leave replacement configurations with stale manifest checksums and subject to deletion during rendering.
3 open issues
16 resolved since last review
Review effort: Balanced
Comment on lines
+148
to
+150
| for name, contents := range map[string][]byte{specName: finalSpec, plan.ConfigName: []byte(toolConfig)} { | ||
| if err := fileutils.WriteFile(p.fs, filepath.Join(outputDir, name), contents, fileperms.PublicFile); err != nil { | ||
| return fmt.Errorf("publishing Go vendor file %#q:\n%w", name, err) |
| "cd " + sourceMount, | ||
| "mkdir -p " + topDir + "/BUILD", | ||
| quote("git", "config", "--global", "--add", "safe.directory", sourceMount), | ||
| quote("rpmautospec", "process-distgit", request.SpecName, request.SpecName), |
Comment on lines
+69
to
+70
| if configSource != "" { | ||
| return prepareExistingGoVendor(tree, configSource) |
The Go vendor configuration intentionally extends the schema, but scenarioUpdate refreshed only the local baseline when Docker was unavailable. CI reached the container schema check and failed on the missing 37 lines. Update the container baseline to match the actual generated CLI schema and document that incomplete Docker runs can leave this second snapshot stale. Both stdout snapshots and the checked-in schema match the rebuilt CLI output byte-for-byte. Local scenario/scenarioUpdate runs remain blocked by the unavailable Docker provider. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Critical issues in module-directory handling, shared configuration isolation, and Git-history preservation can break RPM builds, leak settings between components, and fail preparation for automatic-release specs.
4 open issues
Review effort: Balanced
Comment on lines
+499
to
+500
| if other.GoVendor != nil && other.GoVendor.Enabled != nil { | ||
| config := *c.GoVendor |
Antonio Salinas (Tonisal-byte)
marked this pull request as ready for review
November 2, 2026 23:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds per-component Go vendoring while preserving upstream packaging. Custom settings live in a separate TOML file, whose contents are included in change detection.
Configuration
The file replaces upstream settings without merging. Omit
config-fileto reuse upstream settings when available. Missing or invalid configured files fail explicitly.Workflow
prepare-sourcesruns%prepand generates the vendor archive—not a full RPM build.