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

Version Tag w/ Same Name As Release Branch Does Not Finish Properly #221

Closed
jfelchner opened this issue Jun 1, 2012 · 13 comments
Closed

Comments

@jfelchner
Copy link

Our current git flow configuration:

[gitflow "branch"]
  master = master
  develop = development
[gitflow "prefix"]
  feature = feature/
  release = release/
  hotfix = hotfix/
  support = support/
  versiontag = release/

Notice that the versiontag and release are the same. When finishing a release using this configuration, git flow seems to:

  • Merge the branch into master
  • Create the version tag
  • Merge the branch into development
  • Delete the release branch ref

With this flow, the user receives release/foo is not unambiguous ref name or something similar. Additionally (as of my git version 1.7.9.5), this causes the tag's ref to be merged into development instead of the branch's ref. Very undesirable behavior.

I believe what needs to happen instead is:

  • Merge the branch into development
  • Merge the branch into master
  • Create the version tag
  • Delete the release branch ref
@sirlancelot
Copy link

just change versiontag to release-

@jfelchner
Copy link
Author

@sirlancelot Obviously the bug won't happen if I don't do the thing that causes the bug. :)

And I specifically want to use release/ because most good git GUIs will organize your tags/branches into a hierarchy if they have slashes in them.

@petervanderdoes
Copy link

@jfelchner Technically I think you are right with your proposal. By the letter of "law" the implemented way is the right way.

When creating a release, you update your production code (master branch), you tag it and you are done.
When you start development again, you would have to make sure you have the latest production release so you update your development source code (development branch).

@petervanderdoes
Copy link

@jfelchner Here's a solution for you, instead of messing with the order,

Open the file git-flow-release and edit line 256

git merge --no-ff "$BRANCH" || \

to look like

git merge --no-ff "refs/heads/$BRANCH" || \

@jfelchner
Copy link
Author

Excellent, that should do it. Is this something I should do as a PR?

@petervanderdoes
Copy link

I believe this change is very personal and not needed for public release.

In general it's not a good idea to name tags and branches the same. Besides the "problem" you showed here it means there are other commands that will give the same warning message. If you are not keeping the the release branch, why not use release- as a prefix for the release branch

git config gitflow.prefix.release release-

I'm not sure about this statement "the tag's ref to be merged into development instead of the branch's ref. Very undesirable behavior." Not sure why it is undesirable to merge the tag instead of the branch. In fact in the source code of git-flow-feature the following comment suggests preferring merging tags over branches

"Actually, accounting for 'git describe' pays, so we should ideally git merge --no-ff $tagname here, instead!"

With the current setup, git describe will not find a tag in the develop branch.

Just my two cents on this.

@jfelchner
Copy link
Author

I'm not sure I understand why one would not want to fix a situation where a warning would show up if the fix is relatively simple and non-invasive.

And it is a problem. Even the git flow page shows the release branch being merged both into develop and master. Not the release branch being merged into master and then master being merged into develop.

http://nvie.com/posts/a-successful-git-branching-model/

It also specifically says:

May branch off from: develop
Must merge back into: develop and master

And

$ git checkout master
Switched to branch 'master'
$ git merge --no-ff release-1.2
Merge made by recursive.
(Summary of changes)
$ git tag -a 1.2

$ git checkout develop
Switched to branch 'develop'
$ git merge --no-ff release-1.2
Merge made by recursive.
(Summary of changes)

This is clearly the branch being merged into develop and not the tag. Which is the only thing that makes sense.

If @nvie has decided to change the way it works, that's his prerogative as it's his project. But everything in the docs say the branch has priority over the tag.

And all I'm saying is that if the release and the versiontag are the same, that doesn't happen. So if the desired result is to have the branch take priority when merging a release branch (as I feel it should), then there is a bug.

If there is a bug, then the solution to the bug may be "We've decided not to fix that.", "We've changed the specifications for the application and this is no longer a bug" or "Let's get it fixed". "Don't do that", in my opinion, is not a proper solution.

And as I said, I don't want to use release- because I want the auto-organizing folders in my Git GUI. I get that with release/. I don't get that with release-.

@jfelchner
Copy link
Author

One more point I forgot to address:

The reason that merging the tag over merging the branch is "Very undesirable behavior." is because this potentially allows for commits to be made directly on master which are brought over to develop in a haphazard way.

By "condoning" commits to be made directly on master, you're allowing for surprises when merging a release branch into master.

What surprises? I'm glad you asked:

Let's assume we follow these steps:

  1. Make a commit on master which breaks a test
  2. Start a release branch
  3. Run tests on the release branch (assume all pass)
  4. Finish a release branch

Scenario 1 (The way git-flow is "supposed" to work):

Tests on develop pass. Tests on master fail. This is "correct" because if you commit directly to master, you're not following git-flow and you deserve to have your branches out of sync.

Scenario 2 (What happens with this bug):

Tests on develop fail. Tests on master fail. This is "incorrect" because (assuming no other commits were made to develop) a passing test suite on the release branch should result in a passing test suite on develop after the release branch is finished.

@petervanderdoes
Copy link

