GitVersion icon indicating copy to clipboard operation
GitVersion copied to clipboard

Published 20 hours ago • verified publisher icon GitTools

[Bug] Closing pull request from hotfix to support failed to inherit Increment branch configuration

Open micdenny opened this issue 2 years ago • 22 comments

The version chosen when closing a pull request from hotfix branch to support branch is wrong because gitversion doesn't choose the right parent branch.

GitVersion.yml

mode: ContinuousDeployment
branches:
  master:
    tag: beta
    increment: Minor
  pull-request:
    tag: alpha-pr
  support:
    tag: beta
ignore:
  sha: []
merge-message-formats: {}

I have a couple of case that you can look at it.

case 1 remote + local branches

image

WARN [02/28/22 16:27:04:24] Failed to inherit Increment branch configuration, ended up with: support/v1.0.x, origin/hotfix/v1.0.2
Falling back to master branch config

case 2 remote/local only

image

WARN [02/28/22 16:13:00:16] Failed to inherit Increment branch configuration, ended up with: master, support/v1.0.x
Falling back to master branch config

The pull/x/merge is the branch that is automatically created from azure devops pull request, and the build pipeline use that branch to build from, and I would like to have a proper version even in that case to generate a very early pre-release version.

Expected Behavior

case 1 remote + local branches

"FullSemVer": "1.0.2-alpha-pr0002.2"

case 2 remote/local only

"FullSemVer": "1.0.1-alpha-pr0002.2"

Actual Behavior

case 1 remote + local branches

"FullSemVer": "1.1.0-alpha-pr0002.3"

case 2 remote/local only

"FullSemVer": "1.1.0-alpha-pr0002.2"

Steps to Reproduce

case 1 remote + local branches

git init remote-case1
cd remote-case1

# create a file GitVersion.yml using the configuration up above

git add .
git commit -m "First commit"
git tag -a -m "Release v1.0.0" v1.0.0
git commit --allow-empty -m "Merged PR 1: new feature"
git checkout tags/v1.0.0 -b support/v1.0.x
git commit --allow-empty -m "hotfix 1"
git tag -a -m "Release v1.0.1" v1.0.1
git checkout -b hotfix/v1.0.2
git commit --allow-empty -m "hotfix 2"
git checkout support/v1.0.x
git checkout -b pull/2/merge
git merge --no-ff -m "Merge pull request 2 from hotfix/v1.0.2 into support/v1.0.x"  hotfix/v1.0.2

cd ..
git clone remote-case1 local-case1
cd local-case1
git checkout master
git checkout support/v1.0.x
git checkout hotfix/v1.0.2
git checkout pull/2/merge

# dotnet gitversion /nocache /output json /output buildserver
# gitversion response is wrong 1.1.0-alpha-pr0002.3 expected 1.0.2-alpha-pr0002.2

case 2 remote/local only

git init remote-case2
cd remote-case2

# create a file GitVersion.yml using the configuration up above

git add .
git commit -m "First commit"
git tag -a -m "Release v1.0.0" v1.0.0
git commit --allow-empty -m "Merged PR 1: new feature"
git checkout tags/v1.0.0 -b support/v1.0.x
git checkout -b hotfix/v1.0.1
git commit --allow-empty -m "hotfix"
git checkout support/v1.0.x
git checkout -b pull/2/merge
git merge --no-ff -m "Merge pull request 2 from hotfix/v1.0.1 into support/v1.0.x"  hotfix/v1.0.1

# dotnet gitversion /nocache /output json /output buildserver
# gitversion response is wrong 1.1.0-alpha-pr0002.2 expected 1.0.1-alpha-pr0002.2

Your Environment

  • Version Used: 5.10.3+Branch.support-5.x.Sha.bc9c9d003e655385e3dd1ba3bd013e04062d2f9b
  • Operating System and version: Windows 10 21H2 (19044.1566)

micdenny avatar Feb 28 '22 16:02 micdenny

