Add a configuration mechanism to allow users to point Omnisharp to a …

…custom .NET CLI, with a fallback to the DOTNET_ROOT environment variable before falling back to the PATH. I've updated the tests to use this functionality for locating the .NET CLI instead of using the old test-only code-path. As this is my first contribution to Omnisharp, I'm not quite sure how this exposes the new setting through to omnisharp.json, so if anyone has any pointers on that, they would be well appreciated.
OmniSharp · Sep 2, 2021 · 0c74963 · 0c74963
1 parent 8144773
commit 0c74963
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 24 deletions.
diff --git a/src/OmniSharp.Host/CompositionHostBuilder.cs b/src/OmniSharp.Host/CompositionHostBuilder.cs
@@ -140,6 +140,12 @@ private static IEnumerable<Type> SafeGetTypes(Assembly a)
             services.AddSingleton<IAnalyzerAssemblyLoader, AnalyzerAssemblyLoader>();
             services.AddOptions();
 
+            // Setup the options from configuration
+            services.Configure<OmniSharpOptions>(configuration)
+                .PostConfigure<OmniSharpOptions>(OmniSharpOptions.PostConfigure);
+            services.AddSingleton(configuration);
+            services.AddSingleton<IConfiguration>(configuration);
+
             services.AddSingleton<IDotNetCliService, DotNetCliService>();
 
             // MSBuild
@@ -149,13 +155,6 @@ private static IEnumerable<Type> SafeGetTypes(Assembly a)
                     assemblyLoader: sp.GetService<IAssemblyLoader>(),
                     msbuildConfiguration: configuration.GetSection("msbuild")));
 
-
-            // Setup the options from configuration
-            services.Configure<OmniSharpOptions>(configuration)
-                .PostConfigure<OmniSharpOptions>(OmniSharpOptions.PostConfigure);
-            services.AddSingleton(configuration);
-            services.AddSingleton<IConfiguration>(configuration);
-
             services.AddLogging(builder =>
             {
                 var workspaceInformationServiceName = typeof(WorkspaceInformationService).FullName;

diff --git a/src/OmniSharp.Host/Services/DotNetCliService.cs b/src/OmniSharp.Host/Services/DotNetCliService.cs
@@ -2,11 +2,16 @@
 using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.IO;
+using System.Runtime.InteropServices;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Options;
 using NuGet.Versioning;
 using OmniSharp.Eventing;
+using OmniSharp.Options;
 using OmniSharp.Utilities;
 
 namespace OmniSharp.Services
@@ -20,14 +25,45 @@ internal class DotNetCliService : IDotNetCliService
 
         public string DotNetPath { get; }
 
-        public DotNetCliService(ILoggerFactory loggerFactory, IEventEmitter eventEmitter, string dotnetPath = null)
+        public DotNetCliService(ILoggerFactory loggerFactory, IEventEmitter eventEmitter, IOptions<DotNetCliOptions> dotNetCliOptions, IOmniSharpEnvironment environment)
         {
             _logger = loggerFactory.CreateLogger<DotNetCliService>();
             _eventEmitter = eventEmitter;
             _locks = new ConcurrentDictionary<string, object>();
             _semaphore = new SemaphoreSlim(Environment.ProcessorCount / 2);
 
-            DotNetPath = dotnetPath ?? "dotnet";
+            // Check if any of the provided paths have a dotnet executable.
+            string executableExtension = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".exe" : string.Empty;
+            foreach (var path in dotNetCliOptions.Value.GetNormalizedLocationPaths(environment))
+            {
+                if (File.Exists(Path.Combine(path, $"dotnet{executableExtension}")))
+                {
+                    // We'll take the first path that has a dotnet executable.
+                    DotNetPath = Path.Combine(path, "dotnet");
+                }
+                else
+                {
+                    _logger.LogInformation($"Provided dotnet CLI path does not contain the dotnet executable: '{path}'.");
+                }
+            }
+
+            // If we still haven't found a dotnet CLI, check the DOTNET_ROOT environment variable.
+            if (DotNetPath is null)
+            {
+                _logger.LogInformation("Checking the 'DOTNET_ROOT' environment variable to find a .NET SDK");
+                string dotnetRoot = Environment.GetEnvironmentVariable("DOTNET_ROOT");
+                if (!string.IsNullOrEmpty(dotnetRoot) && File.Exists(Path.Combine(dotnetRoot, $"dotnet{executableExtension}")))
+                {
+                    DotNetPath = Path.Combine(dotnetRoot, "dotnet");
+                }
+            }
+
+            // If we still haven't found the CLI, use the one on the PATH.
+            if (DotNetPath is null)
+            {
+                _logger.LogInformation("Using the 'dotnet' on the PATH.");
+                DotNetPath = "dotnet";
+            }
 
             _logger.LogInformation($"DotNetPath set to {DotNetPath}");
         }

diff --git a/src/OmniSharp.Shared/Options/DotNetCliOptions.cs b/src/OmniSharp.Shared/Options/DotNetCliOptions.cs
@@ -0,0 +1,12 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace OmniSharp.Options
+{
+    public class DotNetCliOptions : OmniSharpExtensionsOptions
+    {
+    }
+}
diff --git a/src/OmniSharp.Shared/Options/OmniSharpOptions.cs b/src/OmniSharp.Shared/Options/OmniSharpOptions.cs
@@ -15,6 +15,8 @@ public class OmniSharpOptions
 
         public ImplementTypeOptions ImplementTypeOptions { get; set; } = new ImplementTypeOptions();
 
+        public DotNetCliOptions DotNetCliOptions { get; set; } = new DotNetCliOptions();
+
         public OmniSharpExtensionsOptions Plugins { get; set; } = new OmniSharpExtensionsOptions();
 
         public override string ToString() => JsonConvert.SerializeObject(this);
@@ -26,6 +28,7 @@ public static void PostConfigure(OmniSharpOptions options)
             options.FileOptions ??= new FileOptions();
             options.RenameOptions ??= new RenameOptions();
             options.ImplementTypeOptions ??= new ImplementTypeOptions();
+            options.DotNetCliOptions ??= new DotNetCliOptions();
             options.Plugins ??= new OmniSharpExtensionsOptions();
         }
     }

