Fix Copy task MSB3030 failure for long-path files in non-longPathAware processes#13697
Conversation
There was a problem hiding this comment.
Pull request overview
This PR aims to make MSBuild's long-path handling work in non-longPathAware Windows hosts by introducing an extended-length path helper and applying it to GetFileAttributesEx/File.Copy, plus adding regression tests around FileState and Copy.
Changes:
- Added
NativeMethods.EnsureExtendedLengthPath()and routed several Windows file-attribute probes through it. - Updated
CopyandFileStateto use the helper for long-path scenarios. - Added regression tests covering long-path
FileStateexistence checks andCopytask behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
src/Tasks/FileState.cs |
Uses extended-length paths for native file attribute lookup in FileState. |
src/Tasks/Copy.cs |
Uses extended-length paths for the direct File.Copy call in the Copy task. |
src/Tasks.UnitTests/FileStateTests.cs |
Adds a Windows long-path regression test for FileState.FileExists. |
src/Tasks.UnitTests/Copy_Tests.cs |
Adds a Windows long-path regression test for Copy. |
src/Framework/NativeMethods.cs |
Introduces the shared extended-length path helper and applies it to several native existence/timestamp probes. |
| internal static string EnsureExtendedLengthPath(string path) | ||
| { | ||
| if (!IsWindows || path == null || path.Length < MAX_PATH || | ||
| path.StartsWith(@"\\?\", StringComparison.Ordinal)) | ||
| { | ||
| return path; | ||
| } | ||
|
|
||
| return path.StartsWith(@"\\", StringComparison.Ordinal) | ||
| ? @"\\?\UNC\" + path.Substring(2) | ||
| : @"\\?\" + path; |
| return path.StartsWith(@"\\", StringComparison.Ordinal) | ||
| ? @"\\?\UNC\" + path.Substring(2) | ||
| : @"\\?\" + path; |
| File.Copy( | ||
| NativeMethodsShared.EnsureExtendedLengthPath(sourceFileState.Path), | ||
| NativeMethodsShared.EnsureExtendedLengthPath(destinationFileState.Path), | ||
| true); |
| if (longFilePath.Length <= NativeMethodsShared.MAX_PATH) | ||
| { | ||
| return; // path not long enough on this machine; nothing to test |
| [Fact] | ||
| public void ExistsWithLongPath() | ||
| { | ||
| if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
| { | ||
| return; // long-path Win32 behaviour is Windows-only | ||
| } |
| [Fact] | ||
| public void CopyFileWithLongPath() | ||
| { | ||
| if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) | ||
| { | ||
| return; | ||
| } | ||
|
|
| File.Copy( | ||
| NativeMethodsShared.EnsureExtendedLengthPath(sourceFileState.Path), | ||
| NativeMethodsShared.EnsureExtendedLengthPath(destinationFileState.Path), | ||
| true); |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
MSBuild Code Review — PR #13697: Extended-Length Path (\\?\) Support
Summary
This PR addresses a real pain point — MSB3030 errors when copying long-path files in non-longPathAware hosts like devenv.exe. The core idea of prepending \\?\ to paths ≥ MAX_PATH before Win32 API calls is sound. However, the fix is incomplete in several dimensions that could lead to partial failures, and the EnsureExtendedLengthPath method has a correctness gap around path normalization.
🔴 Critical Issues
1. EnsureExtendedLengthPath doesn't validate or normalize paths
The \\?\ prefix disables ALL Win32 path normalization — forward slashes, ./.. segments, relative paths, and trailing spaces are all passed literally. If a path contains / separators (common in cross-platform MSBuild code) or is relative, prepending \\?\ produces an invalid path. At minimum, add Path.IsPathRooted() guard and Replace('/', '\\'). See inline comment for suggested fix.
2. Missed call sites in NativeMethods.cs
GetLastWriteDirectoryUtcTime(line 998):GetFileAttributesEx(fullPath, ...)— not wrapped.OpenFileThroughSymlinks(line 1229):CreateFile(fullPath, ...)— not wrapped. This breaksGetContentLastWriteFileUtcTimefor long-path symlinks.
3. Incomplete fix in the Copy task pipeline
Only File.Copy is wrapped, but the surrounding operations are not:
Directory.CreateDirectory(line 299) — fails on net472 for long pathsFileUtilities.DeleteNoThrow(line 325) — callsFile.Deletewithout prefixFile.SetAttributesinMakeFileWriteable(line 423) — fails for long-path destinationsCreateHardLink/CreateSymbolicLinkinTryCopyViaLink(line 405) — Win32 APIs with MAX_PATH limits
This means a copy can succeed but post-copy fixup (setting attributes, pre-copy deletion) will fail, potentially leaving files in an inconsistent state.
🟡 Medium Issues
4. No unit tests for EnsureExtendedLengthPath itself
The method has 5+ code paths (null, short, already-prefixed, UNC, normal) but no isolated unit tests. Only integration tests exist. A parameterized [Theory] test would be easy to add and would catch regressions in the prefix logic.
5. Tests may silently pass without testing anything
Both tests have if (path.Length <= MAX_PATH) return; guards. On CI runners with short temp paths, the tests exit early without exercising the fix. Consider constructing paths that guarantee they exceed MAX_PATH.
6. Tests should use [WindowsOnlyFact] instead of runtime platform checks
Both test files already use [WindowsOnlyFact] elsewhere. The runtime if (!IsOSPlatform) return; pattern silently skips rather than marking the test as skipped in test results.
🟢 Minor / Nits
- Use
is nullinstead of== nullper MSBuild style guide. FileStateTests.ExistsWithLongPathcleanup usesDirectory.Delete(longDir, recursive: false)which will throw if the file deletion silently failed.- Consider whether
MSBuildTaskHost/Utilities/NativeMethods.cs(the .NET Framework task host copy) also needs the fix.
ChangeWave Consideration
Since this fixes a bug (paths that previously failed now work), this is arguably a pure bug fix rather than a behavioral change. However, the \\?\ prefix semantics are subtly different from normal paths (no normalization, case-sensitive on some filesystem configs). If there's any concern about compatibility, gating behind a ChangeWave would be prudent.
Overall Assessment
Good direction, but the fix needs to be more comprehensive to avoid partial-failure scenarios in the copy pipeline, and EnsureExtendedLengthPath needs path normalization to be safe.
Note
🔒 Integrity filter blocked 1 item
The following item were blocked because they don't meet the GitHub integrity level.
- #13697
pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".
To allow these resources, lower min-integrity in your GitHub frontmatter:
tools:
github:
min-integrity: approved # merged | approved | unapproved | noneGenerated by Expert Code Review (on open) for issue #13697 · ● 5.7M
| try | ||
| { | ||
| if (sourcePath.Length <= NativeMethodsShared.MAX_PATH) |
There was a problem hiding this comment.
Fragile test: early return if path is not long enough
The if (sourcePath.Length <= NativeMethodsShared.MAX_PATH) return; guard means this test silently passes without testing anything if the temp path is shorter than expected. This will happen on CI runners with short temp paths. Consider asserting that the path is long enough — or constructing the path to guarantee it exceeds MAX_PATH (e.g., by nesting enough subdirectories).
| internal static string EnsureExtendedLengthPath(string path) | ||
| { | ||
| if (!IsWindows || path == null || path.Length < MAX_PATH || | ||
| path.StartsWith(@"\\?\", StringComparison.Ordinal)) | ||
| { | ||
| return path; | ||
| } | ||
|
|
||
| return path.StartsWith(@"\\", StringComparison.Ordinal) | ||
| ? @"\\?\UNC\" + path.Substring(2) | ||
| : @"\\?\" + path; |
There was a problem hiding this comment.
Bug: \\?\ prefix requires fully-qualified absolute paths
The \\?\ prefix disables all Win32 path normalization — including forward-slash-to-backslash conversion, removal of ./.. segments, and relative path resolution. If path is relative or contains / separators (which MSBuild allows and normalizes elsewhere), prepending \\?\ will produce an invalid path that Win32 will reject with ERROR_INVALID_NAME.
Suggestion: add a guard for rooted paths and normalize separators:
internal static string EnsureExtendedLengthPath(string path)
{
if (!IsWindows || path is null || path.Length < MAX_PATH ||
path.StartsWith(@"\\?\", StringComparison.Ordinal))
{
return path;
}
if (!Path.IsPathRooted(path))
{
return path; // \\?\ requires absolute paths
}
// \\?\ disables Win32 path normalization; ensure backslashes.
path = path.Replace('/', '\\');
return path.StartsWith(@"\\", StringComparison.Ordinal)
? @"\\?\UNC\" + path.Substring(2)
: @"\\?\" + path;
}Also consider guarding against \\.\ device paths — e.g. path.StartsWith(@"\\.\", StringComparison.Ordinal) — though these are unlikely to exceed MAX_PATH in practice.
| /// </summary> | ||
| internal static string EnsureExtendedLengthPath(string path) | ||
| { | ||
| if (!IsWindows || path == null || path.Length < MAX_PATH || |
There was a problem hiding this comment.
Nit: Prefer is null pattern
Per MSBuild coding conventions, use is null / is not null instead of == null / != null:
if (!IsWindows || path is null || path.Length < MAX_PATH ||| /// Regression test: MSB3030 when copying a long-path file in a multi-targeting build. | ||
| /// devenv.exe is not longPathAware; the net472 test host has the same condition. | ||
| /// </summary> | ||
| [Fact] |
There was a problem hiding this comment.
Use [WindowsOnlyFact] instead of runtime platform check
This file already uses [WindowsOnlyFact] (lines 2663, 2769) and [WindowsOnlyTheory] (lines 1755, 2567) for Windows-specific tests. Per MSBuild testing conventions, prefer platform-conditional attributes over runtime if checks that silently skip assertions:
[WindowsOnlyFact(additionalMessage: "Extended-length path (\\\\?\\) support is Windows-only.")]
public void CopyFileWithLongPath()
{
// ... (remove the RuntimeInformation check)Same applies to ExistsWithLongPath in FileStateTests.cs.
| File.Copy( | ||
| NativeMethodsShared.EnsureExtendedLengthPath(sourceFileState.Path), | ||
| NativeMethodsShared.EnsureExtendedLengthPath(destinationFileState.Path), | ||
| true); |
There was a problem hiding this comment.
Incomplete fix within the Copy pipeline
File.Copy is fixed, but several other file operations in this same method will still fail on long paths:
-
Line 299:
Directory.CreateDirectory(destinationFolder)— On .NET Framework (net472),Directory.CreateDirectorycalls Win32CreateDirectorywhich has MAX_PATH limits. The\\?\prefix is needed there too. -
Line 325:
FileUtilities.DeleteNoThrow(destinationFileState.Path)→ callsFile.Delete()which will fail for long paths on net472. -
Line 423:
File.SetAttributes(file.Path, FileAttributes.Normal)inMakeFileWriteable— will fail for long-path destinations after a successful copy. -
Line 405:
TryCopyViaLinkpassessourceFileState.PathanddestinationFileState.Pathdirectly toNativeMethods.MakeHardLinkandNativeMethodsShared.MakeSymbolicLink, both of which call Win32 APIs (CreateHardLink,CreateSymbolicLink) subject to MAX_PATH.
If any of these paths exceed MAX_PATH, the copy will fail after the File.Copy succeeds but before post-copy fixup completes — potentially leaving the destination in an inconsistent state (e.g., still read-only).
|
|
||
| WIN32_FILE_ATTRIBUTE_DATA data = new WIN32_FILE_ATTRIBUTE_DATA(); | ||
| bool success = NativeMethods.GetFileAttributesEx(path, 0, ref data); | ||
| bool success = NativeMethods.GetFileAttributesEx(EnsureExtendedLengthPath(path), 0, ref data); |
There was a problem hiding this comment.
Missed call site: GetLastWriteDirectoryUtcTime (line 998)
EnsureExtendedLengthPath was applied here in LastWriteFileUtcTime, but the sibling method GetLastWriteDirectoryUtcTime (line 998) also calls GetFileAttributesEx(fullPath, 0, ref data) without the fix:
// Line 998 — NOT fixed:
bool success = GetFileAttributesEx(fullPath, 0, ref data);Additionally, OpenFileThroughSymlinks (line 1229) calls CreateFile(fullPath, ...) without the prefix. CreateFile has the same MAX_PATH limitation. This means that for symlinked files with long paths, GetContentLastWriteFileUtcTime (called from this very method at line 1206) will still fail.
| finally | ||
| { | ||
| try { File.Delete(@"\\?\" + sourcePath); } catch { } | ||
| try { File.Delete(@"\\?\" + destPath); } catch { } | ||
| try { Directory.Delete(tempBase, true); } catch { } | ||
| } |
There was a problem hiding this comment.
Test cleanup: Consider using TestEnvironment
MSBuild testing conventions recommend using TestEnvironment to manage temporary files and directories instead of manual try/finally blocks. TestEnvironment automatically reverts state on dispose.
Also, the Directory.Delete(tempBase, true) in the finally block may itself fail on a long path if the test host isn't longPathAware. Consider using the \\?\ prefix for the cleanup too, or use TestEnvironment.CreateFolder() which handles cleanup automatically.
| // Delete using \\?\ because the test host may not be longPathAware. | ||
| try { File.Delete(@"\\?\" + longFilePath); } catch { } | ||
| } | ||
| Directory.Delete(longDir, recursive: false); |
There was a problem hiding this comment.
Cleanup bug: Directory.Delete(longDir, recursive: false) may fail
If File.Delete(@"\\?\" + longFilePath) in the catch block silently fails, the directory won't be empty and Directory.Delete(longDir, recursive: false) will throw IOException. This should either use recursive: true or be wrapped in a try/catch.
Fixes #
Fixes https://developercommunity.visualstudio.com/t/Copy-file-to-OutputDirectory-fails-with-/11074178
Context
With (plural), inner per-framework builds run inside devenv.exe, which is not longPathAware. Win32 APIs (GetFileAttributesEx, CopyFileW) reject paths > 260 chars in such processes, causing MSB3030 even when LongPathsEnabled=1 is set. Single builds run in dotnet.exe (longPathAware), so it works fine there.
Changes Made
NativeMethods.cs: Added EnsureExtendedLengthPath() helper (prepends \?\ for paths ≥ MAX_PATH); applied to all GetFileAttributesEx call sites.
FileState.cs: Use EnsureExtendedLengthPath() before GetFileAttributesEx.
Copy.cs: Use EnsureExtendedLengthPath() on source and destination before File.Copy.
Testing
CopyFileWithLongPath (Copy_Tests.cs): copies a file matching the exact bug report path structure; runs under net472 test host (not longPathAware, same as devenv.exe).
ExistsWithLongPath (FileStateTests.cs): verifies FileState.FileExists is true for a long-path file.
End-to-end verified by replacing DLLs in VS Insiders — bug resolved.
Notes