Re-apply internal staging versions of .NET in sync-internal-release command

eng/update-dependencies/BaseCommand.cs

@@ -3,13 +3,12 @@
 
 using System.CommandLine;
 using System.CommandLine.NamingConventionBinder;
-using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
 
 namespace Dotnet.Docker;
 
-public abstract class BaseCommand<TOptions>() where TOptions : IOptions
+public abstract class BaseCommand<TOptions>() : ICommand<TOptions> where TOptions : IOptions
 {
     public abstract Task<int> ExecuteAsync(TOptions options);
 
@@ -34,7 +33,7 @@ public static Command Create(string name, string description)
     private static BindingHandler Handler =>
         CommandHandler.Create<TOptions, IHost>(async (options, host) =>
             {
-                var thisCommand = host.Services.GetRequiredService<BaseCommand<TOptions>>();
+                var thisCommand = host.Services.GetRequiredService<ICommand<TOptions>>();
                 return await thisCommand.ExecuteAsync(options);
             });
 }

eng/update-dependencies/BaseUrlUpdater.cs

@@ -17,48 +17,44 @@ internal class BaseUrlUpdater : FileRegexUpdater
 {
     private const string BaseUrlGroupName = "BaseUrlValue";
     private readonly SpecificCommandOptions _options;
-    private readonly JObject _manifestVariables;
+    private readonly ManifestVariables _manifestVariables;
     private readonly string _manifestVariableName;
 
     /// <summary>
     /// Creates a new <see cref="IDependencyUpdater"/> for updating base URLs.
     /// If the base URL variable cannot be found in the manifest, the updater
     /// won't do anything.
     /// </summary>
-    public static IDependencyUpdater Create(string repoRoot, SpecificCommandOptions options)
+    public static IDependencyUpdater Create(ManifestVariables manifestVariables, SpecificCommandOptions options)
     {
-        // Load manifest and extract variables once so the constructor doesn't duplicate this logic
-        var manifest = ManifestHelper.LoadManifest(SpecificCommand.VersionsFilename);
-        var manifestVariables = (JObject?)manifest["variables"];
-
         if (manifestVariables is null)
         {
             Trace.TraceWarning("BaseUrlUpdater: manifest variables missing - skipping base URL update.");
             return new EmptyDependencyUpdater();
         }
 
+        var upstreamBranch = manifestVariables.GetValue("branch");
         string baseUrlVarName = ManifestHelper.GetBaseUrlVariableName(
-            options.DockerfileVersion,
-            options.SourceBranch,
-            options.VersionSourceName,
-            options.IsSdkOnly);
+            dockerfileVersion: options.DockerfileVersion,
+            branch: upstreamBranch,
+            versionSourceName: options.VersionSourceName,
+            sdkOnlyRelease: options.IsSdkOnly);
 
-        if (!manifestVariables.ContainsKey(baseUrlVarName))
+        if (!manifestVariables.HasValue(baseUrlVarName))
         {
             Trace.TraceWarning($"BaseUrlUpdater: variable '{baseUrlVarName}' not found - skipping base URL update.");
             return new EmptyDependencyUpdater();
         }
 
-        return new BaseUrlUpdater(repoRoot, options, manifestVariables, baseUrlVarName);
+        return new BaseUrlUpdater(options, manifestVariables, baseUrlVarName);
     }
 
     private BaseUrlUpdater(
-        string repoRoot,
         SpecificCommandOptions options,
-        JObject manifestVariables,
+        ManifestVariables manifestVariables,
         string manifestVariableName)
     {
-        Path = System.IO.Path.Combine(repoRoot, SpecificCommand.VersionsFilename);
+        Path = options.GetManifestVersionsFilePath();
         VersionGroupName = BaseUrlGroupName;
         _options = options;
         _manifestVariables = manifestVariables;
@@ -72,7 +68,7 @@ protected override string TryGetDesiredValue(IEnumerable<IDependencyInfo> depend
         usedDependencyInfos = Enumerable.Empty<IDependencyInfo>();
 
         string baseUrlVersionVarName = _manifestVariableName;
-        string unresolvedBaseUrl = _manifestVariables[baseUrlVersionVarName]?.ToString() ??
+        string unresolvedBaseUrl = _manifestVariables.Variables[baseUrlVersionVarName]?.ToString() ??
             throw new InvalidOperationException($"Variable with name '{baseUrlVersionVarName}' is missing.");
 
         if (_options.IsInternal)

eng/update-dependencies/CreatePullRequestOptions.cs

@@ -8,7 +8,10 @@ namespace Dotnet.Docker;
 
 public abstract record CreatePullRequestOptions
 {
-    private string? _targetBranch = null;
+    /// <summary>
+    /// The root of the dotnet-docker repo to run against.
+    /// </summary>
+    public string RepoRoot { get; init; } = Directory.GetCurrentDirectory();
 
     public string User { get; init; } = "";
     public string Email { get; init; } = "";
@@ -17,24 +20,21 @@ public abstract record CreatePullRequestOptions
     public string AzdoProject { get; init; } = "";
     public string AzdoRepo { get; init; } = "";
     public string VersionSourceName { get; init; } = "";
-    public string SourceBranch { get; init; } = "nightly";
-    public string TargetBranch
-    {
-        get => _targetBranch ?? SourceBranch;
-        init => _targetBranch = value;
-    }
+    public string SourceBranch { get; init; } = "";
+    public string TargetBranch { get; init; } = "nightly";
 
     public static List<Option> Options =>
     [
+        new Option<string>("--repo-root") { Description = "The root of the dotnet-docker repo to run against (defaults to current working directory)" },
         new Option<string>("--user") { Description = "GitHub or AzDO user used to make PR (if not specified, a PR will not be created)" },
         new Option<string>("--email") { Description = "GitHub or AzDO email used to make PR (if not specified, a PR will not be created)" },
         new Option<string>("--password") { Description = "GitHub or AzDO password used to make PR (if not specified, a PR will not be created)" },
         new Option<string>("--azdo-organization") { Description = "Name of the AzDO organization" },
         new Option<string>("--azdo-project") { Description = "Name of the AzDO project" },
         new Option<string>("--azdo-repo") { Description = "Name of the AzDO repo" },
         new Option<string>("--version-source-name") { Description = "The name of the source from which the version information was acquired." },
-        new Option<string>("--source-branch") { Description = "Branch where the Dockerfiles are hosted" },
-        new Option<string>("--target-branch") { Description = "Target branch of the generated PR (defaults to value of source-branch)" },
+        new Option<string>("--source-branch") { Description = "If synchronizing multiple branches, the branch to pull updates from" },
+        new Option<string>("--target-branch") { Description = "Pull request will be submitted targeting this branch" },
     ];
 
     public static List<Argument> Arguments => [];

eng/update-dependencies/CreatePullRequestOptionsExtensions.cs

@@ -0,0 +1,24 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+namespace Dotnet.Docker;
+
+internal static class CreatePullRequestOptionsExtensions
+{
+    public static string GetManifestVersionsFilePath(this CreatePullRequestOptions options) =>
+        Path.Combine(options.RepoRoot, "manifest.versions.json");
+
+    public static string GetAzdoRepoUrl(this CreatePullRequestOptions options)
+    {
+        // Validate that we have all the required pieces to construct the repo URL.
+        ArgumentException.ThrowIfNullOrWhiteSpace(options.AzdoOrganization);
+        ArgumentException.ThrowIfNullOrWhiteSpace(options.AzdoProject);
+        ArgumentException.ThrowIfNullOrWhiteSpace(options.AzdoRepo);
+        ArgumentException.ThrowIfNullOrWhiteSpace(options.TargetBranch);
+        ArgumentException.ThrowIfNullOrWhiteSpace(options.SourceBranch);
+
+        // AzdoOrganization is a URL like https://dev.azure.com/<org>
+        // A valid Azure DevOps repository URL is formatted like https://dev.azure.com/<org>/<project>/_git/<repo>
+        return $"{options.AzdoOrganization.TrimEnd('/')}/{options.AzdoProject}/_git/{options.AzdoRepo}";
+    }
+}

eng/update-dependencies/DependencyInjectionExtensions.cs

@@ -17,7 +17,7 @@ public static void AddCommand<TCommand, TOptions>(
             where TCommand : BaseCommand<TOptions>
             where TOptions : IOptions
     {
-        serviceCollection.AddSingleton<BaseCommand<TOptions>, TCommand>();
+        serviceCollection.AddSingleton<ICommand<TOptions>, TCommand>();
     }
 
     /// <summary>

eng/update-dependencies/DockerfileShaUpdater.cs

@@ -40,10 +40,17 @@ public class DockerfileShaUpdater : FileRegexUpdater
         private readonly SpecificCommandOptions _options;
         private readonly string _versions;
         private readonly Dictionary<string, string> _urls;
-        private readonly Lazy<JObject> _manifestVariables;
+        private readonly ManifestVariables _manifestVariables;
 
         public DockerfileShaUpdater(
-            string productName, string dockerfileVersion, string? buildVersion, string arch, string os, string versions, SpecificCommandOptions options)
+            string productName,
+            string dockerfileVersion,
+            string? buildVersion,
+            string arch,
+            string os,
+            string versions,
+            SpecificCommandOptions options,
+            ManifestVariables manifestVariables)
         {
             _productName = productName;
             _dockerfileVersion = new Version(dockerfileVersion);
@@ -52,6 +59,7 @@ public DockerfileShaUpdater(
             _os = os;
             _versions = versions;
             _options = options;
+            _manifestVariables = manifestVariables;
 
             // Maps a product name to a set of one or more candidate URLs referencing the associated artifact. The order of the URLs
             // should be in priority order with each subsequent URL being the fallback.
@@ -69,18 +77,6 @@ public DockerfileShaUpdater(
                 { "sdk",                            "$DOTNET_BASE_URL/Sdk/$VERSION_DIR/dotnet-sdk-$VERSION_FILE-$OS-$ARCH.$ARCHIVE_EXT" },
             };
 
-            _manifestVariables = new Lazy<JObject>(
-                () =>
-                {
-                    const string VariablesProperty = "variables";
-                    JToken? variables = ManifestHelper.LoadManifest(SpecificCommand.VersionsFilename)[VariablesProperty];
-                    if (variables is null)
-                    {
-                        throw new InvalidOperationException($"'{VariablesProperty}' property missing in '{SpecificCommand.VersionsFilename}'");
-                    }
-                    return (JObject)variables;
-                });
-
             if (!string.IsNullOrEmpty(_options.InternalAccessToken))
             {
                 s_httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue(
@@ -91,9 +87,12 @@ public DockerfileShaUpdater(
         }
 
         public static IEnumerable<IDependencyUpdater> CreateUpdaters(
-            string productName, string dockerfileVersion, string repoRoot, SpecificCommandOptions options)
+            string productName,
+            string dockerfileVersion,
+            SpecificCommandOptions options,
+            ManifestVariables variables)
         {
-            string versionsPath = System.IO.Path.Combine(repoRoot, SpecificCommand.VersionsFilename);
+            string versionsPath = options.GetManifestVersionsFilePath();
             string versions = File.ReadAllText(versionsPath);
 
             // The format of the sha variable name is '<productName>|<dockerfileVersion>|<os>|<arch>|sha'.
@@ -116,7 +115,8 @@ public static IEnumerable<IDependencyUpdater> CreateUpdaters(
                         GetArch(parts),
                         GetOs(parts),
                         versions,
-                        options)
+                        options,
+                        variables)
                     {
                         Path = versionsPath,
                         VersionGroupName = ShaValueGroupName
@@ -137,7 +137,7 @@ public static IEnumerable<IDependencyUpdater> CreateUpdaters(
         {
             usedBuildInfos = [dependencyBuildInfos.First(info => info.SimpleName == _productName)];
 
-            string baseUrl = ManifestHelper.GetBaseUrl(_manifestVariables.Value, _options);
+            string baseUrl = ManifestHelper.GetBaseUrl(_manifestVariables.Variables, _options);
             // Remove Aspire Dashboard case once https://github.com/dotnet/aspire/issues/2035 is fixed.
             string archiveExt = _os.Contains("win") || _productName.Contains("aspire-dashboard") ? "zip" : "tar.gz";
             string versionDir = _buildVersion ?? "";
@@ -275,7 +275,7 @@ private static string GetArch(string[] variableParts)
             // corresponding build in the daily build location, for example, will not be signed due. So when we're targeting
             // the daily build location, we wouldn't use the release checksums file and instead use the other means of
             // retrieving the checksums.
-            string baseUrl = ManifestHelper.GetBaseUrl(_manifestVariables.Value, _options);
+            string baseUrl = ManifestHelper.GetBaseUrl(_manifestVariables.Variables, _options);
             if (baseUrl != ReleaseDotnetBaseCdnUrl)
             {
                 return null;

eng/update-dependencies/FromStagingPipelineCommand.cs

@@ -1,17 +1,20 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using Dotnet.Docker.Sync;
 using Microsoft.Extensions.Logging;
 
 namespace Dotnet.Docker;
 
 internal partial class FromStagingPipelineCommand(
     ILogger<FromStagingPipelineCommand> logger,
-    PipelineArtifactProvider pipelineArtifactProvider)
+    PipelineArtifactProvider pipelineArtifactProvider,
+    IInternalVersionsService internalVersionsService)
     : BaseCommand<FromStagingPipelineOptions>
 {
     private readonly ILogger<FromStagingPipelineCommand> _logger = logger;
     private readonly PipelineArtifactProvider _pipelineArtifactProvider = pipelineArtifactProvider;
+    private readonly IInternalVersionsService _internalVersionsService = internalVersionsService;
 
     public override async Task<int> ExecuteAsync(FromStagingPipelineOptions options)
     {
@@ -44,7 +47,10 @@ public override async Task<int> ExecuteAsync(FromStagingPipelineOptions options)
         string dockerfileVersion = VersionHelper.ResolveMajorMinorVersion(releaseConfig.RuntimeBuild).ToString();
 
         // Record pipeline run ID for this dockerfileVersion, for later use by sync-internal-release command
-        RecordInternalVersion(dockerfileVersion, options.StagingPipelineRunId.ToString());
+        _internalVersionsService.RecordInternalStagingBuild(
+            options.RepoRoot,
+            dockerfileVersion,
+            options.StagingPipelineRunId);
 
         var productVersions = (options.Internal, releaseConfig.SdkOnly) switch
         {
@@ -90,6 +96,7 @@ public override async Task<int> ExecuteAsync(FromStagingPipelineOptions options)
         var updateDependencies = new SpecificCommand();
         var updateDependenciesOptions = new SpecificCommandOptions()
         {
+            RepoRoot = options.RepoRoot,
             DockerfileVersion = dockerfileVersion.ToString(),
             ProductVersions = productVersions,
 
@@ -109,59 +116,6 @@ public override async Task<int> ExecuteAsync(FromStagingPipelineOptions options)
         return await updateDependencies.ExecuteAsync(updateDependenciesOptions);
     }
 
-    /// <summary>
-    /// Records the staging pipeline run ID in an easy to parse format. This
-    /// can be used by the sync-internal-release pipeline to record and
-    /// re-apply the same staging builds after resetting the state of the repo
-    /// to match the public release branch.
-    /// </summary>
-    /// <remarks>
-    /// This will only store one staging pipeline run ID per dockerfileVersion
-    /// </remarks>
-    /// <param name="dockerfileVersion">major-minor version</param>
-    /// <param name="stagingPipelineRunId">the build ID of the staging pipeline run</param>
-    private void RecordInternalVersion(string dockerfileVersion, string stagingPipelineRunId)
-    {
-        const string InternalVersionsFile = "internal-versions.txt";
-
-        // Internal versions file should have one line per dockerfileVersion
-        // Each line should be formatted as: <dockerfileVersion>=<stagingPipelineRunId>
-        //
-        // The preferable way to do this would be to record the version in
-        // manifest.versions.json, however that would require one of the following:
-        // 1) round-trip serialization, which would remove any whitespace/blank lines - which are
-        //    important for keeping the file readable and reducing git merge conflicts
-        // 2) lots of regex JSON manipulation which is error-prone and harder to maintain
-        //
-        // So for now, the separate file and format is a compromise.
-
-        var versionsFilePath = Path.GetFullPath(SpecificCommand.VersionsFilename);
-        var versionsFileDir = Path.GetDirectoryName(versionsFilePath) ?? "";
-        var internalVersionFile = Path.Combine(versionsFileDir, InternalVersionsFile);
-        Dictionary<string, string> versions = [];
-
-        _logger.LogInformation(
-            "Recording staging pipeline build ID in {internalVersionFile}",
-            internalVersionFile);
-
-        try
-        {
-            // File already exists - read existing versions
-            versions = File.ReadAllLines(internalVersionFile)
-                .Select(line => line.Split('=', 2))
-                .Where(parts => parts.Length == 2)
-                .ToDictionary(parts => parts[0], parts => parts[1]);
-        }
-        catch (FileNotFoundException)
-        {
-            // File doesn't exist - it will be created
-        }
-
-        versions[dockerfileVersion] = stagingPipelineRunId;
-        var versionLines = versions.Select(kv => $"{kv.Key}={kv.Value}");
-        File.WriteAllLines(internalVersionFile, versionLines);
-    }
-
     /// <summary>
     /// Formats a storage account URL has a specific format:
     /// - Starts with "https://"

eng/update-dependencies/FromStagingPipelineOptions.cs

@@ -8,7 +8,7 @@ namespace Dotnet.Docker;
 
 internal record FromStagingPipelineOptions : CreatePullRequestOptions, IOptions
 {
-    public const string StagingStorageAccountOption = "--staging-storage-account";
+    public const string StagingStorageAccountOptionName = "--staging-storage-account";
     public const string InternalOption = "--internal";
 
     /// <summary>
@@ -37,14 +37,16 @@ internal record FromStagingPipelineOptions : CreatePullRequestOptions, IOptions
         ..CreatePullRequestOptions.Arguments,
     ];
 
+    public static Option<string> StagingStorageAccountOption = new(StagingStorageAccountOptionName)
+    {
+        Description = "The Azure Storage Account to use as a source for the update."
+            + " This should be one of two storage accounts: dotnetstagetest or dotnetstage."
+            + " For example: https://dotnetstagetest.blob.core.windows.net/"
+    };
+
     public static new List<Option> Options { get; } =
     [
-        new Option<string>(StagingStorageAccountOption)
-        {
-            Description = "The Azure Storage Account to use as a source for the update."
-                + " This should be one of two storage accounts: dotnetstagetest or dotnetstage."
-                + " For example: https://dotnetstagetest.blob.core.windows.net/"
-        },
+        StagingStorageAccountOption,
         new Option<bool>(InternalOption)
         {
             Description = "Whether or not to use the internal versions of the staged build. When not using an internal"

eng/update-dependencies/Git/GitRemoteInfo.cs

@@ -8,4 +8,4 @@ namespace Dotnet.Docker.Git;
 /// </summary>
 /// <param name="Name">The name of the remote (e.g., "origin").</param>
 /// <param name="Url">The URL of the remote repository.</param>
-public record GitRemoteInfo(string Name, string Url);
+internal sealed record GitRemoteInfo(string Name, string Url);