diff --git a/tests/OmniSharp.MSBuild.Tests/ProjectLoadListenerTests.cs b/tests/OmniSharp.MSBuild.Tests/ProjectLoadListenerTests.cs
@@ -129,9 +129,9 @@ public async Task Given_a_restored_project_the_references_are_emitted()
             var emitter = new ProjectLoadTestEventEmitter();
 
             using var testProject = await TestAssets.Instance.GetTestProjectAsync("HelloWorld");
-            var dotnetCliService = new DotNetCliService(LoggerFactory, emitter);
-            await dotnetCliService.RestoreAsync(testProject.Directory);
             using var host = CreateMSBuildTestHost(testProject.Directory, emitter.AsExportDescriptionProvider(LoggerFactory));
+            var dotnetCliService = host.GetExport<IDotNetCliService>();
+            await dotnetCliService.RestoreAsync(testProject.Directory);
             Assert.Single(emitter.ReceivedMessages);
             Assert.NotEmpty(emitter.ReceivedMessages[0].References.Where(reference => reference == GetHashedReference("system.core")));
         }

diff --git a/tests/OmniSharp.MSBuild.Tests/ProjectWithAnalyzersTests.cs b/tests/OmniSharp.MSBuild.Tests/ProjectWithAnalyzersTests.cs
@@ -9,6 +9,7 @@
 using OmniSharp.FileWatching;
 using OmniSharp.Models.Diagnostics;
 using OmniSharp.Models.FilesChanged;
+using OmniSharp.Options;
 using OmniSharp.Services;
 using TestUtility;
 using Xunit;
