Include default RSP files for core compilers #80993
Conversation
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <None Include="$(MSBuildThisFileDirectory)..\..\..\CSharp\csc\csc.rsp" CopyToOutputDirectory="PreserveNewest" /> |
There was a problem hiding this comment.
This is inspired by other projects that include csc.rsp:
It works by not copying the file if it's not modified, so the builds are more performant
There was a problem hiding this comment.
This is inspired by other projects that include csc.rsp
That is great, but I still would like to understand how exactly this option works. Other projects could have made a mistake. Since these files are not build artifacts, it doesn't feel appropriate to me to copy files only when they are newer. BTW, what exactly does it mean for a file to be newer for the purpose of this operation?
There was a problem hiding this comment.
The file is copied to the output directory only if the source file has newer timestamp than the target file (the one in the output directory of the project). I think this is the common default for many MSBuild operations.
It's documented here: https://learn.microsoft.com/en-us/visualstudio/msbuild/common-msbuild-project-items?view=vs-2022#none
CopyToOutputDirectory Optional string. Determines whether to copy the file to the output directory. Values are:
1. Never
2. Always
3. PreserveNewest
4. IfDifferent
Defaults to Never if DefineExplicitDefaults is set to true; otherwise, defaults to the empty string.
Looks like we could use IfDifferent (but that's a newer option: dotnet/docs#44059); or Always.
| <UsingTask TaskName="Microsoft.CodeAnalysis.BuildTasks.Csc" AssemblyFile="{_buildTaskDll}" /> | ||
| <Target Name="CustomTarget"> | ||
| <Csc Sources="File.cs" UseSharedCompilation="{useSharedCompilation}" DisableSdkPath="{disableSdkPath}" /> | ||
| <Csc Sources="File.cs" UseSharedCompilation="{useSharedCompilation}" DisableSdkPath="{disableSdkPath}" NoConfig="{noConfig}" /> |
There was a problem hiding this comment.
It avoids exponential amount of test runs since I added new input. See https://aarnott.github.io/Xunit.Combinatorial/docs/combinatorial-vs-pairwise.html. The pairwise ones should be the interesting ones (I've verified the test still fails without the product change for example).
There was a problem hiding this comment.
I do not find 8 cases too much and would be more comfortable with covering full matrix. I am uncomfortable with a situation when an "external" tool decides what combinations our tests cover and that combinations possibly changing between versions of that tool. My recommendation is to use full matrix or explicitly specifying combinations covered by the test.
Could we use more precise validation like we have for VB? #Closed Refers to: src/Compilers/Core/MSBuildTaskTests/TestUtilities/IntegrationTestBase.cs:221 in 35e173a. [](commit_id = 35e173a, deletion_comment = False) |
| var rspFile = Path.Combine(Path.GetDirectoryName(typeof(ManagedCompiler).Assembly.Location)!, "csc.rsp"); | ||
| if (File.Exists(rspFile)) | ||
| { | ||
| commandLine.AppendSwitchIfNotNull("@", rspFile); |
|
Done with review pass (commit 5) |
| }; | ||
|
|
||
| AssertEx.Equal($"/sdkpath:{RuntimeEnvironment.GetRuntimeDirectory()} /out:test.exe test.cs", csc.GenerateResponseFileContents()); | ||
| AssertEx.Equal($"/sdkpath:{RuntimeEnvironment.GetRuntimeDirectory()} @{RspFilePath} /out:test.exe test.cs", csc.GenerateResponseFileContents()); |
| }; | ||
|
|
||
| AssertEx.Equal($"/sdkpath:{RuntimeEnvironment.GetRuntimeDirectory()} /optionstrict:custom /out:test.exe test.vb", vbc.GenerateResponseFileContents()); | ||
| AssertEx.Equal($"/sdkpath:{RuntimeEnvironment.GetRuntimeDirectory()} @{RspFilePath} /optionstrict:custom /out:test.exe test.vb", vbc.GenerateResponseFileContents()); |
|
Done with review pass (commit 6) |
2 participants
Follow up on #79911.
Fixes another inconsistency between the core and fx compilers: if the
<Csc>task is used manually on a C# file which hasusing System.Linq;(for example), the fx compiler handles that fine (because there iscsc.rspdeployed with it which has a set of default references) whereas the core compiler fails. A real-world use-case can be seen in https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/684436.The fix is to include this default
csc.rspin scenarios where we invoke the core compiler on fx msbuild (to avoid a break there). We continue to not include thatcsc.rspin scenarios where just the core csc is being used (again, to avoid a break in those scenarios).@jaredpar fyi