Merge pull request #2268 from eerhardt/UpdateDepsMessage

Enhance the update-dependencies script

scripts/update-dependencies/BuildContextProperties.cs

@@ -0,0 +1,30 @@
+// 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 System.Collections.Generic;
+using Microsoft.DotNet.Cli.Build.Framework;
+
+namespace Microsoft.DotNet.Scripts
+{
+    public static class BuildContextProperties
+    {
+        public static List<DependencyInfo> GetDependencyInfos(this BuildTargetContext c)
+        {
+            const string propertyName = "DependencyInfos";
+
+            List<DependencyInfo> dependencyInfos;
+            object dependencyInfosObj;
+            if (c.BuildContext.Properties.TryGetValue(propertyName, out dependencyInfosObj))
+            {
+                dependencyInfos = (List<DependencyInfo>)dependencyInfosObj;
+            }
+            else
+            {
+                dependencyInfos = new List<DependencyInfo>();
+                c.BuildContext[propertyName] = dependencyInfos;
+            }
+
+            return dependencyInfos;
+        }
+    }
+}

scripts/update-dependencies/Config.cs

@@ -23,6 +23,7 @@ namespace Microsoft.DotNet.Scripts
     /// GITHUB_UPSTREAM_OWNER - The owner of the GitHub base repo to create the PR to. (ex. "dotnet")
     /// GITHUB_PROJECT - The repo name under the ORIGIN and UPSTREAM owners. (ex. "cli")
     /// GITHUB_UPSTREAM_BRANCH - The branch in the GitHub base repo to create the PR to. (ex. "rel/1.0.0")
