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

Stub tasks that are not supported on .NET #9153

Merged
merged 11 commits into from Sep 27, 2023

Conversation

jrdodds
Copy link
Contributor

@jrdodds jrdodds commented Aug 22, 2023

Fixes #3499

Context

Following the pattern used for the ResolveComReference task, create stubs for .NET that provide a clear error message indicating the task is not supported on .NET.

Changes Made

The following tasks have been updated:

  • AL
  • AspNetCompiler
  • GenerateBootstrapper
  • GenerateTrustInfo
  • GetFrameworkSdkPath
  • RegisterAssembly
  • ResolveComReference
  • ResolveNativeReference
  • UnregisterAssembly
  • UpdateManifest
  • WinMDExp

Additionally, the Utilities project has been updated so that AppDomainIsolatedTask (base class of AppDomainIsolatedTaskExtension and a dependency of ResolveComReference) is included for '.NETFramework' only.

Testing

Tested on macOS and Windows 11 with test project files (attached as TaskTest.zip.)

Notes

I don't have a full understanding of the interactions of the preprocessor symbols. (e.g. Is FEATURE_APPDOMAIN only valid for NETFRAMEWORK?) There may be improvements that can be made to the conditionals in the source and in the .targets file.

Should unit tests be added for the stubs?

@jrdodds
Copy link
Contributor Author

jrdodds commented Aug 22, 2023

I added a TaskRequiresFramework type and that is breaking the Microsoft.NET.ApiCompat.ValidatePackage. Can someone review the PR and determine if the "APICompat suppression file" should be updated?

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

The strategy looks good--a few nits mentioned below, but I'm ok with adding the CP0007 exceptions for this, once the classes are static.

src/Tasks/Microsoft.Common.tasks Outdated Show resolved Hide resolved
src/Utilities/Microsoft.Build.Utilities.csproj Outdated Show resolved Hide resolved
src/Tasks/Al.cs Outdated Show resolved Hide resolved
src/Tasks/UpdateManifest.cs Outdated Show resolved Hide resolved
src/Tasks/TaskRequiresFramework.cs Outdated Show resolved Hide resolved
src/Tasks/ResolveNativeReference.cs Outdated Show resolved Hide resolved
@jrdodds
Copy link
Contributor Author

jrdodds commented Aug 28, 2023

... once the classes are static.

Did you mean sealed?

@AR-May AR-May self-requested a review August 29, 2023 13:35
@rainersigwald
Copy link
Member

... once the classes are static.

Did you mean sealed?

Sure did!

Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@AR-May AR-May left a comment

Choose a reason for hiding this comment

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

LGTM too

@rainersigwald rainersigwald added this to the VS 17.9 milestone Sep 19, 2023
Copy link
Member

@rainersigwald rainersigwald left a comment

Choose a reason for hiding this comment

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

Thanks for keeping this up to date. We'll be taking a bunch of PRs now that we've forked for .NET 8/VS 17.8.

@rainersigwald rainersigwald merged commit b454470 into dotnet:main Sep 27, 2023
8 checks passed
@jrdodds jrdodds deleted the StubRegUnregAssemblyTasks branch September 28, 2023 23:11
bulatgrzegorz pushed a commit to bulatgrzegorz/selective-condition-evaluator that referenced this pull request Oct 16, 2023
Following the pattern used for the `ResolveComReference` task, create stubs for .NET that provide a clear error message indicating the task is not supported on .NET.

Fixes dotnet#3499.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Emit clearer error for built-in tasks that don't work on .NET Core
3 participants