Don't build a solution when -preprocess or -targets is passed on the command line#8588
Conversation
JanKrivanek
left a comment
There was a problem hiding this comment.
Looks good.
In case of running on solution - do we want to output a message indicating that operation is not currently supported? Silently not performing anything might seem confusing.
I agree that there should be a message. Just pushed a commit to add messages. |
|
Isn't /targets supported for a solution? It certainly used to be. There are many targets in the generated meta project? |
Specifying |
| if (isTargets) | ||
| { | ||
| success = PrintTargets(projectFile, toolsVersion, globalProperties, targetsWriter, projectCollection); | ||
| // TODO: Support /targets for solution files. https://github.com/dotnet/msbuild/issues/7697 |
There was a problem hiding this comment.
I don't know what the repo standard is (it's not my repo) but IMO todo comments and links to issues usually just make the code cluttered (often for years) when the same info could be found from the original PR or commit message.
Eg in this case I'd a simply comment something like "// /targets could be supported for solution files in future"
There was a problem hiding this comment.
I added the TODO comments. I'm actively working on the issue to support /targets and /preprocess for solution files and this is intended to be an interim change. An earlier review requested adding the links. Regardless I have no issue with changing the comments if that's the standard and/or the consensus. Thanks
There was a problem hiding this comment.
I personally prefer to have the comment in the file, as it is now, as long as the link is to a permalink (we still have some that are like "regression test for #1283456" and I have no idea even what database that was in . . .)
|
Thanks @jrdodds! |
Fixes #7697 (Partially)
Context
The -preprocess or -targets options are not supported for .sln files and, when these options are supplied, the .sln is built.
With this change the .sln will not be built.
Changes Made
Re-worked logic for handling the -preprocess or -targets options.
Testing
Tested on Windows 11 and macOS 12
Tested building and running unit tests and 'live' tested running MSBuild against a solution (.sln) file with the options.
Notes
Implementing -preprocess and -targets for solution files will be a separate PR.