@@ -348,7 +349,14 @@ private static async Task NotifyFileChanged(OmniSharpTestHost host, string file)
 
         private static async Task RestoreProject(ITestProject testProject)
         {
-            await new DotNetCliService(new LoggerFactory(), NullEventEmitter.Instance).RestoreAsync(testProject.Directory);
+            DotNetCliOptions options = new DotNetCliOptions
+            {
+                LocationPaths = new[]
+                {
+                    Path.Combine(TestAssets.Instance.RootFolder, DotNetCliVersion.Current.GetFolderName())
+                }
+            };
+            await new DotNetCliService(new LoggerFactory(), NullEventEmitter.Instance, Microsoft.Extensions.Options.Options.Create(options), new OmniSharpEnvironment(testProject.Directory)).RestoreAsync(testProject.Directory);
         }
     }
 }
diff --git a/tests/TestUtility/TestServiceProvider.cs b/tests/TestUtility/TestServiceProvider.cs
@@ -3,12 +3,14 @@
 using System.IO;
 using System.Linq;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Options;
 using Microsoft.Extensions.Caching.Memory;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.DependencyInjection.Extensions;
 using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Options;
+using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces;
 using OmniSharp;
 using OmniSharp.Eventing;
 using OmniSharp.FileWatching;
@@ -81,7 +83,7 @@ public class TestServiceProvider : DisposableObject, IServiceProvider
             eventEmitter = eventEmitter ?? NullEventEmitter.Instance;
 
             var assemblyLoader = CreateAssemblyLoader(loggerFactory);
-            var dotNetCliService = CreateDotNetCliService(dotNetCliVersion, loggerFactory, eventEmitter);
+            var dotNetCliService = CreateDotNetCliService(dotNetCliVersion, loggerFactory, environment, eventEmitter);
             var configuration = CreateConfiguration(configurationData);
             var msbuildLocator = CreateMSBuildLocator(loggerFactory, assemblyLoader);
             var sharedTextWriter = CreateSharedTextWriter(testOutput);
@@ -105,7 +107,7 @@ public class TestServiceProvider : DisposableObject, IServiceProvider
         {
             eventEmitter = eventEmitter ?? NullEventEmitter.Instance;
 
-            var dotNetCliService = CreateDotNetCliService(dotNetCliVersion, loggerFactory, eventEmitter);
+            var dotNetCliService = CreateDotNetCliService(dotNetCliVersion, loggerFactory, environment, eventEmitter);
             var configuration = CreateConfiguration(configurationData);
             var sharedTextWriter = CreateSharedTextWriter(testOutput);
 
@@ -142,26 +144,25 @@ private static IConfigurationRoot CreateConfiguration(IConfiguration configurati
             return builder.Build();
         }
 
-        private static IDotNetCliService CreateDotNetCliService(DotNetCliVersion dotNetCliVersion,
-            ILoggerFactory loggerFactory, IEventEmitter eventEmitter)
+        private static IDotNetCliService CreateDotNetCliService(
+            DotNetCliVersion dotNetCliVersion,
+            ILoggerFactory loggerFactory,
+            IOmniSharpEnvironment environment,
+            IEventEmitter eventEmitter)
         {
             var dotnetPath = Path.Combine(
                 TestAssets.Instance.RootFolder,
-                dotNetCliVersion.GetFolderName(),
-                "dotnet");
+                dotNetCliVersion.GetFolderName());
 
-            if (!File.Exists(dotnetPath))
-            {
-                dotnetPath = Path.ChangeExtension(dotnetPath, ".exe");
-            }
+            var options = new DotNetCliOptions { LocationPaths = new[] { dotnetPath } };
 
-            if (!File.Exists(dotnetPath))
+            if (!Directory.Exists(dotnetPath))
             {
                 throw new InvalidOperationException(
                     $"Local .NET CLI path does not exist. Did you run build.(ps1|sh) from the command line?");
             }
 
-            return new DotNetCliService(loggerFactory, NullEventEmitter.Instance, dotnetPath);
+            return new DotNetCliService(loggerFactory, NullEventEmitter.Instance, Options.Create(options), environment);
         }
 
         private static IMSBuildLocator CreateMSBuildLocator(ILoggerFactory loggerFactory,