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

Add netcoreapp3.1 inbox information to package baseline #49781

Closed

Conversation

GrabYourPitchforks
Copy link
Member

@GrabYourPitchforks GrabYourPitchforks commented Mar 17, 2021

Our package baselines don't include inbox information for the netcoreapp3.1 runtime. This means that if a library targets netcoreapp3.1, the build system will attempt to resolve a package reference instead of an inbox assembly reference, which may cause the build to fail. (See this failing PR for one such example.)

This PR updates the package baselines by adding netcoreapp3.1 assembly inbox information to the baseline file. Assemblies which did not have a preexisting netcoreapp3.0 entry in the baseline file are not modified by this change.

Assembly information was pulled from the file C:\Program Files\dotnet\shared\Microsoft.NETCore.App\3.1.13\Microsoft.NETCore.App.deps.json on my local box.

Edit: Updated the PR to be based on Eric St John's auto-script.

@@ -6689,6 +6707,7 @@
"BaselineVersion": "6.0.0",
"InboxOn": {
"netcoreapp3.0": "4.0.4.0",
"netcoreapp3.1": "4.0.5.1",
Copy link
Member

Choose a reason for hiding this comment

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

We should never list serviced versions here. This has to represent what's inbox in the GA product since packages can't target a serviced release (only TFM). A better source of these versions would be "C:\Program Files\dotnet\packs\Microsoft.NETCore.App.Ref\3.1.0\data\FrameworkList.xml", or just merge the index values from release/3.1. cc @Anipik

Copy link
Member Author

Choose a reason for hiding this comment

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

I can merge the values from https://github.com/dotnet/corefx/blob/release/3.1/pkg/Microsoft.Private.PackageBaseline/packageIndex.json, even if there was never any "netcoreapp3.0" entry. Would that suffice?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Well then, time to overwrite this PR. :)

@GrabYourPitchforks
Copy link
Member Author

@ericstj CI's still failing. Do we also need to update StableVersions or AssemblyVersionInPackageVersion?

@GrabYourPitchforks
Copy link
Member Author

For instance, setting PackageFolders = <directory where 3.1.0 RTM packages were dropped > results in these two lines being merged into the index file:

     "System.Text.Json": {
       "StableVersions": [
         "4.6.0",
         "4.7.0",
         "4.7.1",
         "4.7.2",
         "5.0.0"
       ],
       "BaselineVersion": "6.0.0",
       "InboxOn": {
         "netcoreapp3.0": "4.0.0.0",
+        "netcoreapp3.1": "4.0.1.0",
         "net5.0": "5.0.0.0"
       },
       "AssemblyVersionInPackageVersion": {
         "4.0.0.0": "4.6.0",
+        "4.0.1.0": "4.7.0",
         "5.0.0.0": "5.0.0",
         "6.0.0.0": "6.0.0"
       }
     },

@ericstj
Copy link
Member

ericstj commented Mar 18, 2021

Those should only be relevant when we have a package reference. The assembly-to-package-reference lookup logic is circumvented by setting baseline versions to 6.0.0.

The error you're seeing is more around the build of individual packages not understanding that the packages are OOBing an implementation for those frameworks.

... That said I can't explain why just adding a version for 3.1 is causing these failures. Need to debug. Building locally.

@ericstj
Copy link
Member

ericstj commented Mar 18, 2021

I have to step away for a bit, but @Anipik might know why this is failing.

@ericstj
Copy link
Member

ericstj commented Mar 18, 2021

I think I have a fix. I just went ahead and created a new PR since I couldn't push to yours.

@GrabYourPitchforks
Copy link
Member Author

Superseded by #49792.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 17, 2021
@karelz karelz added this to the 6.0.0 milestone May 20, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-Meta packaging Related to packaging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants