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

CopyUpToDateMarker path too long #9346

Closed
JanKrivanek opened this issue Oct 19, 2023 · 9 comments · Fixed by #9387
Closed

CopyUpToDateMarker path too long #9346

JanKrivanek opened this issue Oct 19, 2023 · 9 comments · Fixed by #9387
Assignees
Labels
needs-triage Have yet to determine what bucket this goes in. Partner request

Comments

@JanKrivanek
Copy link
Member

reported by @olgaark:

We’ve recently had to change the IntDir (IntermediateOutputPath) default value for some vc projects (which made it longer by 17 chars max) and now hitting “CopyUpToDateMarker path too long” problem in some of our tests.

Bug 1900392: Unable to read the project file because the expression exceeds the OS max path limit

The item is added in MSBuild\Current\Bin\amd64\Microsoft.Common.CurrentVersion.targets

So two questions:

  1. Can the path be shortened by using short(er) file name? $(MSBuildProjectFile) can be quite long and using it in other paths is often problematic. VC is currently using ShortProjectName in path’s defaults. Can something similar or Windows short 8.3 file name be used in msbuild common targets for paths too?
  2. VC projects override GetTargetPathWithTargetPlatformMoniker target where it is used in the CurrentVersion.targets’ implementation. Do we need to add it in the VC’s override?

The item in question: https://github.com/dotnet/msbuild/blob/main/src/Tasks/Microsoft.Common.CurrentVersion.targets#L392

@JanKrivanek JanKrivanek added the needs-triage Have yet to determine what bucket this goes in. label Oct 19, 2023
@AR-May
Copy link
Member

AR-May commented Oct 24, 2023

MSBuild team triage: @drewnoakes Do you know of any reason we could not change this file name?

@drewnoakes
Copy link
Member

I can't think of a technical reason not to change the name. The .NET Project System obtains the name via the CopyUpToDateMarker property, so will respond to any changes correctly.

That said, that approach is not a real fix. What component is hitting this limit, and can it be changed to support long paths properly?

@JanKrivanek
Copy link
Member Author

@drewnoakes Thank you for confirming.
Yes - this is a short/mid term workaround untill devenv onboards onto long path support.
At least I suppose the scenario where this is happening involves the in-proc MSBuild node in VS - is that correct @olgaark?

@olgaark
Copy link
Collaborator

olgaark commented Nov 2, 2023

Yes, this happens during evaluation, so project load in the IDE is affected. And yes, the proposed change is just to reduce the probability that people will hit the limit when long path are not supported.

@JanKrivanek
Copy link
Member Author

I'm wondering whether we can do this in reasonable and correct way.
VC uses a $(ProjectGuid) to generate the short name: (source):

<PropertyGroup Condition ="'$(ShortProjectName)' == '' and '$(ProjectName)' != '' and '$(ProjectGuid)' != ''">
    <ShortProjectName>$(ProjectName)</ShortProjectName>
    <ShortProjectName Condition="'$(ProjectName.Length)' &gt; '16'">$(ProjectName.Substring(0,8)).$(ProjectGuid.Substring(1,8))</ShortProjectName>
</PropertyGroup>

However ProjectGuid is no longer required/generated with the CPS. Relaying only on the shortened filename (or even MSBuildProjectName in order to get rid at least of the extension) can lead to collisions - so we'd need something uniquifying. Content hash can come to the rescue, but it's unreasonable (perf impact).

So we can possbily shorten the marker in case the ProjectGuid exists - to remedy just the non-CPS cases - e.g. like this: #9387 - but it rather feels as a nonsystematic hack (what about CPS? What about $(CleanFile), $(SuggestedBindingRedirectsCacheFile), @(CustomAdditionalCompileInputs ) etc.?)

Thoughts on this @rainersigwald?

@drewnoakes
Copy link
Member

Within a solution, a max length project name prefix cannot be considered unique. Project names are often hierarchical and only differ at the end.

@rainersigwald
Copy link
Member

That said, that approach is not a real fix. What component is hitting this limit, and can it be changed to support long paths properly?

It's VS/devenv.exe itself, which is tracked by https://developercommunity.visualstudio.com/idea/351628/allow-building-running-and-debugging-a-net-applica.html

