diff --git a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageAssetsTask.cs b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageAssetsTask.cs index e0aaa508409c..c75c1fe46a67 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageAssetsTask.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/GivenAResolvePackageAssetsTask.cs @@ -4,7 +4,6 @@ using FluentAssertions; using Microsoft.Build.Framework; using System; -using System.Collections; using System.Collections.Generic; using System.IO; using System.Linq; @@ -117,15 +116,11 @@ public void It_does_not_error_on_duplicate_package_names() new CacheWriter(task); // Should not error } - [Fact] - public void It_warns_on_invalid_culture_codes_of_resources() - { - string projectAssetsJsonPath = Path.GetTempFileName(); - var assetsContent = @" + private static string AssetsFileWithInvalidLocale(string tfm, string locale) => @" { `version`: 3, `targets`: { - `net7.0`: { + `{tfm}`: { `JavaScriptEngineSwitcher.Core/3.3.0`: { `type`: `package`, `compile`: { @@ -136,59 +131,65 @@ public void It_warns_on_invalid_culture_codes_of_resources() }, `resource`: { `lib/netstandard2.0/ru-ru/JavaScriptEngineSwitcher.Core.resources.dll`: { - `locale`: `what is this even` + `locale`: `{locale}` } } } } + }, + `project`: { + `version`: `1.0.0`, + `frameworks`: { + `{tfm}`: { + `targetAlias`: `{tfm}` + } + } } -}".Replace("`", "\""); +}".Replace("`", "\"").Replace("{tfm}", tfm).Replace("{locale}", locale); + + [InlineData("net7.0", true)] + [InlineData("net6.0", false)] + [Theory] + public void It_warns_on_invalid_culture_codes_of_resources(string tfm, bool shouldHaveWarnings) + { + string projectAssetsJsonPath = Path.GetTempFileName(); + var assetsContent = AssetsFileWithInvalidLocale(tfm, "what is this even"); File.WriteAllText(projectAssetsJsonPath, assetsContent); var task = InitializeTask(out _); task.ProjectAssetsFile = projectAssetsJsonPath; - task.TargetFramework = "net7.0"; - var writer = new CacheWriter(task); - writer.WriteToCacheFile(); - var messages = (task.BuildEngine as MockBuildEngine).Warnings; - var invalidContextMessages = messages.Where(msg => msg.Code == "NETSDK1188"); - invalidContextMessages.Should().HaveCount(1); + task.TargetFramework = tfm; + var writer = new CacheWriter(task, new MockPackageResolver()); + writer.WriteToMemoryStream(); + var engine = task.BuildEngine as MockBuildEngine; + + var invalidContextWarnings = engine.Warnings.Where(msg => msg.Code == "NETSDK1188"); + invalidContextWarnings.Should().HaveCount(shouldHaveWarnings ? 1 : 0); + + var invalidContextMessages = engine.Messages.Where(msg => msg.Code == "NETSDK1188"); + invalidContextMessages.Should().HaveCount(shouldHaveWarnings ? 0 : 1); + } - - [Fact] - public void It_warns_on_incorrectly_cased_culture_codes_of_resources() + + [InlineData("net7.0", true)] + [InlineData("net6.0", false)] + [Theory] + public void It_warns_on_incorrectly_cased_culture_codes_of_resources(string tfm, bool shouldHaveWarnings) { string projectAssetsJsonPath = Path.GetTempFileName(); - var assetsContent = @" -{ - `version`: 3, - `targets`: { - `net7.0`: { - `JavaScriptEngineSwitcher.Core/3.3.0`: { - `type`: `package`, - `compile`: { - `lib/netstandard2.0/JavaScriptEngineSwitcher.Core.dll`: {} - }, - `runtime`: { - `lib/netstandard2.0/JavaScriptEngineSwitcher.Core.dll`: {} - }, - `resource`: { - `lib/netstandard2.0/ru-ru/JavaScriptEngineSwitcher.Core.resources.dll`: { - `locale`: `ru-ru` - } - } - } - } - } -}".Replace("`", "\""); + var assetsContent = AssetsFileWithInvalidLocale(tfm, "ru-ru"); File.WriteAllText(projectAssetsJsonPath, assetsContent); var task = InitializeTask(out _); task.ProjectAssetsFile = projectAssetsJsonPath; - task.TargetFramework = "net7.0"; - var writer = new CacheWriter(task); - writer.WriteToCacheFile(); - var messages = (task.BuildEngine as MockBuildEngine).Warnings; - var invalidContextMessages = messages.Where(msg => msg.Code == "NETSDK1187"); - invalidContextMessages.Should().HaveCount(1); + task.TargetFramework = tfm; + var writer = new CacheWriter(task, new MockPackageResolver()); + writer.WriteToMemoryStream(); + var engine = task.BuildEngine as MockBuildEngine; + + var invalidContextWarnings = engine.Warnings.Where(msg => msg.Code == "NETSDK1187"); + invalidContextWarnings.Should().HaveCount(shouldHaveWarnings ? 1 : 0); + + var invalidContextMessages = engine.Messages.Where(msg => msg.Code == "NETSDK1187"); + invalidContextMessages.Should().HaveCount(shouldHaveWarnings ? 0 : 1); } private ResolvePackageAssets InitializeTask(out IEnumerable inputProperties) diff --git a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Mocks/MockPackageResolver.cs b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Mocks/MockPackageResolver.cs index 85ccb9da17c2..1945185459ba 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Mocks/MockPackageResolver.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks.UnitTests/Mocks/MockPackageResolver.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using NuGet.ProjectModel; using NuGet.Versioning; using System.IO; @@ -24,5 +25,7 @@ public string GetPackageDirectory(string packageId, NuGetVersion version, out st packageRoot = _root; return Path.Combine(_root, packageId, version.ToNormalizedString(), "path"); } + + public string ResolvePackageAssetPath(LockFileTargetLibrary package, string relativePath) => Path.Combine(GetPackageDirectory(package.Name, package.Version), relativePath); } -} \ No newline at end of file +} diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/IPackageResolver.cs b/src/Tasks/Microsoft.NET.Build.Tasks/IPackageResolver.cs index 1540f00e5089..e208ba1bcfca 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/IPackageResolver.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/IPackageResolver.cs @@ -1,6 +1,7 @@ // Copyright (c) .NET Foundation and contributors. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using NuGet.ProjectModel; using NuGet.Versioning; namespace Microsoft.NET.Build.Tasks @@ -9,5 +10,6 @@ internal interface IPackageResolver { string GetPackageDirectory(string packageId, NuGetVersion version); string GetPackageDirectory(string packageId, NuGetVersion version, out string packageRoot); + string ResolvePackageAssetPath(LockFileTargetLibrary package, string relativePath); } } diff --git a/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs b/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs index a9880dbd8fc7..bcdcf048ffc6 100644 --- a/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs +++ b/src/Tasks/Microsoft.NET.Build.Tasks/ResolvePackageAssets.cs @@ -657,8 +657,8 @@ internal sealed class CacheWriter : IDisposable private ResolvePackageAssets _task; private BinaryWriter _writer; private LockFile _lockFile; - private NuGetPackageResolver _packageResolver; private LockFileTarget _compileTimeTarget; + private IPackageResolver _packageResolver; private LockFileTarget _runtimeTarget; private Dictionary _stringTable; private List _metadataStrings; @@ -677,6 +677,15 @@ internal sealed class CacheWriter : IDisposable private const char RelatedPropertySeparator = ';'; + /// + /// This constructor should only be used for testing - IPackgeResolver carries a lot of + /// state so using mocks really help with testing this component. + /// + public CacheWriter(ResolvePackageAssets task, IPackageResolver resolver) : this(task) + { + _packageResolver = resolver; + } + public CacheWriter(ResolvePackageAssets task) { _targetFramework = task.TargetFramework; @@ -1425,14 +1434,30 @@ private void WriteResourceAssemblies() var normalizedLocale = System.Globalization.CultureInfo.GetCultureInfo(locale).Name; if (normalizedLocale != locale) { - _task.Log.LogWarning(Strings.PackageContainsIncorrectlyCasedLocale, package.Name, package.Version.ToNormalizedString(), locale, normalizedLocale); + var tfm = _lockFile.GetTargetAndThrowIfNotFound(_targetFramework, null).TargetFramework; + if (tfm.Version.Major >= 7) + { + _task.Log.LogWarning(Strings.PackageContainsIncorrectlyCasedLocale, package.Name, package.Version.ToNormalizedString(), locale, normalizedLocale); + } + else + { + _task.Log.LogMessage(Strings.PackageContainsIncorrectlyCasedLocale, package.Name, package.Version.ToNormalizedString(), locale, normalizedLocale); + } } locale = normalizedLocale; } catch (System.Globalization.CultureNotFoundException cnf) { - _task.Log.LogWarning(Strings.PackageContainsUnknownLocale, package.Name, package.Version.ToNormalizedString(), cnf.InvalidCultureName); - // We could potentially strip this unknown locales at this point, but we do not. + var tfm = _lockFile.GetTargetAndThrowIfNotFound(_targetFramework, null).TargetFramework; + if (tfm.Version.Major >= 7) + { + _task.Log.LogWarning(Strings.PackageContainsUnknownLocale, package.Name, package.Version.ToNormalizedString(), cnf.InvalidCultureName); + } else + { + _task.Log.LogMessage(Strings.PackageContainsUnknownLocale, package.Name, package.Version.ToNormalizedString(), cnf.InvalidCultureName); + } + + // We could potentially strip this unknown locale at this point, but we do not. // Locale data can change over time (it's typically an OS database that's kept updated), // and the data on the system running the build may not be the same data as // the system executing the built code. So we should be permissive for this case.