Skip to content

Commit abe60d9

Browse files
jevansaksCopilot
andcommitted
Replace changelog system with automated API diff for PR review
Remove the ChangesSinceLastRelease.txt system (which caused merge conflicts and lost history on every release) in favor of an automated approach: 1. WinmdUtils 'dump' command: Decompiles a winmd to sorted C# declarations using ICSharpCode.Decompiler, producing deterministic output suitable for text diffing. Shows full type declarations with attributes, method signatures, constants, and nested types. 2. Local diff script (scripts/DiffWinmdToBaseline.ps1): Dumps both the current build and last-release winmd, shows a unified diff via git diff. 3. CI artifact: The PR Validation workflow now publishes the winmd and its API dump as a 'winmd' artifact (retained 30 days). 4. PR API Diff workflow (.github/workflows/pr-api-diff.yml): Triggers after PR Validation succeeds, generates the baseline dump, diffs against the PR's dump, and posts/updates a comment on the PR with the full diff in a collapsible section. Removed: - scripts/ChangesSinceLastRelease.txt (no longer needed) - scripts/UpdateChangesSinceLastRelease.ps1 (no longer needed) - The build no longer fails on API diffs; reviewer sees them in PR comment Changed: - TestWinmdBinary.ps1: Compare is now informational (non-failing) - CompareBinToLastRelease.ps1: Simplified (no knownDiffs/update logic) - Set-LastReleaseVersion.ps1: Removed file-clearing logic Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 1e9cdf1 commit abe60d9

14 files changed

Lines changed: 391 additions & 138 deletions

.github/copilot-instructions.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ Each arch job scrapes ALL partitions for that architecture.
6666

6767
**NuGet feed:** The repo uses a private ADO Artifact Feed (`Win32Metadata-Dependencies`) as its sole package source. New packages from nuget.org must be saved to this feed first.
6868

69-
**Winmd equivalence:** After any change, the winmd must be compared against the baseline. The `NoSuggestedRemappings` test in `Windows.Win32.Tests` validates that no new unhandled remappings were introduced. Changes to the winmd are tracked in `scripts/ChangesSinceLastRelease.txt`.
69+
**Winmd equivalence:** After any change, the winmd must be compared against the baseline. The `NoSuggestedRemappings` test in `Windows.Win32.Tests` validates that no new unhandled remappings were introduced. API diffs are automatically posted as PR comments by CI. Run `.\scripts\DiffWinmdToBaseline.ps1` locally to see a full C# declaration diff against the last release.
7070

7171
**Cross-partition remaps:** Remaps in `scraper.settings.rsp` are global — they apply to all partitions. Auto-discovered remaps (from `Win32MetadataScraper`) are per-partition. If a tag name is used across partitions but the typedef is only in one partition's headers, the remap must be global (in `scraper.settings.rsp`) OR the referencing partition must `#include` the header with the typedef.
7272

.github/workflows/pr-validation.yml

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ on:
2222
- 'docs/**'
2323
workflow_dispatch:
2424

25+
permissions:
26+
contents: read
27+
pull-requests: write
28+
2529
concurrency:
2630
group: pr-${{ github.event.pull_request.number || github.ref }}
2731
cancel-in-progress: true
@@ -135,6 +139,166 @@ jobs:
135139
shell: pwsh
136140
run: .\scripts\DoTests.ps1
137141

