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
Vezzra
Release Manager, Design
Posts: 6095
Joined: Wed Nov 16, 2011 12:56 pm
Location: Sol III

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

#31 Post by Vezzra »

Geoff the Medio wrote:I may just avoid using branches...
That would defeat much of the advantages git has to offer. Just be very careful with rebasing. Merging should be no problem, you can't mess up with merging. Use rebasing only with local branches you did not push to the repo, and you should be fine.

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

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

#32 Post by Geoff the Medio »

I'm actually using a combination of TortoiseGit and Github for Windows. The latter doesn't seem to have any way to do anything complicated... it just has a single Sync button (and ways to switch between branches which works some of the time, and ways to create commits by selecting which modified files to include, and ways to easily add to the gitignore list).

Just now, I deleted my local branch copies, and switched to the logging-migration branch again. Then using TortoiseGit, I pulled from origin/master. this mostly worked except for a few conflicts in HumanClientApp.cpp. I don't understand why it can't merge these as would happen with SVN, but fine, I resolved them (I think) manually.

Then back in Github for Windows, it shows a bunch of modified files, and the default commit name for committing these changes is "Merge remote-tracking branch 'origin/logging-migration' into logging-migration", which doesn't make any sense to me.

Using the TortoiseSVN "commit ->logging-migration" command though gives a reasonable default commit description of

Code: Select all

Merge branch 'master' of https://github.com/freeorion/freeorion into logging-migration

Conflicts:
	client/human/HumanClientApp.cpp
After doing that, going back into Github for Windows, it has a reasonable list of commits, including the merge commit. I can save those as a serial set of patches, which I've zipped and attached. Not sure if I should attempt to push or sync this merge, though...
Attachments
freeorion-logging-merge-commits.zip
commits involved in attempted merging
(11.4 KiB) Downloaded 130 times

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

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

#33 Post by Vezzra »

Edit: I misread your explanation of what you did, see posts below.
Geoff the Medio wrote:Just now, I deleted my local branch copies, and switched to the logging-migration branch again. Then using TortoiseGit, I pulled from origin/master. this mostly worked except for a few conflicts in HumanClientApp.cpp. I don't understand why it can't merge these as would happen with SVN, but fine, I resolved them (I think) manually.
I do not understand what's going on here. First, you should not be able to switch to logging-migration after you deleted the corresponding local branch, because the branch does not exist in your local repo anymore. So I wonder what you really did when you switched to the no longer existing logging-migration.

Second, there shouldn't be any conflicts when you pull in a remote branch for which no local tracking branch exists - what should it conflict with?

Which leads me to the suspicion that when you thought you had switched to logging-migration, actually you didn't (or didn't realize the switch failed), and maybe still have been on master. You then tried to pull the remote logging-migration into your local master, which gave you the conflict. I don't know how TortoiseGit works, but it should offer you an option where you can pull a remote branch into a new local branch that gets created in the process, is set as a local tracking branch for the remote branch, and immediately checked out, so your working copy is now swicthed to your local logging-migration branch.

That should go smoothly without any conflicts. Can you delete your local branches once more and try again pulling in the remote logging-migration? As long as conflicts happen, something isn't right.
Then back in Github for Windows, it shows a bunch of modified files, and the default commit name for committing these changes is "Merge remote-tracking branch 'origin/logging-migration' into logging-migration", which doesn't make any sense to me.
To me neither, I'm hitting the limit of my git proficiency here.
Using the TortoiseSVN "commit ->logging-migration" command though gives a reasonable default commit description of

Code: Select all

Merge branch 'master' of https://github.com/freeorion/freeorion into logging-migration

Conflicts:
	client/human/HumanClientApp.cpp
After doing that, going back into Github for Windows, it has a reasonable list of commits, including the merge commit. I can save those as a serial set of patches, which I've zipped and attached. Not sure if I should attempt to push or sync this merge, though...
That doesn't sound right - what series of commits? There should be only the merge commit...

