-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Update runtimeconfig.json and deps.json paths when these break past the MAX_PATH threshold #56224
Conversation
Tagging subscribers to this area: @vitek-karas, @agocke, @VSadov Issue DetailsWe use the path of an app to build the paths of the runtimeconfig.dev.json, runtimeconfig.json and deps.json files. If the length of the app's path is behind the MAX_PATH threshold (< 260) but close enough ([246-251]) so that the length of any of the above paths break this limit, the app breaks with:
This is because we check for the existence of the above paths (which do exist) but we don't update them if we find that they need to be treated as long paths. Fixes #53223.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. But I'm wondering if there are other places where we might have a similar problem...
Could you maybe look through the code base a little bit and see if there's something "interesting"?
// is just 1 char behind MAX_PATH (260) so that the runtimeconfig(.dev).json files | ||
// break this threshold. This will cause hostfxr to normalize these paths -- here we | ||
// are checking that the updated paths are used. | ||
var dirName = new string('a', 259 - outputDir.Length - appExeName.Length - 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: In theory this would break if the location of the repo was in a too deep. Maybe we should create a folder in temp (which is basically guaranteed to the short enough on all systems).
We do have other tests creating long paths, but those don't need specific path length, they just need path longer than something. This test needs a specific path length and so the arithmetic above could end up with negative size (and the test will fail).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this totally relies on the user having the repo on a path not so far from root. Wasn't sure if it was good to put this on %TMP%, now I remember that we do that for bundling tests... changing this.
I'll have a look at all places where we call |
Fix return value of realpath
…bug_tests * origin/main: (274 commits) Disable test ConnectWithCertificateForDifferentName_Throws (dotnet#56456) Update dependencies from https://github.com/mono/linker build 20210726.2 (dotnet#56374) Cleanup disabled test conditions (dotnet#56381) [mono] Add GC unsafe transition to mono_unhandled_exception (dotnet#56380) don't fail the file extraction when we can't set last write time (dotnet#56370) Properly rebuild optimization data when it changes (dotnet#56397) Make open function calls in coreclr EINTR resilient on macOS (dotnet#56403) Fix dependency from EventLog to TraceSource ref (dotnet#56417) Fix comments in asm with JitDiffableDasm=1 (dotnet#56416) Catch TcpClient ctor exceptions in FtpWebRequest.CreateConnectionAsync (dotnet#56379) Add interop between serializer and DOMs (dotnet#56112) Fix type loader not recognizing overridden method (dotnet#56337) Prevent Segfault in EventPipe on disable (dotnet#56104) Update runtimeconfig.json and deps.json paths when these break past the MAX_PATH threshold (dotnet#56224) Use native allocator instead of pinning when decompressing embedded PDB (dotnet#56336) Specify win-x64 as a valid platform in the microsoft-net-runtime-* workloads for iOS/tvOS/MacCatalyst (dotnet#56311) Fix FailFast message formatting race (dotnet#56388) Try to fix finalizer-based async tests (dotnet#56384) Fix MetricsEventSource tests (dotnet#56382) Remove invalid Castle.DynamicProxy.Internal.AbstractInvocation from ILLink descriptor files (dotnet#56392) ...
* origin/main: (95 commits) Disable test ConnectWithCertificateForDifferentName_Throws (dotnet#56456) Update dependencies from https://github.com/mono/linker build 20210726.2 (dotnet#56374) Cleanup disabled test conditions (dotnet#56381) [mono] Add GC unsafe transition to mono_unhandled_exception (dotnet#56380) don't fail the file extraction when we can't set last write time (dotnet#56370) Properly rebuild optimization data when it changes (dotnet#56397) Make open function calls in coreclr EINTR resilient on macOS (dotnet#56403) Fix dependency from EventLog to TraceSource ref (dotnet#56417) Fix comments in asm with JitDiffableDasm=1 (dotnet#56416) Catch TcpClient ctor exceptions in FtpWebRequest.CreateConnectionAsync (dotnet#56379) Add interop between serializer and DOMs (dotnet#56112) Fix type loader not recognizing overridden method (dotnet#56337) Prevent Segfault in EventPipe on disable (dotnet#56104) Update runtimeconfig.json and deps.json paths when these break past the MAX_PATH threshold (dotnet#56224) Use native allocator instead of pinning when decompressing embedded PDB (dotnet#56336) Specify win-x64 as a valid platform in the microsoft-net-runtime-* workloads for iOS/tvOS/MacCatalyst (dotnet#56311) Fix FailFast message formatting race (dotnet#56388) Try to fix finalizer-based async tests (dotnet#56384) Fix MetricsEventSource tests (dotnet#56382) Remove invalid Castle.DynamicProxy.Internal.AbstractInvocation from ILLink descriptor files (dotnet#56392) ...
We use the path of an app to build the paths of the runtimeconfig.dev.json, runtimeconfig.json and deps.json files. If the length of the app's path is behind the MAX_PATH threshold (< 260) but close enough ([246-251]) so that the length of any of the above paths break this limit, the app breaks with:
This is because we check for the existence of the above paths (which do exist) but we don't update them if we find that they need to be treated as long paths.
Fixes #53223.