With GitVersion 5.10.3, this is the output I get:

{
  "Major": 1,
  "Minor": 0,
  "Patch": 2,
  "PreReleaseTag": "PullRequest0002.2",
  "PreReleaseTagWithDash": "-PullRequest0002.2",
  "PreReleaseLabel": "PullRequest0002",
  "PreReleaseLabelWithDash": "-PullRequest0002",
  "PreReleaseNumber": 2,
  "WeightedPreReleaseNumber": 30002,
  "BuildMetaData": null,
  "BuildMetaDataPadded": "",
  "FullBuildMetaData": "Branch.pull-2-merge.Sha.cde794da99c75e1997ad38167e0fec05fabfb1e7",
  "MajorMinorPatch": "1.0.2",
  "SemVer": "1.0.2-PullRequest0002.2",
  "LegacySemVer": "1.0.2-PullRequest0002-2",
  "LegacySemVerPadded": "1.0.2-PullRequest0002-0002",
  "AssemblySemVer": "1.0.2.0",
  "AssemblySemFileVer": "1.0.2.0",
  "FullSemVer": "1.0.2-PullRequest0002.2",
  "InformationalVersion": "1.0.2-PullRequest0002.2+Branch.pull-2-merge.Sha.cde794da99c75e1997ad38167e0fec05fabfb1e7",
  "BranchName": "pull/2/merge",
  "EscapedBranchName": "pull-2-merge",
  "Sha": "cde794da99c75e1997ad38167e0fec05fabfb1e7",
  "ShortSha": "cde794d",
  "NuGetVersionV2": "1.0.2-pullrequest0002-0002",
  "NuGetVersion": "1.0.2-pullrequest0002-0002",
  "NuGetPreReleaseTagV2": "pullrequest0002-0002",
  "NuGetPreReleaseTag": "pullrequest0002-0002",
  "VersionSourceSha": "919c847f718e25c6669717146e11a0542fdc709f",
  "CommitsSinceVersionSource": 2,
  "CommitsSinceVersionSourcePadded": "0002",
  "UncommittedChanges": 0,
  "CommitDate": "2022-09-03"
}

Perhaps you can write up your test case and expected version assertions as a RepositoryFixture test? Then it would be easier to understand what your expectations are and debug why they are not met by GitVersion.

asbjornu avatar Sep 03 '22 22:09 asbjornu

I think I am facing the same issue here when I create a PR from a hotfix branch int master. I am not a C# developer but here's how I imagine the test would be.

[Test]
public void CalculatesCorrectVersionAfterHotfixBranchMergedToMain()
{
    using var fixture = new EmptyRepositoryFixture();
    fixture.Repository.MakeATaggedCommit("1.0.0");
    fixture.Repository.MakeACommit();
    Commands.Checkout(fixture.Repository, fixture.Repository.CreateBranch("hotfix/1.0.1"));
    fixture.Repository.MakeACommit();
    fixture.Repository.MakeACommit();

    fixture.Repository.CreatePullRequestRef("hotfix/1.0.1", MainBranch, normalise: true);

    fixture.AssertFullSemver("1.0.1-PullRequest2.2");
}

Hope this help. Please keep us posted for any workaround. Thank you

NightFox7 avatar Sep 07 '22 13:09 NightFox7

I managed to execute the test above. I actually get the required result that is 1.0.1-PullRequest. Unfortunately, this is not the case when the PR is created by AzureDevops. Maybe the test fixture does not match what is actually done by Azure Devops. I am available if you need any further information.

NightFox7 avatar Sep 07 '22 14:09 NightFox7

@NightFox7, I suppose you will have to create a remote repository and then clone that to a local one to reproduce AzDO's behavior. You can see an example of how that's done in the following test:

https://github.com/GitTools/GitVersion/blob/9998780a8bf9bb0725d67fb37189768bdbbdd435/src/GitVersion.Core.Tests/IntegrationTests/MainlineDevelopmentMode.cs#L499-L510