I'll take a look at the patches.

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

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

#34 Post by Vezzra »

Vezzra wrote:That doesn't sound right - what series of commits? There should be only the merge commit...

I'll take a look at the patches.
Ok, I take that back, that actually might be correct. The patches seem to be the commits in master that need to be merged into logging-migration. So that looks ok.

I'm still puzzled about you being able to switch to logging-migration after you deleted your local branches, and getting a conflict when pulling in a remote branch for which you don't have a local tracking branch. Weird.

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

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

#35 Post by Vezzra »

Geoff the Medio wrote:I deleted my local branch copies, and switched to the logging-migration branch again. Then using TortoiseGit, I pulled from origin/master.
Gah, sorry, obviously I didn't read carefully - I thought you switched to logging-migration and then tried to pull in remote logging-migration.

So you might have done everything right after all, depending on what really happened. When "switched to the logging-migration branch again" means you pulled the remote logging-migration branch into a new local one, then everything should be right so far. After that you tried to merge master using TortoiseGit, and that gave you the conflict, right? Now, I can't tell you why you'd get a conflict here, the only reason for that would be conflicting changes in both branches.

Other than that, everything should be right. Why GitHub for Windows gives you this strange default commit message, I don't know - maybe Marcel has an explanation.

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

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

#36 Post by Vezzra »

I just checked, I get the conflict too when I try to merge master into logging-migration, the offending piece of code being:

Code: Select all

}

void HumanClientApp::HandleWindowMove(GG::X w, GG::Y h) {
    //DebugLogger() << "HumanClientApp::HandleWindowMove(" << Value(w) << ", " << Value(h) << ")";

    if (!Fullscreen()) {
<<<<<<< HEAD
        GetOptionsDB().Set<int> ("app-left-windowed", Value (w));
        GetOptionsDB().Set<int> ("app-top-windowed", Value (h));
=======
        GetOptionsDB().Set<int>("app-left-windowed", Value(w));
        GetOptionsDB().Set<int>("app-top-windowed", Value(h));
>>>>>>> master
        GetOptionsDB().Commit();
    }
}

