Merge pull request #70875 from dibarbet/remove_npm

Ship Devkit dependencies as a nuget package to support directly including in the C# extension
dotnet · Nov 21, 2023 · d1b2e28 · d1b2e28
2 parents ce6f532 + d7a1d67
commit d1b2e28
Show file tree

Hide file tree

Showing 18 changed files with 37 additions and 167 deletions.
diff --git a/Roslyn.sln b/Roslyn.sln
@@ -1639,8 +1639,6 @@ Global
 		src\Compilers\Server\VBCSCompiler\VBCSCompilerCommandLine.projitems*{9508f118-f62e-4c16-a6f4-7c3b56e166ad}*SharedItemsImports = 5
 		src\Compilers\VisualBasic\vbc\VbcCommandLine.projitems*{975cd834-45f4-4ea0-a395-cb60dbd0e214}*SharedItemsImports = 5
 		src\Workspaces\SharedUtilitiesAndExtensions\Workspace\Core\WorkspaceExtensions.projitems*{99f594b1-3916-471d-a761-a6731fc50e9a}*SharedItemsImports = 13
-		src\Dependencies\Collections\Microsoft.CodeAnalysis.Collections.projitems*{9b7ac5c2-293d-438d-b9a2-1eddc2c6bf00}*SharedItemsImports = 5
-		src\Dependencies\PooledObjects\Microsoft.CodeAnalysis.PooledObjects.projitems*{9b7ac5c2-293d-438d-b9a2-1eddc2c6bf00}*SharedItemsImports = 5
 		src\Analyzers\VisualBasic\CodeFixes\VisualBasicCodeFixes.projitems*{9f9ccc78-7487-4127-9d46-db23e501f001}*SharedItemsImports = 13
 		src\Analyzers\CSharp\CodeFixes\CSharpCodeFixes.projitems*{a07abcf5-bc43-4ee9-8fd8-b2d77fd54d73}*SharedItemsImports = 5
 		src\Workspaces\SharedUtilitiesAndExtensions\Workspace\CSharp\CSharpWorkspaceExtensions.projitems*{a07abcf5-bc43-4ee9-8fd8-b2d77fd54d73}*SharedItemsImports = 5

diff --git a/azure-pipelines-official.yml b/azure-pipelines-official.yml
@@ -301,21 +301,6 @@ stages:
         ArtifactName: 'PackageArtifacts'
       condition: succeeded()
 
-    # Publish language server package
-    - powershell: Write-Host "##vso[task.setvariable variable=NPMFileName]$((ls -file $(Build.SourcesDirectory)\artifacts\packages\Release\NPM\ | select -First 1).FullName)"
-      displayName: Setting NPM Package Variable
-
-    # Authenticates the .npmrc file for publishing to the internal AzDO feed.
-    # See: https://learn.microsoft.com/azure/devops/pipelines/tasks/package/npm-authenticate?view=azure-devops
-    - task: npmAuthenticate@0
-      displayName: Authenticate NPM Feed
-      inputs:
-        workingFile: $(Build.SourcesDirectory)/src/VisualStudio/DevKit/Impl/.npmrc
-        customEndpoint: devdiv-vs-green-npm-package-feed
-
-    - script: npm publish --userconfig $(Build.SourcesDirectory)\src\VisualStudio\DevKit\Impl\.npmrc $(NPMFileName)
-      displayName: Publish Language Server NPM Package
-
     # Publish Asset Manifests for Build Asset Registry job
     - task: PublishBuildArtifacts@1
       displayName: Publish Asset Manifests

diff --git a/eng/Versions.props b/eng/Versions.props
@@ -164,7 +164,6 @@
     <MicrosoftVisualStudioImageCatalogVersion>$(MicrosoftVisualStudioShellPackagesVersion)</MicrosoftVisualStudioImageCatalogVersion>
     <MicrosoftVisualStudioImagingVersion>$(MicrosoftVisualStudioShellPackagesVersion)</MicrosoftVisualStudioImagingVersion>
     <MicrosoftVisualStudioImagingInterop140DesignTimeVersion>17.8.36726</MicrosoftVisualStudioImagingInterop140DesignTimeVersion>
