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

Clean up error experience when downloading non-tools #43045

Merged
merged 12 commits into from
Sep 16, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ Task<string> DownloadPackageAsync(PackageId packageId,
bool includePreview = false,
bool? includeUnlisted = null,
DirectoryPath? downloadFolder = null,
PackageSourceMapping packageSourceMapping = null);
PackageSourceMapping packageSourceMapping = null,
bool isTool = false);

Task<string> GetPackageUrl(PackageId packageId,
NuGetVersion packageVersion = null,
Expand Down
3 changes: 3 additions & 0 deletions src/Cli/dotnet/NugetPackageDownloader/LocalizableStrings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@
<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>
Expand Down
34 changes: 30 additions & 4 deletions src/Cli/dotnet/NugetPackageDownloader/NuGetPackageDownloader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,19 +83,45 @@ public async Task<string> DownloadPackageAsync(PackageId packageId,
bool includePreview = false,
bool? includeUnlisted = null,
DirectoryPath? downloadFolder = null,
PackageSourceMapping packageSourceMapping = null)
PackageSourceMapping packageSourceMapping = null,
bool isTool = false)
{
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);

resource = await repository.GetResourceAsync<FindPackageByIdResource>(cancellationToken)
.ConfigureAwait(false);
// 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";
joeloff marked this conversation as resolved.
Show resolved Hide resolved

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)
.ConfigureAwait(false);
if (resource == null)
{
throw new NuGetPackageNotFoundException(
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

22 changes: 15 additions & 7 deletions src/Cli/dotnet/ToolPackage/ToolPackageDownloader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -134,13 +134,21 @@ public IToolPackage InstallPackage(PackageLocation packageLocation, PackageId pa
{
DownloadAndExtractPackage(packageId, nugetPackageDownloader, toolDownloadDir.Value, packageVersion, packageSourceLocation, includeUnlisted: givenSpecificVersion).GetAwaiter().GetResult();
}
else if(isGlobalTool)
else
{
throw new ToolPackageException(
string.Format(
CommonLocalizableStrings.ToolPackageConflictPackageId,
packageId,
packageVersion.ToNormalizedString()));
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()));
}
}

CreateAssetFile(packageId, packageVersion, toolDownloadDir, assetFileDirectory, _runtimeJsonPath, targetFramework);
Expand Down Expand Up @@ -284,7 +292,7 @@ private static async Task<NuGetVersion> DownloadAndExtractPackage(
bool includeUnlisted = false
)
{
var packagePath = await nugetPackageDownloader.DownloadPackageAsync(packageId, packageVersion, packageSourceLocation, includeUnlisted: includeUnlisted).ConfigureAwait(false);
var packagePath = await nugetPackageDownloader.DownloadPackageAsync(packageId, packageVersion, packageSourceLocation, includeUnlisted: includeUnlisted, isTool: true).ConfigureAwait(false);

// look for package on disk and read the version
NuGetVersion version;
Expand Down
11 changes: 11 additions & 0 deletions src/Cli/dotnet/ToolPackage/ToolPackageInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,17 @@ 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ public Task<string> DownloadPackageAsync(PackageId packageId, NuGetVersion packa
bool includePreview = false,
bool? includeUnlisted = null,
DirectoryPath? downloadFolder = null,
PackageSourceMapping packageSourceMapping = null)
PackageSourceMapping packageSourceMapping = null,
bool isTool = false)
{
var mockPackagePath = Path.Combine(MockPackageDir, $"{packageId}.{packageVersion}.nupkg");
File.WriteAllText(mockPackagePath, string.Empty);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public Task<string> DownloadPackageAsync(PackageId packageId,
bool includePreview = false,
bool? includeUnlisted = null,
DirectoryPath? downloadFolder = null,
PackageSourceMapping packageSourceMapping = null)
PackageSourceMapping packageSourceMapping = null,
bool isTool = false)
{
DownloadCallParams.Add((packageId, packageVersion, downloadFolder, packageSourceLocation));

Expand Down
Loading