Skip to content

Commit

Permalink
fix: Evaluate RootLayout.ForCurrentDirectory lazily
Browse files Browse the repository at this point in the history
This prevents start-up failures in the container environment (and
any other time we're not in a repo).

Fixes #14005
  • Loading branch information
jskeet committed Dec 14, 2024
1 parent 7468d5b commit 97aa134
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 14 deletions.
9 changes: 7 additions & 2 deletions tools/Google.Cloud.Tools.ReleaseManager/CommandBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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<RootLayout> _lazyLayout = new(RootLayout.ForCurrentDirectory, LazyThreadSafetyMode.ExecutionAndPublication);

private readonly int _minArgs;
private readonly int _maxArgs;

Expand All @@ -43,7 +49,7 @@ public abstract class CommandBase : ICommand
/// <remarks>
/// Currently this is always derived from the current directory, but we may later have a way of specifying it separately.
/// </remarks>
protected RootLayout RootLayout { get; }
protected RootLayout RootLayout => _lazyLayout.Value;

protected CommandBase(string command, string description, int minArgs, int maxArgs, string expectedArguments)
{
Expand All @@ -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)
Expand Down
19 changes: 7 additions & 12 deletions tools/Google.Cloud.Tools.ReleaseManager/DetectPrChangesCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,13 @@ public class DetectPrChangesCommand : ICommand

public string ExpectedArguments => "<pre-PR git directory> <[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();
Expand All @@ -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<string> LoadRepositoryTags()
{
using var repo = new Repository(_rootLayout.RepositoryRoot);
using var repo = new Repository(rootLayout.RepositoryRoot);
return new HashSet<string>(repo.Tags.Select(tag => tag.FriendlyName));
}
}
Expand All @@ -76,7 +71,7 @@ HashSet<string> 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.</remarks>
/// <returns>Whether the comparison has failed in some way.</returns>
private bool CompareApi(string oldCommitDirectory, HashSet<string> tags, ApiMetadata api)
private bool CompareApi(RootLayout rootLayout, string oldCommitDirectory, HashSet<string> tags, ApiMetadata api)
{
string id = api.Id;
LogHeader($"Finding changes in {id}...");
Expand Down Expand Up @@ -129,7 +124,7 @@ private bool CompareApi(string oldCommitDirectory, HashSet<string> tags, ApiMeta
LogHeader($"Comparing with previous NuGet package");
try
{
CheckVersionCompatibilityCommand.CheckCompatibilityWithPreviousRelease(_rootLayout, tags, api);
CheckVersionCompatibilityCommand.CheckCompatibilityWithPreviousRelease(rootLayout, tags, api);
}
catch (UserErrorException ex)
{
Expand Down

0 comments on commit 97aa134

Please sign in to comment.