142+
- name: Generate API surface dumps
143+
id: apidump
144+
shell: pwsh
145+
run: |
146+
$winmdUtils = "bin\Release\net8.0\WinmdUtils.dll"
147+
$currentWinmd = "bin\Windows.Win32.winmd"
148+
149+
# Get the baseline winmd from the last release NuGet package
150+
. .\scripts\CommonUtils.ps1
151+
$ErrorActionPreference = 'Continue'
152+
$PSNativeCommandUseErrorActionPreference = $false
153+
$baselineWinmd = Get-Win32MetadataLastReleaseWinmdPath
154+
155+
Write-Host "Baseline: $baselineWinmd"
156+
Write-Host "Current: $currentWinmd"
157+
158+
# Dump baseline
159+
Write-Host "Dumping baseline..."
160+
& dotnet $winmdUtils dump --winmd $baselineWinmd --output bin\baseline.apidump.cs
161+
if ($LASTEXITCODE -ne 0) { throw "Failed to dump baseline" }
162+
163+
# Dump current build
164+
Write-Host "Dumping current build..."
165+
& dotnet $winmdUtils dump --winmd $currentWinmd --output bin\current.apidump.cs
166+
if ($LASTEXITCODE -ne 0) { throw "Failed to dump current" }
167+
168+
# Generate diff — git diff exits 1 when differences exist, which is expected
169+
# Use cmd /c to isolate from PowerShell's native command error handling
170+
& cmd /c "git diff --no-index --unified=3 bin\baseline.apidump.cs bin\current.apidump.cs > bin\api-diff.patch 2>&1"
171+
$diffExitCode = $LASTEXITCODE
172+
173+
if ($diffExitCode -eq 0) {
174+
Write-Host "No API differences found."
175+
"has_diff=false" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
176+
} else {
177+
Write-Host "API differences detected."
178+
"has_diff=true" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
179+
180+
# Count additions/deletions from the patch file
181+
$diffLines = Get-Content bin\api-diff.patch
182+
$additions = ($diffLines | Where-Object { $_ -match '^\+[^+]' }).Count
183+
$deletions = ($diffLines | Where-Object { $_ -match '^\-[^-]' }).Count
184+
"additions=$additions" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
185+
"deletions=$deletions" | Out-File -FilePath $env:GITHUB_OUTPUT -Append
186+
187+
Write-Host "+$additions additions / -$deletions deletions"
188+
189+
# Truncate if too large for a GH comment
190+
$diffText = $diffLines -join "`n"
191+
$maxLen = 60000
192+
if ($diffText.Length -gt $maxLen) {
193+
$diffText = $diffText.Substring(0, $maxLen) + "`n`n... (diff truncated - see full artifact)"
194+
Set-Content -Path bin\api-diff.patch -Value $diffText -Encoding UTF8
195+
}
196+
}
197+
198+
# Reset LASTEXITCODE so GitHub Actions doesn't treat this step as failed
199+
# (git diff exits 1 when differences exist, which is expected/success for us)
200+
$global:LASTEXITCODE = 0
201+
202+
- name: Post API diff comment on PR
203+
if: github.event_name == 'pull_request' && steps.apidump.outputs.has_diff == 'true'
204+
uses: actions/github-script@v7
205+
with:
206+
script: |
207+
const fs = require('fs');
208+
const diff = fs.readFileSync('bin/api-diff.patch', 'utf8');
209+
const prNumber = context.payload.pull_request.number;
210+
const additions = ${{ steps.apidump.outputs.additions || 0 }};
211+
const deletions = ${{ steps.apidump.outputs.deletions || 0 }};
212+
const runUrl = `https://github.com/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`;
213+
214+
const marker = '<!-- win32metadata-api-diff -->';
215+
const body = `${marker}
216+
## 📋 API Surface Diff
217+
218+
**+${additions} additions / -${deletions} deletions** vs last release ([full build log](${runUrl}))
219+
220+
<details>
221+
<summary>Click to expand API diff</summary>
222+
223+
\`\`\`diff
224+
${diff}
225+
\`\`\`
226+
227+
</details>
228+
229+
> This comment is automatically updated on each push.`;
230+
231+
const comments = await github.rest.issues.listComments({
232+
owner: context.repo.owner,
233+
repo: context.repo.repo,
234+
issue_number: prNumber,
235+
per_page: 100
236+
});
237+
238+
const existing = comments.data.find(c => c.body.includes(marker));
239+
240+
if (existing) {
241+
await github.rest.issues.updateComment({
242+
owner: context.repo.owner,
243+
repo: context.repo.repo,
244+
comment_id: existing.id,
245+
body: body
246+
});
247+
} else {
248+
await github.rest.issues.createComment({
249+
owner: context.repo.owner,
250+
repo: context.repo.repo,
251+
issue_number: prNumber,
252+
body: body
253+
});
254+
}
255+
256+
- name: Post no-diff comment on PR
257+
if: github.event_name == 'pull_request' && steps.apidump.outputs.has_diff == 'false'
258+
uses: actions/github-script@v7
259+
with:
260+
script: |
261+
const prNumber = context.payload.pull_request.number;
262+
const marker = '<!-- win32metadata-api-diff -->';
263+
const body = `${marker}\n## 📋 API Surface Diff\n\n✅ No API differences vs last release.\n\n> This comment is automatically updated on each push.`;
264+
265+
const comments = await github.rest.issues.listComments({
266+
owner: context.repo.owner,
267+
repo: context.repo.repo,
268+
issue_number: prNumber,
269+
per_page: 100
270+
});
271+
272+
const existing = comments.data.find(c => c.body.includes(marker));
273+
274+
if (existing) {
275+
await github.rest.issues.updateComment({
276+
owner: context.repo.owner,
277+
repo: context.repo.repo,
278+
comment_id: existing.id,
279+
body: body
280+
});
281+
} else {
282+
await github.rest.issues.createComment({
283+
owner: context.repo.owner,
284+
repo: context.repo.repo,
285+
issue_number: prNumber,
286+
body: body
287+
});
288+
}
289+
290+
- name: Upload winmd and API dump
291+
uses: actions/upload-artifact@v4
292+
if: always()
293+
with:
294+
name: winmd
295+
path: |
296+
bin/Windows.Win32.winmd
297+
bin/current.apidump.cs
298+
bin/baseline.apidump.cs
299+
bin/api-diff.patch
300+
retention-days: 30
301+
138302
- name: Upload build logs
139303
uses: actions/upload-artifact@v4
140304
if: always()

CONTRIBUTING.md

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -252,20 +252,30 @@ Run `./DoAll.ps1 -ExcludePackages -ExcludeSamples` in [PowerShell 7](https://aka
252252

253253
Note that stale artifacts on your system may sometimes result in cryptic errors when attempting incremental builds. If you do encounter cryptic errors during incremental builds that you suspect are the result of previously built changes, reset your system state by running a clean build with `./DoAll.ps1 -Clean`.
254254

255-
### Comparing against the last release
255+
### Reviewing API changes
256256

257-
A list of accumulated changes since the last release is kept at [ChangesSinceLastRelease.txt](scripts/ChangesSinceLastRelease.txt). New changes are reported by `./scripts/TestWinmdBinary.ps1` which is called during both full and incremental builds if you follow the steps above.
257+
API differences between your build and the last release are automatically detected during the build. When you submit a PR, the CI pipeline posts a comment showing the full API diff as decompiled C# declarations, making it easy for reviewers to see exactly what changed.
258258

259-
When validating changes, it's important to evaluate the diffs to ensure all changes are intentional. Common patterns to expect in the diffs include:
259+
#### Viewing changes locally
260+
261+
To see a full diff of your changes locally before submitting a PR:
262+
263+
```powershell
264+
.\scripts\DiffWinmdToBaseline.ps1
265+
```
266+
267+
This decompiles both the current build's winmd and the last released winmd to sorted C# declarations, then displays a unified diff. The output shows full type declarations with attributes, method signatures, and constants — similar to what you'd see in ILSpy.
268+
269+
#### What to look for
270+
271+
When validating changes, it's important to evaluate the diffs to ensure all changes are intentional. Common patterns to expect include:
260272

261273
* APIs were added to the baseline
262274
* APIs were removed from the baseline
263275
* APIs were moved to different namespaces
264276

265277
Additionally, it is useful to load the winmd in [ILSpy](https://github.com/icsharpcode/ILSpy) and navigate through the APIs as another means to identify additional changes that may be required to achieve the desired end result. You may notice that two related APIs are in different namespaces or that a type that an API depends on was not moved as you would have expected. If that happens, search the repo for the API or its header file to identify where it may be being mapped to another namespace.
266278

267-
Once all the changes are validated, update the list of known changes since the last release by following the steps reported in the build output. When a new release is made, the list of changes in [ChangesSinceLastRelease.txt](scripts/ChangesSinceLastRelease.txt) will get reset and will start accumulating again until the next release.
268-
269279
## Releasing
270280

271281
The main branch must have a clean build to publish a new release. Run the [release pipeline](https://github-private.visualstudio.com/microsoft/_build?definitionId=750) to publish new packages to nuget.org and create a new draft release on GitHub autopopulated with the list of resolved issues.

docs/copilot/research/win32metadata-detailed-research.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,7 @@ DoAll.ps1
475475

476476
6. **Allow-list testing pattern**: Tests use `.rsp` allow-list files to track known acceptable violations. This enables progressive quality improvement without blocking builds.
477477

478-
7. **Winmd diff as release gate**: Every build compares the new .winmd against the last release via `WinmdUtils compare`. The diff is tracked in `ChangesSinceLastRelease.txt` to ensure all changes are intentional.
478+
7. **Winmd diff for PR review**: Every build compares the new .winmd against the last release via `WinmdUtils compare`. The full API diff (decompiled C# declarations) is posted as a PR comment by CI for reviewer visibility.
479479

480480
8. **Separate pipelines for metadata vs docs**: The API documentation pipeline (`azure-pipelines-apidocs.yml`) is completely independent, triggered only by changes in `apidocs/`.
481481

docs/copilot/research/win32metadata-research-summary.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ Tasks are loaded via `UsingTask AssemblyFile` (in-process), but net8.0 assemblie
123123
- No full winmd binary snapshots (only ~100 interfaces)
124124
- No cross-build consistency tests ("build A vs build B")
125125
- No hash-based regression detection
126-
- `ChangesSinceLastRelease.txt` is manually maintained (400+ lines)
126+
- API diffs are posted as PR comments (full C# declarations via `WinmdUtils dump`)
127127

128128
### 3.5 Metadata Defined After-the-Fact
129129

0 commit comments

Comments
 (0)