@JanKrivanek
Copy link
Member Author

Within a solution, a max length project name prefix cannot be considered unique. Project names are often hierarchical and only differ at the end.

Correct.
In a presence of ProjectGuid it can be uniquidied by that. Without ProjectGuid it would not be shortened. (#9387)
That means we can fix the particular problem here (CopyUpToDateMarker path too long for VC project), but other scenarios (CPS, other obj metafiles etc.) are left untouched - so question is whether we want such a 'tactical' change

@KalleOlaviNiemitalo
Copy link

It would be possible to generate a GUID by hashing the project name, when ProjectGuid is not defined.

@JanKrivanek JanKrivanek self-assigned this Nov 3, 2023
rainersigwald pushed a commit that referenced this issue Dec 12, 2023
…entVersion.targets (#9520)

#9387 introduced improved hashing for [`[MSBuild]::StableStringHash`](https://learn.microsoft.com/en-us/visualstudio/msbuild/property-functions?view=vs-2022#msbuild-stablestringhash), that however broke internal functionality relying on the hash to be stable between versions (despite documented otherwise).

### Changes Made
 * Reverted all the hashing changes
 * Kept the UTD marker change (fixing #9346)

### Note
Longer term fix: #9519
ladipro pushed a commit that referenced this issue Apr 25, 2024
Fixes #10066
[GB18030] Failed to run the WebAPI project that named as GB18030 provided level3 strings

### Context
OS: Windows 11 Enterprise 22H2 ZH-CN
Affected Build: 9.0.0-preview.4.24209.5, Aso repro on 8.0.300-preview.24203.14(8.0.2)

Steps to reproduce:

Use CLI to create WebAPI project copying GB18030 characters, for example:
```
dotnet new webapi -controllers -o 㐇𠁠𪨰𫠊𫦠𮚮⿕#
cd 㐇𠁠𪨰𫠊𫦠𮚮⿕#
dotnet run
```

Without this PR:
```
dotnet run
Building...
Restore complete (0.5s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  㐇𠁠𪨰𫠊𫦠��⿕# failed with 2 error(s) (2.9s) → bin/Debug/net9.0/㐇𠁠𪨰𫠊𫦠��⿕#.dll
    /workspaces/runtime/.dotnet/sdk/9.0.100-preview.3.24204.13/Microsoft.Common.CurrentVersion.targets(5578,5): error MSB3491: Could not write lines to file "obj/Debug/net9.0/㐇𠁠𪨰𫠊𫦠��⿕#.csproj.FileListAbsolute.txt". Unable to translate Unicode character \\uD86E at index 2368 to specified code page.
    /workspaces/runtime/.dotnet/sdk/9.0.100-preview.3.24204.13/Microsoft.Common.CurrentVersion.targets(5823,5): error MSB3491: Could not write lines to file "obj/Debug/net9.0/㐇𠁠𪨰𫠊𫦠��⿕#.csproj.FileListAbsolute.txt". Unable to translate Unicode character \\uD86E at index 2368 to specified code page.

Build failed with 2 error(s) in 3.8s

The build failed. Fix the build errors and run again.
```

With this PR:
```
dotnet run
Building...
Restore complete (0.5s)
You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
  㐇𠁠𪨰𫠊𫦠��⿕# succeeded (0.6s) → bin/Debug/net9.0/㐇𠁠𪨰𫠊𫦠��⿕#.dll

Build succeeded in 1.5s
info: Microsoft.Hosting.Lifetime[14]
      Now listening on: http://localhost:5210
info: Microsoft.Hosting.Lifetime[0]
      Application started. Press Ctrl+C to shut down.
info: Microsoft.Hosting.Lifetime[0]
      Hosting environment: Development
info: Microsoft.Hosting.Lifetime[0]
      Content root path: /workspaces/reproGB/㐇𠁠𪨰𫠊𫦠��⿕#
^Cinfo: Microsoft.Hosting.Lifetime[0]
      Application is shutting down...
```

### Changes Made
Changed the way we create a substring to prevent cutting surrogates in half.

### Testing
Added a test for new substring function.

### Regerssion
Yes, after #9346.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-triage Have yet to determine what bucket this goes in. Partner request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants