Skip to content

Commit 9fb66f1

Browse files
authored
ci: auto-prune stale suppression entries on baseline bump (#352)
1 parent 258e449 commit 9fb66f1

2 files changed

Lines changed: 307 additions & 5 deletions

File tree

.github/workflows/release.yml

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,21 @@ jobs:
126126
runs-on: ubuntu-latest
127127
# Budgeted for the deliberate nuget.org indexing wait below (up to ~20
128128
# minutes of polling plus a 60s grace sleep) on top of the PR open/
129-
# approve/merge steps, not just the promotion script itself.
130-
timeout-minutes: 35
129+
# approve/merge steps, plus ~10 min headroom for the suppression refresh
130+
# step's own `dotnet pack` per suppression-owning project.
131+
timeout-minutes: 45
131132
permissions:
132133
contents: write # create the promotion branch
133134
pull-requests: write # open the PR
134135
steps:
135136
- uses: actions/checkout@v7
137+
with:
138+
# Full history: the suppression refresh below runs `dotnet pack`,
139+
# and Nerdbank.GitVersioning's shallow-clone warning would become a
140+
# build failure under this repo's repo-wide TreatWarningsAsErrors.
141+
fetch-depth: 0
142+
143+
- uses: ./.github/actions/setup-build
136144

137145
- name: Promote unshipped public API
138146
run: python3 eng/mark_api_shipped.py --version ${{ needs.release.outputs.version }}
@@ -172,6 +180,16 @@ jobs:
172180
echo "::error::nuget.org did not index $PACKAGE_ID $VERSION within ~20 minutes. This PR's build will fail with NU1102 until indexing completes -- re-run the failed check once nuget.org catches up."
173181
exit 1
174182
183+
# Runs after the baseline bump above and after nuget.org has indexed the
184+
# new baseline version (regeneration below restores that baseline
185+
# package), so its changes land in the same commit as the rest of this
186+
# PR. Prune-only: this only ever removes entries that ApiCompat no
187+
# longer needs against the new baseline; if regenerating would ADD an
188+
# entry anywhere, it fails the job instead of writing anything, since
189+
# that means a real API break needs a human suppression decision.
190+
- name: Refresh compatibility suppressions for the new baseline
191+
run: python3 eng/refresh_compat_suppressions.py
192+
175193
- name: Open public API promotion PR
176194
id: cpr
177195
uses: peter-evans/create-pull-request@5f6978faf089d4d20b00c7766989d076bb2fc7f1 # v8
@@ -182,6 +200,7 @@ jobs:
182200
token: ${{ secrets.AUTOMATION_TOKEN }}
183201
add-paths: |
184202
src/**/PublicAPI.*.txt
203+
src/**/CompatibilitySuppressions.xml
185204
Directory.Build.props
186205
branch: chore/mark-api-shipped-${{ needs.release.outputs.version }}
187206
delete-branch: true
@@ -193,6 +212,8 @@ jobs:
193212
194213
Each `PublicAPI.Unshipped.txt` was folded into its sibling `PublicAPI.Shipped.txt` and reset to the `#nullable enable` header. Merge this so the next development cycle starts from a clean unshipped surface and package validation compares against the release that just shipped.
195214
215+
Any `CompatibilitySuppressions.xml` entry the new baseline no longer needs was pruned (or the file removed, if none remain) in the same commit.
216+
196217
# Hands-off promotion, mirroring the dependabot auto-approve-and-merge flow.
197218
# Approve with GITHUB_TOKEN (the github-actions[bot] identity) so the review
198219
# comes from someone other than the AUTOMATION_TOKEN that opened the PR --
@@ -228,15 +249,20 @@ jobs:
228249
runs-on: ubuntu-latest
229250
# Same deliberate nuget.org indexing wait as mark-api-shipped's gate when
230251
# a bump is actually needed; skipped output is fast, but this must budget
231-
# for the slow path (see that job's comment for the exact math).
232-
timeout-minutes: 35
252+
# for the slow path (see that job's comment for the exact math), plus the
253+
# same suppression-refresh `dotnet pack` headroom when a bump does happen.
254+
timeout-minutes: 45
233255
permissions:
234256
contents: write # create the propagation branch
235257
pull-requests: write # open the PR
236258
steps:
237259
- uses: actions/checkout@v7
238260
with:
239261
ref: main
262+
# Full history: see mark-api-shipped's checkout comment -- the
263+
# suppression refresh below runs `dotnet pack`, which needs it for
264+
# Nerdbank.GitVersioning under this repo's TreatWarningsAsErrors.
265+
fetch-depth: 0
240266

241267
- name: Compute propagated baseline
242268
id: propagate
@@ -276,6 +302,19 @@ jobs:
276302
echo "::error::nuget.org did not index $PACKAGE_ID $VERSION within ~20 minutes. This PR's build will fail with NU1102 until indexing completes -- re-run the failed check once nuget.org catches up."
277303
exit 1
278304
305+
# Only set up the .NET SDK / NuGet cache when there's actually a bump to
306+
# refresh suppressions against -- skipped along with everything below
307+
# when propagate_baseline.py found nothing to do.
308+
- if: steps.propagate.outputs.bumped == 'true'
309+
uses: ./.github/actions/setup-build
310+
311+
# See mark-api-shipped's identically-named step: same script, same
312+
# prune-only contract. Gated the same as everything else in this job so
313+
# the refresh never runs when the bump itself was skipped.
314+
- name: Refresh compatibility suppressions for the new baseline
315+
if: steps.propagate.outputs.bumped == 'true'
316+
run: python3 eng/refresh_compat_suppressions.py
317+
279318
- name: Open baseline propagation PR
280319
id: cpr
281320
if: steps.propagate.outputs.bumped == 'true'
@@ -285,7 +324,9 @@ jobs:
285324
# the default GITHUB_TOKEN, pushes from a workflow do not trigger CI.
286325
# Enforced by the "Require automation token" step above -- no fallback.
287326
token: ${{ secrets.AUTOMATION_TOKEN }}
288-
add-paths: Directory.Build.props
327+
add-paths: |
328+
Directory.Build.props
329+
src/**/CompatibilitySuppressions.xml
289330
branch: chore/baseline-${{ needs.release.outputs.version }}-to-main
290331
delete-branch: true
291332
commit-message: "chore: validate packages against the ${{ needs.release.outputs.version }} baseline"
@@ -294,6 +335,8 @@ jobs:
294335
body: |
295336
**${{ github.ref_name }}** released **v${{ needs.release.outputs.version }}**. This raises main's `PackageValidationBaselineVersion` to match, so main keeps validating its packages against the newest release it must stay compatible with.
296337
338+
Any `CompatibilitySuppressions.xml` entry the new baseline no longer needs was pruned (or the file removed, if none remain) in the same commit.
339+
297340
- name: Approve the propagation PR
298341
if: steps.cpr.outputs.pull-request-number
299342
run: gh pr review --approve "${{ steps.cpr.outputs.pull-request-number }}"

eng/refresh_compat_suppressions.py

Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
#!/usr/bin/env python3
2+
"""Prune-only refresh of CompatibilitySuppressions.xml against the current baseline.
3+
4+
Package validation (APICompat) diffs each package against two things: the
5+
published nupkg pinned by Directory.Build.props' PackageValidationBaselineVersion,
6+
and (for multi-targeted packages) the package's own net9.0 vs. net10.0 surface.
7+
Both kinds of findings that a maintainer has accepted live side by side in the
8+
same per-project CompatibilitySuppressions.xml, distinguished only by whether
9+
IsBaselineSuppression is true.
10+
11+
Every time PackageValidationBaselineVersion is bumped -- see
12+
eng/mark_api_shipped.py and eng/propagate_baseline.py, both of which bump it
13+
in a working tree they are about to open a PR from -- some baseline-suppressed
14+
entries can go stale: the API they suppressed a diagnostic for is no longer
15+
different from the new baseline. On this SDK, an unnecessary suppression is
16+
not a warning, it is a hard `dotnet pack` error ("Unnecessary suppressions
17+
found"). Left alone, that error would surface on the *next* release's pack,
18+
attributed to whatever innocent commit happened to trigger it. This script
19+
fixes that by pruning stale entries in the same commit that bumps the
20+
baseline, right after the working tree already reflects the new version.
21+
22+
Safety property (non-negotiable): this refresh may only REMOVE suppression
23+
entries, never add one. `-p:ApiCompatGenerateSuppressionFile=true` regenerates
24+
a project's suppression file from scratch -- it prunes entries no longer
25+
needed, but it will just as happily ADD an entry for a genuine, previously
26+
unsuppressed API break (e.g. a real regression that landed after the release
27+
this baseline now points at). Blindly writing that regenerated file back would
28+
silently bless a breaking change. So every project's regenerated entry set is
29+
compared against its current file first; only if the regenerated set is a
30+
subset of the current one (pure removal) is anything written. If regeneration
31+
would add even one entry anywhere, NOTHING is written and the script fails
32+
loudly -- that case needs a human suppression decision.
33+
34+
Mechanics, verified empirically against the 10.0.302 SDK:
35+
- `ApiCompatSuppressionOutputFile` (Microsoft.NET.ApiCompat.Common.targets)
36+
redirects where a regenerated file is WRITTEN, independently of
37+
`ApiCompatSuppressionFile`, which controls what is READ for comparison and
38+
still defaults to the project's own CompatibilitySuppressions.xml. Passing
39+
just `-p:ApiCompatSuppressionOutputFile=<scratch path>` therefore
40+
regenerates against the real, on-disk suppression file while never writing
41+
to it -- the original is untouched unless and until this script itself
42+
decides to copy a vetted result over it. This is used instead of a
43+
snapshot/regenerate-in-place/restore dance: with a redirect there is
44+
nothing to restore, because nothing real was ever written to in the first
45+
place. (A single global override can't be used for a solution-wide pack --
46+
every project would collide on the same output path -- so each project
47+
with a suppression file is packed individually, which also keeps this
48+
targeted rather than paying for a full-solution pack.)
49+
- When the regenerated set is empty, the tool still writes a file, just with
50+
a self-closing `<Suppressions ... />` root and no children.
51+
- Each project's own obj/ is deleted before its regeneration pack.
52+
RunPackageValidation is an incremental MSBuild target keyed off a semaphore
53+
file under obj/; without this, a second pack in the same tree can skip
54+
revalidation entirely and silently report the prior result.
55+
56+
Cost of a rewrite (only paid on a real change, never on a no-op): the
57+
regenerated file the SDK writes carries only its own boilerplate header
58+
comment, not whatever hand-written justification comment a maintainer had
59+
added above the suppression list. That context is lost from the file on a
60+
prune and needs to live in the PR description or commit history instead.
61+
"""
62+
import argparse
63+
import glob
64+
import os
65+
import shutil
66+
import subprocess
67+
import sys
68+
import tempfile
69+
import xml.etree.ElementTree as ET
70+
71+
SUPPRESSIONS_FILENAME = "CompatibilitySuppressions.xml"
72+
73+
74+
def discover_suppression_files(root):
75+
pattern = os.path.join(root, "**", SUPPRESSIONS_FILENAME)
76+
return sorted(glob.glob(pattern, recursive=True))
77+
78+
79+
def find_project_file(project_dir):
80+
candidates = sorted(glob.glob(os.path.join(project_dir, "*.csproj")))
81+
if len(candidates) != 1:
82+
raise RuntimeError(
83+
f"{project_dir}: expected exactly one .csproj next to {SUPPRESSIONS_FILENAME}, "
84+
f"found {len(candidates)}."
85+
)
86+
return candidates[0]
87+
88+
89+
def _element_text(suppression, tag):
90+
element = suppression.find(tag)
91+
return element.text.strip() if element is not None and element.text else ""
92+
93+
94+
def parse_entries(path):
95+
"""Parses a suppression file into a set of (DiagnosticId, Target, Left,
96+
Right, IsBaselineSuppression) tuples -- the same identity ApiCompat itself
97+
uses to match a diagnostic to a suppression. Comparing this tuple set
98+
instead of raw XML text is what lets a file that lost its hand-written
99+
comments, or whose entries came out in a different order, still compare
100+
equal to what was there before.
101+
"""
102+
if not os.path.exists(path):
103+
return frozenset()
104+
105+
root = ET.parse(path).getroot()
106+
entries = set()
107+
for suppression in root.findall("Suppression"):
108+
entries.add((
109+
_element_text(suppression, "DiagnosticId"),
110+
_element_text(suppression, "Target"),
111+
_element_text(suppression, "Left"),
112+
_element_text(suppression, "Right"),
113+
_element_text(suppression, "IsBaselineSuppression").lower() == "true",
114+
))
115+
return frozenset(entries)
116+
117+
118+
def clean_build_output(project_dir):
119+
for name in ("obj", "bin"):
120+
path = os.path.join(project_dir, name)
121+
if os.path.isdir(path):
122+
shutil.rmtree(path)
123+
124+
125+
def regenerate(csproj_path, configuration, work_dir):
126+
"""Packs a single project with suppression regeneration redirected into
127+
work_dir, and returns the resulting entry set. Never writes to the
128+
project's real CompatibilitySuppressions.xml.
129+
"""
130+
project_dir = os.path.dirname(csproj_path)
131+
clean_build_output(project_dir)
132+
133+
output_file = os.path.join(work_dir, "regenerated.xml")
134+
pack_out_dir = os.path.join(work_dir, "pack-out")
135+
command = [
136+
"dotnet", "pack", csproj_path,
137+
"-c", configuration,
138+
"-p:ApiCompatGenerateSuppressionFile=true",
139+
f"-p:ApiCompatSuppressionOutputFile={output_file}",
140+
"-o", pack_out_dir,
141+
]
142+
result = subprocess.run(command, capture_output=True, text=True, check=False)
143+
if result.stdout:
144+
print(result.stdout)
145+
if result.stderr:
146+
print(result.stderr, file=sys.stderr)
147+
if result.returncode != 0:
148+
raise RuntimeError(
149+
f"dotnet pack failed while regenerating suppressions for {csproj_path} "
150+
f"(exit code {result.returncode}). This is a build/restore problem, not a "
151+
f"suppression finding -- see the captured output above."
152+
)
153+
154+
return parse_entries(output_file)
155+
156+
157+
def plan_refresh(suppression_path, original_entries, regenerated_entries):
158+
"""Decides the action for one suppression file. Returns (action, added)
159+
where action is one of "noop", "delete", "prune", or "violation".
160+
"""
161+
added = regenerated_entries - original_entries
162+
if added:
163+
return "violation", added
164+
165+
removed = original_entries - regenerated_entries
166+
if not removed:
167+
return "noop", frozenset()
168+
if not regenerated_entries:
169+
return "delete", frozenset()
170+
return "prune", frozenset()
171+
172+
173+
def format_entry(entry):
174+
diagnostic_id, target, left, right, is_baseline = entry
175+
return f"{diagnostic_id} {target} (Left={left}, Right={right}, IsBaselineSuppression={is_baseline})"
176+
177+
178+
def refresh(root, configuration):
179+
suppression_files = discover_suppression_files(root)
180+
if not suppression_files:
181+
print(f"No {SUPPRESSIONS_FILENAME} files found under {root}; nothing to refresh.")
182+
return True, False
183+
184+
violations = []
185+
actions = []
186+
187+
with tempfile.TemporaryDirectory(prefix="apicompat-suppression-refresh-") as tmp_root:
188+
for index, suppression_path in enumerate(suppression_files):
189+
project_dir = os.path.dirname(suppression_path)
190+
csproj_path = find_project_file(project_dir)
191+
original_entries = parse_entries(suppression_path)
192+
193+
print(f"Regenerating suppressions for {csproj_path}...")
194+
work_dir = os.path.join(tmp_root, str(index))
195+
os.makedirs(work_dir, exist_ok=True)
196+
regenerated_entries = regenerate(csproj_path, configuration, work_dir)
197+
198+
action, added = plan_refresh(suppression_path, original_entries, regenerated_entries)
199+
if action == "violation":
200+
violations.append((suppression_path, added))
201+
else:
202+
actions.append((action, suppression_path, os.path.join(work_dir, "regenerated.xml")))
203+
204+
if violations:
205+
print(
206+
"::error::Regenerating one or more suppression files would ADD entries "
207+
"that are not present today. That means a real API break landed after "
208+
"the release this baseline now points at, and needs a human suppression "
209+
"decision -- auto-suppressing it would silently bless a breaking change. "
210+
"No suppression files were modified."
211+
)
212+
for suppression_path, added in violations:
213+
print(f"::error::{suppression_path}: would newly require {len(added)} suppression(s):")
214+
for entry in sorted(added):
215+
print(f"::error:: {format_entry(entry)}")
216+
return False, False
217+
218+
# Nothing tripped the invariant, so every real suppression file is
219+
# still exactly as it was on disk (regeneration above only ever wrote
220+
# into the temporary directory). This is the only point where a real
221+
# file is touched.
222+
changed = False
223+
for action, suppression_path, regenerated_file in actions:
224+
if action == "noop":
225+
print(f"Up to date, no changes: {suppression_path}")
226+
continue
227+
changed = True
228+
if action == "delete":
229+
os.remove(suppression_path)
230+
print(f"Deleted (no suppressions still needed): {suppression_path}")
231+
elif action == "prune":
232+
shutil.copyfile(regenerated_file, suppression_path)
233+
print(f"Pruned stale suppressions: {suppression_path}")
234+
235+
return True, changed
236+
237+
238+
def main():
239+
parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter)
240+
parser.add_argument("--root", default="src", help="Directory to scan for CompatibilitySuppressions.xml (default: src)")
241+
parser.add_argument("--configuration", default="Release", help="Build configuration to pack with (default: Release)")
242+
args = parser.parse_args()
243+
244+
try:
245+
ok, changed = refresh(args.root, args.configuration)
246+
except RuntimeError as error:
247+
print(f"::error::{error}")
248+
ok, changed = False, False
249+
250+
output = os.environ.get("GITHUB_OUTPUT")
251+
if output:
252+
with open(output, "a", encoding="utf-8") as handle:
253+
handle.write(f"changed={'true' if changed else 'false'}\n")
254+
255+
return 0 if ok else 1
256+
257+
258+
if __name__ == "__main__":
259+
sys.exit(main())

0 commit comments

Comments
 (0)