-    <MicrosoftVisualStudioInternalMicroBuildNpmPackVersion>2.0.93</MicrosoftVisualStudioInternalMicroBuildNpmPackVersion>
     <MicrosoftVisualStudioInteropVersion>17.8.36711</MicrosoftVisualStudioInteropVersion>
     <MicrosoftVisualStudioLanguageVersion>$(VisualStudioEditorPackagesVersion)</MicrosoftVisualStudioLanguageVersion>
     <MicrosoftVisualStudioLanguageCallHierarchyVersion>15.8.27812-alpha</MicrosoftVisualStudioLanguageCallHierarchyVersion>

diff --git a/eng/build.ps1 b/eng/build.ps1
@@ -258,8 +258,6 @@ function BuildSolution() {
 
   $restoreUseStaticGraphEvaluation = $true
 
-  $isNpmAvailable = IsNpmAvailable
-
   try {
     MSBuild $toolsetBuildProj `
       $bl `
@@ -282,7 +280,6 @@ function BuildSolution() {
       /p:RestoreUseStaticGraphEvaluation=$restoreUseStaticGraphEvaluation `
       /p:VisualStudioIbcDrop=$ibcDropName `
       /p:VisualStudioDropAccessToken=$officialVisualStudioDropAccessToken `
-      /p:IsNpmPackable=$isNpmAvailable `
       $suppressExtensionDeployment `
       $msbuildWarnAsError `
       $buildFromSource `
@@ -713,14 +710,6 @@ function List-Processes() {
   Get-Process -Name "devenv" -ErrorAction SilentlyContinue | Out-Host
 }
 
-function IsNpmAvailable() {
-  if (Get-Command "npm" -ErrorAction SilentlyContinue) {
-    return $true
-  }
-
-  return $false;
-}
-
 try {
   if ($PSVersionTable.PSVersion.Major -lt "5") {
     Write-Host "PowerShell version must be 5 or greater (version $($PSVersionTable.PSVersion) detected)"

diff --git a/eng/config/PublishData.json b/eng/config/PublishData.json
@@ -45,7 +45,7 @@
       "Microsoft.CodeAnalysis.EditorFeatures.Wpf": "vssdk",
       "Microsoft.CodeAnalysis.EditorFeatures.Cocoa": "vssdk",
       "Microsoft.VisualStudio.IntegrationTest.Utilities": "vs-impl",
-      "Microsoft.VisualStudio.LanguageServices.DevKit": "npm",
+      "Microsoft.VisualStudio.LanguageServices.DevKit": "vs-impl",
       "Microsoft.CodeAnalysis.LanguageServer": "vs-impl",
       "Microsoft.CodeAnalysis.LanguageServer.alpine-arm64": "vs-impl",
       "Microsoft.CodeAnalysis.LanguageServer.alpine-x64": "vs-impl",

diff --git a/eng/publish-assets.ps1 b/eng/publish-assets.ps1
@@ -92,11 +92,6 @@ function Publish-Nuget($publishData, [string]$packageDir) {
         continue
       }
 
-      if ($feedName.equals("npm")) {
-        Write-Host "Skipping publishing for $nupkg as it is published in a separate step as an NPM package"
-        continue
-      }
-
       # Use the feed name to get the source to upload the package to.
       if (-not (Get-Member -InputObject $feedData -Name $feedName)) {
         throw "$feedName has no configured source feed"

diff --git a/eng/targets/Settings.props b/eng/targets/Settings.props
@@ -51,11 +51,6 @@
     <PublishWindowsPdb>false</PublishWindowsPdb>
     <EnforceExtendedAnalyzerRules>false</EnforceExtendedAnalyzerRules>
     <EnforceExtendedAnalyzerRules Condition="'$(IsAnalyzer)' == 'true'">true</EnforceExtendedAnalyzerRules>
-
-    <!-- NPM packaging settings -->
-    <NpmPackageOutputPath>$(ArtifactsDir)\packages\$(Configuration)\NPM\</NpmPackageOutputPath>
-    <!-- By default don't attempt to do npm packaging during pack since it requires NPM to be installed -->
-    <IsNpmPackable Condition="'$(IsNpmPackable)'==''">false</IsNpmPackable>
   </PropertyGroup>
 
   <!-- 

diff --git a/...eAnalysis.LanguageServer.UnitTests/Microsoft.CodeAnalysis.LanguageServer.UnitTests.csproj b/...eAnalysis.LanguageServer.UnitTests/Microsoft.CodeAnalysis.LanguageServer.UnitTests.csproj
@@ -27,7 +27,7 @@
   -->
   <Target Name="_CopyDevKitExtensionFiles" AfterTargets="ResolveProjectReferences">
     <MSBuild Projects="..\..\..\VisualStudio\DevKit\Impl\Microsoft.VisualStudio.LanguageServices.DevKit.csproj"
-             Targets="CollectNpmInputs">
+             Targets="CollectPackInputs">
       <Output TaskParameter="TargetOutputs" ItemName="_DevKitExtensionFile"/>
     </MSBuild>
 

diff --git a/...icrosoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs b/...icrosoft.CodeAnalysis.LanguageServer.UnitTests/Utilities/LanguageServerTestComposition.cs
@@ -20,5 +20,5 @@ private static string GetDevKitExtensionPath()
         => Path.Combine(AppContext.BaseDirectory, DevKitExtensionSubdirectory, DevKitAssemblyFileName);
 
     public static Task<ExportProvider> CreateExportProviderAsync(ILoggerFactory loggerFactory, bool includeDevKitComponents)
-        => ExportProviderBuilder.CreateExportProviderAsync(extensionAssemblyPaths: includeDevKitComponents ? [GetDevKitExtensionPath()] : Array.Empty<string>(), sharedDependenciesPath: null, loggerFactory);
+        => ExportProviderBuilder.CreateExportProviderAsync(extensionAssemblyPaths: includeDevKitComponents ? [GetDevKitExtensionPath()] : Array.Empty<string>(), loggerFactory: loggerFactory);
 }
diff --git a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ExportProviderBuilder.cs
@@ -14,7 +14,7 @@ namespace Microsoft.CodeAnalysis.LanguageServer;
 
 internal sealed class ExportProviderBuilder
 {
-    public static async Task<ExportProvider> CreateExportProviderAsync(IEnumerable<string> extensionAssemblyPaths, string? sharedDependenciesPath, ILoggerFactory loggerFactory)
+    public static async Task<ExportProvider> CreateExportProviderAsync(IEnumerable<string> extensionAssemblyPaths, ILoggerFactory loggerFactory)
     {
         var logger = loggerFactory.CreateLogger<ExportProviderBuilder>();
 
@@ -48,7 +48,7 @@ public static async Task<ExportProvider> CreateExportProviderAsync(IEnumerable<s
 
         foreach (var extensionAssemblyPath in extensionAssemblyPaths)
         {
-            if (AssemblyLoadContextWrapper.TryLoadExtension(extensionAssemblyPath, sharedDependenciesPath, logger, out var extensionAssembly))
+            if (AssemblyLoadContextWrapper.TryLoadExtension(extensionAssemblyPath, logger, out var extensionAssembly))
             {
                 assemblies.Add(extensionAssembly);
             }

diff --git a/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs b/src/Features/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/Program.cs
@@ -76,7 +76,7 @@ static async Task RunAsync(ServerConfiguration serverConfiguration, Cancellation
 
     logger.LogTrace($".NET Runtime Version: {RuntimeInformation.FrameworkDescription}");
 
-    using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(serverConfiguration.ExtensionAssemblyPaths, serverConfiguration.SharedDependenciesPath, loggerFactory);
+    using var exportProvider = await ExportProviderBuilder.CreateExportProviderAsync(serverConfiguration.ExtensionAssemblyPaths, loggerFactory);
 
     // The log file directory passed to us by VSCode might not exist yet, though its parent directory is guaranteed to exist.
     Directory.CreateDirectory(serverConfiguration.ExtensionLogDirectory);
@@ -179,12 +179,6 @@ static CliRootCommand CreateCommandLineParser()
         Required = false
     };
 
-    var sharedDependenciesOption = new CliOption<string?>("--sharedDependencies")
-    {
-        Description = "Full path of the directory containing shared assemblies (optional).",
-        Required = false
-    };
-
     var extensionAssemblyPathsOption = new CliOption<string[]?>("--extension", "--extensions") // TODO: remove plural form
     {
         Description = "Full paths of extension assemblies to load (optional).",
@@ -199,7 +193,6 @@ static CliRootCommand CreateCommandLineParser()
         starredCompletionsPathOption,
         telemetryLevelOption,
         sessionIdOption,
-        sharedDependenciesOption,
         extensionAssemblyPathsOption,
         extensionLogDirectoryOption
     };
@@ -210,7 +203,6 @@ static CliRootCommand CreateCommandLineParser()
         var starredCompletionsPath = parseResult.GetValue(starredCompletionsPathOption);
         var telemetryLevel = parseResult.GetValue(telemetryLevelOption);
         var sessionId = parseResult.GetValue(sessionIdOption);
-        var sharedDependenciesPath = parseResult.GetValue(sharedDependenciesOption);
         var extensionAssemblyPaths = parseResult.GetValue(extensionAssemblyPathsOption) ?? Array.Empty<string>();
         var extensionLogDirectory = parseResult.GetValue(extensionLogDirectoryOption)!;
 
@@ -220,7 +212,6 @@ static CliRootCommand CreateCommandLineParser()
             StarredCompletionsPath: starredCompletionsPath,
             TelemetryLevel: telemetryLevel,
             SessionId: sessionId,
-            SharedDependenciesPath: sharedDependenciesPath,
             ExtensionAssemblyPaths: extensionAssemblyPaths,
             ExtensionLogDirectory: extensionLogDirectory);
 

diff --git a/...atures/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ServerConfigurationFactory.cs b/...atures/LanguageServer/Microsoft.CodeAnalysis.LanguageServer/ServerConfigurationFactory.cs
@@ -34,8 +34,8 @@ public void InitializeConfiguration(ServerConfiguration serverConfiguration)
 
         // Update any other global options based on the configuration the server was started with.
 
-        // Use the SharedDependenciesPath option as a proxy for whether or not devkit is running.
-        var isDevkitEnabled = !string.IsNullOrEmpty(serverConfiguration.SharedDependenciesPath);
+        // Check if the devkit extension is included to see if devkit is enabled.
+        var isDevkitEnabled = serverConfiguration.ExtensionAssemblyPaths.Any(path => Path.GetFileName(path) == "Microsoft.VisualStudio.LanguageServices.DevKit.dll");
         // Set the standalone option so other features know whether devkit is running.
         _globalOptionService.SetGlobalOption(LspOptionsStorage.LspUsingDevkitFeatures, isDevkitEnabled);
     }
@@ -47,6 +47,5 @@ internal record class ServerConfiguration(
     string? StarredCompletionsPath,
     string? TelemetryLevel,
     string? SessionId,
-    string? SharedDependenciesPath,
     IEnumerable<string> ExtensionAssemblyPaths,
     string ExtensionLogDirectory);
diff --git a/...nguageServer/Microsoft.CodeAnalysis.LanguageServer/Services/AssemblyLoadContextWrapper.cs b/...nguageServer/Microsoft.CodeAnalysis.LanguageServer/Services/AssemblyLoadContextWrapper.cs
@@ -20,8 +20,6 @@ namespace Microsoft.CodeAnalysis.LanguageServer.Services
 {
     internal sealed class AssemblyLoadContextWrapper : IDisposable
     {
-        private static readonly ConcurrentDictionary<AssemblyName, Assembly?> s_loadedSharedAssemblies = new(AssemblyNameComparer.Default);
-
         private AssemblyLoadContext? _assemblyLoadContext;
         private readonly ImmutableDictionary<string, Assembly> _loadedAssemblies;
         private readonly ILogger? _logger;
@@ -33,7 +31,7 @@ private AssemblyLoadContextWrapper(AssemblyLoadContext assemblyLoadContext, Immu
             _logger = logger;
         }
 
-        public static bool TryLoadExtension(string assemblyFilePath, string? sharedDependenciesPath, ILogger? logger, [NotNullWhen(true)] out Assembly? assembly)
+        public static bool TryLoadExtension(string assemblyFilePath, ILogger? logger, [NotNullWhen(true)] out Assembly? assembly)
         {
             var dir = Path.GetDirectoryName(assemblyFilePath);
             var fileName = Path.GetFileName(assemblyFilePath);
@@ -43,7 +41,7 @@ public static bool TryLoadExtension(string assemblyFilePath, string? sharedDepen
             Contract.ThrowIfNull(fileName);
             Contract.ThrowIfNull(fileNameNoExt);
 
-            var loadContext = TryCreate(fileNameNoExt, dir, sharedDependenciesPath, logger);
+            var loadContext = TryCreate(fileNameNoExt, dir, logger);
             if (loadContext != null)
             {
                 assembly = loadContext.GetAssembly(fileName);
@@ -54,11 +52,11 @@ public static bool TryLoadExtension(string assemblyFilePath, string? sharedDepen
             return false;
         }
 
-        public static AssemblyLoadContextWrapper? TryCreate(string name, string assembliesDirectoryPath, string? sharedDependenciesPath, ILogger? logger)
+        public static AssemblyLoadContextWrapper? TryCreate(string name, string assembliesDirectoryPath, ILogger? logger)
         {
             try
             {
-                var loadContext = CreateLoadContext(name, sharedDependenciesPath);
+                var loadContext = CreateLoadContext(name);
                 var directory = new DirectoryInfo(assembliesDirectoryPath);
                 var builder = new Dictionary<string, Assembly>();
                 foreach (var file in directory.GetFiles("*.dll"))
@@ -75,39 +73,9 @@ public static bool TryLoadExtension(string assemblyFilePath, string? sharedDepen
             }
         }
 
-        private static AssemblyLoadContext CreateLoadContext(string name, string? sharedDependenciesPath)
+        private static AssemblyLoadContext CreateLoadContext(string name)
         {
             var loadContext = new AssemblyLoadContext(name);
-
-            if (sharedDependenciesPath != null)
-            {
-                loadContext.Resolving += (_, assemblyName) =>
-                {
-                    if (assemblyName.Name is null)
-                    {
-                        return null;
-                    }
-
-                    if (s_loadedSharedAssemblies.TryGetValue(assemblyName, out var loadedAssembly))
-                    {
-                        return loadedAssembly;
-                    }
-
-                    var candidatePath = assemblyName.CultureName is not null
-                        ? Path.Combine(sharedDependenciesPath, assemblyName.CultureName, $"{assemblyName.Name}.dll")
-                        : Path.Combine(sharedDependenciesPath, $"{assemblyName.Name}.dll");
-
-                    if (File.Exists(candidatePath))
-                    {
-                        loadedAssembly = loadContext.LoadFromAssemblyPath(candidatePath);
-                    }
-
-                    s_loadedSharedAssemblies.TryAdd(assemblyName, loadedAssembly);
-
-                    return s_loadedSharedAssemblies[assemblyName];
-                };
-            }
-
             return loadContext;
         }
 

diff --git a/...deAnalysis.LanguageServer/Services/StarredCompletions/StarredCompletionsAssemblyHelper.cs b/...deAnalysis.LanguageServer/Services/StarredCompletions/StarredCompletionsAssemblyHelper.cs
@@ -88,7 +88,7 @@ internal static void InitializeInstance(string? completionsAssemblyLocation, ILo
 
             try
             {
-                var alc = AssemblyLoadContextWrapper.TryCreate(ALCName, s_completionsAssemblyLocation, sharedDependenciesPath: null, s_logger);
+                var alc = AssemblyLoadContextWrapper.TryCreate(ALCName, s_completionsAssemblyLocation, s_logger);
                 if (alc is null)
                 {
                     s_previousCreationFailed = true;

diff --git a/src/VisualStudio/DevKit/Impl/.npmrc b/src/VisualStudio/DevKit/Impl/.npmrc