Skip to content

Failing inheritance fixes #456

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.

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
merged 8 commits into from
Jun 7, 2015
Merged
5 changes: 0 additions & 5 deletions GitVersionCore.Tests/Fixtures/EmptyRepositoryFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ public EmptyRepositoryFixture(Config configuration) :
{
}

public void DumpGraph()
{
Repository.DumpGraph();
}

static IRepository CreateNewRepository(string path)
{
LibGit2Sharp.Repository.Init(path);
Expand Down
12 changes: 11 additions & 1 deletion GitVersionCore.Tests/Fixtures/RepositoryFixtureBase.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Diagnostics;
using GitVersion;
using LibGit2Sharp;
using Shouldly;
Expand Down Expand Up @@ -30,7 +31,16 @@ public void AssertFullSemver(string fullSemver, IRepository repository = null, s
gitVersionContext.Configuration.VersioningMode,
gitVersionContext.Configuration.ContinuousDeploymentFallbackTag,
gitVersionContext.IsCurrentCommitTagged);
variables.FullSemVer.ShouldBe(fullSemver);
try
{
variables.FullSemVer.ShouldBe(fullSemver);
}
catch (Exception)
{
Trace.WriteLine("Test failing, dumping repository graph");
repository.DumpGraph();
throw;
}
}

private SemanticVersion ExecuteGitVersion(GitVersionContext context)
Expand Down
2 changes: 1 addition & 1 deletion GitVersionCore.Tests/Helpers/GitTestExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public static void DumpGraph(this IRepository repository)
e => output.AppendLineFormat("ERROR: {0}", e),
null,
"git",
@"log --graph --abbrev-commit --decorate --date=relative --all",
@"log --graph --abbrev-commit --decorate --date=relative --all --remotes=*",
repository.Info.Path);

Trace.Write(output.ToString());
Expand Down
57 changes: 57 additions & 0 deletions GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,63 @@
[TestFixture]
public class FeatureBranchScenarios
{
[Test]
public void ShouldInheritIncrementCorrectlyWithMultiplePossibleParentsAndWeirdlyNamedDevelopBranch()
{
using (var fixture = new EmptyRepositoryFixture(new Config()))
{
fixture.Repository.MakeATaggedCommit("1.0.0");
fixture.Repository.CreateBranch("development");
fixture.Repository.Checkout("development");

//Create an initial feature branch
var feature123 = fixture.Repository.CreateBranch("feature/JIRA-123");
fixture.Repository.Checkout("feature/JIRA-123");
fixture.Repository.MakeCommits(1);

//Merge it
fixture.Repository.Checkout("development");
fixture.Repository.Merge(feature123, SignatureBuilder.SignatureNow());

//Create a second feature branch
fixture.Repository.CreateBranch("feature/JIRA-124");
fixture.Repository.Checkout("feature/JIRA-124");
fixture.Repository.MakeCommits(1);

fixture.AssertFullSemver("1.1.0-JIRA-124.1+2");
}
}

[Test]
public void BranchCreatedAfterFastForwardMergeShouldInheritCorrectly()
{
var config = new Config();
config.Branches.Add("unstable", config.Branches["develop"]);

using (var fixture = new EmptyRepositoryFixture(config))
{
fixture.Repository.MakeATaggedCommit("1.0.0");
fixture.Repository.CreateBranch("unstable");
fixture.Repository.Checkout("unstable");

//Create an initial feature branch
var feature123 = fixture.Repository.CreateBranch("feature/JIRA-123");
fixture.Repository.Checkout("feature/JIRA-123");
fixture.Repository.MakeCommits(1);

//Merge it
fixture.Repository.Checkout("unstable");
fixture.Repository.Merge(feature123, SignatureBuilder.SignatureNow());

//Create a second feature branch
fixture.Repository.CreateBranch("feature/JIRA-124");
fixture.Repository.Checkout("feature/JIRA-124");
fixture.Repository.MakeCommits(1);

fixture.AssertFullSemver("1.1.0-JIRA-124.1+2");
}
}

[Test]
public void ShouldNotUseNumberInFeatureBranchAsPreReleaseNumberOffDevelop()
{
Expand Down
6 changes: 3 additions & 3 deletions GitVersionCore.Tests/IntegrationTests/PullRequestScenarios.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ public void CanCalculatePullRequestChanges()

fixture.Repository.CreatePullRequest("feature/Foo", "master");

fixture.DumpGraph();
fixture.Repository.DumpGraph();
fixture.AssertFullSemver("0.1.1-PullRequest.2+2");
}
}
Expand All @@ -34,7 +34,7 @@ public void CanCalculatePullRequestChangesInheritingConfig()

fixture.Repository.CreatePullRequest("feature/Foo", "develop", 44);

fixture.DumpGraph();
fixture.Repository.DumpGraph();
fixture.AssertFullSemver("0.2.0-PullRequest.44+3");
}
}
Expand All @@ -51,7 +51,7 @@ public void CanCalculatePullRequestChangesFromRemoteRepo()