asbjornu avatar Sep 08 '22 11:09 asbjornu

@asbjornu Yes I have done that. But I created a new issue #3187 because I now think these might be different issues. Here is the PR for the test #3186.

NightFox7 avatar Sep 08 '22 11:09 NightFox7

I will try to re-run the issue with newer version, I didn't clone a repo from azdo, the test case I've submitted it's a vanilla git repository created exactly with the repro steps I've attached.

If @asbjornu results is 1.0.2 with the case 1, maybe something has changed in newer version and I'm going to check as soon as I found some spare time. Thank you!

micdenny avatar Sep 08 '22 17:09 micdenny

I've re-run it the exact same repro steps with version 5.10.3+Branch.support-5.x.Sha.bc9c9d003e655385e3dd1ba3bd013e04062d2f9b and it does the same

dotnet gitversion /nocache /output json /output buildserver

INFO [09/08/22 20:03:32:73] Working directory: C:\temp\gittools-bug\local-case1
INFO [09/08/22 20:03:32:76] Project root is: C:\temp\gittools-bug\local-case1\
INFO [09/08/22 20:03:32:76] DotGit directory is: C:\temp\gittools-bug\local-case1\.git
INFO [09/08/22 20:03:32:77] Branch from build environment:
INFO [09/08/22 20:03:32:80] Using latest commit on specified branch
INFO [09/08/22 20:03:32:91] Begin: Attempting to inherit branch configuration from parent branch
  INFO [09/08/22 20:03:32:92] HEAD is merge commit, this is likely a pull request using hotfix/v1.0.2 as base
  INFO [09/08/22 20:03:32:92] Begin: Finding branch source of 'hotfix/v1.0.2'
    INFO [09/08/22 20:03:32:93] Begin: Finding merge base between 'hotfix/v1.0.2' and 'master'.
      INFO [09/08/22 20:03:32:94] Found merge base of 0c1ce4d First commit
      INFO [09/08/22 20:03:32:95] Merge base of hotfix/v1.0.2' and 'master is 0c1ce4d First commit
    INFO [09/08/22 20:03:32:95] End: Finding merge base between 'hotfix/v1.0.2' and 'master'. (Took: 21.71ms)
    INFO [09/08/22 20:03:32:95] Begin: Finding merge base between 'hotfix/v1.0.2' and 'support/v1.0.x'.
      INFO [09/08/22 20:03:32:95] Found merge base of 7c7ff0a hotfix 1
      INFO [09/08/22 20:03:32:95] Merge base of hotfix/v1.0.2' and 'support/v1.0.x is 7c7ff0a hotfix 1
    INFO [09/08/22 20:03:32:95] End: Finding merge base between 'hotfix/v1.0.2' and 'support/v1.0.x'. (Took: 2.23ms)
    INFO [09/08/22 20:03:32:96] Multiple source branches have been found, picking the first one (master).
This may result in incorrect commit counting.
Options were:
master, support/v1.0.x
  INFO [09/08/22 20:03:32:96] End: Finding branch source of 'hotfix/v1.0.2' (Took: 41.04ms)
  INFO [09/08/22 20:03:32:96] Begin: Getting branches containing the commit '0c1ce4d'.
    INFO [09/08/22 20:03:32:97] Trying to find direct branches.
    INFO [09/08/22 20:03:32:97] No direct branches found, searching through all branches.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'master'.
    INFO [09/08/22 20:03:32:97] The branch 'master' has a matching commit.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'support/v1.0.x'.
    INFO [09/08/22 20:03:32:97] The branch 'support/v1.0.x' has a matching commit.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'origin/hotfix/v1.0.2'.
    INFO [09/08/22 20:03:32:97] The branch 'origin/hotfix/v1.0.2' has a matching commit.
  INFO [09/08/22 20:03:32:97] End: Getting branches containing the commit '0c1ce4d'. (Took: 5.51ms)
  INFO [09/08/22 20:03:32:97] Begin: Getting branches containing the commit '34dba13'.
    INFO [09/08/22 20:03:32:97] Trying to find direct branches.
    INFO [09/08/22 20:03:32:97] No direct branches found, searching through all branches.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'master'.
    INFO [09/08/22 20:03:32:97] The branch 'master' has no matching commits.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'support/v1.0.x'.
    INFO [09/08/22 20:03:32:97] The branch 'support/v1.0.x' has no matching commits.
    INFO [09/08/22 20:03:32:97] Searching for commits reachable from 'origin/hotfix/v1.0.2'.
    INFO [09/08/22 20:03:32:98] The branch 'origin/hotfix/v1.0.2' has no matching commits.
  INFO [09/08/22 20:03:32:98] End: Getting branches containing the commit '34dba13'. (Took: 5.77ms)
  INFO [09/08/22 20:03:32:98] Found possible parent branches: master, support/v1.0.x, origin/hotfix/v1.0.2
  WARN [09/08/22 20:03:32:98] Failed to inherit Increment branch configuration, ended up with: master, support/v1.0.x, origin/hotfix/v1.0.2
Falling back to master branch config
  INFO [09/08/22 20:03:32:98] End: Attempting to inherit branch configuration from parent branch (Took: 76.37ms)
  INFO [09/08/22 20:03:33:02] Running against branch: pull/2/merge (34dba13 Merge pull request 2 from hotfix/v1.0.2 into support/v1.0.x)
  INFO [09/08/22 20:03:33:02] Begin: Calculating base versions
    INFO [09/08/22 20:03:33:02] Fallback base version: 0.1.0 with commit count source 0c1ce4d0bcfcc466c1d0ed4a99aab8922c5aa332
    INFO [09/08/22 20:03:33:10] Git tag 'v1.0.0': 1.0.0 with commit count source 0c1ce4d0bcfcc466c1d0ed4a99aab8922c5aa332
    INFO [09/08/22 20:03:33:10] Git tag 'v1.0.1': 1.0.1 with commit count source 7c7ff0ad35b47a90c167084d5cf4488197a141e3
    INFO [09/08/22 20:03:33:12] Found multiple base versions which will produce the same SemVer (1.1.0), taking oldest source for commit counting (Git tag 'v1.0.1')
    INFO [09/08/22 20:03:33:13] Base version used: Git tag 'v1.0.1': 1.0.1 with commit count source 7c7ff0ad35b47a90c167084d5cf4488197a141e3
  INFO [09/08/22 20:03:33:13] End: Calculating base versions (Took: 106.09ms)
  INFO [09/08/22 20:03:33:13] 2 commits found between 7c7ff0a hotfix 1 and 34dba13 Merge pull request 2 from hotfix/v1.0.2 into support/v1.0.x
  INFO [09/08/22 20:03:33:13] 2 commits found between 7c7ff0a hotfix 1 and 34dba13 Merge pull request 2 from hotfix/v1.0.2 into support/v1.0.x
  INFO [09/08/22 20:03:33:13] Begin: Getting version tags from branch 'refs/heads/pull/2/merge'.
  INFO [09/08/22 20:03:33:14] End: Getting version tags from branch 'refs/heads/pull/2/merge'. (Took: 5.20ms)
Executing GenerateSetVersionMessage for 'LocalBuild'.

