Skip to content

Commit 8658c56

Browse files
authored
feat: migrate to tools directive (#13844)
#### Description This PR migrates the existing `internal/tools` to the new [recommended approach for managing tool dependencies](https://go.dev/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module). The makefile targets have been updated, including test/build scripts as well. All tools have been migrated to the to the new `go tool` approach, with one exception - `modernize` tool. The reason why `modernize` has been left out is because of [this issue](golang/go#73279). > The issue here is that the modernize command, currently (but not fundamentally) part of the gopls module, is governed by the gopls release process, and you can't just upgrade the x/tools package to a later one, as happens when installing stringer, because the gopls release branch needs a particular version and x/tools does not present a stable internal API to gopls. In short, no-one should ever add gopls as a module dependency, and that means you cannot use go get -tool with gopls. I will update the documentation to make this clear. Please check this issue for full details: - golang/go#73279 Based on the comment above it would appear that even in the [current internal/tools/go.mod](https://github.com/open-telemetry/opentelemetry-collector/blob/f729e7b55a6b9cf703c74f13d9f48ac8b655ced7/internal/tools/go.mod#L22) we have a dependency on `gopls`, which we should not. cc: @mx-psi #### Link to tracking issue Fixes #13721 #### Testing Manual testing and running the various Makefile targets, including `all` target. #### Documentation N/A
1 parent cd46300 commit 8658c56

File tree

10 files changed

+155
-195
lines changed

10 files changed

+155
-195
lines changed

.gitignore

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
bin/
22
dist/
3-
.tools/
43
/local
54

65
# GoLand IDEA
@@ -37,4 +36,3 @@ go.work*
3736
# npm (used for markdown-link-check)
3837
node_modules/*
3938
package-lock.json
40-

Makefile

Lines changed: 52 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,11 @@ ALL_DOC := $(shell find . \( -name "*.md" -o -name "*.yaml" \) \
1313
-type f | sort)
1414

1515
# ALL_MODULES includes ./* dirs (excludes . dir)
16-
ALL_MODULES := $(shell find . -type f -name "go.mod" -exec dirname {} \; | sort | grep -E '^./' )
16+
ALL_MODULES := $(shell find . -mindepth 2 \
17+
-type f \
18+
-name "go.mod" \
19+
-not -path "./internal/tools/*" \
20+
-exec dirname {} \; | sort )
1721

1822
CMD?=
1923

@@ -61,8 +65,8 @@ gotest-with-junit:
6165
@$(MAKE) for-all-target TARGET="test-with-junit"
6266

6367
.PHONY: goporto
64-
goporto: $(PORTO)
65-
$(PORTO) -w --include-internal --skip-dirs "^cmd/mdatagen/third_party$$" ./
68+
goporto:
69+
$(GO_TOOL) porto -w --include-internal --skip-dirs "^cmd/mdatagen/third_party$$" ./
6670

6771
.PHONY: for-all
6872
for-all:
@@ -101,8 +105,8 @@ govulncheck:
101105
@$(MAKE) for-all-target TARGET="vulncheck"
102106

103107
.PHONY: addlicense
104-
addlicense: $(ADDLICENSE)
105-
@ADDLICENSEOUT=`$(ADDLICENSE) -s=only -y "" -c "The OpenTelemetry Authors" $(ALL_SRC) 2>&1`; \
108+
addlicense:
109+
@ADDLICENSEOUT=`$(GO_TOOL) addlicense -s=only -y "" -c "The OpenTelemetry Authors" $(ALL_SRC) 2>&1`; \
106110
if [ "$$ADDLICENSEOUT" ]; then \
107111
echo "$(ADDLICENSE) FAILED => add License errors:\n"; \
108112
echo "$$ADDLICENSEOUT\n"; \
@@ -112,7 +116,7 @@ addlicense: $(ADDLICENSE)
112116
fi
113117

114118
.PHONY: checklicense
115-
checklicense: $(ADDLICENSE)
119+
checklicense:
116120
@licRes=$$(for f in $$(find . -type f \( -iname '*.go' -o -iname '*.sh' \) ! -path '**/third_party/*') ; do \
117121
awk '/Copyright The OpenTelemetry Authors|generated|GENERATED/ && NR<=3 { found=1; next } END { if (!found) print FILENAME }' $$f; \
118122
awk '/SPDX-License-Identifier: Apache-2.0|generated|GENERATED/ && NR<=4 { found=1; next } END { if (!found) print FILENAME }' $$f; \
@@ -123,12 +127,12 @@ checklicense: $(ADDLICENSE)
123127
fi
124128

125129
.PHONY: misspell
126-
misspell: $(MISSPELL)
127-
$(MISSPELL) -error $(ALL_DOC)
130+
misspell:
131+
$(GO_TOOL) misspell -error $(ALL_DOC)
128132

129133
.PHONY: misspell-correction
130-
misspell-correction: $(MISSPELL)
131-
$(MISSPELL) -w $(ALL_DOC)
134+
misspell-correction:
135+
$(GO_TOOL) misspell -w $(ALL_DOC)
132136

133137
.PHONY: run
134138
run: otelcorecol
@@ -159,13 +163,13 @@ otelcorecol:
159163
pushd cmd/otelcorecol && CGO_ENABLED=0 $(GOCMD) build -trimpath -o ../../bin/otelcorecol_$(GOOS)_$(GOARCH) -tags "grpcnotrace" ./... && popd
160164

161165
.PHONY: genotelcorecol
162-
genotelcorecol: install-tools
166+
genotelcorecol:
163167
pushd cmd/builder/ && $(GOCMD) run ./ --skip-compilation --config ../otelcorecol/builder-config.yaml --output-path ../otelcorecol && popd
164168
$(MAKE) -C cmd/otelcorecol fmt
165169

166170
.PHONY: actionlint
167-
actionlint: $(ACTIONLINT)
168-
$(ACTIONLINT) -config-file .github/actionlint.yaml -color .github/workflows/*.yml .github/workflows/*.yaml
171+
actionlint:
172+
$(GO_TOOL) actionlint -config-file .github/actionlint.yaml -color .github/workflows/*.yml .github/workflows/*.yaml
169173

170174
.PHONY: ocb
171175
ocb:
@@ -246,13 +250,10 @@ genproto_sub:
246250
@rm -rf $(OPENTELEMETRY_PROTO_SRC_DIR)/*
247251
@rm -rf $(OPENTELEMETRY_PROTO_SRC_DIR)/.* > /dev/null 2>&1 || true
248252

249-
remove-pdatagen:
250-
rm -f .tools/pdatagen
251-
252253
# Generate structs, functions and tests for pdata package. Must be used after any changes
253254
# to proto and after running `make genproto`
254-
genpdata: remove-pdatagen $(PDATAGEN)
255-
$(PDATAGEN)
255+
genpdata:
256+
cd internal/cmd/pdatagen && $(GOCMD) run main.go -C $(SRC_ROOT)
256257
$(MAKE) -C pdata fmt
257258

258259
INTERNAL_PROTO_SRC_DIRS := exporter/exporterhelper/internal/queue pdata/xpdata/request/internal
@@ -321,17 +322,17 @@ certs-dryrun:
321322
@internal/buildscripts/gen-certs.sh -d
322323

323324
.PHONY: checkapi
324-
checkapi: $(CHECKAPI)
325-
$(CHECKAPI) -folder . -config .checkapi.yaml
325+
checkapi:
326+
$(GO_TOOL) checkapi -folder . -config .checkapi.yaml
326327

327328
# Verify existence of READMEs for components specified as default components in the collector.
328329
.PHONY: checkdoc
329-
checkdoc: $(CHECKFILE)
330-
$(CHECKFILE) --project-path $(CURDIR) --component-rel-path $(COMP_REL_PATH) --module-name $(MOD_NAME) --file-name "README.md"
330+
checkdoc:
331+
$(GO_TOOL) checkfile --project-path $(CURDIR) --component-rel-path $(COMP_REL_PATH) --module-name $(MOD_NAME) --file-name "README.md"
331332

332333
# Construct new API state snapshots
333334
.PHONY: apidiff-build
334-
apidiff-build: $(APIDIFF)
335+
apidiff-build:
335336
@$(foreach pkg,$(ALL_PKGS),$(call exec-command,./internal/buildscripts/gen-apidiff.sh -p $(pkg)))
336337

337338
# If we are running in CI, change input directory
@@ -343,33 +344,33 @@ endif
343344

344345
# Compare API state snapshots
345346
.PHONY: apidiff-compare
346-
apidiff-compare: $(APIDIFF)
347+
apidiff-compare:
347348
@$(foreach pkg,$(ALL_PKGS),$(call exec-command,./internal/buildscripts/compare-apidiff.sh -p $(pkg)))
348349

349350
.PHONY: multimod-verify
350-
multimod-verify: $(MULTIMOD)
351+
multimod-verify:
351352
@echo "Validating versions.yaml"
352-
$(MULTIMOD) verify
353+
$(GO_TOOL) multimod verify
353354

354355
MODSET?=stable
355356
.PHONY: multimod-prerelease
356-
multimod-prerelease: $(MULTIMOD)
357-
$(MULTIMOD) prerelease -s=true -b=false -v ./versions.yaml -m ${MODSET}
357+
multimod-prerelease:
358+
$(GO_TOOL) multimod prerelease -s=true -b=false -v ./versions.yaml -m ${MODSET}
358359
$(MAKE) gotidy
359360

360361
COMMIT?=HEAD
361362
REMOTE?[email protected]:open-telemetry/opentelemetry-collector.git
362363
.PHONY: push-tags
363-
push-tags: $(MULTIMOD)
364-
$(MULTIMOD) verify
365-
set -e; for tag in `$(MULTIMOD) tag -m ${MODSET} -c ${COMMIT} --print-tags | grep -v "Using" `; do \
364+
push-tags:
365+
$(GO_TOOL) multimod verify
366+
set -e; for tag in `$(GO_TOOL) multimod tag -m ${MODSET} -c ${COMMIT} --print-tags | grep -v "Using" `; do \
366367
echo "pushing tag $${tag}"; \
367368
git push ${REMOTE} $${tag}; \
368369
done;
369370

370371
.PHONY: check-changes
371-
check-changes: $(MULTIMOD)
372-
$(MULTIMOD) diff -p $(PREVIOUS_VERSION) -m $(MODSET)
372+
check-changes:
373+
$(GO_TOOL) multimod diff -p $(PREVIOUS_VERSION) -m $(MODSET)
373374

374375
.PHONY: prepare-release
375376
prepare-release:
@@ -429,29 +430,29 @@ checklinks:
429430
# error message "failed to sync logger: sync /dev/stderr: inappropriate ioctl for device"
430431
# is a known issue but does not affect function.
431432
.PHONY: crosslink
432-
crosslink: $(CROSSLINK)
433+
crosslink:
433434
@echo "Executing crosslink"
434-
$(CROSSLINK) --root=$(shell pwd) --prune
435+
$(GO_TOOL) crosslink --root=$(shell pwd) --prune
435436

436437
FILENAME?=$(shell git branch --show-current)
437438
.PHONY: chlog-new
438-
chlog-new: $(CHLOGGEN)
439-
$(CHLOGGEN) new --config $(CHLOGGEN_CONFIG) --filename $(FILENAME)
439+
chlog-new:
440+
$(GO_TOOL) chloggen new --config $(CHLOGGEN_CONFIG) --filename $(FILENAME)
440441

441442
.PHONY: chlog-validate
442-
chlog-validate: $(CHLOGGEN)
443-
$(CHLOGGEN) validate --config $(CHLOGGEN_CONFIG)
443+
chlog-validate:
444+
$(GO_TOOL) chloggen validate --config $(CHLOGGEN_CONFIG)
444445

445446
.PHONY: chlog-preview
446-
chlog-preview: $(CHLOGGEN)
447-
$(CHLOGGEN) update --config $(CHLOGGEN_CONFIG) --dry
447+
chlog-preview:
448+
$(GO_TOOL) chloggen update --config $(CHLOGGEN_CONFIG) --dry
448449

449450
.PHONY: chlog-update
450-
chlog-update: $(CHLOGGEN)
451-
$(CHLOGGEN) update --config $(CHLOGGEN_CONFIG) --version $(VERSION)
451+
chlog-update:
452+
$(GO_TOOL) chloggen update --config $(CHLOGGEN_CONFIG) --version $(VERSION)
452453

453454
.PHONY: builder-integration-test
454-
builder-integration-test: $(ENVSUBST)
455+
builder-integration-test:
455456
cd ./cmd/builder && ./test/test.sh
456457

457458
.PHONY: mdatagen-test
@@ -462,16 +463,16 @@ mdatagen-test:
462463
cd cmd/mdatagen && $(GOCMD) test ./...
463464

464465
.PHONY: generate-gh-issue-templates
465-
generate-gh-issue-templates: $(GITHUBGEN)
466-
$(GITHUBGEN) issue-templates
466+
generate-gh-issue-templates:
467+
$(GO_TOOL) githubgen issue-templates
467468

468469
.PHONY: generate-codeowners
469-
generate-codeowners: $(GITHUBGEN)
470-
$(GITHUBGEN) --default-codeowner "open-telemetry/collector-approvers" codeowners
470+
generate-codeowners:
471+
$(GO_TOOL) githubgen --default-codeowner "open-telemetry/collector-approvers" codeowners
471472

472473
.PHONY: gengithub
473-
gengithub: $(GITHUBGEN) generate-codeowners generate-gh-issue-templates
474+
gengithub: generate-codeowners generate-gh-issue-templates
474475

475476
.PHONY: gendistributions
476-
gendistributions: $(GITHUBGEN)
477-
$(GITHUBGEN) distributions
477+
gendistributions:
478+
$(GO_TOOL) githubgen distributions

Makefile.Common

Lines changed: 37 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -17,101 +17,84 @@ GOTEST_OPT?= -timeout $(GOTEST_TIMEOUT) $(if $(and $(filter windows,$(GOOS)), $(
1717
SRC_ROOT := $(shell git rev-parse --show-toplevel)
1818

1919
TOOLS_MOD_DIR := $(SRC_ROOT)/internal/tools
20-
TOOLS_BIN_DIR := $(SRC_ROOT)/.tools
21-
TOOLS_MOD_REGEX := "\s+_\s+\".*\""
22-
TOOLS_PKG_NAMES := $(shell grep -E $(TOOLS_MOD_REGEX) < $(TOOLS_MOD_DIR)/tools.go | tr -d " _\"" | grep -vE '/v[0-9]+$$')
23-
TOOLS_BIN_NAMES := $(addprefix $(TOOLS_BIN_DIR)/, $(notdir $(shell echo $(TOOLS_PKG_NAMES))))
20+
TOOLS_MOD_FILE := $(TOOLS_MOD_DIR)/go.mod
21+
GO_TOOL := $(GOCMD) tool -modfile $(TOOLS_MOD_FILE)
2422
CHLOGGEN_CONFIG := .chloggen/config.yaml
2523
# no trailing slash
2624
JUNIT_OUT_DIR ?= $(TOOLS_MOD_DIR)/testresults
2725

28-
ACTIONLINT := $(TOOLS_BIN_DIR)/actionlint
29-
ADDLICENSE := $(TOOLS_BIN_DIR)/addlicense
30-
APIDIFF := $(TOOLS_BIN_DIR)/apidiff
31-
CHECKAPI := $(TOOLS_BIN_DIR)/checkapi
32-
CHECKFILE := $(TOOLS_BIN_DIR)/checkfile
33-
CHLOGGEN := $(TOOLS_BIN_DIR)/chloggen
34-
CROSSLINK := $(TOOLS_BIN_DIR)/crosslink
35-
ENVSUBST := $(TOOLS_BIN_DIR)/envsubst
36-
GITHUBGEN := $(TOOLS_BIN_DIR)/githubgen
37-
GOFUMPT := $(TOOLS_BIN_DIR)/gofumpt
38-
GOIMPORTS := $(TOOLS_BIN_DIR)/goimports
39-
GOVULNCHECK := $(TOOLS_BIN_DIR)/govulncheck
40-
LINT := $(TOOLS_BIN_DIR)/golangci-lint
41-
PDATAGEN := $(TOOLS_BIN_DIR)/pdatagen
42-
IMPI := $(TOOLS_BIN_DIR)/impi
43-
MISSPELL := $(TOOLS_BIN_DIR)/misspell
44-
MULTIMOD := $(TOOLS_BIN_DIR)/multimod
45-
PORTO := $(TOOLS_BIN_DIR)/porto
46-
GOTESTSUM := $(TOOLS_BIN_DIR)/gotestsum
47-
MODERNIZE := $(TOOLS_BIN_DIR)/modernize
48-
49-
.PHONY: install-tools
50-
install-tools: $(TOOLS_BIN_NAMES)
51-
52-
$(TOOLS_BIN_DIR):
53-
mkdir -p $@
54-
55-
$(TOOLS_BIN_NAMES): $(TOOLS_BIN_DIR) $(TOOLS_MOD_DIR)/go.mod
56-
cd $(TOOLS_MOD_DIR) && GOOS="" GOARCH="" $(GOCMD) build -o $@ -trimpath $(filter %/$(notdir $@),$(TOOLS_PKG_NAMES))
57-
5826
.PHONY: test
59-
test: $(GOTESTSUM)
60-
$(GOTESTSUM) --packages="./..." -- $(GOTEST_OPT)
27+
test:
28+
$(GO_TOOL) gotestsum --packages="./..." -- $(GOTEST_OPT)
6129

6230
.PHONY: test-with-cover
63-
test-with-cover: $(GOTESTSUM)
31+
test-with-cover:
6432
mkdir -p $(PWD)/coverage/unit
65-
$(GOTESTSUM) --packages="./..." -- $(GOTEST_OPT) -cover -covermode=atomic -coverpkg $(COVER_PKGS) -args -test.gocoverdir="$(PWD)/coverage/unit"
33+
$(GO_TOOL) gotestsum \
34+
--packages="./..." -- \
35+
$(GOTEST_OPT) -cover -covermode=atomic -coverpkg $(COVER_PKGS) -args -test.gocoverdir="$(PWD)/coverage/unit"
6636

6737
.PHONY: test-with-junit
68-
test-with-junit: $(GOTESTSUM)
38+
test-with-junit:
6939
mkdir -p $(JUNIT_OUT_DIR)
70-
$(GOTESTSUM) --packages="./..." --junitfile $(JUNIT_OUT_DIR)/$(CURR_MOD)-junit.xml -- $(GOTEST_OPT) ./...
40+
$(GO_TOOL) gotestsum \
41+
--packages="./..." --junitfile $(JUNIT_OUT_DIR)/$(CURR_MOD)-junit.xml -- \
42+
$(GOTEST_OPT) ./...
7143

7244
.PHONY: benchmark
73-
benchmark: $(GOTESTSUM)
74-
$(GOTESTSUM) --packages="$(ALL_PKGS)" -- -bench=. -run=notests ./... | tee benchmark.txt
45+
benchmark:
46+
$(GO_TOOL) gotestsum \
47+
--packages="$(ALL_PKGS)" -- \
48+
-bench=. -run=notests ./... | tee benchmark.txt
7549

7650
.PHONY: fmt
7751
fmt: common/gofmt common/goimports common/gofumpt
7852

53+
# `modernize' cannot be installed as a Go tool via `go get -tool'.
54+
#
55+
# See [1] for more details.
56+
#
57+
# [1]: https://github.com/golang/go/issues/73279
7958
.PHONY: modernize
80-
modernize: $(MODERNIZE)
81-
$(MODERNIZE) -fix -test -v ./...
59+
modernize:
60+
$(GOCMD) run \
61+
golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest \
62+
-fix -test -v ./...
8263

8364
.PHONY: tidy
8465
tidy:
8566
rm -fr go.sum
8667
$(GOCMD) mod tidy -compat=1.24.0
8768

8869
.PHONY: lint
89-
lint: $(LINT)
90-
$(LINT) run
70+
lint:
71+
$(GO_TOOL) golangci-lint run
9172

9273
.PHONY: common/gofmt
9374
common/gofmt:
9475
gofmt -w -s ./
9576

9677
.PHONY: common/goimports
97-
common/goimports: $(GOIMPORTS)
98-
$(GOIMPORTS) -w -local go.opentelemetry.io/collector ./
78+
common/goimports:
79+
$(GO_TOOL) goimports -w -local go.opentelemetry.io/collector ./
9980

10081
.PHONY: common/gofumpt
101-
common/gofumpt: $(GOFUMPT)
102-
$(GOFUMPT) -l -w -extra .
82+
common/gofumpt:
83+
$(GO_TOOL) gofumpt -l -w -extra .
10384

10485
.PHONY: vulncheck
105-
vulncheck: $(GOVULNCHECK)
106-
$(GOVULNCHECK) ./...
86+
vulncheck:
87+
$(GO_TOOL) govulncheck ./...
10788

10889
.PHONY: generate
10990
generate:
11091
$(GOCMD) generate ./...
11192

11293
.PHONY: impi
113-
impi: $(IMPI)
114-
@$(IMPI) --local go.opentelemetry.io/collector --scheme stdThirdPartyLocal ./...
94+
impi:
95+
$(GO_TOOL) impi \
96+
--local go.opentelemetry.io/collector \
97+
--scheme stdThirdPartyLocal ./...
11598

11699
.PHONY: moddownload
117100
moddownload:

cmd/builder/test/test.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ test_build_config() {
3333
echo "Starting test '${test}' at $(date)" >> "${out}/test.log"
3434

3535
final_build_config=$(basename "${build_config}")
36-
"${WORKSPACE_DIR}/.tools/envsubst" -o "${out}/${final_build_config}" -i <(cat "$build_config" "$replaces")
36+
go tool -modfile "${WORKSPACE_DIR}/internal/tools/go.mod" envsubst \
37+
-o "${out}/${final_build_config}" -i <(cat "$build_config" "$replaces")
3738
if ! go run . --config "${out}/${final_build_config}" --output-path "${out}" > "${out}/builder.log" 2>&1; then
3839
echo "❌ FAIL ${test}. Failed to compile the test ${test}. Build logs:"
3940
cat "${out}/builder.log"

0 commit comments

Comments
 (0)