From 695bf56afc992ffb8d17a601001223e3997ed9f7 Mon Sep 17 00:00:00 2001 From: Nate McMaster Date: Tue, 8 May 2018 16:51:26 -0700 Subject: [PATCH] Run VerifyCoherentVersions check (#1156) All packages were being ignored. This adds an error if 0 packages are found, and also fixes the folder scanned for coherence. This also required removing the 'Private' designation from external dependencies, because we no longer scan just shipping packages, Removed unused NoWarn metadata as well. --- build/external-dependencies.props | 86 +++++++++---------- build/repo.targets | 2 +- build/tasks/ProjectModel/PackageInfo.cs | 2 + .../ProjectModel/PackageReferenceInfo.cs | 4 +- .../tasks/ProjectModel/ProjectInfoFactory.cs | 6 +- build/tasks/VerifyCoherentVersions.cs | 40 ++++----- 6 files changed, 63 insertions(+), 77 deletions(-) diff --git a/build/external-dependencies.props b/build/external-dependencies.props index dea8368864..6554f43a9d 100644 --- a/build/external-dependencies.props +++ b/build/external-dependencies.props @@ -5,10 +5,6 @@ - - - - false false @@ -17,13 +13,13 @@ - - + + - - - - + + + + @@ -39,29 +35,29 @@ - + - + - - + + - - - - - + + + + + - + - + @@ -72,29 +68,29 @@ - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + - - - + + + @@ -116,11 +112,11 @@ - + - + @@ -159,6 +155,6 @@ - + diff --git a/build/repo.targets b/build/repo.targets index 4d8f5246f8..32e0796db9 100644 --- a/build/repo.targets +++ b/build/repo.targets @@ -205,7 +205,7 @@ - + diff --git a/build/tasks/ProjectModel/PackageInfo.cs b/build/tasks/ProjectModel/PackageInfo.cs index 463fab690a..bb499a1b21 100644 --- a/build/tasks/ProjectModel/PackageInfo.cs +++ b/build/tasks/ProjectModel/PackageInfo.cs @@ -38,5 +38,7 @@ namespace RepoTasks.ProjectModel /// public string Source { get; } public IReadOnlyList DependencyGroups { get; } + + public override string ToString() => $"{Id}/{Version}"; } } diff --git a/build/tasks/ProjectModel/PackageReferenceInfo.cs b/build/tasks/ProjectModel/PackageReferenceInfo.cs index d01089d594..cea7d566ef 100644 --- a/build/tasks/ProjectModel/PackageReferenceInfo.cs +++ b/build/tasks/ProjectModel/PackageReferenceInfo.cs @@ -8,7 +8,7 @@ namespace RepoTasks.ProjectModel { internal class PackageReferenceInfo { - public PackageReferenceInfo(string id, string version, bool isImplicitlyDefined, IReadOnlyList noWarn) + public PackageReferenceInfo(string id, string version, bool isImplicitlyDefined) { if (string.IsNullOrEmpty(id)) { @@ -18,12 +18,10 @@ namespace RepoTasks.ProjectModel Id = id; Version = version; IsImplicitlyDefined = isImplicitlyDefined; - NoWarn = noWarn; } public string Id { get; } public string Version { get; } public bool IsImplicitlyDefined { get; } - public IReadOnlyList NoWarn { get; } } } diff --git a/build/tasks/ProjectModel/ProjectInfoFactory.cs b/build/tasks/ProjectModel/ProjectInfoFactory.cs index 42c0bd3fee..350fed398d 100644 --- a/build/tasks/ProjectModel/ProjectInfoFactory.cs +++ b/build/tasks/ProjectModel/ProjectInfoFactory.cs @@ -106,12 +106,8 @@ namespace RepoTasks.ProjectModel foreach (var item in project.GetItems("PackageReference")) { bool.TryParse(item.GetMetadataValue("IsImplicitlyDefined"), out var isImplicit); - var noWarn = item.GetMetadataValue("NoWarn"); - IReadOnlyList noWarnItems = string.IsNullOrEmpty(noWarn) - ? Array.Empty() - : MSBuildListSplitter.SplitItemList(noWarn).ToArray(); - var info = new PackageReferenceInfo(item.EvaluatedInclude, item.GetMetadataValue("Version"), isImplicit, noWarnItems); + var info = new PackageReferenceInfo(item.EvaluatedInclude, item.GetMetadataValue("Version"), isImplicit); if (references.ContainsKey(info.Id)) { diff --git a/build/tasks/VerifyCoherentVersions.cs b/build/tasks/VerifyCoherentVersions.cs index e6800f03c8..a9b0f92644 100644 --- a/build/tasks/VerifyCoherentVersions.cs +++ b/build/tasks/VerifyCoherentVersions.cs @@ -25,21 +25,23 @@ namespace RepoTasks public override bool Execute() { + if (PackageFiles.Length == 0) + { + Log.LogError("Did not find any packages to verify for version coherence"); + return false; + } + var packageLookup = new Dictionary(StringComparer.OrdinalIgnoreCase); - var dependencyMap = new Dictionary>(StringComparer.OrdinalIgnoreCase); + var dependencyMap = new Dictionary>(StringComparer.OrdinalIgnoreCase); foreach (var dep in ExternalDependencies) { if (!dependencyMap.TryGetValue(dep.ItemSpec, out var list)) { - dependencyMap[dep.ItemSpec] = list = new List(); + dependencyMap[dep.ItemSpec] = list = new List(); } - var externalDep = new ExternalDependency - { - Version = dep.GetMetadata("Version"), - IsPrivate = bool.TryParse(dep.GetMetadata("Private"), out var isPrivate) && isPrivate, - }; - list.Add(externalDep); + + list.Add(dep.GetMetadata("Version")); } foreach (var file in PackageFiles) @@ -56,11 +58,12 @@ namespace RepoTasks if (packageLookup.TryGetValue(package.Id, out var existingPackage)) { - throw new Exception("Multiple copies of the following package were found: " + + Log.LogError("Multiple copies of the following package were found: " + Environment.NewLine + existingPackage + Environment.NewLine + package); + continue; } packageLookup[package.Id] = package; @@ -75,15 +78,9 @@ namespace RepoTasks return !Log.HasLoggedErrors; } - private class ExternalDependency - { - public string Version { get; set; } - public bool IsPrivate { get; set; } - } - private void Visit( IReadOnlyDictionary packageLookup, - IReadOnlyDictionary> dependencyMap, + IReadOnlyDictionary> dependencyMap, PackageInfo packageInfo) { Log.LogMessage(MessageImportance.Low, $"Processing package {packageInfo.Id}"); @@ -95,20 +92,17 @@ namespace RepoTasks { PackageInfo dependencyPackageInfo; var depVersion = dependency.VersionRange.MinVersion.ToString(); - if (dependencyMap.TryGetValue(dependency.Id, out var externalDependencies)) + if (dependencyMap.TryGetValue(dependency.Id, out var externalDepVersions)) { - var matchedVersion = externalDependencies.FirstOrDefault(d => depVersion.Equals(d.Version)); + var matchedVersion = externalDepVersions.FirstOrDefault(d => depVersion.Equals(d)); if (matchedVersion == null) { - var versions = string.Join(" or ", externalDependencies.Select(d => d.Version)); + var versions = string.Join(" or ", externalDepVersions); Log.LogError($"Package {packageInfo.Id} has an external dependency on the wrong version of {dependency.Id}. " + $"It uses {depVersion} but only {versions} is allowed."); } - else if (matchedVersion.IsPrivate) - { - Log.LogError($"Package {packageInfo.Id} has an external dependency on {dependency.Id}/{depVersion} which is marked as Private=true."); - } + continue; } else if (!packageLookup.TryGetValue(dependency.Id, out dependencyPackageInfo))