Skip to content

chore: stop vendoring dependencies (MAPCO-11521) - #38

Open
NivGreenstein wants to merge 3 commits into
developmentfrom
chore/drop-vendor
Open

NivGreenstein wants to merge 3 commits into
developmentfrom
chore/drop-vendor

Conversation

@NivGreenstein

@NivGreenstein NivGreenstein commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Jira: MAPCO-11521 · Docs PR: MapColonies/shigola-docs#19

What and why

vendor/ was 88% of the tracked files and 72% of the bytes. The fork inherited it from Tegola and kept it only while it tracked Tegola. It bought availability, not integrity, because go.sum already pins every module. Meanwhile it cost a formatting gate that had to carve it out, and a gofmt -s -w . at the root that rewrote 257 vendored files.

Dependencies now resolve from the module cache and are checked against go.sum.

  • 7caa4403 removes -mod vendor from the workflows and docs first. The flag was redundant: with vendor/modules.txt present, Go selects vendor mode on its own. So every build still works through the next commit.
  • 0cbfd0e8 removes vendor/ and adds /vendor/ to .gitignore. The Dockerfile now downloads modules (go mod download) in a layer of its own, before copying the source. The gofmt gate becomes git ls-files -z '*.go' | xargs -0 gofmt -s -l. A new "Dependencies" section in CONTRIBUTING.md records the decision and gives the offline-build recipe (seeded GOMODCACHE + GOPROXY=off, or an internal proxy) so nobody reinstates vendor/ by reflex.
  • df6d61d7 adds vendor to .dockerignore, a gap review found. go build switches to vendor mode whenever vendor/modules.txt exists, so without this entry a local go mod vendor would silently become what the image builds from. It also fixes prose the removal left stale.

Base is development, not master: the current work lives there and feature branches are cut from it.

Two things the ticket had wrong

  • The Docker build does need network now. The ticket says the container build "is not hermetic today — the Dockerfile runs apk add build-base". That stopped being true when the build went CGO_ENABLED=0, so before this PR the build stage was offline-capable. After it, go mod download is the build stage's one network dependency. CI and the release builds are networked, so nothing breaks, but it is a real change for anyone building air-gapped.
  • There is no Amazon Linux build action to change. It was already removed, and the Lambda jobs build natively.

Verification

All in golang:1.26.6 containers:

  • go mod download, go mod verify and go mod tidy (no diff).
  • go build ./..., plus CGO_ENABLED=0 build and test-link.
  • go vet with both tag sets. The gofmt gate is clean.
  • Cross builds: Lambda amd64/arm64 (-tags lambda.norpc), darwin/arm64 and windows.
  • docker build produces an image, and shigola version runs from it. This also passes with a bogus local vendor/modules.txt present.
  • GOPROXY=off go build from a seeded cache (the documented offline recipe).
  • go install github.com/mattn/goveralls, which the coverage upload uses.
  • Full go test -race ./... on linux/amd64 with RUN_POSTGIS_TESTS and RUN_REDIS_TESTS on: green.

CI has not run yet, so this PR's first run is the check for the release and CITE workflows.

Left out

  • The -mod vendor / -mod=vendor strings in internal/build/*_test.go. They are arbitrary test inputs, not build assumptions.
  • Tegola attribution in LICENSE.md and NOTICE.md is untouched. Only the paragraph that pointed at vendor/ was reworded.

🤖 Generated with Claude Code

NivGreenstein and others added 3 commits September 23, 2026 21:21
The flag is redundant while vendor/ is committed: with vendor/modules.txt
present and a go directive of 1.14 or later, the go command selects vendor
mode on its own. Dropping it first keeps every build path working through
the removal of vendor/ that follows, rather than breaking them in the same
commit (MAPCO-11521).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
vendor/ was 88% of the tracked files and 72% of the bytes, inherited from
Tegola and kept only while the fork tracked it. It bought availability,
not integrity -- go.sum already pins every module -- and it cost a
formatting gate that had to carve it out and a `gofmt -s -w .` that
rewrote 257 vendored files.

Dependencies now resolve from the module cache against go.sum, and
/vendor/ is ignored so a local `go mod vendor` cannot creep back in. The
Dockerfile downloads modules in a layer of its own before copying source,
so a source-only change reuses them. That download is now the container
build's one network dependency; CONTRIBUTING.md records the decision and
how to build offline from a seeded module cache or an internal proxy
instead of reinstating vendor/ (MAPCO-11521).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A local `go mod vendor` is ignored by git but was still sent to the
daemon, and with vendor/modules.txt present go build switches to vendor
mode on its own -- the image would have built from that local copy
rather than the go.sum-verified download. Exclude it, and say so in
CONTRIBUTING.md for local builds.

Also fixes prose the removal left stale: the .dockerignore header, the
"vendored SDK" comment in cache/s3, the LICENSE/NOTICE wording, and the
pipefail rationale the gofmt gate comment had dropped (MAPCO-11521).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 0

Coverage remained the same at 56.828%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: Could not be determined — this PR's diff is too large for GitHub to return (406 error at GitHub).
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 10611
Covered Lines: 6030
Line Coverage: 56.83%
Coverage Strength: 121.3 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants