Skip to content

Commit 9ee240d

Browse files
Fix apidiff CI (#15617)
#### Description Running [the "Inform Incompatible PRs" CI](https://github.com/open-telemetry/opentelemetry-collector/blob/main/.github/workflows/api-compatibility.yml) locally, it seems it only checks for API breakage for the packages inside the root module. This PR rewrites the relevant Makefiles to actually check all non-internal packages inside all stable Collector modules, and updates the CI check to match. #### Testing I briefly tested the `make` commands on #15589, confirming that the expected breaking change is now correctly flagged. #### Authorship - [x] I, a human, wrote this pull request description myself.
1 parent 32a2c6e commit 9ee240d

6 files changed

Lines changed: 43 additions & 16 deletions

File tree

.github/workflows/api-compatibility.yml

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -56,17 +56,15 @@ jobs:
5656
# Compare apidiff states of Main with PR
5757
- name: Compare-States
5858
env:
59-
CI: true
60-
COMPARE_OPTS: -d "../${{ github.base_ref }}/internal/data/apidiff"
59+
APIDIFF_DATA_DIR: "../${{ github.base_ref }}/internal/data/apidiff"
6160
run: |
6261
cd $HEAD_REF
6362
make apidiff-compare
6463
6564
# Fail GitHub Action if there are incompatible changes
6665
- name: Check-States
6766
env:
68-
CI: true
69-
COMPARE_OPTS: -d "../${{ github.base_ref }}/internal/data/apidiff" -c
67+
APIDIFF_DATA_DIR: "../${{ github.base_ref }}/internal/data/apidiff"
7068
run: |
7169
cd $HEAD_REF
72-
make apidiff-compare
70+
make apidiff-check

Makefile

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -270,22 +270,26 @@ checkapi:
270270
checkdoc:
271271
$(GO_TOOL) checkfile --project-path $(CURDIR) --component-rel-path $(COMP_REL_PATH) --module-name $(MOD_NAME) --file-name "README.md"
272272

273+
# Extract the relative path of every module listed between "stable:" and "beta:" in versions.yaml
274+
STABLE_MODULES := $(shell sed -n -e '/stable:/,/beta:/ s/.*- go.opentelemetry.io\/collector/./p' versions.yaml)
275+
276+
.PHONY: for-all-stable-target
277+
for-all-stable-target: $(STABLE_MODULES)
278+
273279
# Construct new API state snapshots
274280
.PHONY: apidiff-build
275281
apidiff-build:
276-
@$(foreach pkg,$(ALL_PKGS),$(call exec-command,./internal/buildscripts/gen-apidiff.sh -p $(pkg)))
277-
278-
# If we are running in CI, change input directory
279-
ifeq ($(CI), true)
280-
APICOMPARE_OPTS=$(COMPARE_OPTS)
281-
else
282-
APICOMPARE_OPTS=-d "./internal/data/apidiff"
283-
endif
282+
@$(MAKE) --silent for-all-stable-target TARGET="apidiff-build-mod"
284283

285-
# Compare API state snapshots
284+
# Compare API state snapshots and log differences
286285
.PHONY: apidiff-compare
287286
apidiff-compare:
288-
@$(foreach pkg,$(ALL_PKGS),$(call exec-command,./internal/buildscripts/compare-apidiff.sh -p $(pkg)))
287+
@$(MAKE) --silent for-all-stable-target TARGET="apidiff-compare-mod"
288+
289+
# Compare API state snapshots and fail if there are differences
290+
.PHONY: apidiff-check
291+
apidiff-check:
292+
@$(MAKE) --silent for-all-stable-target TARGET="apidiff-check-mod"
289293

290294
.PHONY: multimod-verify
291295
multimod-verify:

Makefile.Common

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,3 +110,24 @@ FLAGS ?= '-o=$(CURDIR)/'
110110
schemagen:
111111
@echo "Running schemagen with filepath='$(SRC)' and flags='$(FLAGS)'"
112112
cd $(SRC_ROOT)/cmd/schemagen && go run . $(FLAGS) $(SRC)
113+
114+
EXTERNAL_PKGS := $(shell go list ./... | grep -v -E '/internal($|/)')
115+
APIDIFF_DATA_DIR ?= internal/data/apidiff
116+
117+
.PHONY: apidiff-build-mod
118+
apidiff-build-mod:
119+
@for pkg in $(EXTERNAL_PKGS); do \
120+
$(SRC_ROOT)/internal/buildscripts/gen-apidiff.sh -p $$pkg -o $(SRC_ROOT)/$(APIDIFF_DATA_DIR) || exit 1; \
121+
done
122+
123+
.PHONY: apidiff-compare-mod
124+
apidiff-compare-mod:
125+
@for pkg in $(EXTERNAL_PKGS); do \
126+
$(SRC_ROOT)/internal/buildscripts/compare-apidiff.sh -p $$pkg -d $(SRC_ROOT)/$(APIDIFF_DATA_DIR) || exit 1; \
127+
done
128+
129+
.PHONY: apidiff-check-mod
130+
apidiff-check-mod:
131+
@for pkg in $(EXTERNAL_PKGS); do \
132+
$(SRC_ROOT)/internal/buildscripts/compare-apidiff.sh -p $$pkg -d $(SRC_ROOT)/$(APIDIFF_DATA_DIR) -c || exit 1; \
133+
done

internal/buildscripts/compare-apidiff.sh

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,6 @@ if [ -e "$input_dir"/"$package"/apidiff.state ]; then
5757
echo "$changes"
5858
fi
5959
fi
60+
else
61+
echo "WARNING: No snapshot exists for package $package; either it is newly stable or snapshot creation failed"
6062
fi

internal/buildscripts/gen-apidiff.sh

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ if [ -z "$package" ]; then
4242
usage
4343
fi
4444

45-
set -ex
45+
set -e
4646

4747
# Create temp dir for generated files.
4848
# Source: https://unix.stackexchange.com/a/84980
@@ -67,4 +67,5 @@ if [ $dry_run = false ]; then
6767
mkdir -p "$output_dir/$package" && \
6868
cp "$tmp_dir/$package/apidiff.state" \
6969
"$output_dir/$package"
70+
echo "Wrote snapshot to $output_dir/$package"
7071
fi

internal/data/apidiff/.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
*

0 commit comments

Comments
 (0)