Let me start with stating I'm not in a position to give decisive answer what to do with a bug in this project here :)

The blog post was posted two years ago and not updated since. Currently git flow supports support branches which weren't mentioned in the original document.
This is from nvie's FAQ:

Why does git-describe not work for me?
When finishing release and hotfix branches, that branch's HEAD is first merged into master and then into develop. It is not the
resulting new merge commit from master that is merged back into develop. This means that a linear path from the new develop
branch to the new master commit is not created. Even worse, a linear path is created from the new develop branch to the previous > master commit. Although unintended, this is simply an effect of using the current branching rules.

When using git-describe in these cases, you can get very confusing and misleading results, since git-describe scans the current
commits linear history for the most recent tag it finds, which will always be the previous tag.

I will change this behaviour in the next version of the branching model explicitly and I will include this behavioural change in the first > version of the Python rewrite.

Now fact is that this has been open and not addressed for almost two years.

Personally I don't agree with your statement :

This is clearly the branch being merged into develop and not the tag. Which is the only thing that makes sense.

The reason I don't think it makes sense is the fact that if you have a release branch and you deploy a hotfix, those changes are merged in the master and develop branch. When you deploy your release branch the hotfix changes in the develop branch will disappear and the master and develop branch will always be diverged.
And this is just using the git flow commands, god forbid somebody actually develops on master shiver

As this issue has been open for almost 2 years I guess the conclusion we can draw that the solution for this bug is "We've decided not to fix that."

P.S. I had written this comment and only then saw your 2nd follow up post.
Isn't your starting develop branch suppose to be a exact replica of your production branch?

Test on develop passes
You create a release branch
Bug detected in production, a 3rd developer deploys a hotfix, hotfix passes, merged in master and develop.
You deploy the release branch, merged into master and develop, The master passes but the new development will have the same bug that was fixed by the hotfix.

@jfelchner
Copy link
Author

@petervanderdoes I think we're going to just have to agree to disagree on this one. If what you say is right, it looks like nvie is also thinking along your lines. The fact that I don't agree doesn't really matter. ;)

I'm going to leave this bug open though because I feel that the code should reflect that fact and I don't think it's correct to show the users a warning message in the event that the user chooses to have the release and versiontag config as the same thing.

I did want to have a further discussion about your last examples.

The reason I don't think it makes sense is the fact that if you have a release branch and you deploy a hotfix, those changes are merged in the master and develop branch. When you deploy your release branch the hotfix changes in the develop branch will disappear and the master and develop branch will always be diverged.
And this is just using the git flow commands, god forbid somebody actually develops on master shiver

I believe here it may be a difference of our workflows. For us, we never deploy code directly from a user's local machine. All deployed code is pulled from a central repo (most of the time Github), and because of this, we get the extra security that, if another user has pushed changes (say a hotfix) to master, as soon as we try to push the finished release branch to master, we'll get an error.

...which I believe is also how you arrived at:

The master passes but the new development will have the same bug that was fixed by the hotfix.

In my situation, again, using a central repo as the canonical source of truth, your last scenario would look like this:

-*-*-*-*-*-*-----*--3---   Develop
      \         /  /
       *-*-1---/--/        Release branch
-*-*-2--------/    \       Hotfix branch
      \             \
-------*-------------4-    Master

At this point, they should both be completely synced. Although it is possible that the release branch merge caused a regression, merging master into develop would not alleviate that potential problem.

-*-*-*-*-*-*-----*-----3-  Develop
      \         /      |
       *-*-1---/--\    |   Release branch
-*-*-2--------/    \   |   Hotfix branch
      \             \ /
-------*-------------4-    Master

In the above examples, if tests are all passing at commits 1 and 2, in neither example does it necessarily mean that tests are passing at points 3 or 4, (and I can't think of an easy way around that issue) but the code to fix the bug addressed in the hotfix branch would be on both branches.)

Thoughts?

@petervanderdoes
Copy link

I love drawing diagrams as it tells me I was mistaken in the loss of a commit.

I guess it doesn't really matter, branch or tag merging, except for when you merge the branch, you have the git describe problem on the develop branch. The develop branch has no knowledge of release tags.

BTW, a solution about the hotfix branch existing with a release branch and passing test "4" would be to merge the hotfix in the release branch and not in the develop branch. There is an issue for that one issue #177
Test 4 would exist on the Release branch

-*-*-*-*-*-*--------3---   Develop
      \            /
       *-*-1---*-4/        Release branch
-*-*-2--------/    \       Hotfix branch
      \             \
-------*-------------*-    Master

@jfelchner
Copy link
Author

Agreed on merging into the release branch instead of develop!

I honestly never use git-describe so I currently don't care that develop doesn't know anything about the release tags, that being said, after I look into it, I may change my mind. ;)

Good discussion. Thanks @petervanderdoes.

@jfelchner
Copy link
Author

@petervanderdoes Just got back to this and I did, indeed, change my mind. Release branches and hotfixes should be merged into master and then master merged into develop. I still don't think it should display the error, but I'm going to close this issue anyway.

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

No branches or pull requests

3 participants