-
Notifications
You must be signed in to change notification settings - Fork 420
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Re-architect how MSBuild is loaded by OmniSharp #988
Merged
DustinCampbell
merged 17 commits into
OmniSharp:master
from
DustinCampbell:locate-msbuild
Oct 27, 2017
Merged
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
a03aba6
Re-organize MSBuild folder
DustinCampbell 661430f
Add MSBuildLocator service to handle locating MSBuild
DustinCampbell 0a0264a
Remove Travis script code to install newer Ruby
DustinCampbell f73d789
Ensure that 'CscToolPath' and 'CscToolExe' are set on Mono
DustinCampbell 8a1a11c
Improve MSBuild discovery robustness and delete MSBuildEnvironment class
DustinCampbell 06a41c3
Set 'CscToolPath' to OmniSharp's local MSBuild on Mono
DustinCampbell 4305d28
Log MSBuild property overrides
DustinCampbell ad85cd6
Use correct path to MSBuild Roslyn folder on Mono
DustinCampbell 23d32e2
MSBuild discovery on Mono should only succeed if Mono >= 5.2.0 is loc…
DustinCampbell c9488a2
Introduce helper for 'ImmutableArray<MSBuildInstance>.Empty'
DustinCampbell 74c21f7
Use nameof in a few places
DustinCampbell 14a273a
Add a bit more logging to MSBuild discovery
DustinCampbell 7c4b7c8
Fix build to kill OmniSharp unix processes after running scripts
DustinCampbell f3f2b68
Update Mono MSBuild discovery to only succeed if OmniSharp is launche…
DustinCampbell 1c5f21f
Increase indent slightly in output
DustinCampbell 7f263dc
Use IAssemblyLoader.LoadFrom(...)
DustinCampbell 9bdf658
Add more detailed comment about Mono MSBuild discovery
DustinCampbell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
10 changes: 10 additions & 0 deletions
10
src/OmniSharp.Abstractions/MSBuild/Discovery/DiscoveryType.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
namespace OmniSharp.MSBuild.Discovery | ||
{ | ||
public enum DiscoveryType | ||
{ | ||
StandAlone = 0, | ||
DeveloperConsole = 1, | ||
VisualStudioSetup = 2, | ||
Mono = 3 | ||
} | ||
} |
12 changes: 12 additions & 0 deletions
12
src/OmniSharp.Abstractions/MSBuild/Discovery/IMSBuildLocator.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
using System.Collections.Immutable; | ||
|
||
namespace OmniSharp.MSBuild.Discovery | ||
{ | ||
public interface IMSBuildLocator | ||
{ | ||
MSBuildInstance RegisteredInstance { get; } | ||
|
||
void RegisterInstance(MSBuildInstance instance); | ||
ImmutableArray<MSBuildInstance> GetInstances(); | ||
} | ||
} |
31 changes: 31 additions & 0 deletions
31
src/OmniSharp.Abstractions/MSBuild/Discovery/MSBuildInstance.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
using System; | ||
using System.Collections.Immutable; | ||
|
||
namespace OmniSharp.MSBuild.Discovery | ||
{ | ||
public class MSBuildInstance | ||
{ | ||
public string Name { get; } | ||
public string MSBuildPath { get; } | ||
public Version Version { get; } | ||
public DiscoveryType DiscoveryType { get; } | ||
public ImmutableDictionary<string, string> PropertyOverrides { get; } | ||
public bool SetMSBuildExePathVariable { get; } | ||
|
||
public MSBuildInstance( | ||
string name, string msbuildPath, Version version, DiscoveryType discoveryType, | ||
ImmutableDictionary<string, string> propertyOverrides = null, | ||
bool setMSBuildExePathVariable = false) | ||
{ | ||
Name = name; | ||
MSBuildPath = msbuildPath; | ||
Version = version ?? new Version(0, 0); | ||
DiscoveryType = discoveryType; | ||
PropertyOverrides = propertyOverrides ?? ImmutableDictionary<string, string>.Empty; | ||
SetMSBuildExePathVariable = setMSBuildExePathVariable; | ||
} | ||
|
||
public override string ToString() | ||
=> $"{Name} {Version} - \"{MSBuildPath}\""; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
using System.Runtime.CompilerServices; | ||
|
||
[assembly: InternalsVisibleTo("TestUtility")] | ||
[assembly: InternalsVisibleTo("OmniSharp.Http.Tests")] | ||
[assembly: InternalsVisibleTo("TestUtility")] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we supposed to do anything if we couldn't locate any instances?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably report an error to the log. Will add that.