Skip to content

Bugfix/fix only tracked branches parameter implementation #1702

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
asbjornu merged 3 commits into from
Jan 19, 2020
Merged

Bugfix/fix only tracked branches parameter implementation #1702

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
179870b
Fixed wrong implementation of parameter onlyTrackedBranches in GetBra…
crazycrank Jun 5, 2019
e03b27b
Fix 3 broken tests which require tracked branches to function
crazycrank Jun 5, 2019
54baccd
Align naming: renamed isForTrackedBranchOnly and onlyEvaluateTrackedB…
crazycrank Jan 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions src/GitVersionCore.Tests/GitToolsTestingExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public static class GitToolsTestingExtensions

static GitToolsTestingExtensions() => sp = ConfigureService();

public static VersionVariables GetVersion(this RepositoryFixtureBase fixture, Config configuration = null, IRepository repository = null, string commitId = null, bool isForTrackedBranchOnly = true, string targetBranch = null)
public static VersionVariables GetVersion(this RepositoryFixtureBase fixture, Config configuration = null, IRepository repository = null, string commitId = null, bool onlyTrackedBranches = true, string targetBranch = null)
{
if (configuration == null)
{
Expand All @@ -31,7 +31,7 @@ public static VersionVariables GetVersion(this RepositoryFixtureBase fixture, Co
var variableProvider = sp.GetService<IVariableProvider>();
var versionFinder = sp.GetService<IGitVersionFinder>();

var gitVersionContext = new GitVersionContext(repository ?? fixture.Repository, log, targetBranch, configuration, isForTrackedBranchOnly, commitId);
var gitVersionContext = new GitVersionContext(repository ?? fixture.Repository, log, targetBranch, configuration, onlyTrackedBranches, commitId);
var executeGitVersion = versionFinder.FindVersion(gitVersionContext);
var variables = variableProvider.GetVariablesFor(executeGitVersion, gitVersionContext.Configuration, gitVersionContext.IsCurrentCommitTagged);

Expand All @@ -47,19 +47,19 @@ public static VersionVariables GetVersion(this RepositoryFixtureBase fixture, Co
}
}

public static void AssertFullSemver(this RepositoryFixtureBase fixture, string fullSemver, IRepository repository = null, string commitId = null, bool isForTrackedBranchOnly = true, string targetBranch = null)
public static void AssertFullSemver(this RepositoryFixtureBase fixture, string fullSemver, IRepository repository = null, string commitId = null, bool onlyTrackedBranches = true, string targetBranch = null)
{
fixture.AssertFullSemver(new Config(), fullSemver, repository, commitId, isForTrackedBranchOnly, targetBranch);
fixture.AssertFullSemver(new Config(), fullSemver, repository, commitId, onlyTrackedBranches, targetBranch);
}

public static void AssertFullSemver(this RepositoryFixtureBase fixture, Config configuration, string fullSemver, IRepository repository = null, string commitId = null, bool isForTrackedBranchOnly = true, string targetBranch = null)
public static void AssertFullSemver(this RepositoryFixtureBase fixture, Config configuration, string fullSemver, IRepository repository = null, string commitId = null, bool onlyTrackedBranches = true, string targetBranch = null)
{
configuration.Reset();
Console.WriteLine("---------");

try
{
var variables = fixture.GetVersion(configuration, repository, commitId, isForTrackedBranchOnly, targetBranch);
var variables = fixture.GetVersion(configuration, repository, commitId, onlyTrackedBranches, targetBranch);
variables.FullSemVer.ShouldBe(fullSemver);
}
catch (Exception)
Expand Down
2 changes: 1 addition & 1 deletion src/GitVersionCore.Tests/IntegrationTests/DevelopScenarios.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ public void WhenDevelopBranchedFromMasterDetachedHeadMinorIsIncreased()
var commit = fixture.Repository.Head.Tip;
fixture.Repository.MakeACommit();
Commands.Checkout(fixture.Repository, commit);
fixture.AssertFullSemver("1.1.0-alpha.1");
fixture.AssertFullSemver("1.1.0-alpha.1", onlyTrackedBranches: false);
}

[Test]
Expand Down
6 changes: 3 additions & 3 deletions src/GitVersionCore.Tests/IntegrationTests/MasterScenarios.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using GitTools.Testing;
using GitTools.Testing;
using LibGit2Sharp;
using NUnit.Framework;
using GitVersion.Configuration;
Expand Down Expand Up @@ -93,7 +93,7 @@ public void GivenARepositoryWithCommitsButNoTagsWithDetachedHeadVersionShouldBe0
Commands.Checkout(fixture.Repository, commit);

// When
fixture.AssertFullSemver("0.1.0+2");
fixture.AssertFullSemver("0.1.0+2", onlyTrackedBranches: false);
}

[Test]
Expand Down Expand Up @@ -207,4 +207,4 @@ public void AreTagsNotAdheringToTagPrefixIgnored()
fixture.AssertFullSemver(config, "0.1.0+6"); //Fallback version + 6 commits since tag
}
}
}
}
2 changes: 1 addition & 1 deletion src/GitVersionCore.Tests/IntegrationTests/RemoteRepositoryScenarios.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void GivenARemoteGitRepositoryWhenCheckingOutDetachedheadUsingExistingImp
fixture.LocalRepositoryFixture.Repository,
fixture.LocalRepositoryFixture.Repository.Head.Tip);