Executing GenerateBuildLogOutput for 'LocalBuild'.
{
  "Major": 1,
  "Minor": 1,
  "Patch": 0,
  "PreReleaseTag": "alpha-pr0002.2",
  "PreReleaseTagWithDash": "-alpha-pr0002.2",
  "PreReleaseLabel": "alpha-pr0002",
  "PreReleaseLabelWithDash": "-alpha-pr0002",
  "PreReleaseNumber": 2,
  "WeightedPreReleaseNumber": 30002,
  "BuildMetaData": null,
  "BuildMetaDataPadded": "",
  "FullBuildMetaData": "Branch.pull-2-merge.Sha.34dba133c6929851d44eb9616a677202aa8a0e56",
  "MajorMinorPatch": "1.1.0",
  "SemVer": "1.1.0-alpha-pr0002.2",
  "LegacySemVer": "1.1.0-alpha-pr0002-2",
  "LegacySemVerPadded": "1.1.0-alpha-pr0002-0002",
  "AssemblySemVer": "1.1.0.0",
  "AssemblySemFileVer": "1.1.0.0",
  "FullSemVer": "1.1.0-alpha-pr0002.2",
  "InformationalVersion": "1.1.0-alpha-pr0002.2+Branch.pull-2-merge.Sha.34dba133c6929851d44eb9616a677202aa8a0e56",
  "BranchName": "pull/2/merge",
  "EscapedBranchName": "pull-2-merge",
  "Sha": "34dba133c6929851d44eb9616a677202aa8a0e56",
  "ShortSha": "34dba13",
  "NuGetVersionV2": "1.1.0-alpha-pr0002-0002",
  "NuGetVersion": "1.1.0-alpha-pr0002-0002",
  "NuGetPreReleaseTagV2": "alpha-pr0002-0002",
  "NuGetPreReleaseTag": "alpha-pr0002-0002",
  "VersionSourceSha": "7c7ff0ad35b47a90c167084d5cf4488197a141e3",
  "CommitsSinceVersionSource": 2,
  "CommitsSinceVersionSourcePadded": "0002",
  "UncommittedChanges": 0,
  "CommitDate": "2022-09-08"
}
  INFO [09/08/22 20:03:33:19] Done writing

with the same warning:

WARN [09/08/22 20:03:32:98] Failed to inherit Increment branch configuration, ended up with: master, support/v1.0.x, origin/hotfix/v1.0.2 Falling back to master branch config

remember to add the GitVersion.yml as follow:

mode: ContinuousDeployment
branches:
  master:
    tag: beta
    increment: Minor
  pull-request:
    tag: alpha-pr
  support:
    tag: beta
ignore:
  sha: []
merge-message-formats: {}

micdenny avatar Sep 08 '22 18:09 micdenny

@asbjornu I manage to create a repro as fixture here:

  • https://github.com/micdenny/GitVersion/tree/issue/3020/repro
  • https://github.com/GitTools/GitVersion/compare/5.10.3...micdenny:GitVersion:issue/3020/repro?expand=1

micdenny avatar Sep 09 '22 18:09 micdenny

I have taken a look to the following test:

    [Test]
    public void WithRemote()
    {
        var fixture = PrepareLocalRepo();

        var local = fixture.CloneRepository();
        local.Checkout("origin/hotfix/v1.0.2");
        local.BranchTo("hotfix/v1.0.2");
        local.Checkout("origin/support/v1.0.x");
        local.BranchTo("support/v1.0.x");
        local.Checkout("origin/main");
        local.BranchTo("main", "main");
        local.AssertFullSemver("1.1.0-beta.1", config);
        local.Checkout("pull/2/merge");

        local.AssertFullSemver("1.0.2-alpha-pr0002.2", config);

        local.Repository.DumpGraph();
    }

~~Why are you expecting the version 1.0.2-alpha-pr0002.2 for this scenario? You have configured the main branch Increment = IncrementStrategy.Minor. You have to set the value to IncrementStrategy.Patch if you ask me.~~ I didn't realize that you are trying to merge a pull-request from hotfix to the support branch. I agree with you that the expected result should be 1.0.2. This is because the Configuration (EffectedConfiguration) in the GitVersionContext is not correct. It has for this scenario the value IncrementStrategy.Minor but should be Patch.

HHobeck avatar Sep 09 '22 20:09 HHobeck

