Making MSBuild great again #709
Conversation
…be copied to output
Merge 'dev' into 'new-msbuild'
Merge dev into new msbuild
Merge dev into new-msbuild
Merge dev into new msbuild
…est!) This is a fairly significant change in that it adds our first MSBuild project test. This requires some gymnastics to ensure that MSBuild.exe can to set up MSBuild's build environment. In the tests, we set the MSBUILD_EXE_PATH environment variable to properly locate MSBuild during test runs.
…sproj-in-solution add support for new .net core csproj guid in .sln files.
MSBuild unit test refactoring
Update to MSBuild project model and fix for cross-targeting projects
| @@ -5,7 +5,6 @@ | |||
| }, | |||
| "dependencies": { | |||
| "OmniSharp.Abstractions": "1.0.0", | |||
| "OmniSharp.Nuget": "1.0.0", | |||
There was a problem hiding this comment.
@david-driscoll: Please note this change. I think we need to invest in some effort to make this endpoint great again.
There was a problem hiding this comment.
oh god, no more trump references... I only hope he doesn't follow through repealing NAFTA (though looking at all his other promises... it might disappear!)
| var specificDiagnosticOptions = new Dictionary<string, ReportDiagnostic>(projectFileInfo.SuppressedDiagnosticIds.Count); | ||
|
|
||
| // Always suppress CS1701 (Assuming assembly reference 'x' used by 'y' matches identity 'z'. you may need to supply runtime policy) | ||
| specificDiagnosticOptions.Add("CS1701", ReportDiagnostic.Suppress); |
There was a problem hiding this comment.
.net cli project system disables also CS1702 and CS1705 - should these be aligned?
There was a problem hiding this comment.
Actually, this probably isn't needed anymore. Now that we properly support the <NoWarn> attribute in the project file, we actually inherit CS1701, CS1702 and CS1705 from the targets.
|
this is a hugely welcome change! 🎉 🎈 🍰 |
| return; | ||
| } | ||
|
|
||
| if (File.Exists(msbuildExePath)) |
There was a problem hiding this comment.
this check is not necessary anymore, as we will have exited already if the file was not there
similarly, the following else is unreachable anymore and can be removed
There was a problem hiding this comment.
I really should read my code after refactoring 😄
Fixing...
| msbuildFolder = FindMSBuildFolderFromSolution(); | ||
| } | ||
|
|
||
| if (msbuildFolder == null || !Directory.Exists(msbuildFolder)) |
There was a problem hiding this comment.
FindMSBuildFolderFromSolution() already checks for directory existence internally so no need to recheck again, just null check would be enough
There was a problem hiding this comment.
Good catch! I'd forgotten to remove that after some refactoring.
makes sense 👍 |
🚢 | @@ -5,7 +5,6 @@ | |||
| }, | |||
| "dependencies": { | |||
| "OmniSharp.Abstractions": "1.0.0", | |||
| "OmniSharp.Nuget": "1.0.0", | |||
There was a problem hiding this comment.
oh god, no more trump references... I only hope he doesn't follow through repealing NAFTA (though looking at all his other promises... it might disappear!)
|
OK. In it goes! 😄 |
This PR integrates the new-msbuild branch back into dev, which gets us one large step of the way to #666. Essentially, this integrates MSBuild in a much better way with OmniSharp, using official MSBuild packages and a custom-build for Mono.
Adding a couple of smart people to review, but others are welcome to chime in.