Skip obj→bin copy on build failure in CSharpCompilerCommand#51614
Skip obj→bin copy on build failure in CSharpCompilerCommand#51614jjonescz merged 9 commits intorelease/10.0.2xxfrom
Conversation
Modified CSharpCompilerCommand to only copy the obj dll to bin when exitCode == 0. Added test CscOnly_CompilationFailure_NoCopyToBin to verify bin dll is not updated on failure. Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
Removed unnecessary Thread.Sleep(100) from the test as we're comparing timestamps for equality, not testing for differences after a delay. Co-authored-by: jjonescz <3669664+jjonescz@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds a safeguard to prevent copying DLL files from the obj directory to the bin directory when C# compilation fails. The change ensures that spurious errors don't occur from attempting to copy non-existent or incomplete DLL files after compilation errors.
Key Changes
- Modified the copy logic in
CSharpCompilerCommand.csto only copy files whenexitCode == 0 - Added comprehensive test coverage for the compilation failure scenario
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Cli/dotnet/Commands/Run/CSharpCompilerCommand.cs | Added exit code check to prevent copying DLLs when compilation fails |
| test/dotnet.Tests/CommandTests/Run/RunFileTests.cs | Added test to verify DLLs are not copied when compilation fails |
|
Is there a specific issue that is being solved by this PR? If so, please link to it. |
|
There is not an existing issue afaik; I noticed the problem while working on another PR. The added test has the "repro" for the issue though. |
| File.Delete(binDll); | ||
|
|
||
| // Write invalid code that causes compilation to fail | ||
| code = code.Replace(";", ""); |
There was a problem hiding this comment.
Rather than a compile error that could potentially be adjusted in the future, let's just add #error error to the file. Then we can know exactly what the content will be and be resilient to future compiler changes.
There was a problem hiding this comment.
@jjonescz it looks like we're still asserting a specific error code.
There was a problem hiding this comment.
Right, I see what you mean now, I will adjust, thanks.
There was a problem hiding this comment.
Btw, this is just a test relying on a compiler error code; whereas for example in #51609, I'm relying on compiler code in product code (although if the error code changes, only that one scenario might stop working optimally, it shouldn't break anything else; also I'm not sure how else to do it).
|
@RikkiGibson for another look, thanks |
2 similar comments
|
@RikkiGibson for another look, thanks |
|
@RikkiGibson for another look, thanks |
CSharpCompilerCommand.Execute()was copying the obj dll to bin regardless of build success, causing unnecessary work and potential errors when the dll doesn't exist due to compilation failures.Changes
exitCode == 0checkCscOnly_CompilationFailure_NoCopyToBintest verifying bin dll is not updated after failed compilationThe fix ensures failed builds don't produce or modify output artifacts:
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.