Skip to content

Fix APIDiff to include NETCore references for AspNetCore and WindowsDesktop #9989

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 3 commits into from
Jul 21, 2025
Merged

Fix APIDiff to include NETCore references for AspNetCore and WindowsDesktop #9989

merged 3 commits into from
Jul 21, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jul 21, 2025 •
edited
Loading

The APIDiff script RunApiDiff.ps1 was not passing the reference path to the Microsoft.NETCore.App.Ref package when running diffs on AspNetCore and WindowsDesktop SDKs. This caused Roslyn to lack core assembly references, leading to incorrect symbol information for enums, basic types, and other fundamental .NET constructs.

Changes Made

Modified ProcessSdk function

  • Added logic to download Microsoft.NETCore.App.Ref packages when processing AspNetCore and WindowsDesktop SDKs
  • The NETCore references are downloaded for both "Before" and "After" versions to ensure consistent comparison

Updated RunApiDiff function

  • Added optional beforeReferenceFolder and afterReferenceFolder parameters
  • Included -rb and -ra flags in the API diff command when reference folders are provided
  • Maintains backward compatibility - NETCore processing works unchanged without reference parameters

Example of the fix in action:

Before (missing core references):

apidiff.exe -b 'aspnetcore-before' -a 'aspnetcore-after' -o 'output' ...
# Roslyn can't understand enums or basic types

After (with core references):

apidiff.exe -b 'aspnetcore-before' -a 'aspnetcore-after' -o 'output' ... -rb 'netcore-before' -ra 'netcore-after'
# Roslyn has full access to core .NET types

Testing

  • Verified PowerShell syntax validity
  • Tested backward compatibility (NETCore processing unchanged)
  • Confirmed new functionality works for AspNetCore and WindowsDesktop
  • Validated proper parameter handling for all SDK types

This ensures that API diffs for AspNetCore and WindowsDesktop will now have proper symbol information and can correctly decode enum values and other fundamental types.

Fixes #9988.

💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] APIDiff has trouble with differences in ASPNET and WindowsDesktop Fix APIDiff to include NETCore references for AspNetCore and WindowsDesktop Jul 21, 2025
Copilot finished work on behalf of ericstj July 21, 2025 16:13
@Copilot Copilot AI requested a review from ericstj July 21, 2025 16:13
ericstj
ericstj approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@ericstj ericstj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. I pulled it down and ran the changes and it resolved the reported issue.

@ericstj ericstj requested a review from tarekgh July 21, 2025 19:23
@tarekgh
Copy link
Member

tarekgh commented Jul 21, 2025

Does this file need to show any diff at all? My understanding is nothing changed in such public APIs, or is this because the APIs moved to a new place?

Refers to: release-notes/10.0/preview/preview6/api-diff/Microsoft.AspNetCore.App/10.0-preview6_System.Formats.Cbor.md:2 in 5485e54. [](commit_id = 5485e54, deletion_comment = False)

tarekgh
tarekgh approved these changes Jul 21, 2025
View reviewed changes
Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left a minor question. LGTM otherwise.

@tarekgh
Copy link
Member

tarekgh commented Jul 21, 2025 

Error: -21 19:24:53 [ERROR]   Errors found in POWERSHELL
Error: -21 19:24:54 [ERROR]   Super-linter detected linting errors

@ericstj
Copy link
Member

ericstj commented Jul 21, 2025

Does this file need to show any diff at all? My understanding is nothing changed in such public APIs, or is this because the APIs moved to a new place?

It was added to the shared framework, which is why all of it's API shows as an addition.

@ericstj ericstj marked this pull request as ready for review July 21, 2025 21:13
@ericstj
Copy link
Member

ericstj commented Jul 21, 2025 

Error: -21 19:24:53 [ERROR]   Errors found in POWERSHELL
Error: -21 19:24:54 [ERROR]   Super-linter detected linting errors

These all appear to be existing issues with the script. I filed #9990 to fix them.

@ericstj ericstj merged commit 6b610a7 into main Jul 21, 2025
6 of 8 checks passed
@ericstj ericstj deleted the copilot/fix-9988 branch July 21, 2025 21:15
@OwnageIsMagic
Copy link

OwnageIsMagic commented Jul 27, 2025 •
edited
Loading

@ericstj could you also regenerate diffs for previous previews?
#9906 (comment)

And also look at #9821 (comment)

@ericstj
Copy link
Member

ericstj commented Jul 29, 2025

Sure, let me give that a try.

@ericstj ericstj mentioned this pull request Aug 1, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tarekgh tarekgh tarekgh approved these changes

@rbhanda rbhanda Awaiting requested review from rbhanda rbhanda is a code owner

@leecow leecow Awaiting requested review from leecow leecow is a code owner

@victorisr victorisr Awaiting requested review from victorisr victorisr is a code owner

+1 more reviewer

@ericstj ericstj ericstj approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees

@ericstj ericstj

Copilot code review Copilot

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

APIDiff has trouble with differences in ASPNET and WindowsDesktop
4 participants
@Copilot @tarekgh @ericstj @OwnageIsMagic