Revert "Clean up error experience when downloading non-tools (#43045)" (

#43872)
dotnet · Oct 4, 2024 · 54abeef · 54abeef
2 parents 5a0344e + 952f719
commit 54abeef
Show file tree

Hide file tree

Showing 20 changed files with 14 additions and 130 deletions.
diff --git a/src/Cli/dotnet/NugetPackageDownloader/INuGetPackageDownloader.cs b/src/Cli/dotnet/NugetPackageDownloader/INuGetPackageDownloader.cs
@@ -16,8 +16,7 @@ Task<string> DownloadPackageAsync(PackageId packageId,
             bool includePreview = false,
             bool? includeUnlisted = null,
             DirectoryPath? downloadFolder = null,
-            PackageSourceMapping packageSourceMapping = null,
-            bool isTool = false);
+            PackageSourceMapping packageSourceMapping = null);
 
         Task<string> GetPackageUrl(PackageId packageId,
             NuGetVersion packageVersion = null,

diff --git a/src/Cli/dotnet/NugetPackageDownloader/LocalizableStrings.resx b/src/Cli/dotnet/NugetPackageDownloader/LocalizableStrings.resx
@@ -126,9 +126,6 @@
   <data name="IsNotFoundInNuGetFeeds" xml:space="preserve">
     <value>{0} is not found in NuGet feeds {1}.</value>
   </data>
-  <data name="NotATool" xml:space="preserve">
-    <value>Package {0} is not a .NET tool.</value>
-  </data>
   <data name="DownloadVersionFailed" xml:space="preserve">
     <value>Downloading {0} version {1} failed.</value>
   </data>

diff --git a/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs b/src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs
@@ -83,45 +83,19 @@ public async Task<string> DownloadPackageAsync(PackageId packageId,
             bool includePreview = false,
             bool? includeUnlisted = null,
             DirectoryPath? downloadFolder = null,
-            PackageSourceMapping packageSourceMapping = null,
-            bool isTool = false)
+            PackageSourceMapping packageSourceMapping = null)
         {
             CancellationToken cancellationToken = CancellationToken.None;
 
             (var source, var resolvedPackageVersion) = await GetPackageSourceAndVersion(packageId, packageVersion,
                 packageSourceLocation, includePreview, includeUnlisted ?? packageVersion is not null, packageSourceMapping).ConfigureAwait(false);
 
+            FindPackageByIdResource resource = null;
             SourceRepository repository = GetSourceRepository(source);
 
-            // TODO: Fix this to use the PackageSearchResourceV3 once https://github.com/NuGet/NuGet.Client/pull/5991 is completed.
-            if (isTool && await repository.GetResourceAsync<ServiceIndexResourceV3>().ConfigureAwait(false) is ServiceIndexResourceV3 serviceIndex)
-            {
-                // See https://learn.microsoft.com/en-us/nuget/api/package-base-address-resource#download-package-manifest-nuspec
-                var uri = serviceIndex.GetServiceEntries("PackageBaseAddress/3.0.0")[0].Uri;
-                var queryUri = uri + $"{packageId}/{packageVersion}/{packageId}.nuspec";
-
-                using HttpClient client = new(new HttpClientHandler() { CheckCertificateRevocationList = true });
-                using HttpResponseMessage response = await client.GetAsync(queryUri).ConfigureAwait(false);
-
-                if (response.IsSuccessStatusCode)
-                {
-                    string nuspec = await response.Content.ReadAsStringAsync().ConfigureAwait(false);
-
-                    XDocument doc = XDocument.Parse(nuspec);
-
-                    if (!ToolPackageInstance.IsToolPackage(doc))
-                    {
-                        throw new GracefulException(string.Format(LocalizableStrings.NotATool, packageId));
-                    }
-                }
-                else
-                {
-                    throw new GracefulException(string.Format(LocalizableStrings.NotATool, packageId));
-                }
-            }
-
-            FindPackageByIdResource resource = await repository.GetResourceAsync<FindPackageByIdResource>(cancellationToken)
+            resource = await repository.GetResourceAsync<FindPackageByIdResource>(cancellationToken)
                 .ConfigureAwait(false);
+
             if (resource == null)
             {
                 throw new NuGetPackageNotFoundException(

diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.cs.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.cs.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.de.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.de.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.es.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.es.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.fr.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.fr.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.it.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.it.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ja.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ja.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ko.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ko.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.pl.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.pl.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.pt-BR.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.pt-BR.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ru.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.ru.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.tr.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.tr.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.zh-Hans.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.zh-Hans.xlf
diff --git a/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.zh-Hant.xlf b/src/Cli/dotnet/NugetPackageDownloader/xlf/LocalizableStrings.zh-Hant.xlf
diff --git a/src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs b/src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs
@@ -134,21 +134,13 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa
                     {
                         DownloadAndExtractPackage(packageId, nugetPackageDownloader, toolDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: givenSpecificVersion).GetAwaiter().GetResult();
                     }
-                    else
+                    else if(isGlobalTool)
                     {
-                        if (!ToolPackageInstance.IsToolPackage(package.Nuspec.Xml))
-                        {
-                            throw new GracefulException(string.Format(NuGetPackageDownloader.LocalizableStrings.NotATool, packageId));
-                        }
-
-                        if (isGlobalTool)
-                        {
-                            throw new ToolPackageException(
-                                string.Format(
-                                    CommonLocalizableStrings.ToolPackageConflictPackageId,
-                                    packageId,
-                                    packageVersion.ToNormalizedString()));
-                        }
+                        throw new ToolPackageException(
+                            string.Format(
+                                CommonLocalizableStrings.ToolPackageConflictPackageId,
+                                packageId,
+                                packageVersion.ToNormalizedString()));
                     }
 
                     CreateAssetFile(packageId, packageVersion, toolDownloadDir, assetFileDirectory, _runtimeJsonPath, targetFramework);
@@ -292,7 +284,7 @@ private static async Task<NuGetVersion> DownloadAndExtractPackage(
             bool includeUnlisted = false
             )
         {
-            var packagePath = await nugetPackageDownloader.DownloadPackageAsync(packageId, packageVersion, packageSourceLocation, includeUnlisted: includeUnlisted, isTool: true).ConfigureAwait(false);
+            var packagePath = await nugetPackageDownloader.DownloadPackageAsync(packageId, packageVersion, packageSourceLocation, includeUnlisted: includeUnlisted).ConfigureAwait(false);
 
             // look for package on disk and read the version
             NuGetVersion version;

diff --git a/src/Cli/dotnet/ToolPackage/ToolPackageInstance.cs b/src/Cli/dotnet/ToolPackage/ToolPackageInstance.cs
@@ -23,17 +23,6 @@ public static ToolPackageInstance CreateFromAssetFile(PackageId id, DirectoryPat
 
             return new ToolPackageInstance(id, version, packageDirectory, assetsJsonParentDirectory);
         }
-
-        /// <summary>
-        /// Validates that the nuspec XML represents a .NET tool package.
-        /// </summary>
-        /// <param name="nuspec">The nuspec XML to check.</param>
-        /// <returns><see langword="true"/> if the nuspec represents a .NET tool package; otherwise, <see langword="false"/>.</returns>
-        public static bool IsToolPackage(XDocument nuspec) =>
-            nuspec.Root.Descendants().Where(
-                e => e.Name.LocalName == "packageType" &&
-                e.Attributes().Where(a => a.Name.LocalName == "name" && a.Value == "DotnetTool").Any()).Any();
-
         private const string PackagedShimsDirectoryConvention = "shims";
 
         public IEnumerable<string> Warnings => _toolConfiguration.Value.Warnings;

diff --git a/test/dotnet-workload-install.Tests/FailingNuGetPackageInstaller.cs b/test/dotnet-workload-install.Tests/FailingNuGetPackageInstaller.cs
@@ -23,8 +23,7 @@ public Task<string> DownloadPackageAsync(PackageId packageId, NuGetVersion packa
             bool includePreview = false,
             bool? includeUnlisted = null,
             DirectoryPath? downloadFolder = null,
-            PackageSourceMapping packageSourceMapping = null,
-            bool isTool = false)
+            PackageSourceMapping packageSourceMapping = null)
         {
             var mockPackagePath = Path.Combine(MockPackageDir, $"{packageId}.{packageVersion}.nupkg");
             File.WriteAllText(mockPackagePath, string.Empty);

diff --git a/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs b/test/dotnet-workload-install.Tests/MockNuGetPackageInstaller.cs
@@ -38,8 +38,7 @@ public Task<string> DownloadPackageAsync(PackageId packageId,
             bool includePreview = false,
             bool? includeUnlisted = null,
             DirectoryPath? downloadFolder = null,
-            PackageSourceMapping packageSourceMapping = null,
-            bool isTool = false)
+            PackageSourceMapping packageSourceMapping = null)
         {
             DownloadCallParams.Add((packageId, packageVersion, downloadFolder, packageSourceLocation));