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.

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
merged 8 commits into from
Jun 7, 2015
Merged

Failing inheritance fixes #456

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3ef0185
Failing test for feature inheriting from parent named development
rcknight Apr 24, 2015
bd005b7
Make develop match work more like config matching
rcknight Apr 24, 2015
dbdbeec
Failing test for broken inheritance after merges
rcknight Apr 24, 2015
730dbfb
Made FindCommitBranchWasBranchedFrom way more efficient
JakeGinnivan Jun 3, 2015
9fdcb69
Fixed test
JakeGinnivan Jun 3, 2015
00f60a4
Fixed all broken existing tests
JakeGinnivan Jun 3, 2015
d68122f
Fixed BranchCreatedAfterFastForwardMergeShouldInheritCorrectly test
JakeGinnivan Jun 3, 2015
6a5d0e1
Fixed broken test
JakeGinnivan Jun 7, 2015
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
57 changes: 57 additions & 0 deletions GitVersionCore.Tests/IntegrationTests/FeatureBranchScenarios.cs
View file
Open in desktop
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
3 changes: 1 addition & 2 deletions GitVersionCore.Tests/IntegrationTests/ReleaseBranchScenarios.cs
View file
Open in desktop
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");
}
}
}
34 changes: 24 additions & 10 deletions GitVersionCore/BranchConfigurationCalculator.cs
View file
Open in desktop
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,24 +74,33 @@ 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)));
Expand Down Expand Up @@ -117,8 +131,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
21 changes: 12 additions & 9 deletions GitVersionCore/LibGitExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,20 @@ 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.Where(b => !b.IsRemote).Except(excludedBranches).Except(new [] { branch }).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);
return mergeBases.OrderByDescending(b => b.Committer.When).FirstOrDefault();
}

public static IEnumerable<Branch> GetBranchesContainingCommit(this Commit commit, IRepository repository, bool onlyTrackedBranches)
Expand Down
2 changes: 1 addition & 1 deletion ...rsionCore/VersionCalculation/BaseVersionCalculators/VersionInBranchBaseVersionStrategy.cs
View file
Open in desktop
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