Improve MSBuild discovery for future scenarios #1328
Conversation
This change adds a method for location an MSBuild tools path and shares it among the various MSBuild discovery providers.
…ion isn't a valid version string
This adds a new OmniSharp option to produce MSBuild binary logs when loading projects
for debugging purposes. The option can be set at the command line or via an omnisharp.json
file:
```JSON
{
"MSBuild": {
"GenerateBinaryLogs": true
}
}
```
|
Linux is failing because Directory.Exists appears to be case-insensitive. Looking for a good solution... |
DustinCampbell
force-pushed
the
future-proof-msbuild-discovery
branch
from
a51bc40
to
c78c151
Compare
DustinCampbell
force-pushed
the
future-proof-msbuild-discovery
branch
from
c78c151
to
c320ddf
Compare
oh wow, thanks for staying on top of that |
| var binlogPath = Path.ChangeExtension(projectInstance.FullPath, ".binlog"); | ||
| var binaryLogger = new MSB.Logging.BinaryLogger() | ||
| { | ||
| CollectProjectImports = MSB.Logging.BinaryLogger.ProjectImportsCollectionMode.Embed, |
There was a problem hiding this comment.
I think this is actually the default behaviour so maybe we can skip explicitly specifying it
There was a problem hiding this comment.
I figured the default behavior was "None": https://github.com/Microsoft/msbuild/blob/master/src/Build/Logging/BinaryLogger/BinaryLogger.cs#L47.
There was a problem hiding this comment.
Oh, I guess it is the default: https://github.com/Microsoft/msbuild/blob/master/src/Build/Logging/BinaryLogger/BinaryLogger.cs#L68
That's OK, let's keep the code specific so there's no question. I think our use case is definitely to Embed.
| if (_options.GenerateBinaryLogs) | ||
| { | ||
| var binlogPath = Path.ChangeExtension(projectInstance.FullPath, ".binlog"); | ||
| var binaryLogger = new MSB.Logging.BinaryLogger() |
There was a problem hiding this comment.
this is a great addition, will definitely be useful in troubleshooting
This change fixes MSBuild discovery to handle scenarios where the MSBuild tools path is not included in a "15.0" folder. In future releases of Visual Studio, MSBuild will move to a folder named "Current" and OmniSharp should support that. In addition, I've added some extra logging during discovery to help indicate why a particular MSBuild instance isn't chosen. Finally, this change introduces a new option that causes OmniSharp to produce MSBuild binary logs when loading projects. This should be a big help for MSBuild debugging. It can be enabled in an omnisharp.json file like so:
{ "MSBuild": { "GenerateBinaryLogs": true } }