Skip to content
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

CommitsSinceVersionSource goes down when deleting a release branch #1526

Closed
DOMZE opened this issue Nov 8, 2018 · 13 comments
Closed

CommitsSinceVersionSource goes down when deleting a release branch #1526

DOMZE opened this issue Nov 8, 2018 · 13 comments

Comments

@DOMZE
Copy link

DOMZE commented Nov 8, 2018

Starting with a clean master branch with only a GitVersion.yml config file

Using the flow described in the following diagram:
image

When deleting the release branch after merging it back to develop causes the CommitsSinceVersionSource to go down from 0006 to 0005

Git commands to reproduce the behavior:

git tag 1.1.0
git checkout -b develop
git commit -m "commit in develop" --allow-empty # 1.2.0-alpha.1
git checkout -b feature/featureA
git commit -m "commit in featureA" --allow-empty # 1.2.0-featureA.2
git commit -m "commit in featureA" --allow-empty # 1.2.0-featureA.3
git checkout develop
git commit -m "commit in develop" --allow-empty # 1.2.0-alpha.2
git commit -m "commit in develop" --allow-empty # 1.2.0-alpha.3
git checkout -b release/1.2.0
git commit -m "commit in release/1.2.0" --allow-empty # 1.2.0-beta.1
git checkout feature/featureA # 1.2.0-featureA.3
git merge develop --no-ff # BUG see issue 1525
git checkout develop
git merge feature/featureA --no-ff # 1.3.0-alpha.4
git checkout master
git merge release/1.2.0 --no-ff
git tag 1.2.0
git checkout develop
git merge release/1.2.0 --no-ff # 1.3.0-alpha.6
git branch -d release/1.2.0 # 1.3.0-alpha.5 <---- ???

GitVersion.yml looks as follow:

mode: ContinuousDeployment
branches: {}
ignore:
  sha: []

Using GitVersion 4.0.0 and ContinuousDeployment in all branches

@DOMZE DOMZE changed the title CommitsSinceVersionSource goes down when deleting release branch CommitsSinceVersionSource goes down when deleting a release branch Nov 8, 2018
@DOMZE
Copy link
Author

DOMZE commented Nov 13, 2018

@arturcic @asbjornu @dazinator @gep13 @pascalberger any thoughts?

@DOMZE
Copy link
Author

DOMZE commented Nov 16, 2018

Please see PR #1533 for the demonstration of this bug.
The test fails

@ruhullahshah
Copy link
Contributor

ruhullahshah commented Feb 22, 2019

After digging into the version calculation code a bit, I figured out that CommitsSinceVersionSource is calculated by LibGit2Sharp here.

Since the commit count is return by LibGit2Sharp, the question that arises is that are we asking the right question in the CommitFilter, if one of the parents of a merge commit is deleted? If no, how should the CommitFilter be updated to accommodate this scenario?

@asbjornu
Copy link
Member

Good question, @ruhullahshah. Do you have any suggestions?

@ruhullahshah
Copy link
Contributor

@asbjornu As of now no. I have never used LibGit2Sharp, but I assume that there are people here who are experienced at this and could provide some suggestions to shorten the time-to-fix. Otherwise, it might take some time to fix this one.

@asbjornu
Copy link
Member

asbjornu commented Feb 26, 2019

Perhaps @ethomson, @bording or @nulltoken receives this and can provide some guidance?

@ruhullahshah
Copy link
Contributor

ruhullahshah commented Feb 26, 2019

Upon further code exploration, I figured out that LibGit2Sharp is doing the right thing. The observed failure is due to GitVersion passing the wrong commit as the base version source here.

With the current version calculation strategies, I do not see as to how GitVersion could resolve this issue.

To be a bit more comprehensive, consider the following GitVersion sequence diagram:

master -> master: commit
master -> master: tag 1.1.0
create participant develop
master -> develop: branch from master
develop -> develop: Commit 'commit in develop - 1' //CommitX
create participant release/1.2.0
develop -> release/1.2.0: branch from develop 
release/1.2.0 -> release/1.2.0: Commit 'commit in release/1.2.0 - 1'
release/1.2.0 -> release/1.2.0: Commit 'commit in release/1.2.0 - 2'
release/1.2.0 -> release/1.2.0: Commit 'commit in release/1.2.0 - 3' //CommitY
note over release/1.2.0 #D3D3D3
  1.2.0-beta.3
end note
release/1.2.0 -> release/1.2.0: tag 1.2.0
develop -> develop: Commit 'commit in develop - 2'
release/1.2.0 -> develop: merge
note over develop #D3D3D3
  1.3.0-alpha.5
end note

In the absence of the release branch, the TrackReleaseBranchesVersionStrategy will not be useful anymore and all the other existing strategies will not use CommitX (see the sequence diagram) as the base version source. Instead, TaggedCommitVersionStrategy will propose CommitY (see the sequence diagram) as the base version source. LibGit2Sharp correctly counts 2 commits from there on, leading to a decreased CommitsSinceVersionSource

Any suggestions to fix this?

@asbjornu
Copy link
Member

asbjornu commented Feb 28, 2019

Good observation and forensics there, @ruhullahshah. Yep, it's obvious that TrackReleaseBranchesVersionStrategy in its current incarnation is pretty useless in the absence of a release branch. Can we somehow deduce that a release branch was merged and then count the number of commits that were made on the release branch?

git rev-list --count HEAD ^release/x won't work since release/x doesn't exist anymore, but the commits are in the graph, so they can be walked from the merge commit if nothing else.

@dazinator
Copy link
Member

dazinator commented Feb 28, 2019

Can we somehow deduce that a release branch was merged and then count the number of commits that were made on the release branch?

I discovered in the past that you can detect a release branch has been merged based on finding the standard merge commit message. However... I found that sometimes when merging release branches locally (I use git flow) - the merge is done as a fast forward.. which avoids any such merge commit and makes the release branch undetectable (as though it never existed). Therefore if you do rely on this strategy, you have to make it a policy when merging release branches to use the --no-ff flag to force a merge commit. Looks like the OP is already using that policy so should be ok.

@ruhullahshah
Copy link
Contributor

@asbjornu and @dazinator, Thanks for the feedback. I have created a fix for this issue in the referenced pull request. It would be nice to have your comments.

@asbjornu
Copy link
Member

#1635 is merged. Can you please test it, @DOMZE?

@DOMZE
Copy link
Author

DOMZE commented Mar 16, 2019

@asbjornu just tried it now with the latest source code. It works. after deleting the branch, the release stays at 1.3.0-alpha.6

@DOMZE DOMZE closed this as completed Mar 16, 2019
@asbjornu
Copy link
Member

@DOMZE: Fantastic! Thanks to @ruhullahshah for fixing this long-standing issue! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants