Skip to content

Commit

Permalink
[chore] Exclude generated modules from make for-all (open-telemetry…
Browse files Browse the repository at this point in the history
…#37755)

#### Context

<details>

The `Makefile` currently has two variables listing "every" module in the
repo:
- `ALL_MODS` contains everything except the non-gitted modules
`cmd/otelcontribcol` and `cmd/oteltestbedcol`;
- `NONROOT_MODS` contains everything except the root module, but
including `cmd/otel*col` if present.

These variables are mostly used through the following helpers:
- `make for-all CMD=blabla` loops through `NONROOT_MODS` + (explicitly)
the root module;
- `make for-all-target` loops through `ALL_MODS`.

The result is that the former includes `cmd/otel*col`, while the latter
does not. This discrepancy is confusing, and can cause issues.

For instance, the `make check-contrib` task in Core (surprise, this is
yet another PR indirectly related to
[core#11167](open-telemetry/opentelemetry-collector#11167))
needs to perform the following steps:
1. replace core dependencies in contrib using `make for-all`
2. run `make gotidy`, which uses `make for-all-target` (or used to until
my recent PR, but the `tidylist`-based version still excludes the
generated modules)
3. run `make generate`, which uses `make for-all`
4. run `make gotest`, which uses `make for-all-target`

The discrepancy causes `make generate` to fail because `cmd/otel*col`
was modified to replace core dependencies, but not tidied.

I don't believe there are many instances where a command needs to be run
on all modules, *including* `cmd/otel*col`, so I decided to standardize
on `ALL_MODS` in this PR.
</details>

#### Description

This PR removes the `NONROOT_MODS` variable from the `Makefile`, and
modifies `make for-all` to use `ALL_MODS` instead.

The practical consequence is that `make generate`, `make
otel-from-tree`, and `make otel-from-lib` no longer apply to the
non-gitted modules `cmd/otelcontribcol` and `cmd/oteltestbedcol`. This
matches the existing behavior of all the `make goXXX` targets, reducing
discrepancies.

I added a new module group `GENERATED_MODS`, which contains the two
problematic modules iff they are present on disk. The new `make
for-generated` target can be used along with `make for-all` to return to
the previous behavior. I don't believe there are any scripts in contrib
or core that require this change (since `cmd/otel*col` don't have
`//go:generate` statements).

I also made some miscellaneous improvements to the Makefile:
- removed a definition of `EXPORTER_MODS_0/1` that is overwritten just
below
- fix the addition of the root module in `ALL_MODS` (currently broken
when calling from another directory with `make -C`, see [this
StackOverflow
answer](https://stackoverflow.com/questions/18136918/how-to-get-current-relative-directory-of-your-makefile))
- updated the output of `make all-groups` with missing groups and
macOS-compatible `-e` option

I can move these to another PR if it seems too messy to include these
here.
  • Loading branch information
jade-guiton-dd authored Feb 12, 2025
1 parent 2abc6d3 commit f6fa33a
Showing 1 changed file with 45 additions and 30 deletions.
75 changes: 45 additions & 30 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ EX_INTERNAL=-not -path "./internal/*"
EX_PKG=-not -path "./pkg/*"
EX_CMD=-not -path "./cmd/*"

# NONROOT_MODS includes ./* dirs (excludes . dir)
NONROOT_MODS := $(shell find . $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
# This includes a final slash
ROOT_DIR := $(dir $(realpath $(lastword $(MAKEFILE_LIST))))

RECEIVER_MODS_0 := $(shell find ./receiver/[a-f]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
RECEIVER_MODS_1 := $(shell find ./receiver/[g-o]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
Expand All @@ -33,9 +33,6 @@ RECEIVER_MODS := $(RECEIVER_MODS_0) $(RECEIVER_MODS_1) $(RECEIVER_MODS_2) $(RECE
PROCESSOR_MODS_0 := $(shell find ./processor/[a-o]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
PROCESSOR_MODS_1 := $(shell find ./processor/[p-z]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
PROCESSOR_MODS := $(PROCESSOR_MODS_0) $(PROCESSOR_MODS_1)
EXPORTER_MODS_0 := $(shell find ./exporter/[a-m]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
EXPORTER_MODS_1 := $(shell find ./exporter/[n-z]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
EXPORTER_MODS := $(EXPORTER_MODS_0) $(EXPORTER_MODS_1)
EXPORTER_MODS_0 := $(shell find ./exporter/[a-c]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
EXPORTER_MODS_1 := $(shell find ./exporter/[d-i]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
EXPORTER_MODS_2 := $(shell find ./exporter/[k-o]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
Expand All @@ -48,42 +45,51 @@ PKG_MODS := $(shell find ./pkg/* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
CMD_MODS_0 := $(shell find ./cmd/[a-m]* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) )
CMD_MODS_1 := $(shell find ./cmd/[n-z]* $(FIND_MOD_ARGS) -not -path "./cmd/otel*col/*" -exec $(TO_MOD_DIR) )
CMD_MODS := $(CMD_MODS_0) $(CMD_MODS_1)
OTHER_MODS := $(shell find . $(EX_COMPONENTS) $(EX_INTERNAL) $(EX_PKG) $(EX_CMD) $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) ) $(PWD)
OTHER_MODS := $(shell find . $(EX_COMPONENTS) $(EX_INTERNAL) $(EX_PKG) $(EX_CMD) $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR) ) $(ROOT_DIR)
ALL_MODS := $(RECEIVER_MODS) $(PROCESSOR_MODS) $(EXPORTER_MODS) $(EXTENSION_MODS) $(CONNECTOR_MODS) $(INTERNAL_MODS) $(PKG_MODS) $(CMD_MODS) $(OTHER_MODS)

CGO_MODS := ./receiver/hostmetricsreceiver

FIND_INTEGRATION_TEST_MODS={ find . -type f -name "*integration_test.go" & find . -type f -name "*e2e_test.go" -not -path "./testbed/*"; }
INTEGRATION_MODS := $(shell $(FIND_INTEGRATION_TEST_MODS) | xargs $(TO_MOD_DIR) | uniq)

# Excluded from ALL_MODS
GENERATED_MODS := $(shell find ./cmd/otel*col/* $(FIND_MOD_ARGS) -exec $(TO_MOD_DIR))

ifeq ($(GOOS),windows)
EXTENSION := .exe
endif

.DEFAULT_GOAL := all

all-modules:
@echo $(NONROOT_MODS) | tr ' ' '\n' | sort
@echo $(ALL_MODS) | tr ' ' '\n' | sort

all-groups:
@echo "receiver-0: $(RECEIVER_MODS_0)"
@echo "\nreceiver-1: $(RECEIVER_MODS_1)"
@echo "\nreceiver-2: $(RECEIVER_MODS_2)"
@echo "\nreceiver-3: $(RECEIVER_MODS_3)"
@echo "\nreceiver: $(RECEIVER_MODS)"
@echo "\nprocessor-0: $(PROCESSOR_MODS_0)"
@echo "\nprocessor-1: $(PROCESSOR_MODS_1)"
@echo "\nprocessor: $(PROCESSOR_MODS)"
@echo "\nexporter-0: $(EXPORTER_MODS_0)"
@echo "\nexporter-1: $(EXPORTER_MODS_1)"
@echo "\nexporter-2: $(EXPORTER_MODS_2)"
@echo "\nexporter-3: $(EXPORTER_MODS_3)"
@echo "\nextension: $(EXTENSION_MODS)"
@echo "\nconnector: $(CONNECTOR_MODS)"
@echo "\ninternal: $(INTERNAL_MODS)"
@echo "\npkg: $(PKG_MODS)"
@echo "\ncmd-0: $(CMD_MODS_0)"
@echo "\ncmd-1: $(CMD_MODS_1)"
@echo "\nother: $(OTHER_MODS)"
@echo -e "receiver-0: $(RECEIVER_MODS_0)"
@echo -e "\nreceiver-1: $(RECEIVER_MODS_1)"
@echo -e "\nreceiver-2: $(RECEIVER_MODS_2)"
@echo -e "\nreceiver-3: $(RECEIVER_MODS_3)"
@echo -e "\nreceiver: $(RECEIVER_MODS)"
@echo -e "\nprocessor-0: $(PROCESSOR_MODS_0)"
@echo -e "\nprocessor-1: $(PROCESSOR_MODS_1)"
@echo -e "\nprocessor: $(PROCESSOR_MODS)"
@echo -e "\nexporter-0: $(EXPORTER_MODS_0)"
@echo -e "\nexporter-1: $(EXPORTER_MODS_1)"
@echo -e "\nexporter-2: $(EXPORTER_MODS_2)"
@echo -e "\nexporter-3: $(EXPORTER_MODS_3)"
@echo -e "\nexporter: $(EXPORTER_MODS)"
@echo -e "\nextension: $(EXTENSION_MODS)"
@echo -e "\nconnector: $(CONNECTOR_MODS)"
@echo -e "\ninternal: $(INTERNAL_MODS)"
@echo -e "\npkg: $(PKG_MODS)"
@echo -e "\ncmd-0: $(CMD_MODS_0)"
@echo -e "\ncmd-1: $(CMD_MODS_1)"
@echo -e "\ncmd: $(CMD_MODS)"
@echo -e "\nother: $(OTHER_MODS)"
@echo -e "\nintegration: $(INTEGRATION_MODS)"
@echo -e "\ncgo: $(CGO_MODS)"
@echo -e "\ngenerated: $(GENERATED_MODS)"

.PHONY: all
all: install-tools all-common goporto multimod-verify gotest otelcontribcol
Expand Down Expand Up @@ -187,9 +193,15 @@ goporto: $(PORTO)

.PHONY: for-all
for-all:
@echo "running $${CMD} in root"
@$${CMD}
@set -e; for dir in $(NONROOT_MODS); do \
@set -e; for dir in $(ALL_MODS); do \
(cd "$${dir}" && \
echo "running $${CMD} in $${dir}" && \
$${CMD} ); \
done

.PHONY: for-generated
for-generated:
@set -e; for dir in $(GENERATED_MODS); do \
(cd "$${dir}" && \
echo "running $${CMD} in $${dir}" && \
$${CMD} ); \
Expand Down Expand Up @@ -285,6 +297,9 @@ for-integration-target: $(INTEGRATION_MODS)
.PHONY: for-cgo-target
for-cgo-target: $(CGO_MODS)

.PHONY: for-generated-target
for-generated-target: $(GENERATED_MODS)

# Debugging target, which helps to quickly determine whether for-all-target is working or not.
.PHONY: all-pwd
all-pwd:
Expand Down Expand Up @@ -320,7 +335,7 @@ docker-telemetrygen:

.PHONY: generate
generate: install-tools
PATH="$$PWD/.tools:$$PATH" $(MAKE) for-all CMD="$(GOCMD) generate ./..."
PATH="$(ROOT_DIR).tools:$$PATH" $(MAKE) for-all CMD="$(GOCMD) generate ./..."
$(MAKE) gofmt

.PHONY: gengithub
Expand Down

0 comments on commit f6fa33a

Please sign in to comment.