Ensure Microsoft.NETCore.App is an implicit dependency for publish. #2881
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peterhuene
changed the title
|
Marking WIP as this needs approval. @livarcocc I'm not marking this or the original issue as servicing consider until the fix is reviewed and we're happy with it. |
|
I did not review in detail, but can we not consider that there is an implicit dependency from Microsoft.AspNetCore.App to Microsoft.NETCore.App? I don't understand why this requires hard-coding of file versions, etc. |
This commit ensures that we exclude files from the `Microsoft.NETCore.App` platform library on publish when the platform library is `Microsoft.AspNetCore.App`, which doesn't have an explicit dependency on `Microsoft.NETCore.App`. When publishing a 2.x ASP.NET application, the `MicrosoftNETPlatformLibrary` property gets changed by ASP.NET to `Microsoft.AspNetCore.App`. This causes the task responsible for determining what files to copy locally to treat files from `Microsoft.NETCore.App` and its dependencies as not being part of the platform. However, when publishing the application as framework-dependent, almost all the platform files get excluded thanks to conflict resolution which has the files as part of the platform manifest and preferred packages. Unfortunately, the apphost, hostpolicy, and hostfxr files are not part of these lists and as a result will be copied locally. This breaks framework-dependent apphost activation because hostfxr and hostpolicy are copied locally. Additionally, these files end up in the deps.json file which also prevents roll-forward activation if the two files are manually deleted. The fix is to treat `Microsoft.NETCore.App` as an implicit dependency of `Microsoft.AspNetCore.App` where we calculate the list of excluded packages for a framework-dependent publish. Fixes dotnet/cli#10602.
peterhuene
force-pushed
the
fix-conflict-detection
branch
from
e210d39 to
db1d373
Compare
peterhuene
changed the title
nguerrera
approved these changes
Jan 29, 2019
nguerrera
reviewed
Jan 29, 2019
… lib names. This commit treats `Microsoft.NETCore.App` as an implicit dependency for any platform library name, allowing the fix to work for both `Microsoft.AspNet.App` and `Microsoft.AspNet.All`.
nguerrera
approved these changes
Jan 30, 2019
|
@peterhuene let me know when this is ready for shiproom. |
|
@livarcocc I think it's ready. I'll try to be available for the next shiproom to help defend, but I may have to do so from my TEALS class. |
|
@peterhuene don't worry. I will talk to you tomorrow morning and get the information I need. Don't worry about it when you are in class. |
peterhuene
added
Approved to merge
Servicing-approved
and removed
Pending Shiproom Approval
labels
peterhuene
changed the title
|
This has been approved but is waiting for the branch to open for the appropriate release before merging. |
|
As the 2.2.1xx branch is now open for the next release, I'm merging this. |
dsplaisted
pushed a commit
to dsplaisted/sdk
that referenced
this pull request
Feb 19, 2020
….10 (dotnet#2881) - Microsoft.DotNet.Cli.Runtime - 3.1.100-preview1.19463.10
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit ensures that we exclude files from the
Microsoft.NETCore.Appplatform library on publish when the platform library is
Microsoft.AspNetCore.App, which doesn't have an explicit dependency onMicrosoft.NETCore.App.When publishing a 2.x ASP.NET application, the
MicrosoftNETPlatformLibraryproperty gets changed by ASP.NET to
Microsoft.AspNetCore.App. This causesthe task responsible for determining what files to copy locally to treat files
from
Microsoft.NETCore.Appand its dependencies as not being part of theplatform.
However, when publishing the application as framework-dependent, almost all the
platform files get excluded thanks to conflict resolution which has the files
as part of the platform manifest and preferred packages. Unfortunately, the
apphost, hostpolicy, and hostfxr files are not part of these lists and as a
result will be copied locally.
This breaks framework-dependent apphost activation because hostfxr and
hostpolicy are copied locally. Additionally, these files end up in the
deps.json file which also prevents roll-forward activation if the two files are
manually deleted.
The fix is to treat
Microsoft.NETCore.Appas an implicit dependency ofMicrosoft.AspNetCore.Appwhere we calculate the list of excluded packages fora framework-dependent publish.
Fixes dotnet/cli#10602.