From 97aa1346af5769b83fe3966dd5d6100d5fbd2d1c Mon Sep 17 00:00:00 2001 From: Jon Skeet Date: Sat, 14 Dec 2024 11:25:38 +0000 Subject: [PATCH] fix: Evaluate RootLayout.ForCurrentDirectory lazily This prevents start-up failures in the container environment (and any other time we're not in a repo). Fixes #14005 --- .../CommandBase.cs | 9 +++++++-- .../DetectPrChangesCommand.cs | 19 +++++++------------ 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/tools/Google.Cloud.Tools.ReleaseManager/CommandBase.cs b/tools/Google.Cloud.Tools.ReleaseManager/CommandBase.cs index a74bd5931003..d944bfdd06e9 100644 --- a/tools/Google.Cloud.Tools.ReleaseManager/CommandBase.cs +++ b/tools/Google.Cloud.Tools.ReleaseManager/CommandBase.cs @@ -14,8 +14,10 @@ using Google.Cloud.Tools.Common; using LibGit2Sharp; +using System; using System.Collections.Generic; using System.Linq; +using System.Threading; namespace Google.Cloud.Tools.ReleaseManager { @@ -28,6 +30,10 @@ public abstract class CommandBase : ICommand // releases, but fundamentally it's "the current source of truth we're basing this release on". internal const string PrimaryBranch = "main"; + // This is lazy so that we can construct instances of CommandBase even when the current directory isn't + // in a repo, but we don't go hunting for the root multiple times. + private readonly Lazy _lazyLayout = new(RootLayout.ForCurrentDirectory, LazyThreadSafetyMode.ExecutionAndPublication); + private readonly int _minArgs; private readonly int _maxArgs; @@ -43,7 +49,7 @@ public abstract class CommandBase : ICommand /// /// Currently this is always derived from the current directory, but we may later have a way of specifying it separately. /// - protected RootLayout RootLayout { get; } + protected RootLayout RootLayout => _lazyLayout.Value; protected CommandBase(string command, string description, int minArgs, int maxArgs, string expectedArguments) { @@ -52,7 +58,6 @@ protected CommandBase(string command, string description, int minArgs, int maxAr _minArgs = minArgs; _maxArgs = maxArgs; ExpectedArguments = expectedArguments; - RootLayout = RootLayout.ForCurrentDirectory(); } protected CommandBase(string command, string description, params string[] argNames) diff --git a/tools/Google.Cloud.Tools.ReleaseManager/DetectPrChangesCommand.cs b/tools/Google.Cloud.Tools.ReleaseManager/DetectPrChangesCommand.cs index 79aa0f09d446..94b2cb06cdab 100644 --- a/tools/Google.Cloud.Tools.ReleaseManager/DetectPrChangesCommand.cs +++ b/tools/Google.Cloud.Tools.ReleaseManager/DetectPrChangesCommand.cs @@ -35,18 +35,13 @@ public class DetectPrChangesCommand : ICommand public string ExpectedArguments => " <[id [id...]]"; - private readonly RootLayout _rootLayout; - - public DetectPrChangesCommand() - { - _rootLayout = RootLayout.ForCurrentDirectory(); - } - public int Execute(string[] args) { + var rootLayout = RootLayout.ForCurrentDirectory(); + string oldCommitDirectory = args[0]; bool anyFailures = false; - var catalog = ApiCatalog.Load(_rootLayout); + var catalog = ApiCatalog.Load(rootLayout); var tags = LoadRepositoryTags(); var apiIds = args.Skip(1).ToList(); @@ -57,14 +52,14 @@ public int Execute(string[] args) LogHeader($"{id} has been deleted"); continue; } - anyFailures |= CompareApi(oldCommitDirectory, tags, api); + anyFailures |= CompareApi(rootLayout, oldCommitDirectory, tags, api); } return anyFailures ? 1 : 0; HashSet LoadRepositoryTags() { - using var repo = new Repository(_rootLayout.RepositoryRoot); + using var repo = new Repository(rootLayout.RepositoryRoot); return new HashSet(repo.Tags.Select(tag => tag.FriendlyName)); } } @@ -76,7 +71,7 @@ HashSet LoadRepositoryTags() /// along the way (TFMs, output locations etc). We could potentially refactor much of it into a class, /// but it's simpler to keep it like this for now. /// Whether the comparison has failed in some way. - private bool CompareApi(string oldCommitDirectory, HashSet tags, ApiMetadata api) + private bool CompareApi(RootLayout rootLayout, string oldCommitDirectory, HashSet tags, ApiMetadata api) { string id = api.Id; LogHeader($"Finding changes in {id}..."); @@ -129,7 +124,7 @@ private bool CompareApi(string oldCommitDirectory, HashSet tags, ApiMeta LogHeader($"Comparing with previous NuGet package"); try { - CheckVersionCompatibilityCommand.CheckCompatibilityWithPreviousRelease(_rootLayout, tags, api); + CheckVersionCompatibilityCommand.CheckCompatibilityWithPreviousRelease(rootLayout, tags, api); } catch (UserErrorException ex) {