Fix csc/vbc misinterpretation of source files in root directory on Unix#80866
Fix csc/vbc misinterpretation of source files in root directory on Unix#80866
Conversation
Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
…rove tests Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
|
The comment for this in Given this change, should we update the comment? Doesn't feel temporary anymore 😄 |
| return null; | ||
|
|
||
| // On Windows, no transformation is needed | ||
| if (Path.DirectorySeparatorChar == '\\') |
There was a problem hiding this comment.
| if (Path.DirectorySeparatorChar == '\\') | |
| if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) |
Believe that works across the frameworks. If not existing code is fine.
There was a problem hiding this comment.
Done in 7448e85. Changed to use RuntimeInformation.IsOSPlatform(OSPlatform.Windows).
| // Copy all items processed so far | ||
| for (int j = 0; j < i; j++) | ||
| { | ||
| transformedSources[j] = sources[j].ItemSpec; | ||
| } |
There was a problem hiding this comment.
| // Copy all items processed so far | |
| for (int j = 0; j < i; j++) | |
| { | |
| transformedSources[j] = sources[j].ItemSpec; | |
| } | |
| sources.AsSpan(0, j).CopyTo(transformedSources.AsSpan()); |
There was a problem hiding this comment.
The suggestion doesn't directly apply here since we're copying from ITaskItem[] to string[] (extracting ItemSpec). We need to iterate to extract the ItemSpec values, so Span.CopyTo can't be used directly.
| } | ||
|
|
||
| [ConditionalFact(typeof(WindowsOnly)), WorkItem("https://github.com/dotnet/roslyn/issues/80865")] | ||
| public void SourceFilePathsOnWindows() |
There was a problem hiding this comment.
This test should be added to VbcTests as well.
There was a problem hiding this comment.
Done in 7448e85. Added SourceFilePathsOnWindows test for VB.
who wants to start a betting pool on the age of the comment? (no cheating). |
| Assert.Contains("/./App.vb", responseFileContents); | ||
| Assert.Contains("src/Test.vb", responseFileContents); | ||
| // /dir/File.vb should NOT be transformed (contains second '/') | ||
| Assert.Contains("/dir/File.vb", responseFileContents); |
There was a problem hiding this comment.
The other test had a leading space for this assertion, it seems reasonable to have it here also.
| Assert.Contains("/dir/File.vb", responseFileContents); | |
| Assert.Contains(" /dir/File.vb", responseFileContents); |
There was a problem hiding this comment.
@copilot add the space, and to other asserts where it makes sense too
There was a problem hiding this comment.
Done in 7448e85. Added leading spaces to assertions for src/Test.vb and /dir/File.vb, as well as corresponding C# test assertions.
I'm guessing that it comes from the "Hello World" commit in Git. Edit well at least I guessed the right person. |
| Assert.Contains("/./App.vb", responseFileContents); | ||
| Assert.Contains("src/Test.vb", responseFileContents); | ||
| // /dir/File.vb should NOT be transformed (contains second '/') | ||
| Assert.Contains("/dir/File.vb", responseFileContents); |
There was a problem hiding this comment.
@copilot add the space, and to other asserts where it makes sense too
| } | ||
|
|
||
| [ConditionalFact(typeof(WindowsOnly)), WorkItem("https://github.com/dotnet/roslyn/issues/80865")] | ||
| public void SourceFilePathsOnWindows() |
| // Copy all items processed so far | ||
| for (int j = 0; j < i; j++) | ||
| { | ||
| transformedSources[j] = sources[j].ItemSpec; | ||
| } |
| return null; | ||
|
|
||
| // On Windows, no transformation is needed | ||
| if (Path.DirectorySeparatorChar == '\\') |
| for (int i = 0; i < sources.Length; i++) | ||
| { | ||
| var itemSpec = sources[i].ItemSpec; | ||
| // Check if this path needs transformation using the compiler's heuristic: |
There was a problem hiding this comment.
@copilot given the heuristic is not "temporary" anymore, can you update the comment in TryParseOption as jaredpar suggested?
…t, improve assertions, update comment Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
Fix csc/vbc misinterpretation of source files in root directory on Unix
Fixes dotnet/sdk#51282
Fixes #80865
Problem
On Unix systems, the C# and VB compilers misinterpret source files in the root directory as command-line switches because they start with
/. For example:This fails with:
This is particularly problematic for file-based apps where the source file lives in the root directory.
Solution
Transform Unix paths that would be misinterpreted as options to use the
/./prefix before passing them to the compiler. This uses the same heuristic as the compiler'sCommandLineParser.TryParseOption: a path starting with/is treated as an option unless it contains another/after the first character.The transformation is applied in
ManagedCompiler.GetTransformedSourcesForCommandLine():RuntimeInformation.IsOSPlatform(OSPlatform.Windows))/Program.cs→/./Program.cs/dir/File.cs(already safe due to second/)string[]overload to minimize allocationsnullwhen no transformation is needed to avoid unnecessary allocationsCompatibility
The fix maintains full compatibility with:
/./pathto/pathwhen resolving file pathsTesting
Added tests for both C# and VB compilers covering:
/dir/file.csformat (verified NOT transformed)/test.cs)All 377 existing MSBuild task tests continue to pass.
Comment Updates
Updated the comment in
CommandLineParser.TryParseOptionto reflect that this heuristic is no longer "temporary" and is now relied upon by the MSBuild tasks.Original prompt
Fixes #80865
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.