NO-ISSUE: Stop tracking go.work.sum to fix recurring merge conflicts - #165
Conversation
|
@eliorerz: This pull request explicitly references no jira issue. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe repository ignores the generated ChangesWorkspace checksum handling
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.gitignore:
- Around line 28-37: Update the repository’s container build flow alongside the
.gitignore change: ensure each relevant Containerfile generates go.work.sum
before its COPY step or otherwise includes the required file so clean builds
succeed. Keep go.work.sum tracked or generated consistently, and revise the
adjacent .gitignore rationale to remove the claim that module go.sum files fully
replace it and accurately qualify golang/go#53502 as guidance about committing
go.work.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: osac-project/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3c376aed-6c50-42f8-bfb4-c6869506ab94
⛔ Files ignored due to path filters (1)
go.work.sumis excluded by!**/*.sum
📒 Files selected for processing (1)
.gitignore
fa7fe5a to
f602d94
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
f602d94 to
f1e81b8
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
f1e81b8 to
aeb0b7a
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
aeb0b7a to
5d3e73c
Compare
go.work.sum is a derived workspace-level checksum cache, regenerated automatically by go build/test from the repo root. Unlike each module's own go.sum (the real, reproducible dependency lock, which stays committed), it grows non-deterministically depending on what each contributor happens to build locally -- causing near-constant merge conflicts unrelated to any actual code change and blocking merges. Matches GitHub's own default Go .gitignore template and does not affect build reproducibility: nothing in CI reads it directly, no workflow sets -mod=readonly/GOFLAGS that would prevent Go from silently regenerating it, and the only direct 'go build' step (CodeQL analysis) builds per-module against each module's own already-correct go.sum.
The previous commit untracked go.work.sum without checking that 4 of 5 component Containerfiles explicitly COPY it by name (COPY go.work go.work.sum ./) -- on a fresh checkout this file no longer exists, so that COPY step fails outright, breaking every one of those image builds. Fix: drop go.work.sum from the COPY line in each affected Containerfile (fulfillment-service, osac-operator, bare-metal-fulfillment-operator, osac-csi-driver). Each already runs a 'go mod download' step immediately after, which regenerates go.work.sum inside the build using the go.sum files already copied in per module -- verified this actually works via a real podman build from a clean checkout (no GOFLAGS=-mod=readonly or similar restriction anywhere in this repo would block it). Also revised the .gitignore comment: removed the overreaching claim that module go.sum files fully replace go.work.sum, and clarified that golang/go#53502 is about whether to commit go.work specifically, not a settled verdict on go.work.sum.
5d3e73c to
b0f28af
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: eliorerz, omer-vishlitzky The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Why
Almost every recent PR touching Go code has been hitting a merge conflict on
go.work.sum, blocking otherwise-unrelated merges. Root cause:go.work.sumis not a real dependency lock file the waygo.sumis -- it's a derived, workspace-level checksum cache thatgo build/go test/go mod downloadsilently regenerates and grows whenever anyone builds from the repo root. Different contributors build different subsets of the workspace, in different orders, at different times, so their localgo.work.sumdiverges non-deterministically. Any two PRs open at the same time end up touching overlapping lines of the same growing file in incompatible ways -- conflicts that have nothing to do with either PR's actual change.Solution
Stop tracking
go.work.sumand add it to.gitignore.go.workitself stays committed -- it's a plain, repo-relative module list (use ./fulfillment-service ./osac-operator ...), not personal/absolute paths, so there's nothing machine-specific about it. Only the derived checksum cache is being untracked.This matches GitHub's own default Go
.gitignoretemplate, which excludesgo.work/go.work.sumby default for this reason. Note the upstream discussion in golang/go#53502 is specifically about whether to commitgo.work-- it isn't a settled, universal verdict ongo.work.sumon its own, and I'm not claiming every module'sgo.sumis a strict superset of whatgo.work.sumrecords (workspace-mode dependency resolution can in principle select versions that don't appear in any single module's owngo.sum). What actually makes untracking safe here is the Containerfile fix below, not a general dependency-lock equivalence argument.Real bug caught in an earlier version of this PR, now fixed: 4 of 5 component Containerfiles (
fulfillment-service,osac-operator,bare-metal-fulfillment-operator,osac-csi-driver) explicitly didCOPY go.work go.work.sum ./. On a fresh checkoutgo.work.sumno longer exists, so thatCOPYfails outright -- breaking every one of those image builds, not just producing a warning. Fixed by droppinggo.work.sumfrom theCOPYline; each Containerfile already runs ago mod downloadstep immediately after (needed regardless, to fetch the module's actual dependencies), which regeneratesgo.work.suminside the build using thego.sumfiles already copied in per module.Verified, not just reasoned about:
go.work.sumby name or content.-mod=readonly/GOFLAGSthat would prevent Go from silently regenerating it.osac-csi-driver's image for real withpodman build, from a cleangit archivecheckout (sogo.work.sumwas genuinely absent, matching what CI sees) -- succeeded end-to-end, confirminggo mod downloadcorrectly regenerates it inside the build.What changed
.gitignore: added/go.work.sumwith an explanatory comment.go.work.sumfrom version control (git rm --cached).fulfillment-service/Containerfile,osac-operator/Containerfile,bare-metal-fulfillment-operator/Containerfile,osac-csi-driver/Containerfile: droppedgo.work.sumfrom theCOPY go.work go.work.sum ./line (keptgo.work), each with a comment explaining why.osac-metering/metering-service/Containerfileneeded no change -- it doesn't referencego.work/go.work.sumat all (standalone module, not part of the shared workspace build).Summary by CodeRabbit