fix: keep the module root a single package (tools.go -> go.mod tool directive) - #452
Open
TangoEnSkai wants to merge 1 commit into
Open
fix: keep the module root a single package (tools.go -> go.mod tool directive)#452TangoEnSkai wants to merge 1 commit into
TangoEnSkai wants to merge 1 commit into
Conversation
…irective) tools.go pinned gotestsum with a //go:build tools file that declared `package tools` next to `package dbsql` in the module root. Tooling that resolves a directory's package without honoring build constraints (bazel/gazelle) — or that deliberately enables the `tools` tag (golangci-lint on a project that uses one internally) — then sees two package names in one directory and fails to load the driver. Go 1.24 added the go.mod `tool` directive, which keeps a build-time tool in the module graph without a placeholder package, so the file is no longer needed. `go build -o bin/gotestsum gotest.tools/gotestsum` (the Makefile's bin/gotestsum target) resolves exactly as before, and the pinned version is unchanged at v1.8.2, so no Makefile or CI change is required. Add a tripwire test that loads the module root with the `tools` tag enabled and fails if the directory ever declares more than one package. Signed-off-by: TangoEnSkai <21152231+TangoEnSkai@users.noreply.github.com>
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.
Context
close #162
tools.gopinned thegotestsumtest runner with the pre-Go-1.24 "tools.go" idiom: a//go:build toolsfile declaringpackage toolsin the module root, next topackage dbsql. Two package names in one directory is only invisible to tools that honor build constraints, and two independent reports say downstream tooling does not:golangci-linton a project that uses atoolsbuild tag internally hits a compile error on the driver's root package (Tools.go causes package conflict #162 (comment)).Reproducible from a clean checkout of
main:What
tools.goand pin the runner with the go.modtooldirective (Go 1.24+) instead. The module already declaresgo 1.25.0, so the directive is available.gotestsumat v1.8.2 — this is a mechanism change, not a version bump, so it does not overlap with Bump gotest.tools/gotestsum from 1.8.2 to 1.13.0 #334.tools_test.go, a tripwire that loads the module root throughgo/buildwith thetoolstag enabled and fails if the directory ever declares more than one package again. Verified it fails whentools.gois restored:No Makefile or CI change is needed:
toolkeeps the module in the build list, so thebin/gotestsumtarget'sgo build -o bin/gotestsum gotest.tools/gotestsumresolves exactly as before.The only go.mod churn beyond the directive is
go mod tidymoving thegotest.tools/gotestsumrequire line into the indirect block — that is what Go itself emits for a tool-only dependency (go get -toolon a scratch module produces the same annotation), since no package in the main module imports it.Why
The
tooldirective is the supported replacement for the tools.go idiom and expresses the intent directly ("build-time tool", not "library the driver imports"), so the placeholder package that broke consumers is simply gone rather than renamed or relocated.One honest caveat:
go build -tags tools ./...still reports collisions, but they now come only fromapache/arrow/go/v12, which ships its owntools.go(found packages arrow (array.go) and tools (tools.go)under the module cache). That is upstream and outside this repo's control; this change removes the one collision this module owns, which is the one both reporters hit on the driver's root package.Completion Criteria
go list -tags tools .succeeds (previously failed with the two-package error)go mod tidyis a no-op on the committed go.modmake test— 1097 tests pass, 2 e2e self-skipped (no warehouse credentials)make test-racepasses./bin/golangci-lint run(v2.12.2, as CI pins) — 0 issuesmake toolsstill buildsbin/gotestsumfrom the pinned v1.8.2tools.gocomes back