Skip to content

[release/5.0] Don't include RuntimeList.xml in sharedFx archive #29109

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

Merged
merged 3 commits into from
Jan 13, 2021

Conversation

wtgodbe
Copy link
Member

@wtgodbe wtgodbe commented Jan 6, 2021

Resolves #27670

dotnet/runtime and dotnet/windowsdesktop don't include this file in their SharedFx archives. Looking at the SDK archive, it contains a runtimelist.xml for aspnetcore, but not for runtime or windowsdesktop. We should remove the file from our archive, but keep it in our .nupkg.

@ghost ghost added the feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform label Jan 6, 2021
@wtgodbe wtgodbe requested review from mthalman, dsplaisted and a team January 6, 2021 23:10
Copy link
Contributor

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

This does what the PR description says.

But, it's not fixing #27670 and instead undoes a fix for that issue. If you're going to mention that issue, say something like "reverts fix for bogus issue #27670".

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 6, 2021

But, it's not fixing #27670 and instead undoes a fix for that issue. If you're going to mention that issue, say something like "reverts fix for bogus issue #27670".

Actually the original issue is bogus - It's always been the case that the public archive doesn't have the file, and the internal archive does - there wasn't an intermediate change that originally "fixed" #27670. I didn't know that when I filed the original issue. I'll edit the issue to reflect the actual state of things

@wtgodbe
Copy link
Member Author

wtgodbe commented Jan 7, 2021

I've looked at the outputs and confirmed that the .nupkg still has runtimeList.xml, but the internal .zip no longer does.

Copy link
Contributor

@dougbu dougbu 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 doing the confirmation…

@@ -154,7 +154,7 @@ This package is an internal implementation of the .NET Core SDK and is not meant
<LocalInstallationOutputPath>$(LocalDotNetRoot)$(SharedRuntimeSubPath)</LocalInstallationOutputPath>

<FrameworkListFileName>RuntimeList.xml</FrameworkListFileName>
<FrameworkListOutputPath>$(SharedFxLayoutTargetDir)$(FrameworkListFileName)</FrameworkListOutputPath>
<FrameworkListOutputPath>$(ArtifactsObjDir)$(FrameworkListFileName)</FrameworkListOutputPath>
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: A comment here indicating we don't always want the RuntimeList.xml file in the layout might help.

@wtgodbe wtgodbe added the tell-mode Indicates a PR which is being merged during tell-mode label Jan 13, 2021
@wtgodbe wtgodbe merged commit fc45191 into release/5.0 Jan 13, 2021
@wtgodbe wtgodbe deleted the wtgodbe/50runtimelist branch January 13, 2021 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform tell-mode Indicates a PR which is being merged during tell-mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants