Skip to content
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.

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

[release/8.0-staging] Move generation of SuggestedBindingRedirects.targets to inner build #112494

Merged

Conversation

ericstj
Copy link
Member

@ericstj ericstj commented Feb 12, 2025

Backport of #112379 to release/9.0-staging
Fixes #111892

/cc @ericstj

In 8.0 we don't need to actually ship this, since we haven't shipped a patch in 8.0 there is nothing to fix. We just want to proactively port this fix so that it's ready should we need to patch in the future.

Customer Impact

  • Customer reported
  • Found internally

.NETFramework project using the latest System.Resources.Extensions package fails to load resources with FileLoadException.

Regression

  • Yes
  • No

9.0.1 - this is the first time we've serviced this package. The package didn't account for the serviced assembly version correctly.

Testing

Build / package / inspect redirects. Manually test consuming package in .NETFramework project.

Automated tests here are difficult with current infrastructure as we don't consume the built packages, nor rely on auto-generated bindingRedirects in the same way a customer project does.

Risk

Low - updating target in a single project that only impacts that library's generation of a targets file. If it builds its good.

…otnet#112379)

* Move generation of SuggestedBindingRedirects.targets to inner build

These targets depend on the AssemblyVersion of the library which is
specific to the inner-build of the library.  Generate them in the inner-build.

* Update src/libraries/System.Resources.Extensions/src/System.Resources.Extensions.csproj
@Copilot Copilot bot review requested due to automatic review settings February 12, 2025 19:18

Choose a reason for hiding this comment

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

Copilot wasn't able to review any files in this pull request.

Files not reviewed (1)
  • src/libraries/System.Resources.Extensions/src/System.Resources.Extensions.csproj: Language not supported
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-resources
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

LGTM. Same change as in main.

@ericstj
Copy link
Member Author

ericstj commented Feb 19, 2025

/ba-g failures are all #112312

@ericstj ericstj merged commit 52ce550 into dotnet:release/8.0-staging Feb 19, 2025
112 of 117 checks passed
</Target>

<ItemGroup Condition="'$(NetFrameworkMinimum)' != ''">
Copy link
Member

Choose a reason for hiding this comment

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

Condition="'$(NetFrameworkMinimum)' != ''"

Does it make any difference checking NetFrameworkMinimum or checking the target framework to be NET Framework?

@tarekgh
Copy link
Member

tarekgh commented Feb 19, 2025

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Resources Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants