dotnet / sdk

Core functionality needed to create .NET Core projects, that is shared between Visual Studio and CLI
https://dot.net/core
MIT License
2.67k stars 1.06k forks source link

Directory separator for TargetPath is not normalized #10627

Open ghost opened 4 years ago

ghost commented 4 years ago

Describe the bug

My application is using NServiceBus as the implementation to the Service Bus pattern.

NServiceBus generates several SQL scripts for its persistence mechanism and then executes those script when starting up (to create the necessary tables), these scripts are generated by an MSBuild task at compile time. The scripts are generated in to a folder called NServiceBus.Persistence.Sql which then has a few more folders for the relevant database types (e.g: MsSqlServer, MySql etc..) as you can see here:

image

NServiceBus will then search for the specific files under NServiceBus.Persistence.Sql directory at the parent app directory - at this point everything seems to be working fine.

The problem begins when moving to a "Single File Executable" with the "PublishSingleFile" parameter set. After reading https://github.com/dotnet/designs/blob/master/accepted/single-file/extract.md it seems like I understood the way the extraction mechanism works but it seems like there's a buggy situation where folders that contains folders inside get extracted faulty (when looking at "/var/tmp/.net/AppName/bov1qdmu.xn3/":

image

and the following error message appears:

appname_1 | 2020-01-27 16:49:54.790 INFO Directory '/var/tmp/.net/AppName/bov1qdmu.xn3/NServiceBus.PersistenceSql/MySql/Sagas' not found so no saga creation scripts will be executed.

To Reproduce

Having a project with NServiceBus using persistence mechanism, reproduce this issue easily by just switching from: RUN dotnet publish AppName.csproj -c Release -r linux-musl-x64 -o /app To RUN dotnet publish AppName.csproj -c Release -r linux-musl-x64 /p:PublishSingleFile=true -o /app

Further technical details

jeffschwMSFT commented 4 years ago

@swaroop-sridhar

swaroop-sridhar commented 4 years ago

Hi @talboren, I tried out a simple app with content within subdirectories, and it seems to extract fine. Can you please share the build logs of the failure (at least the portions relevant to copying the NServiceBus.Persistence.Sql content)?

Here's the repro I tried, can you please see if you can repro the failure with this test?

swaroop-sridhar commented 4 years ago

CC: @lpereira

ghost commented 4 years ago

Hi @talboren, I tried out a simple app with content within subdirectories, and it seems to extract fine. Can you please share the build logs of the failure (at least the portions relevant to copying the NServiceBus.Persistence.Sql content)?

Here's the repro I tried, can you please see if you can repro the failure with this test?

  • Start with a template console app (dotnet new console)
  • Create the NServiceBus.Persistence.Sql directory with the three sub-directories and add some content
  • The csproj file is:
<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp3.0</TargetFramework>
  </PropertyGroup>
  <ItemGroup>
    <Content Include="NServiceBus.Persistence.Sql/**">
      <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
    </Content>
  </ItemGroup>
</Project>

Hey @swaroop-sridhar I was able to reproduce this issue in a simple project I've created using real NServiceBus persistence. Please use the Dockerfile in order to build and run the docker image, after that's done, please take a look at the /var/tmp/.net/Swaroop-2288/.../ directory that is created in that container.

Attached you can find the project I've created. Swaroop-2288.zip

swaroop-sridhar commented 4 years ago

@talboren thanks for the repro. I looked at the failure. The problem here is that:

swaroop-sridhar commented 4 years ago

@dsplaisted can you please move this issue to the SDK? Here is the build log

Thanks.

dasMulli commented 4 years ago

MSBuild itself handles mixed slashes when needed - this is good b/c paths are generally composed upon by different parts of the build scripts authored by different people who can choose between the styles.

MSBuild also needs to preserve the original specification for item identity because items may or may not refer to files on disk, and in case it does not (<FooSpec Include="Foo/1.2.3">) then the original characters need to be preserved for logic parsing the specification. (granted there are a lot of edge cases when you want to use non-file strings that look a lot like paths - e.g. http URLs)

This is why it should be up to the consuming logic (individual MSBuild tasks) to interpret item specifications as file paths or just strings and thus run the proper normalization (i'm not sure if MSBuild has a public API for that though).

The edge cases here is: If an item is created that also happens to match a file on disk, should MSBulid auotmatically normalize its specification? It could, but may have edge cases when it does so accidentally - so we're looking at a potentially breaking change that would be nearly impossible to work around unless there was an env var escape hatch.

ryanbrandenburg commented 4 years ago

We ran into a similar issue while working on RazorTooling. We found that the TargetPath output by AssignTargetPath did not normalize / to \ as we expected based on our understanding that TargetPath must always use \. I wonder if these are connected given that publishing uses AssignTargetPath?

Does anyone here know if:

  1. My guess that these are connected is correct?
  2. Our assumptions about the "contract" of TargetPath is correct?

(Here's a snip of the BinLog from an affected run) TargetPathNormalization