The Git/GitHub Questions, Answers and Howto Thread

Programmers discuss here anything related to FreeOrion programming. Primarily for the developers to discuss.

Moderator: Committer

Message
Author
User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: The Git/GitHub Questions, Answers and Howto Thread

#151 Post by adrian_broher »

Vezzra wrote:Do you think that will work/is a sound policy?
No, because the same reasons mentioned again and again still apply regardless what's the intent of the PR and I'm too lazy to reiterate them again.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#152 Post by Cjkjvfnby »

Suggesting for one commit branches.

1) at pull request creation author adds label :SquashAndMerge:
2) Author rebase master if necessary, but does not squash/edit commits manually. I want to check what was done after review, not whole diff as single commit. It really saves my time.
3) User who close this PR will to choose SquashAndRebase and edit commit message
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Vezzra
Release Manager, Design
Posts: 6090
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: The Git/GitHub Questions, Answers and Howto Thread

#153 Post by Vezzra »

adrian_broher wrote:No, because the same reasons mentioned again and again still apply regardless what's the intent of the PR and I'm too lazy to reiterate them again.
Well, I didn't ask for a reiteration of all the reasons why merging master into PR branches is a bad idea. Actually all I wanted to know was if merging master into a feature branch right before merging that feature branch back into master could be a way to address these issues. Judging by your answer, apparently not.

