Skip to content
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

Require a known good version of .NET SDK be installed. #2302

Merged
merged 2 commits into from
Dec 8, 2021

Conversation

JoeRobich
Copy link
Member

Since we fail when loading SDKs with mismatched NuGet.Framework, we can limit our SDK discovery to a known good SDK version.

catch (MSBuildNotFoundException mnfe)
{
Console.Error.WriteLine(mnfe.Message);
return 0xbad;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can make this a unique exit code.

@@ -72,7 +72,7 @@ public static void RegisterDefaultInstance(this IMSBuildLocator msbuildLocator,
}
else
{
logger.LogError("Could not locate MSBuild instance to register with OmniSharp");
throw new MSBuildNotFoundException("Could not locate MSBuild instance to register with OmniSharp.");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Further loading isn't useful and clutters the output.

@@ -22,9 +22,17 @@ public override ImmutableArray<MSBuildInstance> GetInstances()
{

#if NETCOREAPP
const string DotNetSdkVersion = "6.0.100";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be easy to find and replace this version string.

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes I've suggested this yesterday in a comment somewhere - thanks!

@filipw filipw enabled auto-merge December 8, 2021 07:11
@filipw filipw merged commit 565ba9f into OmniSharp:master Dec 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants