Setting ProvideCommandLineArgs to the Csc/Vbc task includes non-Csc args

[jasonmalinowski]

ProvideCommandLineArgs is a switch on the Csc/Vbc build tasks which lets the task produce as an output the arguments that will be passed to Csc. It doesn't quite work right however in .NET Core: since the tool task in that case doesn't invoke csc.exe but instead tries to invoke "dotnet.exe exec csc.dll"..., the "exec" and "csc.dll" also appear in the ProvideCommandLineArgs. This throws off project systems since we then take the command line args and pass them to the command like parser, so logically you have a source file named "exec"... 😄

Steps to Reproduce:

1. Create a new dotnet console app project.
2. Add <ProvideCommandLineArgs>true</ProvideCommandLineArgs>
3. dotnet build -bl
4. Look at the output items of the Csc task:

In this case, we'd expect the CscCommandLineArgs are just the parts going to csc, excluding the parts going to dotnet.

[tmeschter]

Tangent: I thought those items would only be created during a design-time build. They seem like overhead in a real build. Or does something other than the project system use them?

[jasonmalinowski]

@tmeschter: you have to set the property; although I see right now that GitHub thought my instructions were XML and didn't show them.

[jcouv]

Tagging @jaredpar to advise/triage.

[jaredpar]

They seem like overhead in a real build. Or does something other than the project system use them?

Lots of tools use them in real builds. It is immensely valuable to know what the actual invocation of csc was. Really helpful for recreating Compilation instances. Concrete example is here: https://github.com/jaredpar/basic-compiler-logger

[jaredpar]

In this case, we'd expect the CscCommandLineArgs are just the parts going to csc, excluding the parts going to dotnet.

Sorry but I think this is a case where your expectations are just off. It is important to know the exact command line used by the compiler, particularly when root causing build failures. Other tools depend on this being there.

It's unfortunate that invocation can take so many forms but that is the reality. If the IDE needs to dig past dotnet then you can implement logic similar to here to do so

https://github.com/jaredpar/basic-compiler-logger/blob/main/src/Basic.CompilerLog.Util/BinaryLogUtil.cs#L227

[jasonmalinowski]

case where your expectations are just off

@jaredpar: The support to the build tasks to produce a command line argument was originally added in this pull request to be used for design time builds, I'd absolutely expect that it's still usable for that purpose! Separately, if the item group was "DotnetArguments" I wouldn't be so worried, but it's named Csc arguments specifically.

Other tools depend on this being there.

Which tools? I don't have a sense of what we break if we change this but sounds like you do know of some tools out there. If there's other tools also breaking and having to work around this, then I wonder what the greater good is here.

If the IDE needs to dig past dotnet then you can implement logic similar to here to do so

That looks terribly fragile, especially if any other arguments ever had to get passed to dotnet.exe directly. I can think of other approaches, for example, having the build task mark the created MSBuild items with metadata for the ones that are "not really CSC" and then the tools can filter that more generally that'd be a good approach. If nothing else, if we did want to go with that approach we really need to formalize that approach and have tests in place to ensure it's never broken further, and figure out all the places that fix needs to go into.

I'm going to reactivate this bug just since right now we're not tracking this issue anywhere else, and I don't want this to fall of the radar. And as said, from the project system's perspective the thing that was created for its use isn't working right. If we do have tools that now depend on that behavior and fixing them is worse than the tools that all have to work around this, and we need to figure out a new approach, great, but let's use this bug to track and drive that discussion. (And if we need to spin a up a meeting between impacted parties happy to do that too!)

[jaredpar]

I'd absolutely expect that it's still usable for that purpose!

How is it not usable? Yes there is extra information here but it can easily be bypassed to get the information the design time build needs. I provided sample code to demonstrate where this can be done.

Which tools?

I listed a specific example in my previous comment. You can look to others like binary log viewer for tools that depend on the task args. I've also listed the importantance of this for debugging purposes.

That looks terribly fragile, especially if any other arguments ever had to get passed to dotnet.exe directly.

I disagree. The code is straight forward and there are no plans to change how the dotnet tool is invoked. If it ever does change then we can adjust the logic to work around it.

I'm going to reactivate this bug just since right now we're not tracking this issue anywhere else, and I don't want this to fall of the radar

Okay but at the moment I'm not convinced this code needs to be changed. The existing information is sufficient to solve the problem. The code has been this way for a considerable amount of time so I'm skeptical about the need, urgency around changing this.

[tmat]

@jaredpar

Sorry but I think this is a case where your expectations are just off. It is important to know the exact command line used by the compiler, particularly when root causing build failures. Other tools depend on this being there.

I understand the value of having full command line in the log for investigations.

The binlog actually contains the full command line for the task and using CscCommandLineArgs is not necessary for that:

Isn't that what basic-compilerlog tool is reading (using TaskCommandLineEventArgs binlog type)?

The values passed to CscCommandLineArgs are clearly incorrect.

The comment in
https://github.com/jaredpar/basic-compilerlog/blob/main/src/Basic.CompilerLog.Util/BinaryLogUtil.cs#L287
says: "The argument list is going to include either dotnet exec csc.dll or csc.exe."

It seems to be reading the command line from TaskCommandLineEventArgs, not from CscCommandLineArgs.

When building using dotnet the argument list actually does not include the path to "dotnet.exe". It only includes "exec" and path to "csc.dll".

On desktop it does not include the path to csc.exe.

CscCommandLineArgs thus doesn't inform me what tool is actually running the compiler. It's not the "exact" command line.

Besides, the name CscCommandLineArgs is misleading since "exec" is not an argument of csc.

[jaredpar]

Thanks for the break down there. I was confusing the two items for being one entry.

Dug into this a bit to see where the two values are coming from:

• CommandLineArguments value is coming from ToolTask.Execute. Feel that essentially remain the same because it should be a faithful representation of the command that is executed.
• CscCommandLineArgs is actually command from ManagedCompiler.CommandLineArgs. The targets file is what does the name mapping. Agree that can be modified safely.

Having these values be CommandLineArguments and CommandLineArgs is not confusing at all when reading the code 🤦. I'm tempted to clean that up and give it a better differentiated name.