Copy App-Host for a self-contained app with extension only on windows #1378
Conversation
This file contains hidden or 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
livarcocc
force-pushed
the
fix_publishing_self_contained_apps_with_dots_in_name
branch
2 times, most recently
from
0a2fad0 to
43529b9
Compare
|
@dotnet-bot Test Windows_NT Debug |
nguerrera
suggested changes
Jun 30, 2017
| <ResolvedFileToPublish Include="%(NativeAppHostNETCore.Identity)"> | ||
| <RelativePath>$(AssemblyName)%(Extension)</RelativePath> | ||
| <RelativePath>$(AssemblyName)</RelativePath> | ||
| <RelativePath Condition=" $(RuntimeIdentifier.StartsWith('win')) ">$(AssemblyName)%(Extension)</RelativePath> |
There was a problem hiding this comment.
It is odd that %(Extension) is already the extension of the assembly name here.
I debugged it and it is because NativeAppHostNETCore already gets the name of the app here.
But that is only for 2.0+ apps. The code is quite convoluted. :(
I think the better targeted fix here is:
<ResolvedFileToPublish Include="%(NativeAppHostNETCore.Identity)">
- <RelativePath>$(AssemblyName)%(Extension)</RelativePath>
+ <RelativePath>$(AssemblyName)$(_NativeExecutableExtension)</RelativePath>That at least leaves the handling of executable extensions by platform in one place.
There was a problem hiding this comment.
I agree. I wrote that to you in IM 😄.
…nsion when publishing for windows. Otherwise, for apps with a . in the name, the part that comes after the . ends up being used as an extension and the executable has a duplicated last piece of the name, like sample.console.console, instead of sample.console.
livarcocc
force-pushed
the
fix_publishing_self_contained_apps_with_dots_in_name
branch
from
43529b9 to
3748d66
Compare
nguerrera
approved these changes
Jun 30, 2017
livarcocc
deleted the
fix_publishing_self_contained_apps_with_dots_in_name
branch
|
@livarcocc When will this be released? I just downloaded .NET Core 2.0 from https://www.microsoft.com/net/download/core yesterday and it has this issue. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Customer scenario
Publishing self-contained apps that have a "." in their name ends up publishing an executable with the wrong name.
When copying the App-Host for a self-contained app, only use the extension when publishing for windows. Otherwise, for apps with a . in the name, the part that comes after the . ends up being used as an extension and the executable has a duplicated last piece of the name, like sample.console.console, instead of sample.console.
Bugs this fixes:
Fixes https://github.com/dotnet/cli/issues/6397
Workarounds, if any
Change the name of your application or hack some msbuild to change the file name after it is copied. It is bad.
Risk
Low
Performance impact
None
Is this a regression from a previous update?
Yes.
Root cause analysis:
Lack of test coverage.
How was the bug found?
Reported by the community.
@livarcocc @dotnet/dotnet-cli for review
@MattGertz for approval