Should.Throw<WarningException>(() => fixture.AssertFullSemver("0.1.0+4", fixture.LocalRepositoryFixture.Repository, isForTrackedBranchOnly: false),
Should.Throw<WarningException>(() => fixture.AssertFullSemver("0.1.0+4", fixture.LocalRepositoryFixture.Repository, onlyTrackedBranches: false),
$"It looks like the branch being examined is a detached Head pointing to commit '{fixture.LocalRepositoryFixture.Repository.Head.Tip.Id.ToString(7)}'. Without a proper branch name GitVersion cannot determine the build version.");
}

Expand Down
8 changes: 4 additions & 4 deletions src/GitVersionCore/Configuration/BranchConfigurationCalculator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public BranchConfigurationCalculator(ILog log, GitVersionContext context)
public BranchConfig GetBranchConfiguration(Branch targetBranch, IList<Branch> excludedInheritBranches = null)
{
var matchingBranches = context.FullConfiguration.GetConfigForBranch(targetBranch.NameWithoutRemote());

if (matchingBranches == null)
{
log.Info($"No branch configuration found for branch {targetBranch.FriendlyName}, falling back to default configuration");
Expand Down Expand Up @@ -84,7 +84,7 @@ private BranchConfig InheritBranchConfiguration(Branch targetBranch, BranchConfi
List<Branch> possibleParents;
if (branchPoint == BranchCommit.Empty)
{
possibleParents = context.RepositoryMetadataProvider.GetBranchesContainingCommit(targetBranch.Tip, branchesToEvaluate, true)
possibleParents = context.RepositoryMetadataProvider.GetBranchesContainingCommit(targetBranch.Tip, branchesToEvaluate, false)
// It fails to inherit Increment branch configuration if more than 1 parent;
// therefore no point to get more than 2 parents
.Take(2)
Expand All @@ -93,11 +93,11 @@ private BranchConfig InheritBranchConfiguration(Branch targetBranch, BranchConfi
else
{
var branches = context.RepositoryMetadataProvider
.GetBranchesContainingCommit(branchPoint.Commit, branchesToEvaluate, true).ToList();
.GetBranchesContainingCommit(branchPoint.Commit, branchesToEvaluate, false).ToList();
if (branches.Count > 1)
{
var currentTipBranches = context.RepositoryMetadataProvider
.GetBranchesContainingCommit(context.CurrentCommit, branchesToEvaluate, true).ToList();
.GetBranchesContainingCommit(context.CurrentCommit, branchesToEvaluate, false).ToList();
possibleParents = branches.Except(currentTipBranches).ToList();
}
else
Expand Down
4 changes: 2 additions & 2 deletions src/GitVersionCore/GitRepoMetadataProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ public IEnumerable<Branch> GetBranchesContainingCommit(Commit commit, IList<Bran
// TODO: It looks wasteful looping through the branches twice. Can't these loops be merged somehow? @asbjornu
foreach (var branch in branches)
{
Copy link
Contributor Author

@crazycrank crazycrank Jun 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the change I talked about in the PR. What the original dev probably meant was to use onlyTrackedBranches || !ibranch.IsTracking. I used the longer version (onlyTrackedBranches && branch.IsTracking) || !onlyTrackedBranches as its more verbose and thus makes sure this kind of bug doesn't happen :)

if (branch.Tip != null && branch.Tip.Sha != commit.Sha || (onlyTrackedBranches && !branch.IsTracking))
if (branch.Tip != null && branch.Tip.Sha != commit.Sha || ((onlyTrackedBranches && branch.IsTracking) || !onlyTrackedBranches))
{
continue;
}
Expand All @@ -97,7 +97,7 @@ public IEnumerable<Branch> GetBranchesContainingCommit(Commit commit, IList<Bran
}

log.Info($"No direct branches found, searching through {(onlyTrackedBranches ? "tracked" : "all")} branches.");
foreach (var branch in branches.Where(b => onlyTrackedBranches && !b.IsTracking))
foreach (var branch in branches.Where(b => (onlyTrackedBranches && b.IsTracking) || !onlyTrackedBranches))
{
log.Info($"Searching for commits reachable from '{branch.FriendlyName}'.");

Expand Down
12 changes: 6 additions & 6 deletions src/GitVersionCore/GitVersionContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,18 @@ public class GitVersionContext
{
private readonly ILog log;

public GitVersionContext(IRepository repository, ILog log, string targetBranch, Config configuration, bool onlyEvaluateTrackedBranches = true, string commitId = null)
: this(repository, log, GetTargetBranch(repository, targetBranch), configuration, onlyEvaluateTrackedBranches, commitId)
public GitVersionContext(IRepository repository, ILog log, string targetBranch, Config configuration, bool onlyTrackedBranches = false, string commitId = null)
: this(repository, log, GetTargetBranch(repository, targetBranch), configuration, onlyTrackedBranches, commitId)
{
}

public GitVersionContext(IRepository repository, ILog log, Branch currentBranch, Config configuration, bool onlyEvaluateTrackedBranches = true, string commitId = null)
public GitVersionContext(IRepository repository, ILog log, Branch currentBranch, Config configuration, bool onlyTrackedBranches = false, string commitId = null)
{
this.log = log;
Repository = repository;
RepositoryMetadataProvider = new GitRepoMetadataProvider(repository, log, configuration);
FullConfiguration = configuration;
OnlyEvaluateTrackedBranches = onlyEvaluateTrackedBranches;
OnlyTrackedBranches = onlyTrackedBranches;

if (currentBranch == null)
throw new InvalidOperationException("Need a branch to operate on");
Expand Down Expand Up @@ -53,7 +53,7 @@ public GitVersionContext(IRepository repository, ILog log, Branch currentBranch,

if (currentBranch.IsDetachedHead())
{
CurrentBranch = RepositoryMetadataProvider.GetBranchesContainingCommit(CurrentCommit, repository.Branches.ToList(), OnlyEvaluateTrackedBranches).OnlyOrDefault() ?? currentBranch;
CurrentBranch = RepositoryMetadataProvider.GetBranchesContainingCommit(CurrentCommit, repository.Branches.ToList(), OnlyTrackedBranches).OnlyOrDefault() ?? currentBranch;
}
else
{
Expand All @@ -78,7 +78,7 @@ public GitVersionContext(IRepository repository, ILog log, Branch currentBranch,
/// </summary>
public Config FullConfiguration { get; }
public SemanticVersion CurrentCommitTaggedVersion { get; }
public bool OnlyEvaluateTrackedBranches { get; }
public bool OnlyTrackedBranches { get; }
public EffectiveConfiguration Configuration { get; private set; }
public IRepository Repository { get; }
public Branch CurrentBranch { get; }
Expand Down