Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901

joperezr · 2020-04-08T18:23:18Z

cc: @ericstj @Anipik @safern
Fixes dotnet/runtime#1625

Description

This PR will finish the work on the release/3.1 branch that was started with PR #42849 on release/2.1 branch. It is adding .NET Framework configurations to our OOB packages so that they won't require netsandard shims for projects ingesting them and targeting .NET Framework.

Things that need to happen before merging this PR:

Add Net461 configurations to all projects that today ship as NuGet packages and have a .NET Standard asset when consumer targets .NET Framework 4.6.1/4.6.2/4.7/4.7.1/4.7.2/4.8. (This will be done with the first commit, so that it is easier to review this commit by commit.)
Add Net461 configuration to OpenSSL OOB package which also needs it, but it will be treated specially given that it requires some special types in S.S.C.Algorithms that are not present in 461. (Done with commit 10df782)
Baseline all of the packages that have new configurations from steps 1 and 2, and now service all packages above that depend on them so that all new package versions will depend on these new packages that have the new configs. After doing this, all of our latest NuGet packages should not require the .NET Standard shims. (Done with 14b5db5)
Do some extensive testing to make sure API wasn't regressed by adding these new configurations. (I have more info on how I did this here Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901 (comment)).

Customer Impact

Customers that have hit problems caused by this issue have been trying to add manual workarounds on their projects and custom logic for deploying. Once this is fixed, customers will have to remove those workarounds from their projects and reference the new packages in order to get the benefits of not needing the extra facades or binding redirects for their application.

Regression?

No.

Packaging reviewed?

It will involve packaging changes, so it will require a review by packaging experts.

Risk

Low. We didn't increase Assembly Version in order to not require new binding redirects from apps. The risk here is that we are effectively causing NuGet to use a different asset from the package to pass to the compiler and a different one as well for the implementation, but we did extensive testing to ensure that we were not removing or adding API and that there was no visible difference between the assets that they used to get and the ones they will get now.

joperezr · 2020-04-08T18:24:35Z

@ericstj as you can see from the description above, so far this PR contains the first part of the changes, which include just adding net461 configurations to projects that require it. Now I will work on the next 2 steps.

src/System.ComponentModel.Composition/ref/System.ComponentModel.Composition.csproj

joperezr · 2020-04-09T21:51:07Z

I have now concluded the extensive testing step to ensure that there were no visible API differences between the current latest stable packages and the ones produced by this PR when targeting net461, net462 and net471. I did this by manually comparing the reference asset I was getting out of the package on two projects, one using the latest stable package, and the other one using the one from this PR which would now use the net461 configuration.

joperezr · 2020-04-09T22:58:03Z

@bartonjs PTAL when you have time to commit 10df782

src/System.Security.Cryptography.OpenSsl/ref/System.Security.Cryptography.OpenSsl.csproj

src/System.Security.Cryptography.OpenSsl/ref/System.Security.Cryptography.OpenSsl.cs

ericstj · 2020-04-10T04:17:02Z

src/System.Numerics.Tensors/ref/Configurations.props

@@ -2,9 +2,11 @@
  <PropertyGroup>
    <PackageConfigurations>
      netstandard1.1;
+      net461;


Where is the package version change?

This package isn't stable, it has the BlockStable attribute, so I didn't rev it given that we will just be shipping it as another preview. cc: @tannergooding if you guys want me to rev it up, I can do that too.

We shouldn’t even ship it. Won’t have a valid version (-servicing)

src/System.Resources.Extensions/src/System.Resources.Extensions.csproj

joperezr · 2020-04-10T22:54:39Z

@ericstj this is now ready to go. When you have time do you mind taking another look? If all looks good then we can go ahead and merge.

safern

LGTM

src/System.IO.Pipelines/Directory.Build.props

Anipik

LGTM other than one comment about versioning

safern · 2020-04-13T21:14:35Z

src/System.IO.Pipelines/src/System/IO/Pipelines/PipeAwaitable.cs

@@ -19,7 +19,7 @@ internal struct PipeAwaitable
        private SynchronizationContext _synchronizationContext;
        private ExecutionContext _executionContext;

-#if !netstandard
+#if (!netstandard && !netfx)


You can use NETFRAMEWORK which is predefined by the SDK:
https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/preprocessor-directives/preprocessor-if

Not on release/3.1 at least. Here are the defines I'm getting straight out of the binlog:

,DEBUG,TRACE,netfx;TRACE;RESOURCES_EXTENSIONS;INTERNAL_NULLABLE_ATTRIBUTES;;

I believe release/3.1 is still in the place where we were not using the actual defines from the SDK yet. dotnet/runtime is different.

Dotnet-GitSync-Bot added the area-Infrastructure label Apr 8, 2020

