Skip to content
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

Directory separator for TargetPath is not normalized #10627

Open
talboren opened this issue Jan 28, 2020 · 8 comments
Open

Directory separator for TargetPath is not normalized #10627

talboren opened this issue Jan 28, 2020 · 8 comments

Comments

@talboren
Copy link

@talboren talboren commented Jan 28, 2020

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

  • ASP.NET Core version: mcr.microsoft.com/dotnet/core/runtime-deps:3.1-alpine (3.1.101),
  • Include the output of dotnet --info: No runtime is installed, app is self-contained
  • The IDE (VS / VS Code/ VS4Mac) you're running on, and it's version: Microsoft Visual Studio Professional 2019 Version 16.4.3
@pranavkm pranavkm transferred this issue from dotnet/aspnetcore Jan 28, 2020
@jeffschwMSFT

This comment has been minimized.

Copy link
Member

@jeffschwMSFT jeffschwMSFT commented Jan 30, 2020

@swaroop-sridhar swaroop-sridhar self-assigned this Jan 30, 2020
@swaroop-sridhar

This comment has been minimized.

Copy link
Contributor

@swaroop-sridhar swaroop-sridhar commented Jan 31, 2020

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>
@swaroop-sridhar

This comment has been minimized.

Copy link
Contributor

@swaroop-sridhar swaroop-sridhar commented Jan 31, 2020

CC: @lpereira

@talboren

This comment has been minimized.

Copy link
Author

@talboren talboren commented Feb 3, 2020

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

  • The NServiceBus.Persistence.Sql directory is created during the build process using an MSBuild task so that might be the difference between your example and the reproducible project I've uploaded.
@swaroop-sridhar

This comment has been minimized.

Copy link
Contributor

@swaroop-sridhar swaroop-sridhar commented Feb 11, 2020

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

  • The NServiceBus.Persistence.Sql package includes a target that generates an item with a TargetPath that includes both \ and / in its path here
    • Using a / here would circumvent the problem.
    • Using mixed syntax is not a great idea -- a\b is a valid directory name on Linux.
  • The SDK doesn't normalize the TargetPath which is probably an SDK bug. (CC: @dsplaisted)
  • The Single-file bundler in HostModel uses the TargetPath passed in. The helper libraries used by the SDK shouldn't themselves normalize the path.
@swaroop-sridhar swaroop-sridhar changed the title Buggy extraction mechanism in the Single-file executable feature introduced in .NET Core 3.0 Directory separator for TargetPath is not normalized Feb 11, 2020
@swaroop-sridhar

This comment has been minimized.

Copy link
Contributor

@swaroop-sridhar swaroop-sridhar commented Feb 11, 2020

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

Thanks.

@swaroop-sridhar swaroop-sridhar removed their assignment Feb 11, 2020
@dsplaisted dsplaisted transferred this issue from dotnet/runtime Feb 11, 2020
@dasMulli

This comment has been minimized.

Copy link
Contributor

@dasMulli dasMulli commented Feb 12, 2020

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

This comment has been minimized.

Copy link
Member

@ryanbrandenburg ryanbrandenburg commented Feb 13, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.