search

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.21k stars 1.35k forks source link

Improve ToolTask's usage of response files #4315

Open fadimounir opened 5 years ago

fadimounir commented 5 years ago

In relation to https://github.com/dotnet/sdk/pull/3121

We need a way to: 1) Have a ToolTask implementor to specify a custom response file path (via a property override for example) 2) Boolean property to enable the logging of the tool command using the @"rspfile" format (Ex: mytool.exe @"temppath\myrsp.rsp") instead of the default list of raw arguments.

Regarding the first point, there are some cases where we need to keep the response file around in the obj intermediates. This is especially useful in repros, where we need to quickly reproduce a tool failure for investigation, and it has proven useful when we ask our customers to submit us repros for external bug investigations.

Regarding the second point, there are 2 aspects here: 1) Aesthetic aspect: there are some tools that require huge commands, and can make it very hard to read the msbuild log. I'm not talking about enabling/disabling the logging here, which can be controlled by the msbuild logging verbosity (that's a different thing). I'm talking about logging the exact command that is being executed, which is based on a response file instead of the huge list of raw arguments. Not just would it be easier to copy/paste that command from the msbuild log for a targeted repro, but it would make the log so much easier to read. On another note, given that we can split various raw arguments over multiple lines in the response file, it makes the response file so much easier to read with a human eye compared to a huge string of concatenated raw arguments. Best example: all the /r arguments for the C# compiler. 2) Correctness aspect: we need to know exactly the command that executed. If a tool fails because of a bug in response file argument parsing, there would be no way of easily reproducing that just by copying/pasting the raw arguments logged in the msbuild log.

An example of a tool where we really benefitted from keeping the rsp files and logging the rsp-based command can be found here: https://github.com/dotnet/sdk/pull/3121

fadimounir commented 5 years ago

cc @rainersigwald

yuehuang010 commented 5 years ago

My 2cc, Response file can't be a fixed name for two reasons, 1) security as reusing a file could lead to wrong execution of the tool. 2) parallel build would conflict read/write access to the file. There are solution to addresss #1 and #2, but once you implemented them, you are back to your original issue.

I do agree it may be helpful if there was a mode or tool that the build and logged "rsp" instead of full argument list.

danmoseley commented 5 years ago

The scenarios make sense to me. But many times, the task being executed writes the response file. It seems you need a flag that tells the task to not delete it afterwards.

BTW, it is my lasting regret that we made msbuild.exe default verbosity "normal" rather than "minimal". Most people just want to know the build is progressing, and any problems. They don't need to see the command lines being executed...

fadimounir commented 5 years ago

But many times, the task being executed writes the response file

I think uses a random file name under the %temp% folder

jtschuster commented 1 year ago

1) security as reusing a file could lead to wrong execution of the tool.

Could a ToolTask could specify a path to copy the rsp file to after execution instead of deleting it?

2) parallel build would conflict read/write access to the file.

If we make this a virtual method instead of a property it should be clearer that it shouldn't be a constant path.