dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.23k stars 1.35k forks source link

Inconsistent slashes when creating an asp.net core project #1874

Closed sayedihashimi closed 7 years ago

sayedihashimi commented 7 years ago

After creating an ASP.NET Core project I added a target to print out the Content items. After executing that target I get the following results.

  Content: wwwroot/css\site.css
wwwroot/css\site.min.css
wwwroot/favicon.ico
wwwroot/file.ts
wwwroot/images\banner1.svg
wwwroot/images\banner2.svg
wwwroot/images\banner3.svg
wwwroot/images\banner4.svg
wwwroot/js\site.js
wwwroot/js\site.min.js
wwwroot/lib\bootstrap\.bower.json
wwwroot/lib\bootstrap\dist\css\bootstrap-theme.css
wwwroot/lib\bootstrap\dist\css\bootstrap-theme.css.map
wwwroot/lib\bootstrap\dist\css\bootstrap-theme.min.css
wwwroot/lib\bootstrap\dist\css\bootstrap-theme.min.css.map
wwwroot/lib\bootstrap\dist\css\bootstrap.css
wwwroot/lib\bootstrap\dist\css\bootstrap.css.map
wwwroot/lib\bootstrap\dist\css\bootstrap.min.css
wwwroot/lib\bootstrap\dist\css\bootstrap.min.css.map
wwwroot/lib\bootstrap\dist\fonts\glyphicons-halflings-regular.eot
wwwroot/lib\bootstrap\dist\fonts\glyphicons-halflings-regular.svg
wwwroot/lib\bootstrap\dist\fonts\glyphicons-halflings-regular.ttf
wwwroot/lib\bootstrap\dist\fonts\glyphicons-halflings-regular.woff
wwwroot/lib\bootstrap\dist\fonts\glyphicons-halflings-regular.woff2
wwwroot/lib\bootstrap\dist\js\bootstrap.js
wwwroot/lib\bootstrap\dist\js\bootstrap.min.js
wwwroot/lib\bootstrap\dist\js\npm.js
wwwroot/lib\bootstrap\LICENSE
wwwroot/lib\jquery-validation-unobtrusive\.bower.json
wwwroot/lib\jquery-validation-unobtrusive\jquery.validate.unobtrusive.js
wwwroot/lib\jquery-validation-unobtrusive\jquery.validate.unobtrusive.min.js
wwwroot/lib\jquery-validation\.bower.json
wwwroot/lib\jquery-validation\dist\additional-methods.js
wwwroot/lib\jquery-validation\dist\additional-methods.min.js
wwwroot/lib\jquery-validation\dist\jquery.validate.js
wwwroot/lib\jquery-validation\dist\jquery.validate.min.js
wwwroot/lib\jquery-validation\LICENSE.md
wwwroot/lib\jquery\.bower.json
wwwroot/lib\jquery\dist\jquery.js
wwwroot/lib\jquery\dist\jquery.min.js
wwwroot/lib\jquery\dist\jquery.min.map
wwwroot/lib\jquery\LICENSE.txt
wwwroot/test1.ts
...

The slashes here are not consistent. This is not a big deal because MSBuild will normalize these internally but if I have a custom task then the slashes not being consistent can cause issues I believe. I'd prefer these slashes to be consistent.

I think this is being caused by the globbing patterns for including wwwroot files.

rainersigwald commented 7 years ago

@cdmihai would you consider this a duplicate of #1026?

cdmihai commented 7 years ago

More specific issue might be #1622. #1026 is the more general issue.

@sayedihashimi can you please point me to the msbuild snippet that constructs the Content item? Is it coming from a glob?

sayedihashimi commented 7 years ago

I saw the expressions by using /pp when calling msbuild.exe. It looks to be coming from the .targets file below. I've also included the first snippet itself for Content.

  <!--
============================================================================================================================================
  <Import Project="$(MSBuildThisFileDirectory)..\build\netstandard1.0\Microsoft.NET.Sdk.Web.ProjectSystem.props" Condition="Exists('$(MSBuildThisFileDirectory)..\build\netstandard1.0\Microsoft.NET.Sdk.Web.ProjectSystem.props')">

C:\Program Files\Microsoft Visual Studio\2017\Enterprise\MSBuild\Sdks\Microsoft.NET.Sdk.Web.ProjectSystem\build\netstandard1.0\Microsoft.NET.Sdk.Web.ProjectSystem.props
============================================================================================================================================
-->
    <!-- Publish everything under wwwroot, all JSON files, all web.config files and all Razor files -->
    <Content Include="wwwroot/**" CopyToPublishDirectory="PreserveNewest" Exclude="$(DefaultItemExclude
cdmihai commented 7 years ago

Ah, I remember now. We have a unit test for this exact behaviour: https://github.com/Microsoft/msbuild/blob/master/src/Build.OM.UnitTests/Definition/ProjectItem_Tests.cs#L773

When we were fixing the globbing code to work crossplat we found this behaviour where the slash of the fixed directory part was blindly prepended to the expanded wild card part. We decided to treat it as legacy behaviour and leave it there.

I guess we could just ... "fix" it and remove the test. Opinions?

sayedihashimi commented 7 years ago

When we were fixing the globbing code to work crossplat we found this behaviour where the slash of the fixed directory part was blindly prepended to the expanded wild card part. We decided to treat it as legacy behaviour and leave it there.

In this case I'm suggesting that the Content item be changed to

<Content Include="wwwroot\**" CopyToPublishDirectory="PreserveNewest" Exclude="$(DefaultItemExclude

As opposed to a core MSBuild change here.

cdmihai commented 7 years ago

In this case I'm suggesting that the Content item be changed

As far as I can remember, the slashes in the recursive directory part are the ones generated by Directory.GetDirectories so depending on the OS they will either be back or forward slashes. This means that whatever slash orientation the user chooses for the fixed directory part, it will be inconsistent on either windows or !windows.

I am leaning towards just changing the user's slash to fit the rest. The alternative is to change the recursive slashes to fit the user's slashes, but that will likely cause it to crash on !windows where back-slash is a valid file character. (I remember an early .net core bug where nuget created a file called ~\.nuget\cache, or something like that)

sayedihashimi commented 7 years ago

so depending on the OS they will either be back or forward slashes.

Ok I see. Maybe we should just leave things as they are then.

AndyGerlicher commented 7 years ago

Team Triage: We think that it would be safe to normalize paths in this case where strings come from a glob expansion.