joperezr requested review from ericstj and safern April 8, 2020 18:26

joperezr commented Apr 8, 2020

View reviewed changes

src/System.ComponentModel.Composition/ref/System.ComponentModel.Composition.csproj Outdated Show resolved Hide resolved

WIP Adding netfx configs to packages that need it

51d2f8f

joperezr force-pushed the AddNETFXConfigs branch from c22ae9d to 51d2f8f Compare April 8, 2020 19:05

Fix netfx test build given MEF2 doesn't have ref projects

c4ceae8

joperezr mentioned this pull request Apr 9, 2020

<PackageReference> System.* when targeting NET Framework leads to ambiguity dotnet/standard#1665

Closed

Adding .NET Framework configuration to OpenSsl package

10df782

bartonjs reviewed Apr 10, 2020

View reviewed changes

src/System.Security.Cryptography.OpenSsl/ref/System.Security.Cryptography.OpenSsl.csproj Show resolved Hide resolved

ericstj reviewed Apr 10, 2020

View reviewed changes

ReSpin builds of all packages that need an updated package references

14b5db5

joperezr force-pushed the AddNETFXConfigs branch from 9cfa4fe to 14b5db5 Compare April 10, 2020 22:39

Addressing PR Feedback

9607be2

joperezr added Servicing-consider Issue for next servicing release review Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Apr 13, 2020

safern approved these changes Apr 13, 2020

View reviewed changes

Removing unnecesary DefineConstants and use Built-in instead

0b9393b

Anipik reviewed Apr 13, 2020

View reviewed changes

src/System.IO.Pipelines/Directory.Build.props Show resolved Hide resolved

Anipik approved these changes Apr 13, 2020

View reviewed changes

safern reviewed Apr 13, 2020

View reviewed changes

joperezr added 2 commits April 13, 2020 15:16

Removing one extra DefineConstants to use defaults instead

1523608

Fix incorrect package version bumps

29e070e

ericstj approved these changes Apr 14, 2020

View reviewed changes

Anipik approved these changes Apr 14, 2020

View reviewed changes

joperezr merged commit 76dff3b into dotnet:release/3.1 Apr 14, 2020

joperezr deleted the AddNETFXConfigs branch April 14, 2020 17:27

TimothyMothra mentioned this pull request Apr 14, 2020

Need to update to System.Diagnostics.DiagnosticSource 5.0 microsoft/ApplicationInsights-dotnet#1790

Closed

joperezr mentioned this pull request Apr 22, 2020

[Test-only change]Enable running VerifyReferenceClousre for netfx OOB packages #42909

Merged

MagicAndre1981 mentioned this pull request May 18, 2020

.NET Core May 2020 Update - 2.1.18, 3.1.4 and 3.1.300 SDK dotnet/core#4675

Closed

1 task

This was referenced May 20, 2020

Add NetFramework TFM to System.Reflection.MetadataLoadContext dotnet/runtime#36725

Closed

Add .Net Framework TFM to System.Reflection.MetadataLoadContext dotnet/runtime#30810

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901

Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901

Uh oh!

joperezr commented Apr 8, 2020 •

edited

Loading

Uh oh!

joperezr commented Apr 8, 2020

Uh oh!

Uh oh!

joperezr commented Apr 9, 2020

Uh oh!

joperezr commented Apr 9, 2020

Uh oh!

Uh oh!

Uh oh!

ericstj Apr 10, 2020

Uh oh!

joperezr Apr 10, 2020

Uh oh!

ericstj Apr 14, 2020

Uh oh!

Uh oh!

joperezr commented Apr 10, 2020

Uh oh!

safern left a comment

Uh oh!

Uh oh!

Anipik left a comment

Uh oh!

safern Apr 13, 2020

Uh oh!

joperezr Apr 13, 2020

Uh oh!

joperezr Apr 13, 2020

Uh oh!

Uh oh!

Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901

Adding .NET Framework configurations to OOB packages so that they won't require the netstandard shims when targeting .NET Framework #42901

Uh oh!

Conversation

joperezr commented Apr 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joperezr commented Apr 8, 2020

Uh oh!

Uh oh!

joperezr commented Apr 9, 2020

Uh oh!

joperezr commented Apr 9, 2020

Uh oh!

Uh oh!

Uh oh!

ericstj Apr 10, 2020

Choose a reason for hiding this comment

Uh oh!

joperezr Apr 10, 2020

Choose a reason for hiding this comment

Uh oh!

ericstj Apr 14, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joperezr commented Apr 10, 2020

Uh oh!

safern left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Anipik left a comment

Choose a reason for hiding this comment

Uh oh!

safern Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

joperezr Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

joperezr Apr 13, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joperezr commented Apr 8, 2020 •

edited

Loading