However, Cj's wish to keep the Fighters feature branch up to date with master isn't unreasonable, so how can that be achieved? Frequently rebasing the feature branch obviously also has some issues (we're talking about a feature branch several people will collaborate on, not a PR branch basically only one person works with), what other alternatives are there? Just stating that a certain approach is a bad idea isn't enough, we need to offer a usable alternative.

So my questions are: Is it really impossible to do merges of master into a feature branch in a way that won't result in a big mess once the feature branch gets merged back into master? Or rather, @Cj, is it really that impossible to come up with a workflow that can work sufficiently well with frequent rebasing of the Fighters feature branch (instead of merging master)? I suppose the people working together on that branch would just have to coordinate carefully: each rebase needs to be announced, so all contributors can merge what is already ready, and bring their local checkouts into a state where they also can be easily rebased afterwards... shouldn't be too hard, no?

Please, guys, we need to come to some consensus here (and last but not least, someone needs to make a final decision - I'm not sure if I'm the right one for that, however... Geoff? Sorry if I get a bit annoying sometimes, but you're the programming lead after all...).

User avatar
Vezzra
Release Manager, Design
Posts: 6090
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: The Git/GitHub Questions, Answers and Howto Thread

#154 Post by Vezzra »

Cjkjvfnby wrote:Suggesting for one commit branches...
Sounds reasonable. Anyone any objections against/comments on adopting that policy?

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13586
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: The Git/GitHub Questions, Answers and Howto Thread

#155 Post by Geoff the Medio »

Vezzra wrote:Please, guys, we need to come to some consensus here (and last but not least, someone needs to make a final decision - I'm not sure if I'm the right one for that, however... Geoff? Sorry if I get a bit annoying sometimes, but you're the programming lead after all...).
I don't understand the problem with periodically rebasing a branch on master... The merging conflicts will need to be dealt with eventually either way. Keeping them as up to date as possible is reasonable. If you're worried about rebasing causing problems with others local changes, perhaps schedule a regular time for the rebasing against master?

I don't really know what best practice for this sort of issue is, though; I'd likely defer to adrian broher or someone else with more experience and competence with git / github.

User avatar
MatGB
Creative Contributor
Posts: 3310
Joined: Fri Jun 28, 2013 11:45 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#156 Post by MatGB »

It was my understanding Adrian had already said to rebase the feature branch every so often, but to do so with warnings to other contributors in case of potential issues.

I've been busier than normal this week and not been looking at things but was intending to rebase it this weekend assuming nothing went wrong. I dislike merges generally, and much prefer rebased branches upon merge, ideally with as many superflous commits squashed as possible.
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13586
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: The Git/GitHub Questions, Answers and Howto Thread

#157 Post by Geoff the Medio »

What happens if you merge master into a branch a few times, then at the end before merging back, do a rebase against master?

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#158 Post by Cjkjvfnby »

Geoff the Medio wrote:What happens if you merge master into a branch a few times, then at the end before merging back, do a rebase against master?
Extra work with place for human error on conflict resolve.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: The Git/GitHub Questions, Answers and Howto Thread

#159 Post by adrian_broher »

Sorry for the late response, fever and headache struck me down again the last days.
Vezzra wrote:However, Cj's wish to keep the Fighters feature branch up to date with master isn't unreasonable, so how can that be achieved? Frequently rebasing the feature branch obviously also has some issues (we're talking about a feature branch several people will collaborate on, not a PR branch basically only one person works with), what other alternatives are there? Just stating that a certain approach is a bad idea isn't enough, we need to offer a usable alternative.
I don't understand why do you want to keep the feature branch up-to-date by any means. I can only come up with two reasons beside the obvious I want the feature branch up to date because I like things up to date:
  • master or <other branch> provides a prerequisite the feature branch needs, may it be a bug fix, a new interface, …
  • An external contributor added an PR to master, which provides features only intended for feature branch and this PR was already merged.
The first case should be solved with `git cherry-pick` by picking the commits that are needed on the feature branch from the source branch. When rebasing before the final merge git will sort those already existing commits out, leaving only feature related commits.

The second case is just a special variant of the first case, except we screwed up multiple times in terms of workflow. We didn't say the contributor that he should base his PR against the feature branch and we merged the wrongly based PR anyway. To fix this you can also use cherry-picking.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

User avatar
Vezzra
Release Manager, Design
Posts: 6090
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: The Git/GitHub Questions, Answers and Howto Thread

#160 Post by Vezzra »

Geoff the Medio wrote:I don't understand the problem with periodically rebasing a branch on master
It increases administrative overhead, so to speak. You have coordinate more carefully. If I rebase a branch which others have checked out locally, their local branches become more or less invalid, because the commits in those branches no longer are diffs against the same base. If your local branch contains commits you haven't pushed before the remote branch got rebased, you can't just rebase those "extra" commits on the remote branch, because in case you try that, your whole local branch (extra commits and those which are now "duplicates" of the rebased remote branch) will get rebased on the remote branch, which obviously will end in a big mess.

AFAIK the best way to clean that up in case you find yourself in this situation (having commits you haven't pushed to a branch that got rebased), is to check out a new local copy of the remote branch and cherry pick your commits from the "old" local branch to the new one (Marcel, did I get that right?). No problem if it's only a few commits, but if you've been working on something bigger, not so funny.

To avoid the above scenario, you need to announce a planned rebase and give the involved contributors enough time to wrap up and push whatever they have sitting in their local branches. Once everyone has pushed everything, you can rebase the branch, and everyone can proceed by just deleting their old local copies (as there are no more local changes that would get lost) and make a fresh check out of the rebased branch.

However, during that time you shouldn't of course start on something new locally, but need to wait until the rebase has been done, and then you can resume with whatever you've been working on the feature branch. That's no insurmountable problem, it's just a bit more work in the coordination department, and requires the participants to be sufficiently self-disciplined...
I don't really know what best practice for this sort of issue is, though; I'd likely defer to adrian broher or someone else with more experience and competence with git / github.
That's perfectly fine for me. Having Marcel, who is the most experienced with git/github, in charge of these policies sounds very reasonable. I just wanted to have things clearly stated. So, if no one objects, I'm all for putting him in charge of this.

That doesn't mean I won't argue with him on certain points, like I'm just doing here. ;) But the final call shall be his. :)

User avatar
Vezzra
Release Manager, Design
Posts: 6090
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

Re: The Git/GitHub Questions, Answers and Howto Thread

#161 Post by Vezzra »

adrian_broher wrote:Sorry for the late response, fever and headache struck me down again the last days.
No problem. I'm sitting on a Mt. Everest sized mountain of topics I didn't replied to immediately but still want/wanted to once I find the time to get back to them. The posts/topics at the bottom of that pile can safely be classified as prehistoric by now, so who am I to complain... ;)

Hope you're reasonably well now.
I don't understand why do you want to keep the feature branch up-to-date by any means.
For a feature branch that won't be around very long and will be merged fairly quickly, no problem. But in my experience, once you've got a long running feature branch, not keeping it up to date and trying to resolve all conflicts when doing the final merge back into master is a recipe for headache. The longer a branch is out of sync with master, the more conflicts accumulate over time, and resolving all of them in one big cleanup is a tedious, mind-numbing and error prone task.

If, on the other hand, you've frequently incorporated the changes in master into your branch, conflict resolving is easier and you don't have to clean up a (potential) mountain at the time of the final merge.
master or <other branch> provides a prerequisite the feature branch needs, may it be a bug fix, a new interface
That could work, if we are only dealing with the occasional prerequisite/bug we need to integrate, but that's not what I expect with a feature branch like the one in question here.

First of all, you'll probably want to integrate all bugs that get fixed in master, and as fixed bugs aren't exactly a rare occasion, that's already a fair amount of cherry-picking you'd have to do.

Second, we probably not only want prerequisites in the strict sense (things which the feature branch must integrate to work at all), but all things/features added/changed in master that will in one way or another interact with the feature that gets worked on in the feature branch. Example: if the new supply propagation mechanics had been pushed to master after the Fighters branch got created, we most likely would have wanted to intgrate that into the Fighters branch. It wouldn't have been absolutely necessary, as you could have worked on the fighter and carrier mechanics with the old supply propagation mechanics as well, but that wouldn't be very reasonable IMO. Those mechanics interact, and of course it would be far better to adjust the fighter and carrier to the revised supply propagation mechanics.

However, cherry picking an entire feature branch that got merged into master sounds awfully tedious. Besides, there are probably a lot of other changes you'd want to integrate into the Fighters branch, because they might impact how you implement/balance/adjust things with fighters and carriers.

All of which will lead to an awful lot of cherry picking in the end - at least, that's what I expect will happen. And for that, frequent rebasing (if we don't want to merge master) seems to be the better approach than cherry picking in my eyes.
The first case should be solved with `git cherry-pick` by picking the commits that are needed on the feature branch from the source branch. When rebasing before the final merge git will sort those already existing commits out, leaving only feature related commits.
Now why can't git resolve merges of master into the feature branch like that? According to what you say here, in case I decide to go nuts and cherry pick every single commit in master into my feature branch (those which aren't in feature of course), git would sort those out when finally merging feature into master. What's the difference to merging master into feature? Isn't that basically applying all commits in master which aren't in feature to feature? So that only the commits in feature which aren't in master comprise the difference between feature and master?

Obviously that's not how it works, though, and I assume there is good reason for it, just whining a little bit... ;)

User avatar
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: The Git/GitHub Questions, Answers and Howto Thread

#162 Post by adrian_broher »

Vezzra wrote:AFAIK the best way to clean that up in case you find yourself in this situation (having commits you haven't pushed to a branch that got rebased), is to check out a new local copy of the remote branch and cherry pick your commits from the "old" local branch to the new one (Marcel, did I get that right?).
Yes, that's one way (I would suggest as project default). But as usual in Git there multiple ways + 1 other: One would be `git rebase --onto <local copy>` `treeish containing your commits`. It always depends on what you want to achieve exactly.
Now why can't git resolve merges of master into the feature branch like that? According to what you say here, in case I decide to go nuts and cherry pick every single commit in master into my feature branch (those which aren't in feature of course), git would sort those out when finally merging feature into master. What's the difference to merging master into feature?
That's a good question actually because I never used the suggested `git checkout feature ; git merge master ; git rebase` workflow.
Resident code gremlin
Attached patches are released under GPL 2.0 or later.
Git author: Marcel Metz

User avatar
MatGB
Creative Contributor
Posts: 3310
Joined: Fri Jun 28, 2013 11:45 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#163 Post by MatGB »

I've always simply rebased my personal feature branches against Master (and had master set as the upstream generally) when I've needed to, but I'm rarely collaborating with others as much as we'll need to with Fighters, so I don't know how many extra headaches that causes.

The reason this specific branch needed rebasing at one point was because Geoff had added in some extra stuff that was Fighter specific to master after the branch was created by Morlic on his fork but before we decided to have a specific branch. That's meant that, for example, some of the parts with the new exclusion/exception restrictions weren't so restricted when I was testing various ship ideas and it was going to cause a clash. There was another, different, case (oh, yes, an AI bugfix that applied to both branches and was needed in both).

When I've rebased branches with cherry picked content I've had occasional problems and had read (somewhere, not here) that it wasn't a good workflow but I can't remember where or why), I'm very happy to cherry pick stuff in, and also to rebase on occasions if/when needed, but if all fighter related stuff goes into the fighters branch (*cough* Geoff *cough*) until we're all convinced it's ready then we're good.
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.

User avatar
Cjkjvfnby
AI Contributor
Posts: 539
Joined: Tue Jun 24, 2014 9:55 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#164 Post by Cjkjvfnby »

I have feeling that I made error in rebasing of Fighters branch.

1) Where I can find what was commited before I rewrite history?
2) Can someone who did not pull yet make reserve copy of Fighters branch?
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
MatGB
Creative Contributor
Posts: 3310
Joined: Fri Jun 28, 2013 11:45 pm

Re: The Git/GitHub Questions, Answers and Howto Thread

#165 Post by MatGB »

Have done so, the most recent commits I have are the ones I made before your three from today, I need to log off now but let me know if you want me to do more/push it, etc and I'll get it done at some point tomrrow.
Mat Bowles

Any code or patches in anything posted here is released under the CC and GPL licences in use for the FO project.

Post Reply