Skip to content

Commit 693a701

Browse files
pjanottiChrsMark
authored andcommitted
[chore] Fix make generate on Windows (open-telemetry#43509)
Fix the `generate` target so developers using Windows can do `make generate` for their components on Windows. Main things in this change: 1. Use of path that can be pre-pended to `PATH`, the one being used contains `:` and even escaping it is not enough to get it to correctly work when it is being included in `PATH` 2. Ensure that on Windows all tools used by `go generate` have the `exe` extension. This is needed because the `cmd` package used by `go generate` checks for the known executable extensions on Windows. Notice that `make` and Git Bash don't require that and same extension is not needed for the other tools. 3. The `generate` target now triggers building the tools it needs as various other targets so it can avoid the full `install-tools`. Future work: - `go.*` targets are really slow on Windows since they trigger the re-evaluation of all shell commands that are really expensive on Windows. Basically any recursive make call from `for-all` is currently too expensive on Windows. - Move the definition of components custom tools to the respective components, possibly using the new go.mod `tool` directive. For now I opted to keep the tool used by `githubreceiver` as it is.
1 parent 1e32024 commit 693a701

File tree

2 files changed

+27
-4
lines changed

2 files changed

+27
-4
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,7 @@ goporto: $(PORTO)
222222

223223
.PHONY: for-all
224224
for-all:
225-
@set -e; for dir in $(ALL_MODS); do \
225+
@set -e; for dir in $${ALL_MODS}; do \
226226
(cd "$${dir}" && \
227227
echo "running $${CMD} in $${dir}" && \
228228
$${CMD} ); \

Makefile.Common

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,11 @@ GOTESTSUM := $(TOOLS_BIN_DIR)/gotestsum
9090
CHECKAPI := $(TOOLS_BIN_DIR)/checkapi
9191
MODERNIZE := $(TOOLS_BIN_DIR)/modernize
9292

93+
# Tools used by go generate commands
94+
MDATAGEN := $(TOOLS_BIN_DIR)/mdatagen
95+
# The githubreceiver uses a "go generate" tool that is not used by other components.
96+
GENQLIENT := $(TOOLS_BIN_DIR)/genqlient
97+
9398
GOTESTSUM_OPT?= --rerun-fails=1
9499

95100
# BUILD_TYPE should be one of (dev, release).
@@ -244,13 +249,31 @@ fmt: $(GOFUMPT) $(GOIMPORTS)
244249
$(GOFUMPT) -l -w -extra .
245250
$(GOIMPORTS) -w -local github.com/open-telemetry/opentelemetry-collector-contrib ./
246251

252+
.PHONY: generate-tools
253+
generate-tools: $(MDATAGEN) $(GENQLIENT)
254+
ifeq ($(GOOS),windows)
255+
@echo Adding "exe" extensions to generate tools
256+
@for f in $^; do \
257+
cp -u "$$f" "$$f.exe"; \
258+
done
259+
endif
260+
261+
# On Windows changing path with absolute path includes ':' even escaping it is not enough to get it to correctly
262+
# update PATH, so use a relative path when pre-pending to PATH on Windows. Unfortunately, realpath is not portable
263+
# between mac and GNU based bash so keeping the full path on non-Windows OSes.
264+
ifeq ($(GOOS),windows)
265+
TOOLS_BIN_DIR_PORTABLE?=$(shell realpath --relative-to="$(CURDIR)" "$(TOOLS_BIN_DIR)")
266+
else
267+
TOOLS_BIN_DIR_PORTABLE?=$(TOOLS_BIN_DIR)
268+
endif
269+
247270
.PHONY: generate
248-
generate: install-tools
271+
generate: generate-tools
249272
ifeq ($(CURDIR),$(SRC_ROOT))
250-
PATH="$(TOOLS_BIN_DIR):$$PATH" $(MAKE) for-all CMD="$(GOCMD) generate ./..."
273+
PATH="$(TOOLS_BIN_DIR_PORTABLE):$$PATH" $(MAKE) for-all CMD="$(GOCMD) generate ./..."
251274
$(MAKE) gofmt
252275
else
253-
PATH="$(TOOLS_BIN_DIR):$$PATH" $(GOCMD) generate ./...
276+
PATH="$(TOOLS_BIN_DIR_PORTABLE):$$PATH" $(GOCMD) generate ./...
254277
$(MAKE) fmt
255278
endif
256279

0 commit comments

Comments
 (0)