diff --git a/.editorconfig b/.editorconfig index 56417305..3fdaec25 100644 --- a/.editorconfig +++ b/.editorconfig @@ -82,7 +82,7 @@ dotnet_naming_style.start_underscore_style.required_prefix = _ [*.cs] # Don't prefer "var" when not apparent csharp_style_var_for_built_in_types = false : warning -csharp_style_var_when_type_is_apparent = true : suggestion +csharp_style_var_when_type_is_apparent = false : silent csharp_style_var_elsewhere = false : warning # Prefer method-like constructs to have a block body diff --git a/CHANGELOG.md b/CHANGELOG.md index 48330394..3b6e4159 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Commit: TBD - Adds fileMappings support for filesystem provider - Fixes LIB016 error when using fileMappings - Fixes issue where restore-on-save in one project in VS removes files restored in a separate project +- Fixes issue where CLI install command did not expand templated expressions in defaultDestination ## 3.0.71 Commit: 33c04f70a4f55f1cddbaddad60fc78a282b298d3 diff --git a/src/LibraryManager/Json/LibraryStateToFileConverter.cs b/src/LibraryManager/Json/LibraryStateToFileConverter.cs index 7eede5e7..00024aeb 100644 --- a/src/LibraryManager/Json/LibraryStateToFileConverter.cs +++ b/src/LibraryManager/Json/LibraryStateToFileConverter.cs @@ -6,6 +6,7 @@ using System.Text; using Microsoft.Web.LibraryManager.Contracts; using Microsoft.Web.LibraryManager.LibraryNaming; +using Microsoft.Web.LibraryManager.Utilities; namespace Microsoft.Web.LibraryManager.Json { @@ -30,7 +31,7 @@ public ILibraryInstallationState ConvertToLibraryInstallationState(LibraryInstal string provider = string.IsNullOrEmpty(stateOnDisk.ProviderId) ? _defaultProvider : stateOnDisk.ProviderId; (string name, string version) = LibraryIdToNameAndVersionConverter.Instance.GetLibraryNameAndVersion(stateOnDisk.LibraryId, provider); - string destination = string.IsNullOrEmpty(stateOnDisk.DestinationPath) ? ExpandDestination(_defaultDestination, name, version) : stateOnDisk.DestinationPath; + string destination = string.IsNullOrEmpty(stateOnDisk.DestinationPath) ? PathTemplateUtility.ExpandPathTemplate(_defaultDestination, name, version) : stateOnDisk.DestinationPath; var state = new LibraryInstallationState() { @@ -47,31 +48,6 @@ public ILibraryInstallationState ConvertToLibraryInstallationState(LibraryInstal return state; } - /// - /// Expands [Name] and [Version] tokens in the DefaultDestination - /// - /// The default destination string - /// Package name - /// Package version - /// - [SuppressMessage("Globalization", "CA1307:Specify StringComparison for clarity", Justification = "Not available on net481, not needed here (caseless)")] - private string ExpandDestination(string destination, string name, string version) - { - if (destination is null || !destination.Contains("[")) - { - return destination; - } - - // if the name contains a slash (either filesystem or scoped packages), - // trim that and only take the last segment. - int cutIndex = name.LastIndexOfAny(['/', '\\']); - - StringBuilder stringBuilder = new StringBuilder(destination); - stringBuilder.Replace("[Name]", cutIndex == -1 ? name : name.Substring(cutIndex + 1)); - stringBuilder.Replace("[Version]", version); - return stringBuilder.ToString(); - } - public LibraryInstallationStateOnDisk ConvertToLibraryInstallationStateOnDisk(ILibraryInstallationState state) { if (state == null) diff --git a/src/LibraryManager/Manifest.cs b/src/LibraryManager/Manifest.cs index 6a08006b..5222ad63 100644 --- a/src/LibraryManager/Manifest.cs +++ b/src/LibraryManager/Manifest.cs @@ -12,6 +12,7 @@ using Microsoft.Web.LibraryManager.Helpers; using Microsoft.Web.LibraryManager.Json; using Microsoft.Web.LibraryManager.LibraryNaming; +using Microsoft.Web.LibraryManager.Utilities; using Newtonsoft.Json; namespace Microsoft.Web.LibraryManager @@ -150,7 +151,7 @@ private static void UpdateLibraryProviderAndDestination(ILibraryInstallationStat if (libraryState.DestinationPath == null) { - libraryState.DestinationPath = defaultDestination; + libraryState.DestinationPath = PathTemplateUtility.ExpandPathTemplate(defaultDestination, state.Name, state.Version); libraryState.IsUsingDefaultDestination = true; } } diff --git a/src/LibraryManager/Utilities/PathTemplateUtility.cs b/src/LibraryManager/Utilities/PathTemplateUtility.cs new file mode 100644 index 00000000..fa5566e6 --- /dev/null +++ b/src/LibraryManager/Utilities/PathTemplateUtility.cs @@ -0,0 +1,37 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics.CodeAnalysis; +using System.Linq; + +namespace Microsoft.Web.LibraryManager.Utilities +{ + /// + /// Utility for path template operations. + /// + public static class PathTemplateUtility + { + /// + /// Expands a path template using [Name] and [Version] tokens. + /// + /// Template string + /// Library name + /// Library version + [SuppressMessage("Globalization", "CA1307:Specify StringComparison for clarity", Justification = "Not available on net481, not needed here (caseless)")] + public static string ExpandPathTemplate(string template, string name, string version) + { + if (template is null || !template.Contains('[')) + { + return template; + } + + // if the name contains a slash (either filesystem or scoped packages), + // trim that and only take the last segment as the library name. + int cutIndex = name.LastIndexOfAny(['/', '\\']); + name = cutIndex == -1 ? name : name.Substring(cutIndex + 1); + + return template.Replace("[Name]", name) + .Replace("[Version]", version); + } + } +} diff --git a/src/libman/Commands/InstallCommand.cs b/src/libman/Commands/InstallCommand.cs index c03d8547..4b1ada55 100644 --- a/src/libman/Commands/InstallCommand.cs +++ b/src/libman/Commands/InstallCommand.cs @@ -52,7 +52,6 @@ public InstallCommand(IHostEnvironment hostEnvironment, bool throwOnUnexpectedAr private IProvider _provider; private ILibraryCatalog _catalog; - private string InstallDestination { get; set; } private string ProviderId { get; set; } private IProvider ProviderToUse @@ -116,26 +115,21 @@ protected override async Task ExecuteInternalAsync() _manifest.DefaultProvider = ProviderId; } - InstallDestination = Destination.HasValue() ? Destination.Value() : _manifest.DefaultDestination; - if (string.IsNullOrWhiteSpace(InstallDestination)) + string installDestination = Destination.Value(); + if (string.IsNullOrWhiteSpace(installDestination) && string.IsNullOrWhiteSpace(_manifest.DefaultDestination)) { + // if there isn't a usable destination, prompt the user for one string destinationHint = string.Join('/', Settings.DefaultDestinationRoot, ProviderToUse.GetSuggestedDestination(library)); - InstallDestination = GetUserInputWithDefault( + installDestination = GetUserInputWithDefault( fieldName: nameof(Destination), defaultFieldValue: destinationHint, optionLongName: Destination.LongName); } - string destinationToUse = Destination.Value(); - if (string.IsNullOrWhiteSpace(_manifest.DefaultDestination) && string.IsNullOrWhiteSpace(destinationToUse)) - { - destinationToUse = InstallDestination; - } - - if (destinationToUse is not null) + if (installDestination is not null) { // in case the user changed the suggested default, normalize separator to / - destinationToUse = destinationToUse.Replace('\\', '/'); + installDestination = installDestination.Replace('\\', '/'); } OperationResult result = await _manifest.InstallLibraryAsync( @@ -143,13 +137,13 @@ protected override async Task ExecuteInternalAsync() library.Version, providerIdToUse, files, - destinationToUse, + installDestination, CancellationToken.None); if (result.Success) { await _manifest.SaveAsync(Settings.ManifestFileName, CancellationToken.None); - Logger.Log(string.Format(Resources.Text.InstalledLibrary, libraryId, InstallDestination), LogLevel.Operation); + Logger.Log(string.Format(Resources.Text.InstalledLibrary, libraryId, installDestination), LogLevel.Operation); } else { diff --git a/test/libman.IntegrationTest/CliBaseTest.cs b/test/libman.IntegrationTest/CliBaseTest.cs index 29983ea6..ec338730 100644 --- a/test/libman.IntegrationTest/CliBaseTest.cs +++ b/test/libman.IntegrationTest/CliBaseTest.cs @@ -23,14 +23,34 @@ public class CliTestBase [TestInitialize] public async Task TestInitialize() { + // Create a test directory for the project where we'll run the tool. This isolates it from + // inheriting any build settings from our solution. _testDirectory = Path.Combine(Path.GetTempPath(), "LibmanTest" + Guid.NewGuid().ToString()); Directory.CreateDirectory(_testDirectory); - // create an empty nuget.config with only our package source - await RunDotnetCommandLineAsync("new nugetconfig"); - await RunDotnetCommandLineAsync("nuget remove source nuget"); - await RunDotnetCommandLineAsync("nuget add source ./TestPackages"); - + // Create an empty nuget.config with only our package source + // We need to set packageSourceMappings to override the defaults. + // This is needed because external devs may need to override the root nuget.config + // to build (see https://github.com/aspnet/LibraryManager/issues/728), and those + // settings are inherited in the test directory. + string nugetConfigContent = """ + + + + + + + + + + + + + """; + File.WriteAllText("nuget.config", nugetConfigContent); + + // This installs the tool in the current (test) working directory, not the project directory + // created above. await InstallCliToolAsync(); } diff --git a/test/libman.IntegrationTest/InstallTests.cs b/test/libman.IntegrationTest/InstallTests.cs index 3cb8d852..d80719bc 100644 --- a/test/libman.IntegrationTest/InstallTests.cs +++ b/test/libman.IntegrationTest/InstallTests.cs @@ -15,4 +15,21 @@ public async Task Install_FileSpecified() AssertFileExists("test/jquery/jquery.min.js"); } + + [TestMethod] + public async Task Install_UsingTemplateInDefaultDestination() + { + string manifest = """ + { + "version": "3.0", + "defaultProvider": "cdnjs", + "defaultDestination": "wwwroot/lib/[Name]/" + } + """; + await CreateManifestFileAsync(manifest); + + await ExecuteCliToolAsync("install bootstrap@5.3.2 --provider cdnjs --files css/bootstrap.min.css --files js/bootstrap.bundle.min.js"); + AssertFileExists("wwwroot/lib/bootstrap/css/bootstrap.min.css"); + AssertFileExists("wwwroot/lib/bootstrap/js/bootstrap.bundle.min.js"); + } }