+    /// GITHUB_PULL_REQUEST_NOTIFICATIONS - A semi-colon ';' separated list of GitHub users to notify on the PR.
     /// </remarks>
     public class Config
     {
@@ -36,6 +37,7 @@ namespace Microsoft.DotNet.Scripts
         public string GitHubUpstreamOwner { get; set; }
         public string GitHubProject { get; set; }
         public string GitHubUpstreamBranch { get; set; }
+        public string[] GitHubPullRequestNotifications { get; set; }
 
         private static Config Read()
         {
@@ -52,18 +54,20 @@ namespace Microsoft.DotNet.Scripts
                 GitHubUpstreamOwner = GetEnvironmentVariable("GITHUB_UPSTREAM_OWNER", "dotnet"),
                 GitHubProject = GetEnvironmentVariable("GITHUB_PROJECT", "cli"),
                 GitHubUpstreamBranch = GetEnvironmentVariable("GITHUB_UPSTREAM_BRANCH", "rel/1.0.0"),
+                GitHubPullRequestNotifications = GetEnvironmentVariable("GITHUB_PULL_REQUEST_NOTIFICATIONS", "")
+                    .Split(new[] { ';' }, StringSplitOptions.RemoveEmptyEntries)
             };
         }
 
         private static string GetEnvironmentVariable(string name, string defaultValue = null)
         {
             string value = Environment.GetEnvironmentVariable(name);
-            if (string.IsNullOrEmpty(value))
+            if (value == null)
             {
                 value = defaultValue;
             }
 
-            if (string.IsNullOrEmpty(value))
+            if (value == null)
             {
                 throw new BuildFailureException($"Can't find environment variable '{name}'.");
             }

scripts/update-dependencies/DependencyInfo.cs

@@ -0,0 +1,15 @@
+// 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.
+
+namespace Microsoft.DotNet.Scripts
+{
+    public class DependencyInfo
+    {
+        public string Name { get; set; }
+        public string IdPattern { get; set; }
+        public string IdExclusionPattern { get; set; }
+        public string NewReleaseVersion { get; set; }
+
+        public bool IsUpdated { get; set; }
+    }
+}

scripts/update-dependencies/PushPRTargets.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
+using System.Linq;
 using Microsoft.DotNet.Cli.Build.Framework;
 using Octokit;
 
@@ -14,8 +15,6 @@ namespace Microsoft.DotNet.Scripts
     /// </summary>
     public static class PushPRTargets
     {
-        private const string PullRequestTitle = "Updating dependencies from last known good builds";
-
         private static readonly Config s_config = Config.Instance;
 
         [Target(nameof(CommitChanges), nameof(CreatePR))]
@@ -28,14 +27,31 @@ namespace Microsoft.DotNet.Scripts
         [Target]
         public static BuildTargetResult CommitChanges(BuildTargetContext c)
         {
-            Cmd("git", "add", ".")
+            CommandResult statusResult = Cmd("git", "status", "--porcelain")
-                .Execute()
+                .CaptureStdOut()
-                .EnsureSuccessful();
+                .Execute();
+            statusResult.EnsureSuccessful();
+
+            bool hasModifiedFiles = !string.IsNullOrWhiteSpace(statusResult.StdOut);
+            bool hasUpdatedDependencies = c.GetDependencyInfos().Where(d => d.IsUpdated).Any();
+
+            if (hasModifiedFiles != hasUpdatedDependencies)
+            {
+                return c.Failed($"'git status' does not match DependencyInfo information. Git has modified files: {hasModifiedFiles}. DependencyInfo is updated: {hasUpdatedDependencies}.");
+            }
+
+            if (!hasUpdatedDependencies)
+            {
+                c.Warn("Dependencies are currently up to date");
+                return c.Success();
+            }
 
             string userName = s_config.UserName;
             string email = s_config.Email;
 
-            Cmd("git", "commit", "-m", PullRequestTitle, "--author", $"{userName} <{email}>")
+            string commitMessage = GetCommitMessage(c);
+
+            Cmd("git", "commit", "-a", "-m", commitMessage, "--author", $"{userName} <{email}>")
                 .EnvironmentVariable("GIT_COMMITTER_NAME", userName)
                 .EnvironmentVariable("GIT_COMMITTER_EMAIL", email)
                 .Execute()
@@ -79,12 +95,19 @@ namespace Microsoft.DotNet.Scripts
         public static BuildTargetResult CreatePR(BuildTargetContext c)
         {
             string remoteBranchName = c.GetRemoteBranchName();
+            string commitMessage = c.GetCommitMessage();
 
             NewPullRequest prInfo = new NewPullRequest(
-                PullRequestTitle,
+                commitMessage,
                 s_config.GitHubOriginOwner + ":" + remoteBranchName,
                 s_config.GitHubUpstreamBranch);
 
+            string[] prNotifications = s_config.GitHubPullRequestNotifications;
+            if (prNotifications.Length > 0)
+            {
+                prInfo.Body = $"/cc @{string.Join(" @", prNotifications)}";
+            }
+
             GitHubClient gitHub = new GitHubClient(new ProductHeaderValue("dotnetDependencyUpdater"));
 
             gitHub.Credentials = new Credentials(s_config.Password);
@@ -104,5 +127,36 @@ namespace Microsoft.DotNet.Scripts
         {
             c.BuildContext["RemoteBranchName"] = value;
         }
+
+        private static string GetCommitMessage(this BuildTargetContext c)
+        {
+            const string commitMessagePropertyName = "CommitMessage";
+
+            string message;
+            object messageObject;
+            if (c.BuildContext.Properties.TryGetValue(commitMessagePropertyName, out messageObject))
+            {
+                message = (string)messageObject;
+            }
+            else
+            {
+                DependencyInfo[] updatedDependencies = c.GetDependencyInfos()
+                    .Where(d => d.IsUpdated)
+                    .ToArray();
+
+                string updatedDependencyNames = string.Join(", ", updatedDependencies.Select(d => d.Name));
+                string updatedDependencyVersions = string.Join(", ", updatedDependencies.Select(d => d.NewReleaseVersion));
+
+                message = $"Updating {updatedDependencyNames} to {updatedDependencyVersions}";
+                if (updatedDependencies.Count() > 1)
+                {
+                    message += " respectively";
+                }
+
+                c.BuildContext[commitMessagePropertyName] = message;
+            }
+
+            return message;
+        }
     }
 }

scripts/update-dependencies/UpdateFilesTargets.cs

@@ -34,7 +34,7 @@ namespace Microsoft.DotNet.Scripts
             const string coreFxIdPattern = @"^(?i)((System\..*)|(NETStandard\.Library)|(Microsoft\.CSharp)|(Microsoft\.NETCore.*)|(Microsoft\.TargetingPack\.Private\.(CoreCLR|NETNative))|(Microsoft\.Win32\..*)|(Microsoft\.VisualBasic))$";
             const string coreFxIdExclusionPattern = @"System.CommandLine";
 
-            List<DependencyInfo> dependencyInfos = c.GetDependencyInfo();
+            List<DependencyInfo> dependencyInfos = c.GetDependencyInfos();
             dependencyInfos.Add(new DependencyInfo()
             {
                 Name = "CoreFx",
@@ -46,26 +46,7 @@ namespace Microsoft.DotNet.Scripts
             return c.Success();
         }
 
-        private static List<DependencyInfo> GetDependencyInfo(this BuildTargetContext c)
+        [Target(nameof(ReplaceProjectJson), nameof(ReplaceCrossGen), nameof(ReplaceCoreHostPackaging))]
-        {
-            const string propertyName = "DependencyInfo";
-
-            List<DependencyInfo> dependencyInfos;
-            object dependencyInfosObj;
-            if (c.BuildContext.Properties.TryGetValue(propertyName, out dependencyInfosObj))
-            {
-                dependencyInfos = (List<DependencyInfo>)dependencyInfosObj;
-            }
-            else
-            {
-                dependencyInfos = new List<DependencyInfo>();
-                c.BuildContext[propertyName] = dependencyInfos;
-            }
-
-            return dependencyInfos;
-        }
-
-        [Target(nameof(ReplaceProjectJson), nameof(ReplaceCrossGen))]
         public static BuildTargetResult ReplaceVersions(BuildTargetContext c) => c.Success();
 
         /// <summary>
@@ -74,7 +55,7 @@ namespace Microsoft.DotNet.Scripts
         [Target]
         public static BuildTargetResult ReplaceProjectJson(BuildTargetContext c)
         {
-            List<DependencyInfo> dependencyInfos = c.GetDependencyInfo();
+            List<DependencyInfo> dependencyInfos = c.GetDependencyInfos();
 
             IEnumerable<string> projectJsonFiles = Enumerable.Union(
                 Directory.GetFiles(Dirs.RepoRoot, "project.json", SearchOption.AllDirectories),
@@ -157,6 +138,9 @@ namespace Microsoft.DotNet.Scripts
                                 dependencyProperty.Value = newVersion;
                             }
 
+                            // mark the DependencyInfo as updated so we can tell which dependencies were updated
+                            dependencyInfo.IsUpdated = true;
+
                             return true;
                         }
                     }
@@ -194,40 +178,68 @@ namespace Microsoft.DotNet.Scripts
                 .SelectMany(o => o.Children<JProperty>());
         }
 
-        private class DependencyInfo
-        {
-            public string Name { get; set; }
-            public string IdPattern { get; set; }
-            public string IdExclusionPattern { get; set; }
-            public string NewReleaseVersion { get; set; }
-        }
-
         /// <summary>
         /// Replaces version number that is hard-coded in the CrossGen script.
         /// </summary>
         [Target]
         public static BuildTargetResult ReplaceCrossGen(BuildTargetContext c)
         {
-            DependencyInfo coreFXInfo = c.GetDependencyInfo().Single(d => d.Name == "CoreFx");
+            ReplaceFileContents(@"scripts\dotnet-cli-build\CompileTargets.cs", compileTargetsContent =>
-
-            string compileTargetsPath = Path.Combine(Dirs.RepoRoot, @"scripts\dotnet-cli-build\CompileTargets.cs");
-            string compileTargetsContent = File.ReadAllText(compileTargetsPath);
-
-            Regex regex = new Regex(@"CoreCLRVersion = ""(?<version>\d.\d.\d)-(?<release>.*)"";");
-            compileTargetsContent = regex.Replace(compileTargetsContent, m =>
             {
-                string replacedValue = m.Value;
+                DependencyInfo coreFXInfo = c.GetCoreFXDependency();
-                Group releaseGroup = m.Groups["release"];
+                Regex regex = new Regex(@"CoreCLRVersion = ""(?<version>\d.\d.\d)-(?<release>.*)"";");
 
-                replacedValue = replacedValue.Remove(releaseGroup.Index - m.Index, releaseGroup.Length);
+                return regex.ReplaceGroupValue(compileTargetsContent, "release", coreFXInfo.NewReleaseVersion);
-                replacedValue = replacedValue.Insert(releaseGroup.Index - m.Index, coreFXInfo.NewReleaseVersion);
-
-                return replacedValue;
             });
 
-            File.WriteAllText(compileTargetsPath, compileTargetsContent, Encoding.UTF8);
-
             return c.Success();
         }
+
+        /// <summary>
+        /// Replaces version number that is hard-coded in the corehost packaging dir.props file.
+        /// </summary>
+        [Target]
+        public static BuildTargetResult ReplaceCoreHostPackaging(BuildTargetContext c)
+        {
+            ReplaceFileContents(@"src\corehost\packaging\dir.props", contents =>
+            {
+                DependencyInfo coreFXInfo = c.GetCoreFXDependency();
+                Regex regex = new Regex(@"Microsoft\.NETCore\.Platforms\\(?<version>\d\.\d\.\d)-(?<release>.*)\\runtime\.json");
+
+                return regex.ReplaceGroupValue(contents, "release", coreFXInfo.NewReleaseVersion);
+            });
+
+            return c.Success();
+        }
+
+        private static DependencyInfo GetCoreFXDependency(this BuildTargetContext c)
+        {
+            return c.GetDependencyInfos().Single(d => d.Name == "CoreFx");
+        }
+
+        private static void ReplaceFileContents(string repoRelativePath, Func<string, string> replacement)
+        {
+            string fullPath = Path.Combine(Dirs.RepoRoot, repoRelativePath);
+            string contents = File.ReadAllText(fullPath);
+
+            contents = replacement(contents);
+
+            File.WriteAllText(fullPath, contents, Encoding.UTF8);
+        }
+
+        private static string ReplaceGroupValue(this Regex regex, string input, string groupName, string newValue)
+        {
+            return regex.Replace(input, m =>
+            {
+                string replacedValue = m.Value;
+                Group group = m.Groups[groupName];
+                int startIndex = group.Index - m.Index;
+
+                replacedValue = replacedValue.Remove(startIndex, group.Length);
+                replacedValue = replacedValue.Insert(startIndex, newValue);
+
+                return replacedValue;
+            });
+        }
     }
 }