void HumanClientApp::HandleWindowResize(GG::X w, GG::Y h) {
    //DebugLogger() << "HumanClientApp::HandleWindowResize(" << Value(w) << ", " << Value(h) << ")";
Obviously the lines:

Code: Select all

        GetOptionsDB().Set<int> ("app-left-windowed", Value (w));
        GetOptionsDB().Set<int> ("app-top-windowed", Value (h));
got edited in both branches, resulting of course in a conflict that can't be solved automatically.

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

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

#37 Post by Vezzra »

Further investigation revealed that this is what really happened:

In master these lines:

Code: Select all

    if(!Fullscreen()) {
        GetOptionsDB().Set<int> ("app-left-windowed", Value (w));
        GetOptionsDB().Set<int> ("app-top-windowed", Value (h));
have been changed into:

Code: Select all

    if (!Fullscreen()) {
        GetOptionsDB().Set<int>("app-left-windowed", Value(w));
        GetOptionsDB().Set<int>("app-top-windowed", Value(h));
In logging-migration, only this line:

Code: Select all

    if(!Fullscreen()) {
has been replaced with:

Code: Select all

    if (!Fullscreen()) {
Which resulted in the "if..." line being identical in both branches, hence no conflict, but the "GetOptions..." lines being different, and being part of the whole block, git decided to ask.

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

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

#38 Post by Geoff the Medio »

Geoff the Medio wrote:Just now, I deleted my local branch copies, and switched to the logging-migration branch again.
To clarify, I think switching to a branch in GitHub for Windows for the first time downloads it then. In the TortoiseGit UI, only branches I'd switched to at some point were listed there to be deleted. After deleting the branch, I switched to it again in GitHub for Windows, which re-downloaded it from GitHub.

My confusion with the merging may be just that the TortoiseGit UI for it isn't as nice as what TortoiseSVN does. It's not as clear what version of files it's actually using on a given line, or how to specify this, at least to me. But I also would have expected it to do the merge automatically in the case of the Fullscreen block, though I can somewhat see why it might ask.

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

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

#39 Post by Vezzra »

Geoff the Medio wrote:To clarify, I think switching to a branch in GitHub for Windows for the first time downloads it then. In the TortoiseGit UI, only branches I'd switched to at some point were listed there to be deleted. After deleting the branch, I switched to it again in GitHub for Windows, which re-downloaded it from GitHub.
Ah yes, that makes sense.
My confusion with the merging may be just that the TortoiseGit UI for it isn't as nice as what TortoiseSVN does. It's not as clear what version of files it's actually using on a given line, or how to specify this, at least to me.
Well, you can try other GUI clients for git. I'm quite content with SourceTree, but your personal preference might vary of course.
But I also would have expected it to do the merge automatically in the case of the Fullscreen block, though I can somewhat see why it might ask.
Maybe git is a bit more careful/conservative when it comes to automatically trying to solve merge conflicts...

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

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

#40 Post by Vezzra »

For all of you guys who are looking for a git GUI client:

* http://git-scm.com/downloads/guis

* https://git.wiki.kernel.org/index.php/I ... Interfaces

Should have something for everyone.

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

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

#41 Post by Dilvish »

The somewhat confusing situation Geoff related above seems to me to emphasize that even for us with commit writes to the main repository, it would be best to do most of our work via branches on our own forks, so on our local machines our individual github fork would be (normally) the 'origin' remote, and the main freeorion repository would be the 'upstream' remote. It seems to me that helps keep things organized and clean. Those names are just conventions, but it seems that's the setup that most of the github documentation expects to deal with (at least from what I've seen so far). So just being consistent with what the github help docs expect has a lot of value to all us novices, plus there is presumably the underlying benefits that led to that setup being the one the github help docs focus on.

It seems that one of the big benefits of going our individual work in feature branches that are either just on our local machines or (more commonly) also on our personal forks at GitHub, is that rebasing can be used to clean up the commit history before it gets pulled/merged/pushed to freeorion/freeorion, such as is described in this article. For example, Marcel had agreed it would be good for Morlic to squash the 2 commits of a recent PR down into one, and that seemed to work fine. This article is just one of the many places I have seen this practice recommended. This article notes that this is only appropriate if the changes have not previously been pushed to a public repo that folks might be forking off of. Technically our individual forks on github are public, but I think it's common and reasonable in this kind of situation that unless we've invited folks to use or collaborate on one of our individual branches on our individual forks, that they are essentially private and fair/good to rebase with to keep the commit history cleaner. It does seem like an issue that can get complicated, perhaps someone with more git experience can help guide us to a more clear consensus? When I try to read up on this I do see quite a few different approaches, it seems to me.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

jcantero
Krill Swarm
Posts: 12
Joined: Sun Nov 24, 2013 5:08 pm

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

#42 Post by jcantero »

Dilvish wrote: This article notes that this is only appropriate if the changes have not previously been pushed to a public repo that folks might be forking off of.
Let me explain why is this a problem, and when it's not.

As you already know, every commit (and really every object in the git repo) is identified by a hash string like "20c41600f30267f30d61d6db54f7dd74abad5350". This id is unique and plays a fundamental role in the git model. Every commit keeps in its metadata the id (hash) of its parent commit, or several ids if its a merge commit with several parents. From a tip commit (a commit in the head of a branch) you can access to all its ancestors simply following the hash of its parent, and the parent of its parent, and so on. It's efectively a chain (really a tree) of hashes pointing to other hashes.

git rebase and git rebase -i are operations that can alter some of these hashes because the redo some commits of this chain (git commit --amend also changes a commit, specifically the last commit (tip) of a branch). That is what it means to "rewrite history". The problem is, if anyone has a commit (lets call it Y) which is pointing to (its parent or one of its parents is) a commit (lets call it X) in another public or private branch or repo and that parent commit X is changed (i. e. its hash is now different) then the chain of hashes is now broken (Y is pointing to original X hash, but now X is identified by a different hash), leaving your Y commit in a inconsistent state (and this also happens when X is not the direct parent of Y but a older ancestor). And that's not funny for the owner of the repo with the Y commit, that now it's (partially) broken.

Therefore, when we maintain a repo that someone can potentially clone, we don't want to annoy them by breaking their repos and work changing "the floor under they feet". When our repo is private, we are positively sure that we are not going to break anybody's work by changing commits. But there is also another situations where we can do these type of changes and hope that nobody is disturbed.
Dilvish wrote:Technically our individual forks on github are public, but I think it's common and reasonable in this kind of situation that unless we've invited folks to use or collaborate on one of our individual branches on our individual forks, that they are essentially private and fair/good to rebase with to keep the commit history cleaner.
That's one of these "socially accepted" situations where you can suposse that nobody is basing his/her work on yours, especially if you specifically point out that, or use a convention (like suffix this type of branches with -WIP), or everybody that's using it is warned or in agreement.

Also, you can do a trick to guarantee that nobody is going to be in a inconsistent state: make the rebase in a different, non-published branch and then publish the new branch to do the pull request. The steps are simple:

Code: Select all

git checkout -b my-rebased-branch
git rebase -i
git push -u origin my-rebased-branch
(create the pull request for my-rebased-branch) 
(I've done an example here: https://github.com/javiercantero/test-r ... nteractive)

Note that you can't rename the new branch to the old one, or you acomplish nothing. Because nobody had a commit pointing to the new branch you can guarantee everything is going to be fine. And by keep the old branch, commits based on that are not messed up.
Dilvish wrote:It does seem like an issue that can get complicated, perhaps someone with more git experience can help guide us to a more clear consensus? When I try to read up on this I do see quite a few different approaches, it seems to me.
I'm not a git expert, but I hope this can help you.

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

Re: GitHub migration procedure

#43 Post by MatGB »

Edit by Vezzra: Moved this and follow-up posts from here.

Clarification on procedure please. CJ has asked me to merge in Pull Request #18 · freeorion/freeorion and Pull Request #20 · freeorion/freeorion.

Both appear fine to me (and indeed Dilvish has done 18 as I type this), but both have received comments and tags from others (Vezzra and adrianbroher) without being merged.

Why tag them and not merge them?

Conversely, when I merge stuff in, how should I tag it or should that be left to someone else regardless?
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
adrian_broher
Programmer
Posts: 1156
Joined: Fri Mar 01, 2013 9:52 am
Location: Germany

Re: GitHub migration procedure

#44 Post by adrian_broher »

MatGB wrote:Why tag them …
Because I can see that the pull request provides an enhancement for the project (here: updated translation) and the PR will be part of next release if it is merged.
MatGB wrote:… and not merge them?
I didn't have time to review it.
MatGB wrote:Conversely, when I merge stuff in, how should I tag it or should that be left to someone else regardless?
Please tag the PR if you think this adds useful information to the issue or pull request. This should help in the future when writing change logs or finding old issues/pull requests.
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: GitHub migration procedure

#45 Post by Cjkjvfnby »

adrian_broher wrote:
MatGB wrote:Why tag them …
Because I can see that the pull request provides an enhancement for the project (here: updated translation) and the PR will be part of next release if it is merged.
MatGB wrote:… and not merge them?
I didn't have time to review it.
MatGB wrote:Conversely, when I merge stuff in, how should I tag it or should that be left to someone else regardless?
Please tag the PR if you think this adds useful information to the issue or pull request. This should help in the future when writing change logs or finding old issues/pull requests.
I think it will be good practice to leave some comments if you check changes but has no objections.

Code: Select all

looks fine | compiles | plays fine | tested deep.  
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

Post Reply