fixture.Repository.CreatePullRequest("feature/Foo", "master");

fixture.DumpGraph();
fixture.Repository.DumpGraph();
fixture.AssertFullSemver("0.1.1-PullRequest.2+2");
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,7 @@ public void WhenMergingReleaseBackToDevShouldNotResetBetaVersion()

//but keep working on the release
fixture.Repository.Checkout("release-2.0.0");

fixture.AssertFullSemver("2.0.0-beta.2+3");
fixture.AssertFullSemver("2.0.0-beta.2+2");
}
}
}
40 changes: 24 additions & 16 deletions GitVersionCore/BranchConfigurationCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ public class BranchConfigurationCalculator
{
public static KeyValuePair<string, BranchConfig> GetBranchConfiguration(Commit currentCommit, IRepository repository, bool onlyEvaluateTrackedBranches, Config config, Branch currentBranch, IList<Branch> excludedInheritBranches = null)
{
var matchingBranches = config.Branches.Where(b => Regex.IsMatch(currentBranch.Name, "^" + b.Key, RegexOptions.IgnoreCase)).ToArray();
var matchingBranches = LookupBranchConfiguration(config, currentBranch);

if (matchingBranches.Length == 0)
{
Expand All @@ -33,6 +33,11 @@ public static KeyValuePair<string, BranchConfig> GetBranchConfiguration(Commit c
throw new Exception(string.Format(format, currentBranch.Name, string.Join(", ", matchingBranches.Select(b => b.Key))));
}

static KeyValuePair<string, BranchConfig>[] LookupBranchConfiguration(Config config, Branch currentBranch)
{
return config.Branches.Where(b => Regex.IsMatch(currentBranch.Name, "^" + b.Key, RegexOptions.IgnoreCase)).ToArray();
}

static KeyValuePair<string, BranchConfig> InheritBranchConfiguration(bool onlyEvaluateTrackedBranches, IRepository repository, Commit currentCommit, Branch currentBranch, KeyValuePair<string, BranchConfig> keyValuePair, BranchConfig branchConfiguration, Config config, IList<Branch> excludedInheritBranches)
{
Logger.WriteInfo("Attempting to inherit branch configuration from parent branch");
Expand Down Expand Up @@ -69,34 +74,37 @@ static KeyValuePair<string, BranchConfig> InheritBranchConfiguration(bool onlyEv
}
if (excludedInheritBranches == null)
{
excludedInheritBranches = new List<Branch>();
excludedInheritBranches = repository.Branches.Where(b =>
{
var branchConfig = LookupBranchConfiguration(config, b);
return branchConfig.Length == 1 && branchConfig[0].Value.Increment == IncrementStrategy.Inherit;
}).ToList();
}
excludedBranches.ToList().ForEach(excludedInheritBranches.Add);

var branchPoint = currentBranch.FindCommitBranchWasBranchedFrom(repository, onlyEvaluateTrackedBranches, excludedInheritBranches.ToArray());
var branchPoint = currentBranch.FindCommitBranchWasBranchedFrom(repository, excludedInheritBranches.ToArray());

List<Branch> possibleParents;
if (branchPoint.Sha == currentCommit.Sha)
if (branchPoint == null)
{
possibleParents = currentCommit.GetBranchesContainingCommit(repository, true).Except(excludedInheritBranches).ToList();
}
else
{
var branches = branchPoint.GetBranchesContainingCommit(repository, true).Except(excludedInheritBranches).ToList();
var currentTipBranches = currentCommit.GetBranchesContainingCommit(repository, true).Except(excludedInheritBranches).ToList();
possibleParents = branches
.Except(currentTipBranches)
.ToList();
if (branches.Count > 1)
{
var currentTipBranches = currentCommit.GetBranchesContainingCommit(repository, true).Except(excludedInheritBranches).ToList();
possibleParents = branches.Except(currentTipBranches).ToList();
}
else
{
possibleParents = branches;
}
}

Logger.WriteInfo("Found possible parent branches: " + string.Join(", ", possibleParents.Select(p => p.Name)));

// If it comes down to master and something, master is always first so we pick other branch
if (possibleParents.Count == 2 && possibleParents.Any(p => p.Name == "master"))
{
possibleParents.Remove(possibleParents.Single(p => p.Name == "master"));
}

if (possibleParents.Count == 1)
{
var branchConfig = GetBranchConfiguration(currentCommit, repository, onlyEvaluateTrackedBranches, config, possibleParents[0], excludedInheritBranches).Value;
Expand All @@ -117,8 +125,8 @@ static KeyValuePair<string, BranchConfig> InheritBranchConfiguration(bool onlyEv
else
errorMessage = "Failed to inherit Increment branch configuration, ended up with: " + string.Join(", ", possibleParents.Select(p => p.Name));

var hasDevelop = repository.Branches["develop"] != null;
var branchName = hasDevelop ? "develop" : "master";
var developBranch = repository.Branches.FirstOrDefault(b => Regex.IsMatch(b.Name, "^develop", RegexOptions.IgnoreCase));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why using regex for this one? Maybe it's even better to make this a setting (I see some people using "dev" instead on the inet).

Copy link
Member

Choose a reason for hiding this comment

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

A setting for this is already available (as per #292), no?

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we should use it :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this can be removed. The new logic should allow not having this fallback. Let me look into it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can't be removed at the moment. If the code gets to the point where it cannot figure out which branch to inherit from then this code needs to be able to pick between master and develop.

An example:

* [master, develop]
|
* [feature/foo]

In this scenario there is no way to know if GitVersion should inheirit the config from master or develop, this code makes sure that develop gets selected. It isn't great but I can't think of a better solution?

var branchName = developBranch != null ? developBranch.Name : "master";

Logger.WriteWarning(errorMessage + Environment.NewLine + Environment.NewLine + "Falling back to " + branchName + " branch config");
var value = GetBranchConfiguration(currentCommit, repository, onlyEvaluateTrackedBranches, config, repository.Branches[branchName]).Value;
Expand Down
26 changes: 17 additions & 9 deletions GitVersionCore/LibGitExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,25 @@ public static Branch FindBranch(this IRepository repository, string branchName)
return repository.Branches.FirstOrDefault(x => x.Name == "origin/" + branchName);
}

public static Commit FindCommitBranchWasBranchedFrom(this Branch branch, IRepository repository, bool onlyTrackedBranches, params Branch[] excludedBranches)
public static Commit FindCommitBranchWasBranchedFrom(this Branch branch, IRepository repository, params Branch[] excludedBranches)
{
var currentBranches = branch.Tip.GetBranchesContainingCommit(repository, onlyTrackedBranches).ToList();
var tips = repository.Branches.Except(excludedBranches).Where(b => b != branch && !b.IsRemote).Select(b => b.Tip).ToList();
var branchPoint = branch.Commits.FirstOrDefault(c =>
var otherBranches = repository.Branches.Except(excludedBranches).Where(b => IsSameBranch(branch, b)).ToList();
var mergeBases = otherBranches.Select(b =>
{
if (tips.Contains(c)) return true;
var branchesContainingCommit = c.GetBranchesContainingCommit(repository, onlyTrackedBranches).ToList();
return branchesContainingCommit.Count > currentBranches.Count;
});
return branchPoint ?? branch.Tip;
var otherCommit = b.Tip;
if (b.Tip.Parents.Contains(branch.Tip))
{
otherCommit = b.Tip.Parents.First();
}
var mergeBase = repository.Commits.FindMergeBase(otherCommit, branch.Tip);
return mergeBase;
}).Where(b => b != null).ToList();
return mergeBases.OrderByDescending(b => b.Committer.When).FirstOrDefault();
}

static bool IsSameBranch(Branch branch, Branch b)
{
return (b.IsRemote ? b.Name.Replace(b.Remote.Name + "/", string.Empty) : b.Name) != branch.Name;
}

public static IEnumerable<Branch> GetBranchesContainingCommit(this Commit commit, IRepository repository, bool onlyTrackedBranches)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ public override BaseVersion GetVersion(GitVersionContext context)
var versionInBranch = GetVersionInBranch(context);
if (versionInBranch != null)
{
var commitBranchWasBranchedFrom = context.CurrentBranch.FindCommitBranchWasBranchedFrom(context.Repository, context.OnlyEvaluateTrackedBranches);
var commitBranchWasBranchedFrom = context.CurrentBranch.FindCommitBranchWasBranchedFrom(context.Repository);
var branchNameOverride = context.CurrentBranch.Name.RegexReplace("[-/]" + versionInBranch.Item1, string.Empty);
return new BaseVersion("Version in branch name", false, versionInBranch.Item2, commitBranchWasBranchedFrom, branchNameOverride);
}
Expand Down
1 change: 1 addition & 0 deletions GitVersionCore/VersionCalculation/MetaDataCalculator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ public SemanticVersionBuildMetaData Create(Commit baseVersionSource, GitVersionC

var commitLog = context.Repository.Commits.QueryBy(qf);
var commitsSinceTag = commitLog.Count();
Logger.WriteInfo(string.Format("{0} commits found between {1} and {2}", commitsSinceTag, baseVersionSource.Sha, context.CurrentCommit.Sha));

return new SemanticVersionBuildMetaData(
commitsSinceTag,
Expand Down
2 changes: 1 addition & 1 deletion GitVersionExe.Tests/GitPreparerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public void UpdatesExistingDynamicRepository()

using (var repository = new Repository(dynamicRepositoryPath))
{
mainRepositoryFixture.DumpGraph();
mainRepositoryFixture.Repository.DumpGraph();
repository.DumpGraph();
repository.Commits.ShouldContain(c => c.Sha == newCommit.Sha);
}
Expand Down