Even because if you check the OnlyLocal test it is just working fine, the remote version instead doesn't work, so there's should be a subtle bug somewhere

micdenny avatar Sep 09 '22 21:09 micdenny

@micdenny, If I change the increment strategy to Patch for pull-request, your remote test passes.

{ "pull-request", new BranchConfig { Tag = "alpha-pr", Increment = IncrementStrategy.Patch} },

asbjornu avatar Sep 12 '22 14:09 asbjornu

@micdenny, If I change the increment strategy to Patch for pull-request, your remote test passes.

Sorry @asbjornu, I don't get how this can help me, pull-request has to follow the targeting branch versioning style, in fact in the local version simply works, I don't understand why it doesn't work when there is also one remote in the same repo.

In other words, when I open a PR against master, I want to increase the minor part when I open a PR against a support branch, I want to increase the patch part

so basically the Increment strategy of pull-request should be inherit, as it is by default:

image

micdenny avatar Sep 12 '22 14:09 micdenny

Okay, so setting increment to Inherit gives the correct behavior in the local test, but not in the remote. Setting increment explicitly to Minor (as it should inherit from main) makes both tests fail. Setting increment explicitly to Patch makes both tests succeed.

I'm not sure why Inherit behaves differently in the two tests, but I agree it does seem like a bug. If you are able to figure out why this discrepancy exists, a pull request would be warmly welcome.

asbjornu avatar Sep 12 '22 15:09 asbjornu

Setting increment explicitly to Patch makes both tests succeed.

yes, but then if we add a test closing the PR against main setting increment to patch, that test will fail. I can also add that test too, so that we have to push us to green all those.

I'm not sure why Inherit behaves differently in the two tests, but I agree it does seem like a bug. If you are able to figure out why this discrepancy exists, a pull request would be warmly welcome.

I can give it a try

micdenny avatar Sep 12 '22 16:09 micdenny

I figure out where the problem is, but not why:

image

in somehow the remote is messing up probably with the LocalRemoteBranchEqualityComparer, because in the excludedInheritBranches there is the hotfix/v1.0.2 we don't want

image

will continue investigate later

micdenny avatar Sep 12 '22 18:09 micdenny

Nice find, @micdenny. Thanks for digging into this!

asbjornu avatar Sep 12 '22 21:09 asbjornu

It is not LocalRemoteBranchEqualityComparer, but it's the this.repositoryStore.ExcludingBranches(excludedInheritBranches) and the "why" resides in this equality/comparer lambdas

image

I would not change this behavior, probably you want to know that origin/main it is not equals to main.

I think I'll go with a sanitation of the excludedInheritBranches before passing to the method ExcludingBranches.

Hold on ⏳

micdenny avatar Sep 13 '22 07:09 micdenny

I think I'll go with a sanitation of the excludedInheritBranches before passing to the method ExcludingBranches.

Nope the problem it's not directly related to excludedInheritBranches, the hotfix/v1.0.2 branch is calculated by CalculateWhenMultipleParents that uses this method that excludes remote:

image

If we exclude remotes then the name comparer used in ~GetExcludedInheritBranches~ ExcludingBranches that use Canonical name kicks in and doesn't filter out origin/hotfix/v1.0.2

So basically I'm in a clamp, one of the two flow has to be changed.

~I try to alter GetExcludedInheritBranches because it is used only there.~

micdenny avatar Sep 13 '22 07:09 micdenny

@asbjornu I've opened the PR #3196 let me know if that make sense or you can do it better

micdenny avatar Sep 13 '22 10:09 micdenny

:tada: This issue has been resolved in version 6.0.0 :tada: The release is available on:

Your GitReleaseManager bot :package::rocket:

arturcic avatar Dec 22 '22 15:12 arturcic

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.

github-actions[bot] avatar Jun 27 '23 11:06 github-actions[bot]

This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs.

github-actions[bot] avatar Sep 29 '23 11:09 github-actions[bot]