Fixed up some errors revealed by CI

build/ToolsetPackages/roslynsdk.project.json

@@ -11,8 +11,8 @@
     "Microsoft.CodeAnalysis.VisualBasic": "1.0.0",
     "Microsoft.CodeAnalysis.VisualBasic.Workspaces": "1.0.0",
     "Microsoft.CodeAnalysis.Workspaces.Common": "1.0.0",
-    "System.Reflection.Metadata": "1.4.1-beta-24322-03",
-    "System.Collections.Immutable": "1.2.0",
+    "System.Reflection.Metadata": "1.0.21",
+    "System.Collections.Immutable": "1.1.36"
   },
   "frameworks": {
     ".NETFramework,Version=v4.6": {}

cibuild.cmd

@@ -89,7 +89,7 @@ powershell -noprofile -executionPolicy RemoteSigned -file "%RoslynRoot%\build\sc
 call :TerminateBuildProcesses
 
 REM Verify the state of our project.jsons
-"Binaries\%BuildConfiguration%\RepoUtil\RepoUtil.exe verify" || goto :BuildFailed
+.\Binaries\%BuildConfiguration%\RepoUtil\RepoUtil.exe verify || goto :BuildFailed
 
 REM Ensure caller sees successful exit.
 exit /b 0

src/Tools/RepoUtil/ChangeCommand.cs

@@ -180,7 +180,7 @@ private void ChangeAllCore(ImmutableDictionary<NuGetPackage, NuGetPackage> chang
 
             // Calculate the new set of packages based on the changed information.
             var list = new List<NuGetPackage>();
-            foreach (var cur in _repoData.AllPackages)
+            foreach (var cur in _repoData.FloatingBuildPackages)
             {
                 NuGetPackage newPackage;
                 if (changeMap.TryGetValue(cur, out newPackage))
@@ -208,13 +208,13 @@ private void ChangeProjectJsonFiles(ImmutableDictionary<NuGetPackage, NuGetPacka
             }
         }
 
-        private void ChangeGeneratedFiles(IEnumerable<NuGetPackage> allPackages)
+        private void ChangeGeneratedFiles(IEnumerable<NuGetPackage> floatingPackages)
         {
             var msbuildData = _repoData.RepoConfig.MSBuildGenerateData;
             if (msbuildData.HasValue)
             {
                 var fileName = new FileName(_repoData.SourcesPath, msbuildData.Value.RelativeFileName);
-                var packages = GenerateUtil.GetFilteredPackages(msbuildData.Value, allPackages);
+                var packages = GenerateUtil.GetFilteredPackages(msbuildData.Value, _repoData);
                 GenerateUtil.WriteMSBuildContent(fileName, packages);
             }
         }

src/Tools/RepoUtil/ConsumesCommand.cs

@@ -40,9 +40,9 @@ private JObject GoCore()
         private JProperty GetFixedPackages()
         {
             var obj = new JObject();
-            foreach (var pair in _repoData.FixedPackagesMap.OrderBy(x => x.Key))
+            foreach (var package in _repoData.FixedPackages.OrderBy(x => x.Name))
             {
-                obj.Add(GetProperty(pair.Key, pair.Value));
+                obj.Add(GetProperty(package));
             }
             return new JProperty("fixed", obj);
         }

src/Tools/RepoUtil/GenerateUtil.cs

@@ -22,9 +22,13 @@ internal static class GenerateUtil
         /// <summary>
         /// Get the subset of packages which match the specified filter for the generated file.
         /// </summary>
-        internal static IEnumerable<NuGetPackage> GetFilteredPackages(GenerateData generateData, IEnumerable<NuGetPackage> allPackages)
+        internal static IEnumerable<NuGetPackage> GetFilteredPackages(GenerateData generateData, RepoData repoData)
         {
-            return allPackages
+            // Fixed packages are never included in generated output.  Doing so would create conflicts because it's
+            // possible for two versions to exist for the same package.  Take for example System.Collections.Immuatble
+            // which is both fixed and floating in Roslyn.
+            return repoData
+                .FloatingPackages
                 .Where(x => generateData.Packages.Any(y => y.IsMatch(x.Name)))
                 .ToList();
         }
@@ -32,7 +36,7 @@ internal static IEnumerable<NuGetPackage> GetFilteredPackages(GenerateData gener
         internal static void WriteMSBuildContent(FileName fileName, IEnumerable<NuGetPackage> packages)
         {
             Console.WriteLine($"Generating MSBuild props file {fileName}");
-            using (var stream = File.OpenWrite(fileName.FullPath))
+            using (var stream = File.Open(fileName.FullPath, FileMode.Create, FileAccess.ReadWrite, FileShare.None))
             {
                 WriteMSBuildContent(stream, packages);
             }

src/Tools/RepoUtil/NuGetPackageSource.cs

+using System;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+
+namespace RepoUtil
+{
+    internal struct NuGetPackageSource
+    {
+        internal NuGetPackage NuGetPackage { get; }
+        internal FileName FileName { get; }
+
+        internal NuGetPackageSource(NuGetPackage package, FileName fileName)
+        {
+            NuGetPackage = package;
+            FileName = fileName;
+        }
+    }
+
+    internal struct NuGetPackageConflict
+    {
+        internal NuGetPackageSource Original { get; }
+        internal NuGetPackageSource Conflict { get; }
+        internal string PackageName => Original.NuGetPackage.Name;
+
+        internal NuGetPackageConflict(NuGetPackageSource original, NuGetPackageSource conflict)
+        {
+            Debug.Assert(Constants.NugetPackageNameComparer.Equals(original.NuGetPackage.Name, conflict.NuGetPackage.Name));
+            Original = original;
+            Conflict = conflict;
+        }
+    }
+}

src/Tools/RepoUtil/RepoConfig.cs

@@ -2,6 +2,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -38,8 +39,6 @@ namespace RepoUtil
     internal class RepoConfig
     {
         internal ImmutableArray<NuGetPackage> FixedPackages { get; }
-        // TODO: is this map needed?
-        internal ImmutableDictionary<string, ImmutableArray<string>> FixedPackagesMap { get; }
         internal ImmutableArray<string> ToolsetPackages { get; }
         internal ImmutableArray<Regex> NuSpecExcludes { get; }
         internal GenerateData? MSBuildGenerateData { get; }
@@ -50,11 +49,10 @@ internal class RepoConfig
             IEnumerable<Regex> nuspecExcludes,
             GenerateData? msbuildGenerateData)
         {
+            Debug.Assert(toolsetPackages.Distinct().Count() == toolsetPackages.Count());
             MSBuildGenerateData = msbuildGenerateData;
             FixedPackages = fixedPackages.OrderBy(x => x.Name).ToImmutableArray();
             NuSpecExcludes = nuspecExcludes.ToImmutableArray();
-
-            // TODO: Validate duplicate names in the floating lists
             ToolsetPackages = toolsetPackages.OrderBy(x => x).ToImmutableArray();
 
             var map = new Dictionary<string, List<string>>();
@@ -69,12 +67,6 @@ internal class RepoConfig
 
                 list.Add(nugetRef.Version);
             }
-
-            FixedPackagesMap = ImmutableDictionary<string, ImmutableArray<string>>.Empty;
-            foreach (var pair in map)
-            {
-                FixedPackagesMap = FixedPackagesMap.Add(pair.Key, pair.Value.ToImmutableArray());
-            }
         }
 
         internal static RepoConfig ReadFrom(string jsonFilePath)

src/Tools/RepoUtil/RepoData.cs

@@ -2,6 +2,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using System.IO;
 using System.Linq;
 using System.Text;
@@ -18,9 +19,8 @@ internal sealed class RepoData
         internal ImmutableArray<NuGetPackage> FloatingPackages { get; }
         internal ImmutableArray<NuGetPackage> FixedPackages => RepoConfig.FixedPackages;
         internal ImmutableArray<NuGetPackage> AllPackages { get; }
-        internal ImmutableDictionary<string, ImmutableArray<string>> FixedPackagesMap => RepoConfig.FixedPackagesMap;
 
-        internal RepoData(RepoConfig config, string sourcesPath, IEnumerable<NuGetPackage> floatingPackages)
+        private RepoData(RepoConfig config, string sourcesPath, IEnumerable<NuGetPackage> floatingPackages)
         {
             SourcesPath = sourcesPath;
             RepoConfig = config;
@@ -48,28 +48,58 @@ private static ImmutableArray<NuGetPackage> Combine(params ImmutableArray<NuGetP
 
         /// <summary>
         /// The raw RepoData contains only the fixed + toolset packages that we need to track.  This method will examine the current
-        /// state of the repo and add in the current data.
+        /// state of the repo and add in the current data.  If any conflicting package definitions are detected this method 
+        /// will throw.
         /// </summary>
         internal static RepoData Create(RepoConfig config, string sourcesPath)
         {
-            var set = new HashSet<NuGetPackage>();
-            foreach (var fileName in ProjectJsonUtil.GetProjectJsonFiles(sourcesPath))
+            List<NuGetPackageConflict> conflicts;
+            var repoData = Create(config, sourcesPath, out conflicts);
+            if (conflicts?.Count > 0)
             {
-                foreach (var nuget in ProjectJsonUtil.GetDependencies(fileName))
+                throw new Exception("Creation failed because of conflicting packages");
+            }
+
+            return repoData;
+        }
+
+        internal static RepoData Create(RepoConfig config, string sourcesPath, out List<NuGetPackageConflict> conflicts)
+        {
+            conflicts = null;
+
+            var fixedPackageSet = new HashSet<NuGetPackage>(config.FixedPackages);
+            var floatingPackageMap = new Dictionary<string, NuGetPackageSource>(Constants.NugetPackageNameComparer);
+            foreach (var filePath in ProjectJsonUtil.GetProjectJsonFiles(sourcesPath))
+            {
+                var fileName = FileName.FromFullPath(sourcesPath, filePath);
+                foreach (var package in ProjectJsonUtil.GetDependencies(filePath))
                 {
-                    if (config.FixedPackagesMap.ContainsKey(nuget.Name))
+                    if (fixedPackageSet.Contains(package))
                     {
                         continue;
                     }
 
-                    set.Add(nuget);
+                    // If this is the first time we've seen the package then record where it was found.  Need the source
+                    // information to provide better error messages later.
+                    var packageSource = new NuGetPackageSource(package, fileName);
+                    NuGetPackageSource originalSource;
+                    if (floatingPackageMap.TryGetValue(package.Name, out originalSource))
+                    {
+                        if (originalSource.NuGetPackage.Version != package.Version)
+                        {
+                            var conflict = new NuGetPackageConflict(original: originalSource, conflict: packageSource);
+                            conflicts = conflicts ?? new List<NuGetPackageConflict>();
+                            conflicts.Add(conflict);
+                        }
+                    }
+                    else
+                    {
+                        floatingPackageMap.Add(package.Name, packageSource);
+                    }
                 }
             }
 
-            return new RepoData(
-                config,
-                sourcesPath,
-                set);
+            return new RepoData(config, sourcesPath, floatingPackageMap.Values.Select(x => x.NuGetPackage));
         }
     }
 }

src/Tools/RepoUtil/RepoData.json

@@ -6,7 +6,9 @@
         "Microsoft.VisualStudio.Language.Intellisense": [ "14.3.25407", "15.0.25123-Dev15Preview" ],
         "Newtonsoft.Json": [ "6.0.4", "7.0.1", "8.0.2", "9.0.1" ],
         "xunit.runner.console": [ "2.1.0", "2.2.0-beta1-build3239" ],
-        "Microsoft.VSSDK.BuildTools": [ "14.3.25407", "15.0.25201-Dev15Preview2" ]
+        "Microsoft.VSSDK.BuildTools": [ "14.3.25407", "15.0.25201-Dev15Preview2" ],
+        "System.Reflection.Metadata": "1.0.21",
+        "System.Collections.Immutable": "1.1.36"
     },
 
     "toolsetPackages": [

src/Tools/RepoUtil/RepoUtil.csproj

@@ -50,6 +50,7 @@
     <Compile Include="JsonTypes.cs" />
     <Compile Include="NuGetPackage.cs" />
     <Compile Include="NuGetPackageChange.cs" />
+    <Compile Include="NuGetPackageSource.cs" />
     <Compile Include="NuSpecUtil.cs" />
     <Compile Include="ProducesCommand.cs" />
     <Compile Include="Program.cs" />

src/Tools/RepoUtil/VerifyCommand.cs

@@ -14,18 +14,6 @@ namespace RepoUtil
     /// </summary>
     internal sealed class VerifyCommand : ICommand
     {
-        private struct NuGetPackageSource
-        {
-            internal NuGetPackage NuGetPackage { get; }
-            internal FileName FileName { get; }
-
-            internal NuGetPackageSource(NuGetPackage package, FileName fileName)
-            {
-                NuGetPackage = package;
-                FileName = fileName;
-            }
-        }
-
         private readonly string _sourcesPath;
         private readonly RepoConfig _repoConfig;
 
@@ -37,66 +25,37 @@ internal VerifyCommand(RepoConfig repoConfig, string sourcesPath)
 
         public bool Run(TextWriter writer, string[] args)
         {
-            List<NuGetPackage> allPackages;
-            if (!VerifyProjectJsonContents(writer, out allPackages))
-            {
-                return false;
-            }
-
+            RepoData repoData;
             return
+                VerifyProjectJsonContents(writer, out repoData) &&
                 VerifyRepoConfig(writer) &&
-                VerifyGeneratedFiles(writer, allPackages);
+                VerifyGeneratedFiles(writer, repoData);
         }
 
         /// <summary>
         /// Verify the packages listed in project.json are well formed.  Packages should all either have the same version or 
         /// be explicitly fixed in the config file.
         /// </summary>
-        private bool VerifyProjectJsonContents(TextWriter writer, out List<NuGetPackage> allPackages)
+        private bool VerifyProjectJsonContents(TextWriter writer, out RepoData repoData)
         {
             writer.WriteLine($"Verifying project.json contents");
-            var allGood = true;
-            var fixedPackageSet = new HashSet<NuGetPackage>(_repoConfig.FixedPackages);
-            var floatingPackageMap = new Dictionary<string, NuGetPackageSource>(Constants.NugetPackageNameComparer);
-            foreach (var filePath in ProjectJsonUtil.GetProjectJsonFiles(_sourcesPath))
-            {
-                var fileName = FileName.FromFullPath(_sourcesPath, filePath);
-                foreach (var package in ProjectJsonUtil.GetDependencies(filePath))
-                {
-                    if (fixedPackageSet.Contains(package))
-                    {
-                        continue;
-                    }
-
-                    NuGetPackageSource source;
-
-                    // If this is the first time we've seen the package then record where it was found.  Need the source
-                    // information to provide better error messages later.
-                    if (!floatingPackageMap.TryGetValue(package.Name, out source))
-                    {
-                        source = new NuGetPackageSource(package, fileName);
-                        floatingPackageMap.Add(package.Name, source);
-                        continue;
-                    }
 
-                    if (source.NuGetPackage != package)
-                    {
-                        writer.WriteLine($"Error! Package {package.Name} has different versions:");
-                        writer.WriteLine($"\t{fileName} at {package.Version}");
-                        writer.WriteLine($"\t{source.FileName} at {source.NuGetPackage.Version}");
-                        writer.WriteLine($"The versions must be the same or one must be explicitly listed as fixed in RepoData.json");
-                        allGood = false;
-                    }
+            List<NuGetPackageConflict> conflicts;
+            repoData = RepoData.Create(_repoConfig, _sourcesPath, out conflicts);
+            if (conflicts?.Count > 0)
+            { 
+                foreach (var conflict in conflicts)
+                {
+                    writer.WriteLine($"Error! Package {conflict.PackageName} has different versions:");
+                    writer.WriteLine($"\t{conflict.Original.FileName} at {conflict.Original.NuGetPackage.Version}");
+                    writer.WriteLine($"\t{conflict.Conflict.FileName} at {conflict.Conflict.NuGetPackage.Version}");
+                    writer.WriteLine($"The versions must be the same or one must be explicitly listed as fixed in RepoData.json");
                 }
+
+                return false;
             }
 
-            allPackages = floatingPackageMap
-                .Values
-                .Select(x => x.NuGetPackage)
-                .Concat(fixedPackageSet)
-                .OrderBy(x => x.Name)
-                .ToList();
-            return allGood;
+            return true;
         }
 
         /// <summary>
@@ -125,14 +84,14 @@ private bool VerifyRepoConfig(TextWriter writer)
             return allGood;
         }
 
-        private bool VerifyGeneratedFiles(TextWriter writer, List<NuGetPackage> allPackages)
+        private bool VerifyGeneratedFiles(TextWriter writer, RepoData repoData)
         {
             var allGood = true;
             writer.WriteLine($"Verifying generated files");
             if (_repoConfig.MSBuildGenerateData.HasValue)
             {
                 var data = _repoConfig.MSBuildGenerateData.Value;
-                var packages = GenerateUtil.GetFilteredPackages(data, allPackages);
+                var packages = GenerateUtil.GetFilteredPackages(data, repoData);
                 var fileName = new FileName(_sourcesPath, data.RelativeFileName);
                 var actualContent = File.ReadAllText(fileName.FullPath, GenerateUtil.Encoding);
                 var expectedContent = GenerateUtil.GenerateMSBuildContent(packages);