Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 11, 2013 at 6:38 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? Because as you said, they don't know what that is. That does not answer my question: why ask? If you have to ask, then you haven't read the commit messages, the cover letter, or the relevant discussion. Even Linus Torvalds agreed this was a good change. Look around you what people say about Git. See how many complain about Git not exposing enough complexity to the user. See how many would complain about Git not advertising rebase enough. Then, look how many complain about Git exposing too much complexity and making it too easy to use features like rebase. And see how many are confused by Git doing something they never told it to do, and then being totally lost because they are in the middle of a state they don't understand, and how many do merges by mistake. There's a reason why Git user-base considers 'git pull' dangerous. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: The problem is the newcomers, and the newcomers will most definitely not activate a configuration option to tell them that they are doing something potentially undesirable. I teach Git to 200 newcommers each year. All of them run git pull the first day, but believe me, very few of them want to know what a rebase is at that time. And who says they have to? This is a straw man argument. May of them don't want to know what the staging area is, that's why they run 'git commit --all', and just like that they can run 'git pull --merge'. By the time they learn about pull.mode, they probably already know what a rebase is. So what is the point of the configuration in the first place? [...] That doesn't mean anything, you are assuming the user will do 'git pull --rebase', and there's no rationale as to why they would end up doing that. So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? Because as you said, they don't know what that is. 'git commit' by default prevents users from creating commits without first adding changes to the staging area, and since it's a concept unique to Git, it's fair to say that none of the newcomers understand why 'git commit' is failing, the error messages is not particularly useful either. I don't particularly agree that not defaulting to --all was a good idea, but that's another topic. It the same topic, the project already made a choice, and precisely because of the same reasoning that 'git commit --all' is required, 'git pull --merge' should be required. But the error message is rather clear: no changes added to commit (use git add and/or git commit -a) And we can do the same: Read more with 'git pull --help' or do 'git pull --merge'. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 10, 2013 at 3:26 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? Because as you said, they don't know what that is. That does not answer my question: why ask? Look around you what people say about Git. See how many complain about Git not exposing enough complexity to the user. See how many would complain about Git not advertising rebase enough. Then, look how many complain about Git exposing too much complexity and making it too easy to use features like rebase. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 06:02:35PM -0500, Felipe Contreras wrote: On Mon, Sep 9, 2013 at 3:24 PM, John Keeping j...@keeping.me.uk wrote: On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. And you propose to show that every single time the user does a 'git pull'' that results in a non-fast-forward merge? Isn't that what 'git pull --help' is for? Only if the user has not given an explicit mode (either on the command line or in their config) and possibly if an advice.pullNonFF variable is not set to false. I think that matches what Git does elsewhere. git-pull(1) provides quite a lot more information that I think a new Git user would be comfortable with. There certainly is not a quick way to find out how to fix this error and I don't think it makes sense to add one because we'll still be presenting the user with all of the other content and they won't have any way to know what they can safely ignore and what they have to read and understand. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: The problem is the newcomers, and the newcomers will most definitely not activate a configuration option to tell them that they are doing something potentially undesirable. I teach Git to 200 newcommers each year. All of them run git pull the first day, but believe me, very few of them want to know what a rebase is at that time. (I also work with experienced computer scientists, and actually, very few of them want to know what a rebase is either :-( ) By the time they learn about pull.mode, they probably already know what a rebase is. So what is the point of the configuration in the first place? [...] That doesn't mean anything, you are assuming the user will do 'git pull --rebase', and there's no rationale as to why they would end up doing that. So, you insist in asking the user to chose between rebase and merge, but you also insist that they will not chose rebase? So, why ask? 'git commit' by default prevents users from creating commits without first adding changes to the staging area, and since it's a concept unique to Git, it's fair to say that none of the newcomers understand why 'git commit' is failing, the error messages is not particularly useful either. I don't particularly agree that not defaulting to --all was a good idea, but that's another topic. But the error message is rather clear: no changes added to commit (use git add and/or git commit -a) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Matthieu Moy matthieu@grenoble-inp.fr writes: Junio C Hamano gits...@pobox.com writes: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. Yes. Having an option can't harm anybody, and there's a clear demand for that. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I didn't follow very closely the discussions and patch series, but that would sound right to me. The last version of Felipes' patch series already gives a warning only, but the wording and commit message implies that this will become an error in the future (this is the part with which I disagree). OK, the first step to drop jc/pull-training-wheel from 'next' has been done. I _think_ the one that starts at $gmane/234295 is the newer incarnation of the patches in this thread, but that seems to do a lot more than what the patches in this thread did, and it also badly interacts with another topic in flight that updates git-pull, so I have a topic branch for it but haven't merged to 'pu' yet. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson sand...@crustytoothpaste.net wrote: Yes, that would be me. My hesitance here is that as the one usually driving git updates (which so far have happened once a year), I will end up retraining forty developers. I don't think the current behavior is broken or really problematic at all: merging has always been the default, and people have come to expect that. It may not be broken for you, but it is for other people. Would you be so egocentric as to ignore everybody else because it works for you? It's not a matter of works for me. Git currently works for all use cases because you can already merge or rebase. The proposed changes are not about allowing the behavior that works, but disallowing the behavior that doesn't. I agree that allowing people to reject non-ff merge is a good idea. I strongly disagree that this should eventually become the default, though. I think it should really remain an opt-in (possibly with some non-scary warning advertizing for the feature). First, the discussions on this thread show that it's hard to find the right behavior. My guess is that it's hard because we're trying to think for the users. I've used GNU Arch for a while, and this VCS was trying to impose what the developer thought was good for me. I had to fight with the tool whenever I tried to do something non-standard. I don't want to go back there. Preventing _users_ to do something because _we_ considered it was bad for them is wrong IMHO. I already mentionned another reason in http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 : git rebase is hard to use for many people. With git merge, doing things wrong isn't so bad. If you forget to commit after a conflicted merge, you'll mix your changes with the merge, this is bad, but it works. With git rebase, if you forget to git rebase --continue after a conflict, you end up in detached HEAD, with part of your own changes discarded. If my students end up in this situation, they'll stop using Git and exchange files by email. git pull is one of the first things one learns with Git, and _requiring_ users to chose between merge and rebase is a nonsense at this time of learning. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Matthieu Moy matthieu@grenoble-inp.fr writes: First, the discussions on this thread show that it's hard to find the right behavior. My guess is that it's hard because we're trying to think for the users. I've used GNU Arch for a while, and this VCS was trying to impose what the developer thought was good for me. I had to fight with the tool whenever I tried to do something non-standard. I don't want to go back there. Fond memories of tla comes back to me as well ... ;-) Preventing _users_ to do something because _we_ considered it was bad for them is wrong IMHO. I already mentionned another reason in http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 : git rebase is hard to use for many people. ... git pull is one of the first things one learns with Git, and _requiring_ users to chose between merge and rebase is a nonsense at this time of learning. After I re-read that message, I am starting to think that the topic that has been cooking in 'next' that attempts to catch git pull (no from where, integrate with what parameters) may already be bad by that standard. Brian Carlson's comments on the impact on existing users seems to the same direction to me. You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 11:03:52AM +0100, John Keeping wrote:
I know those are all balanced by some advantages of rebasing, but I also
think they are things that can be troublesome for a user who does not
fully grok the rebase process. I'm just wondering if we should mention
both, but steer people towards merging as the safer alternative (you
might have ugly history, but you are less likely to create a mess with
duplicate commits or badly-resolved conflicts).
The really correct thing to do here is to encourage a feature branch
workflow, but in my experience people are happier to walk through a
rebase than to switch over to feature branches completely.
An alternative pull mode would be:
git reset --keep @{u}
git merge @{-1}
which gets a sensible history shape without any of your disadvantages
above. But that didn't go anywhere last time it came up [1] [2].
FWIW, that approach makes some sense to me. De-coupling for a moment the
idea of what is the default from what are the options, it seems like
doing a reverse-merge would be a good option to have in the toolbox.
It would also have other uses beyond git pull. For example, in
development of GitHub itself, we use topic branches. But before merging
them to master, we often test-deploy the topic to the live site. Before
doing so, you have to merge the topic with the latest master to make
sure you are not un-deploying anybody else's recently graduated topics.
You can do so by creating a temporary merge branch and deploying that,
or you can simply merge master back into the topic. We generally choose
the latter, because it leaves any conflict resolution in an obvious
place (and doesn't need repeating).
-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 03:50:46AM -0400, Jeff King wrote: If you are interested, I can ask the opinion of some of the GitHub trainers. They see a lot of new users and have a sense of what kinds of confusion come up most frequently, what kinds of workflows they tend to see, etc. Their experience may be biased towards corporate-ish users, though, because those are the people who pay for training. Ask. I'm sure they will tell you doing merges by mistake with 'git pull' is an issue. I've sent an email. I'll post the response when I get it. Here is what I sent them (I am leaving both my mail and theirs unedited to avoid any telephone-like confusion in trying to summarize): Right now, running git pull will always create a merge, unless the user has specifically configured it to perform a rebase. Some people find this problematic, because the project may care about the order of merges (e.g., so that --first-parent traversals do the right thing), and some users may accidentally do backwards merges from a main branch into a topic (either because they are clueless, or because they simply forgot). There is a proposal being considered to have git pull do nothing by default, but instead ask the user to specify whether to merge or rebase (with the option of setting a config value if you want it to do one by default). One concern I have is that new users may run across this relatively early. For example, the first time they git push and get a non-fast-forward because somebody else has already pushed, git suggests to run git pull. At which point they will have to decide whether to merge or rebase. So what I'd like your opinions on is: 1. Do new users have trouble with the concept of rebase vs merge? How would they handle this change of behavior? 2. Do new users have trouble with rebases in general? There are some complications over doing a normal merge, but I don't know how often they trip people up in practice. And the responses I got were: 1. New users definitely have trouble distinguishing between rebase and merge. Even people who have been using Git for a while on a basic level are sometimes confused by this. 2. Most people we teach—even the ones who have been using Git for a while—don't know what a rebase is at all. They've heard of it, but they don't get it. It takes careful explanation to get the concept across and explain why it is not the same thing as a merge. Speaking for myself, about half of the time in the Foundations class I'll explain `pull --rebase` and `branch.autosetuprebase`. (Whether we get to it depends on class interest and ability.) When we do address that topic, we always recommend that rebase-on-pull is the right thing to do, since the merges Git creates are just noise that makes history hard to work with in the ways you have pointed out. (For smart classes, I like to make the analogy of Git to a distributed database, and point out how the merge on pull is just Git's mechanism for resolving split-brain writes. I explain that those merges aren't a deficiency in Git; they're just what has to happen by default. The fact that Git handles split-brain writes so well by itself is amazing.) My input would be to continue to have `pull` merge by default. Those merges aren't great, but new users won't have any idea how to make a decision about them at that point. As it is, it just works, and it works quite elegantly. Once you start to learn some things, you can tune Git up to work even more elegantly by rebasing, but having to understand that concept and make a decision on your first (or second or third or twentieth) pull is probably asking too much. and: Just a few more elements to add: * I have been teaching rebase and what it means in _some_ of my Git Foundations classes as of late. But some means there are a majority that do not get it. * These are the people that get formal training on Git. What about all the newbies? They really won't have a foundation for what these two flavors mean. * The merge is very different from what Subversion presents as a default. That's a possible point in the option's favor. * In the end though, the simplest thing that works should be the default without a choice. To me, a choice implies knowledge of the benefits of each option. I would say that the majority of our Git students do not, at the beginning of Git usage, understand the difference. I did not specifically ask in the original about
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. ? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote: I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Yes, that's a good point. I don't know if just git rebase is the right advice, though; it would depend on whether we were actually pulling from the upstream or not. I wonder if we have sufficient information at the time of the warning to print out the actual git rebase invocation that would rebase as if they had run pull --rebase. I think we may have to do a little refactoring around the base selection from the reflog (IIRC, git-pull does not even calculate it at all if you are not using --rebase). It is also depending on git rebase throwing away the merge commit we just created. Which I think should happen always if you have not configured anything (though perhaps we will eventually support a pull mode that does rebase -p, you would not see this warning with that option anyway). But another option would be to simply tell them: git reset --keep HEAD^ git pull --rebase [X...] where [X...] is the arguments they gave to rebase in the first place. That looks a little less friendly, though. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
John Keeping j...@keeping.me.uk writes: I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Sounds good to me. One option is to display the warning on the command-line, and another option is to show it in COMMIT_EDITMSG (since we now default to showing it even for non-conflicted merges). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Junio C Hamano gits...@pobox.com writes: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. Yes. Having an option can't harm anybody, and there's a clear demand for that. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I didn't follow very closely the discussions and patch series, but that would sound right to me. The last version of Felipes' patch series already gives a warning only, but the wording and commit message implies that this will become an error in the future (this is the part with which I disagree). -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 04:44:16PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote: I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Yes, that's a good point. I don't know if just git rebase is the right advice, though; it would depend on whether we were actually pulling from the upstream or not. I wonder if we have sufficient information at the time of the warning to print out the actual git rebase invocation that would rebase as if they had run pull --rebase. I think we may have to do a little refactoring around the base selection from the reflog (IIRC, git-pull does not even calculate it at all if you are not using --rebase). We can probably do something like: opts= if git merge-base --is-ancestor $orig_head $merge_head then opts=$merge_head else opts=$orig_head --onto $merge_head fi so that git rebase $opts is the right thing. Most users then get the simple git rebase $merge_head variant. It is also depending on git rebase throwing away the merge commit we just created. Which I think should happen always if you have not configured anything (though perhaps we will eventually support a pull mode that does rebase -p, you would not see this warning with that option anyway). But another option would be to simply tell them: git reset --keep HEAD^ git pull --rebase [X...] where [X...] is the arguments they gave to rebase in the first place. That looks a little less friendly, though. Yeah, I think we should keep it simple if possible. In my experience people are relatively happy to run a single make things right command but less so if there's a sequence of steps to be performed. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 09, 2013 at 10:50:31PM +0200, Matthieu Moy wrote: John Keeping j...@keeping.me.uk writes: I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Sounds good to me. One option is to display the warning on the command-line, and another option is to show it in COMMIT_EDITMSG (since we now default to showing it even for non-conflicted merges). I hadn't though of that, but showing it in COMMIT_EDITMSG is a great moment, because you are notifying the user _before_ they create a merge commit. So the backout/switch procedure is cancel this by giving an empty message, then re-run git pull --rebase. On the other hand, if we run into conflicts, you'd probably want to let them know before asking them to resolve them all. So perhaps a separate message would be needed for that case (to suggest reset --merge git pull --rebase). -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Jeff King p...@peff.net Sent: Monday, September 09, 2013 9:53 PM On Mon, Sep 09, 2013 at 10:50:31PM +0200, Matthieu Moy wrote: John Keeping j...@keeping.me.uk writes: I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. Sounds good to me. One option is to display the warning on the command-line, and another option is to show it in COMMIT_EDITMSG (since we now default to showing it even for non-conflicted merges). I hadn't though of that, but showing it in COMMIT_EDITMSG is a great moment, because you are notifying the user _before_ they create a merge commit. So the backout/switch procedure is cancel this by giving an empty message, then re-run git pull --rebase. On the other hand, if we run into conflicts, you'd probably want to let them know before asking them to resolve them all. So perhaps a separate message would be needed for that case (to suggest reset --merge git pull --rebase). In fact this [running into conflicts unexpectedly] is usually my use case, which I mis-described as a no-ff in an earlier reply. Usually I'd want a clean rebase before submitting patches, but I can see other uses cases where there is a desire that branches show where they started so rebase wouldn't be appropriate. It should not be necessary to give prescriptions about how to backout of a difficult corner, rather give details about how to go forward after stopping safely and early. The urge to press on (various proposals) may not be the right thing. Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On 2013-09-09 16:44, Jeff King wrote:
On Mon, Sep 09, 2013 at 09:24:35PM +0100, John Keeping wrote:
I think we need to make sure that we give instructions for how to go
back if the default hasn't done what you wanted. Something like this:
Your pull did not fast-forward, so Git has merged '$upstream' into
your branch, which may not be correct for your project. If you
would rather rebase your changes, run
git rebase
See pull.mode in git-config(1) to suppress this message in the
future.
Yes, that's a good point. I don't know if just git rebase is the right
advice, though; it would depend on whether we were actually pulling from
the upstream or not.
Another reason 'git rebase' might not be the right advice: We don't
want to encourage users to flatten intentional merges. For example:
$ git checkout master
$ git merge --no-ff just-finished-feature-branch
$ git push
! [rejected]master - master (non-fast-forward)
$ git pull
WARNING: Your pull did not fast-forward [...] run git rebase
If 'git rebase' is run here, the commits on just-finished-feature-branch
will be linearized onto @{u}, which is not what the user wants.
Perhaps one could argue that a user that gets into this situation and is
normally comfortable running 'git rebase' is already experienced enough
to know to ignore the advice to run 'git rebase'.
(Sidenote: Unfortunately there's not an easy way to recover from this
case without adding another merge commit. But that's a topic for
another thread.)
-Richard
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 9, 2013 at 3:24 PM, John Keeping j...@keeping.me.uk wrote: On Mon, Sep 09, 2013 at 03:52:31PM -0400, Jeff King wrote: On Mon, Sep 09, 2013 at 11:47:45AM -0700, Junio C Hamano wrote: You are in favor of an _option_ to allow people to forbid a pull in a non-ff situation, and I think other people are also in agreement. So perhaps: - drop jc/pull-training-wheel and revert its merge from 'next'; - update Felipe's series with a bit of tweak to make it less impactful by demoting error into warning and advice. would be a good way forward? I think that would address the concern I raised, because it does not create a roadblock to new users accomplishing their task. They can ignore the warning, or choose merge as the default to shut up the warning (and it is easy to choose that if you are confused, because it is what git is doing by default alongside the warning). I think we need to make sure that we give instructions for how to go back if the default hasn't done what you wanted. Something like this: Your pull did not fast-forward, so Git has merged '$upstream' into your branch, which may not be correct for your project. If you would rather rebase your changes, run git rebase See pull.mode in git-config(1) to suppress this message in the future. And you propose to show that every single time the user does a 'git pull'' that results in a non-fast-forward merge? Isn't that what 'git pull --help' is for? -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 9, 2013 at 3:17 PM, Jeff King p...@peff.net wrote: On Sun, Sep 08, 2013 at 03:50:46AM -0400, Jeff King wrote: If you are interested, I can ask the opinion of some of the GitHub trainers. They see a lot of new users and have a sense of what kinds of confusion come up most frequently, what kinds of workflows they tend to see, etc. Their experience may be biased towards corporate-ish users, though, because those are the people who pay for training. Ask. I'm sure they will tell you doing merges by mistake with 'git pull' is an issue. I've sent an email. I'll post the response when I get it. Here is what I sent them (I am leaving both my mail and theirs unedited to avoid any telephone-like confusion in trying to summarize): Right now, running git pull will always create a merge, unless the user has specifically configured it to perform a rebase. Some people find this problematic, because the project may care about the order of merges (e.g., so that --first-parent traversals do the right thing), and some users may accidentally do backwards merges from a main branch into a topic (either because they are clueless, or because they simply forgot). There is a proposal being considered to have git pull do nothing by default, but instead ask the user to specify whether to merge or rebase (with the option of setting a config value if you want it to do one by default). One concern I have is that new users may run across this relatively early. For example, the first time they git push and get a non-fast-forward because somebody else has already pushed, git suggests to run git pull. At which point they will have to decide whether to merge or rebase. So what I'd like your opinions on is: 1. Do new users have trouble with the concept of rebase vs merge? How would they handle this change of behavior? 2. Do new users have trouble with rebases in general? There are some complications over doing a normal merge, but I don't know how often they trip people up in practice. And the responses I got were: 1. New users definitely have trouble distinguishing between rebase and merge. Even people who have been using Git for a while on a basic level are sometimes confused by this. 2. Most people we teach—even the ones who have been using Git for a while—don't know what a rebase is at all. They've heard of it, but they don't get it. It takes careful explanation to get the concept across and explain why it is not the same thing as a merge. Speaking for myself, about half of the time in the Foundations class I'll explain `pull --rebase` and `branch.autosetuprebase`. (Whether we get to it depends on class interest and ability.) When we do address that topic, we always recommend that rebase-on-pull is the right thing to do, since the merges Git creates are just noise that makes history hard to work with in the ways you have pointed out. (For smart classes, I like to make the analogy of Git to a distributed database, and point out how the merge on pull is just Git's mechanism for resolving split-brain writes. I explain that those merges aren't a deficiency in Git; they're just what has to happen by default. The fact that Git handles split-brain writes so well by itself is amazing.) My input would be to continue to have `pull` merge by default. Those merges aren't great, but new users won't have any idea how to make a decision about them at that point. As it is, it just works, and it works quite elegantly. Once you start to learn some things, you can tune Git up to work even more elegantly by rebasing, but having to understand that concept and make a decision on your first (or second or third or twentieth) pull is probably asking too much. and: Just a few more elements to add: * I have been teaching rebase and what it means in _some_ of my Git Foundations classes as of late. But some means there are a majority that do not get it. * These are the people that get formal training on Git. What about all the newbies? They really won't have a foundation for what these two flavors mean. * The merge is very different from what Subversion presents as a default. That's a possible point in the option's favor. * In the end though, the simplest thing that works should be the default without a choice. To me, a choice implies knowledge of the benefits of each option. I would say that the majority of our Git
Re: [PATCH 0/3] Reject non-ff pulls by default
On Mon, Sep 9, 2013 at 2:18 AM, Matthieu Moy matthieu@grenoble-inp.fr wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson sand...@crustytoothpaste.net wrote: Yes, that would be me. My hesitance here is that as the one usually driving git updates (which so far have happened once a year), I will end up retraining forty developers. I don't think the current behavior is broken or really problematic at all: merging has always been the default, and people have come to expect that. It may not be broken for you, but it is for other people. Would you be so egocentric as to ignore everybody else because it works for you? It's not a matter of works for me. Git currently works for all use cases because you can already merge or rebase. The proposed changes are not about allowing the behavior that works, but disallowing the behavior that doesn't. If it works for all use cases why are we discussing this? Hint: because it doesn't. I agree that allowing people to reject non-ff merge is a good idea. I strongly disagree that this should eventually become the default, though. I think it should really remain an opt-in (possibly with some non-scary warning advertizing for the feature). That defeats the whole purpose of the proposal, which means that you don't understand the problem. The problem is the newcomers, and the newcomers will most definitely not activate a configuration option to tell them that they are doing something potentially undesirable. By the time they learn about pull.mode, they probably already know what a rebase is. So what is the point of the configuration in the first place? First, the discussions on this thread show that it's hard to find the right behavior. My guess is that it's hard because we're trying to think for the users. I've used GNU Arch for a while, and this VCS was trying to impose what the developer thought was good for me. I had to fight with the tool whenever I tried to do something non-standard. I don't want to go back there. Preventing _users_ to do something because _we_ considered it was bad for them is wrong IMHO. We are not preventing anybody from anything. The user can do 'git pull --merge', the user can set 'pull.mode = merge', the user can do anything he wants. I already mentionned another reason in http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=229162 : git rebase is hard to use for many people. With git merge, doing things wrong isn't so bad. If you forget to commit after a conflicted merge, you'll mix your changes with the merge, this is bad, but it works. With git rebase, if you forget to git rebase --continue after a conflict, you end up in detached HEAD, with part of your own changes discarded. If my students end up in this situation, they'll stop using Git and exchange files by email. That doesn't mean anything, you are assuming the user will do 'git pull --rebase', and there's no rationale as to why they would end up doing that. git pull is one of the first things one learns with Git, and _requiring_ users to chose between merge and rebase is a nonsense at this time of learning. Let's use another core command as an illustration. 'git commit' by default prevents users from creating commits without first adding changes to the staging area, and since it's a concept unique to Git, it's fair to say that none of the newcomers understand why 'git commit' is failing, the error messages is not particularly useful either. Following your rationale, by default 'git commit' should behave like 'git commit --all', and add all the changes in the work tree to the new commit when there's no changes in the staging area, that would be the easiest for the newcomers, but we don't do that, we force them to understand what the staging area is, or do 'git commit --all', most of the newcomers do the later. So, if we draw a parallel with with pull, 'git pull --merge' is like 'git commit --all'; if they don't know what they are doing, that's what they should type, and when the pull is non-fast-forward we error out, just like we error out when there's nothing on the staging area. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 12:21 AM, Jeff King p...@peff.net wrote: On Sun, Sep 08, 2013 at 12:09:34AM -0500, Felipe Contreras wrote: It's not if you understand the difference between merge-then-commit and commit-then-merge. But for a clueless user who has been told replace svn commit with git commit git push and replace svn update with git pull, it is quite similar. Well, yeah, but if they are so clueless they have to be told what to do, they can be told to do 'git pull --merge' instead, no? I think it's fine to tell them to do git pull --merge. What I'd worry more about is somebody who is suddenly presented with the choice between --rebase and --merge and doesn't know which to choose. We've created a cognitive load on the user, and even more load if they choose --rebase and don't quite understand what it means. If that happens they will go back to the guy that told them to run those commands. Fortunately there probably are very few of these users. The current warning message in jc/pull-training-wheel is quite neutral between the two options. Perhaps we should lean more towards merging? I don't like that message. I would like this for the deprecation period: The pull was not fast-forward, in the future you would have to choose a merge or a rebase, merging automatically for now. Read 'man git pull' for more help. Then when obsolete: The pull was not fast-forward, please either merge or rebase. Any more babysitting with essay long messages is counter-productive to the vast majority of Git users. I guess that works against John's case, though, which is clueless people working on a project that _does_ care about the shape of history. At least they would have to stop and think for a moment, though, which might help (and maybe convince them to ask more clueful project members). I don't know. Or google 'git pull' 'git merge' 'git rebase' or 'git non-fast-forward'. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On 2013-09-07 22:41, Felipe Contreras wrote: On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Which can be solved by adding the above fail option, and then renaming them to pull.integrate and branch.name.integrate to clarify what these variables are about (it is no longer do you rebase or not---if you choose not to rebase, by definition you are going to merge, as there is a third choice to fail), while retaining pull.rebase and branch.name.rebase as a deprecated synonym. All these names are completely unintuitive. First of all, why integrate? Integrate what to what? And then, why fail? Fail on what circumstances? Always? My proposal that does: pull.mode = merge/rebase/merge-ff-only Is way more intuitive. +1 What about something like: pull.mergeoptions (defaults to --ff-only) pull.rebaseoptions (defaults to empty? --preserve-merges?) branch.name.pull.mergeoptions (defaults to pull.mergeoptions) branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions) -Richard -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 01:17:42AM -0500, Felipe Contreras wrote: I think it's fine to tell them to do git pull --merge. What I'd worry more about is somebody who is suddenly presented with the choice between --rebase and --merge and doesn't know which to choose. We've created a cognitive load on the user, and even more load if they choose --rebase and don't quite understand what it means. If that happens they will go back to the guy that told them to run those commands. I think the guy may be git itself. For example, here is a possible session with jc/pull-training-wheel: $ git push To ... ! [rejected]master - master (non-fast-forward) error: failed to push some refs to '...' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. $ git pull The pull does not fast-forward; please specify if you want to merge or rebase. Use either git pull --rebase git pull --merge You can also use 'git config pull.rebase true' (if you want --rebase) or 'git config pull.rebase false' (if you want --merge) to set this once for this project and forget about it. The user is pointed at pull from push, and then gets presented with the merge or rebase choice. It may be that the advice you can find by googling merge vs rebase is enough to then help the person along (and/or we may need to improve the manpages in that respect). I am genuinely curious what people in favor of this feature would want to say in the documentation to a user encountering this choice for the first time. In my experience, rebasing introduces more complications, specifically: 1. the merge is backwards with respect to ours/theirs 2. you may end up with difficult conflict resolution due to repeated changes over the same section of code. E.g., you write some buggy code and then fix it, but upstream has changed the same area. Rebasing involves first resolving your buggy version with the upstream code, and then resolving the fix on top of the previous resolution. 3. rewriting of commits found in other branches, which then need rebased on top of the branch you just rebased 4. a previously bug-free commit can show a bug after the rebase if other parts of the project changed (whereas with a merge, the bug would be attributable to the merge) I know those are all balanced by some advantages of rebasing, but I also think they are things that can be troublesome for a user who does not fully grok the rebase process. I'm just wondering if we should mention both, but steer people towards merging as the safer alternative (you might have ugly history, but you are less likely to create a mess with duplicate commits or badly-resolved conflicts). Fortunately there probably are very few of these users. Maybe. I am not sure how one would measure. If you are interested, I can ask the opinion of some of the GitHub trainers. They see a lot of new users and have a sense of what kinds of confusion come up most frequently, what kinds of workflows they tend to see, etc. Their experience may be biased towards corporate-ish users, though, because those are the people who pay for training. The current warning message in jc/pull-training-wheel is quite neutral between the two options. Perhaps we should lean more towards merging? I don't like that message. I would like this for the deprecation period: The pull was not fast-forward, in the future you would have to choose a merge or a rebase, merging automatically for now. Read 'man git pull' for more help. Then when obsolete: The pull was not fast-forward, please either merge or rebase. A deprecation message helps people who are making the transition from an older behavior to a newer one. It cannot help new users who start with a git version after the deprecation period. Any more babysitting with essay long messages is counter-productive to the vast majority of Git users. I think that is what we have advice.* for. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 1:54 AM, Jeff King p...@peff.net wrote: On Sun, Sep 08, 2013 at 01:17:42AM -0500, Felipe Contreras wrote: I think it's fine to tell them to do git pull --merge. What I'd worry more about is somebody who is suddenly presented with the choice between --rebase and --merge and doesn't know which to choose. We've created a cognitive load on the user, and even more load if they choose --rebase and don't quite understand what it means. If that happens they will go back to the guy that told them to run those commands. I think the guy may be git itself. For example, here is a possible session with jc/pull-training-wheel: $ git push Who told him to use 'git push'? Certainly not git. To ... ! [rejected]master - master (non-fast-forward) error: failed to push some refs to '...' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. $ git pull The pull does not fast-forward; please specify if you want to merge or rebase. Use either git pull --rebase git pull --merge You can also use 'git config pull.rebase true' (if you want --rebase) or 'git config pull.rebase false' (if you want --merge) to set this once for this project and forget about it. Why stop there? Post the whole man page already. Moreover, it's overly verbose on all the wrong and irrelevant information. If you are going to waste precious screen state, explain wth a non fast-forward is; people can figure out what a merge is, and maybe a rebase, but a non fast-forward definitely not. The user is pointed at pull from push, and then gets presented with the merge or rebase choice. It may be that the advice you can find by googling merge vs rebase is enough to then help the person along (and/or we may need to improve the manpages in that respect). Yes, but that's not the use-case we are talking about. You mentioned specifically a svn-like worfklow where the guy was told by somebody else to replace the svn commands with git ones. If we are talking about a guy that is learning git, that's and entirely different case. I am genuinely curious what people in favor of this feature would want to say in the documentation to a user encountering this choice for the first time. In my experience, rebasing introduces more complications, specifically: Yes, but it's what the user might want. I know those are all balanced by some advantages of rebasing, but I also think they are things that can be troublesome for a user who does not fully grok the rebase process. I'm just wondering if we should mention both, but steer people towards merging as the safer alternative (you might have ugly history, but you are less likely to create a mess with duplicate commits or badly-resolved conflicts). The purpose of this change in the code is not to change the user behavior. The choice of merge vs. rebase is entirely up to the user, and we are not changing that. The purpose of this change is to avoid doing a merge when the user wanted a rebase, or maybe something more complicated. So a rebase being complicated is not an issue, because we know that's what the user wants, that's the whole reason we are trying to avoid the automated merge. Fortunately there probably are very few of these users. Maybe. I am not sure how one would measure. If you are interested, I can ask the opinion of some of the GitHub trainers. They see a lot of new users and have a sense of what kinds of confusion come up most frequently, what kinds of workflows they tend to see, etc. Their experience may be biased towards corporate-ish users, though, because those are the people who pay for training. Ask. I'm sure they will tell you doing merges by mistake with 'git pull' is an issue. The current warning message in jc/pull-training-wheel is quite neutral between the two options. Perhaps we should lean more towards merging? I don't like that message. I would like this for the deprecation period: The pull was not fast-forward, in the future you would have to choose a merge or a rebase, merging automatically for now. Read 'man git pull' for more help. Then when obsolete: The pull was not fast-forward, please either merge or rebase. A deprecation message helps people who are making the transition from an older behavior to a newer one. It cannot help new users who start with a git version after the deprecation period. The new users are told to either merge or rebase, if they don't know what that means, they will go on look it up, just like they looked up the 'git pull' command in the first place. Any more babysitting with essay long messages is counter-productive to the vast majority of Git users. I think that is what we have advice.* for. I don't
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 02:15:17AM -0500, Felipe Contreras wrote: I think the guy may be git itself. For example, here is a possible session with jc/pull-training-wheel: $ git push Who told him to use 'git push'? Certainly not git. Any of the hundreds of existing tutorials that teach basic git commands like push? To ... ! [rejected]master - master (non-fast-forward) error: failed to push some refs to '...' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. [...] Why stop there? Post the whole man page already. Moreover, it's overly verbose on all the wrong and irrelevant information. If you are going to waste precious screen state, explain wth a non fast-forward is; people can figure out what a merge is, and maybe a rebase, but a non fast-forward definitely not. Note that I was not trying to defend any of the messages, but only showing a plausible mechanism by which a user with basic knowledge that he wants to push may arrive at the question what is the difference between merge and rebase?. If you want to suggest revisions for the push message, go ahead. The push advice _is_ an attempt to define non-fast-forwards in plain language without taking up too much space, but perhaps you can do better. You could even suggest omitting it entirely, but I'm not sure if that is a good idea. It was not added in a vacuum; we lacked that advice for many years, and people complained about it quite a bit until it was added. The user is pointed at pull from push, and then gets presented with the merge or rebase choice. It may be that the advice you can find by googling merge vs rebase is enough to then help the person along (and/or we may need to improve the manpages in that respect). Yes, but that's not the use-case we are talking about. You mentioned specifically a svn-like worfklow where the guy was told by somebody else to replace the svn commands with git ones. No, I mentioned an svn-like workflow. I didn't say anything about how they were told. They might have been told by a co-worker, or read a brief tutorial on git, or read something like Git-SVN Crash Course. If we are talking about a guy that is learning git, that's and entirely different case. That is certainly what I meant to be talking about. The purpose of this change in the code is not to change the user behavior. The choice of merge vs. rebase is entirely up to the user, and we are not changing that. Right, but by not doing anything by default, you are forcing the user to make a decision. Right now, we strongly encourage merging by making it the default, and you have to learn about rebasing separately. But a message that mentions them both as equals is going to lead to extra work for the user; they have to figure out which one is most appropriate. My concern is that this is non-trivial for new users, and that they may end up arbitrarily picking rebase, which is probably not doing them any favors if they do not understand it. For clueful users, choosing between the two is not hard. But some people seem to have trouble understanding the DAG. I don't know how large a group that is, and how any pain caused by this change might compare to the times it will help. If you are interested, I can ask the opinion of some of the GitHub trainers. They see a lot of new users and have a sense of what kinds of confusion come up most frequently, what kinds of workflows they tend to see, etc. Their experience may be biased towards corporate-ish users, though, because those are the people who pay for training. Ask. I'm sure they will tell you doing merges by mistake with 'git pull' is an issue. I've sent an email. I'll post the response when I get it. Any more babysitting with essay long messages is counter-productive to the vast majority of Git users. I think that is what we have advice.* for. I don't understand what that means. It means that some time ago, after many people complained that git did not give enough hints, we added many hints. Some people who did not need these hints would want to disable them, and we have the advice.* config options to do so. So we can have a longer message for new users, and a shorter one for people who do not want to be bothered with the long advice. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Felipe Contreras felipe.contre...@gmail.com
Sent: Sunday, September 08, 2013 3:34 AM
On Thu, Sep 5, 2013 at 3:06 AM, John Keeping j...@keeping.me.uk
wrote:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this may
be
better off fetching and then updating their local branch,
particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
That's why after recognizing the fact the you can't find the branch
point of a branch in Git, I decided to write patches to support the
@{tail} shorthand, which is basically the point where the branch was
created, or rebased to:
https://github.com/felipec/git/commits/fc/base
And if 'git rebase' was fixed to ignore the commits already in the
rebased onto branch, almost always what you would want to do is 'git
rebase @{tail} --onto @{upstream}'.
The use case that trips me up (i.e. doesn't fit the above) is when I
have a branch that may need rebasing on (onto) pu, or may need rebasing
on master, or next, depending on what others have been doing.
As a Distributed VCS (i.e. others doing work independently), a rebase
always has the possibility that the world has moved on and one has to
adapt to the new world order by moving location (--onto somewhere new),
not just fixing up the house (patch conflicts). When the update order is
unknown there is no guaranteed solution (IIUC).
You are right that mostly what one wants to do is stick with ones
current location and patch up conflicts, it's just that one din't want
any conflicts in the first place (i.e. the fast forward check).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 3:01 AM, Philip Oakley philipoak...@iee.org wrote:
From: Felipe Contreras felipe.contre...@gmail.com
Sent: Sunday, September 08, 2013 3:34 AM
On Thu, Sep 5, 2013 at 3:06 AM, John Keeping j...@keeping.me.uk wrote:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this may be
better off fetching and then updating their local branch, particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
That's why after recognizing the fact the you can't find the branch
point of a branch in Git, I decided to write patches to support the
@{tail} shorthand, which is basically the point where the branch was
created, or rebased to:
https://github.com/felipec/git/commits/fc/base
And if 'git rebase' was fixed to ignore the commits already in the
rebased onto branch, almost always what you would want to do is 'git
rebase @{tail} --onto @{upstream}'.
The use case that trips me up (i.e. doesn't fit the above) is when I have a
branch that may need rebasing on (onto) pu, or may need rebasing on master,
or next, depending on what others have been doing.
Yes, so you would do:
% git rebase --onto pu
Which would be translated to:
% git rebase @{tail} --onto pu
What's the problem?
As a Distributed VCS (i.e. others doing work independently), a rebase always
has the possibility that the world has moved on and one has to adapt to the
new world order by moving location (--onto somewhere new), not just fixing
up the house (patch conflicts). When the update order is unknown there is no
guaranteed solution (IIUC).
Yeah, but almost always you want to rebase onto @{upstream}.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Felipe Contreras felipe.contre...@gmail.com
To: Philip Oakley philipoak...@iee.org
Cc: John Keeping j...@keeping.me.uk; Junio C Hamano
gits...@pobox.com; git@vger.kernel.org; Andreas Krey
a.k...@gmx.de
Sent: Sunday, September 08, 2013 9:16 AM
Subject: Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 3:01 AM, Philip Oakley philipoak...@iee.org
wrote:
From: Felipe Contreras felipe.contre...@gmail.com
Sent: Sunday, September 08, 2013 3:34 AM
On Thu, Sep 5, 2013 at 3:06 AM, John Keeping j...@keeping.me.uk
wrote:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the
other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this
may be
better off fetching and then updating their local branch,
particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
That's why after recognizing the fact the you can't find the branch
point of a branch in Git, I decided to write patches to support the
@{tail} shorthand, which is basically the point where the branch was
created, or rebased to:
https://github.com/felipec/git/commits/fc/base
And if 'git rebase' was fixed to ignore the commits already in the
rebased onto branch, almost always what you would want to do is 'git
rebase @{tail} --onto @{upstream}'.
The use case that trips me up (i.e. doesn't fit the above) is when I
have a
branch that may need rebasing on (onto) pu, or may need rebasing on
master,
or next, depending on what others have been doing.
Yes, so you would do:
% git rebase --onto pu
Which would be translated to:
% git rebase @{tail} --onto pu
What's the problem?
The 'problem' is (would be) that I don't yet know that I would need
the --onto pu until I discover (how?) that the default rebase would
result in conflicts.
As a Distributed VCS (i.e. others doing work independently), a rebase
always
has the possibility that the world has moved on and one has to adapt
to the
new world order by moving location (--onto somewhere new), not just
fixing
up the house (patch conflicts). When the update order is unknown
there is no
guaranteed solution (IIUC).
Yeah, but almost always you want to rebase onto @{upstream}.
Yeah, but almost always you want to check first *before* starting.
That is, 'git rebase --abort' should not be required from the (user's
selected /git's) default invocation.
--
Philip Oakley
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 2:50 AM, Jeff King p...@peff.net wrote: On Sun, Sep 08, 2013 at 02:15:17AM -0500, Felipe Contreras wrote: I think the guy may be git itself. For example, here is a possible session with jc/pull-training-wheel: $ git push Who told him to use 'git push'? Certainly not git. Any of the hundreds of existing tutorials that teach basic git commands like push? You can't use a tutorial out there that just tells you to replace svn commands with git alternatives, go to work and mess up the repository history. I'm trying to take the point of view of your hypothetical user working on a repository where history is not important, but it seems more and more than this person is just not real. If it's OK to push crappy merges, somebody must have told him that was OK and provided him with the commands. If it's just some random person that read some random tutorial from 'svn' - 'git' working on a random repository that happens to accept merges all over the place. Well I think that's a very very exceptional situation. And this person still wouldn't have a problem finding another tutorial explaining what a merge is. To ... ! [rejected]master - master (non-fast-forward) error: failed to push some refs to '...' hint: Updates were rejected because the tip of your current branch is behind hint: its remote counterpart. Integrate the remote changes (e.g. hint: 'git pull ...') before pushing again. hint: See the 'Note about fast-forwards' in 'git push --help' for details. [...] Why stop there? Post the whole man page already. Moreover, it's overly verbose on all the wrong and irrelevant information. If you are going to waste precious screen state, explain wth a non fast-forward is; people can figure out what a merge is, and maybe a rebase, but a non fast-forward definitely not. Note that I was not trying to defend any of the messages, but only showing a plausible mechanism by which a user with basic knowledge that he wants to push may arrive at the question what is the difference between merge and rebase?. Yes, and this person would have to read it online, like everything else, because clearly Git documentation would do a bad job at it. That's why the online documentation was needed in the first place. The first hits of 'git merge vs rebase' are rather useful: http://mislav.uniqpath.com/2013/02/merge-vs-rebase/ http://stackoverflow.com/questions/16336014/git-merge-vs-rebase http://www.derekgourlay.com/archives/428 http://blog.sourcetreeapp.com/2012/08/21/merge-or-rebase/ http://git-scm.com/book/en/Git-Branching-Rebasing Notice how none of the results point to official documentation, precisely because it's not really useful. If you want to suggest revisions for the push message, go ahead. The push advice _is_ an attempt to define non-fast-forwards in plain language without taking up too much space, but perhaps you can do better. I definitely can, but you would disagree. But anyway, you are relying on the user having pushed first, what if he is pulling first, or what if he doesn't have write access and is only pulling? You could even suggest omitting it entirely, but I'm not sure if that is a good idea. It was not added in a vacuum; we lacked that advice for many years, and people complained about it quite a bit until it was added. I would have to see the evidence, as I have never seen any complaints about that. The complains are about the UI, and they still remain. The user is pointed at pull from push, and then gets presented with the merge or rebase choice. It may be that the advice you can find by googling merge vs rebase is enough to then help the person along (and/or we may need to improve the manpages in that respect). Yes, but that's not the use-case we are talking about. You mentioned specifically a svn-like worfklow where the guy was told by somebody else to replace the svn commands with git ones. No, I mentioned an svn-like workflow. I didn't say anything about how they were told. They might have been told by a co-worker, or read a brief tutorial on git, or read something like Git-SVN Crash Course. Once again, this doesn't make any sense. People can't just push crap merges to any repository. If we are talking about a guy that is learning git, that's and entirely different case. That is certainly what I meant to be talking about. If he is learning Git, then he will be looking for the meaning of a merge and a rebase. The fact that the repository accepts crappy merges wouldn't be relevant. The purpose of this change in the code is not to change the user behavior. The choice of merge vs. rebase is entirely up to the user, and we are not changing that. Right, but by not doing anything by default, you are forcing the user to make a decision. No, it would be a warning first, he wouldn't be *forced* to make a decision, only after the deprecation period is over. Then yes, if by then he
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 3:42 AM, Philip Oakley philipoak...@iee.org wrote:
The 'problem' is (would be) that I don't yet know that I would need the
--onto pu until I discover (how?) that the default rebase would result in
conflicts.
I don't see what that has to do with an invocation of 'git rebase'
without arguments, and @{tail}. There's absolutely no way Git can
figure out for you which is the appropriate place for you to rebase
onto.
However, it shouldn't be too difficult to write a tool that checks
multiple commits and tells you on top of which ones a rebase could
work, but I don't think 'git rebase' is the right place.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Felipe Contreras felipe.contre...@gmail.com
Sent: Sunday, September 08, 2013 9:49 AM
On Sun, Sep 8, 2013 at 3:42 AM, Philip Oakley philipoak...@iee.org
wrote:
The 'problem' is (would be) that I don't yet know that I would need
the
--onto pu until I discover (how?) that the default rebase would
result in
conflicts.
I don't see what that has to do with an invocation of 'git rebase'
without arguments, and @{tail}.
There's absolutely no way Git can
figure out for you which is the appropriate place for you to rebase
onto.
.. which was my point. I may not have explained it that well.
Given that Git can't figure it out, we should stop trying in such cases.
However, it shouldn't be too difficult to write a tool that checks
multiple commits and tells you on top of which ones a rebase could
work, but I don't think 'git rebase' is the right place.
That's an SOS approach (Success Oriented Script)[1] that presumes the
user is already better than they are - The Kruger Dunning paper [2]
offers some insight into capability misconceptions (at all levels).
--
regards
Philip
--
[1] in the original it was a Success Oriented Schedule - one of those
plans that hopeful managers put together on late running projects that
amount to wishful thinking that hopefully garners them enough time to
make a little progress and update their 'success stories'.
[2] http://dx.doi.org/10.%2F1467-8721.01235 Why People Fail to
Recognize Their Own Incompetence. Though the corollaries (People fail
to recognise their own skills, and hence they/we mishandle our
communications) are just as (IMHO more) important.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 02:54:20AM -0400, Jeff King wrote:
I am genuinely curious what people in favor of this feature would want
to say in the documentation to a user encountering this choice for the
first time. In my experience, rebasing introduces more complications,
specifically:
1. the merge is backwards with respect to ours/theirs
2. you may end up with difficult conflict resolution due to repeated
changes over the same section of code. E.g., you write some buggy
code and then fix it, but upstream has changed the same area.
Rebasing involves first resolving your buggy version with the
upstream code, and then resolving the fix on top of the previous
resolution.
3. rewriting of commits found in other branches, which then need
rebased on top of the branch you just rebased
4. a previously bug-free commit can show a bug after the rebase if
other parts of the project changed (whereas with a merge, the bug
would be attributable to the merge)
I know those are all balanced by some advantages of rebasing, but I also
think they are things that can be troublesome for a user who does not
fully grok the rebase process. I'm just wondering if we should mention
both, but steer people towards merging as the safer alternative (you
might have ugly history, but you are less likely to create a mess with
duplicate commits or badly-resolved conflicts).
The really correct thing to do here is to encourage a feature branch
workflow, but in my experience people are happier to walk through a
rebase than to switch over to feature branches completely.
An alternative pull mode would be:
git reset --keep @{u}
git merge @{-1}
which gets a sensible history shape without any of your disadvantages
above. But that didn't go anywhere last time it came up [1] [2].
[1] http://article.gmane.org/gmane.comp.version-control.git/210246
[2] http://article.gmane.org/gmane.comp.version-control.git/210625
Fortunately there probably are very few of these users.
Maybe. I am not sure how one would measure.
If you are interested, I can ask the opinion of some of the GitHub
trainers. They see a lot of new users and have a sense of what kinds of
confusion come up most frequently, what kinds of workflows they tend to
see, etc. Their experience may be biased towards corporate-ish users,
though, because those are the people who pay for training.
I expect corporate environments are the ones in which this is relevant.
Open source projects that care about the shape of history can have one
person able to write to the central repository who can enforce the
policy they want. This tends to be more difficult in a corporate
environment, particularly one that was previously using a centralised
VCS.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Philip Oakley philipoak...@iee.org [2] http://dx.doi.org/10.%2F1467-8721.01235 Why People Fail to Recognize Their Own Incompetence. Oops, That's behind a paywall, and a more recent variant. Though the corollaries (People fail to recognise their own skills, and hence they/we mishandle our communications) are just as (IMHO more) important. I believe this is the on-line version of the original 1999 paper http://mastercodeprofessional.com/library_files/Kruger-Dunning---Unskilled_and_Unaware_of_It_(2009).pdf The section 5.1. The Burden of Expertise discusses my point above. they [Experts] fail to realize that their proficiency is not necessarily shared by their peers. Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote: On Sat, Sep 7, 2013 at 11:18 PM, Jeff King p...@peff.net wrote: By svn-like, I mean the people whose workflow is: $ hack hack hack $ git commit $ git push ;# oops, somebody else pushed in the meantime $ git pull $ git push It's possible that some teams at work may be using this workflow. It's more likely that there would be a rebase if the push failed, but some teams might do a merge. I don't know because we don't dictate workflow to individual teams for the reasons I get into below. Regardless, merges are our typical workflow, so forcing rebase mode all the time wouldn't be appropriate for us. $ hack hack hack $ svn commit ;# oops, somebody else committed in the meantime $ svn update $ svn commit Those people would now have to learn enough to choose between merge and rebase when running the git pull. But that's only if they don't care about the shape of history. In my experience the people that cling more to centralized VCS do not like merges, so they rebase everything to make it a straight line. That is much more svn-like. So chances are they are already doing 'git pull --rebase' (or similar), so their workflow wouldn't be affected. We end up squashing each project branch into one commit (usually using git reset --soft), so we don't care about the shape of history. Over the course of a project branch, in fact, there may be many merges from the main release branches (including other projects), so history is going to be very messy otherwise. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/3] Reject non-ff pulls by default
Richard Hansen rhan...@bbn.com writes: On 2013-09-07 22:41, Felipe Contreras wrote: On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Which can be solved by adding the above fail option, and then renaming them to pull.integrate and branch.name.integrate to clarify what these variables are about (it is no longer do you rebase or not---if you choose not to rebase, by definition you are going to merge, as there is a third choice to fail), while retaining pull.rebase and branch.name.rebase as a deprecated synonym. All these names are completely unintuitive. First of all, why integrate? Integrate what to what? And then, why fail? Fail on what circumstances? Always? My proposal that does: pull.mode = merge/rebase/merge-ff-only Is way more intuitive. +1 What about something like: pull.mergeoptions (defaults to --ff-only) pull.rebaseoptions (defaults to empty? --preserve-merges?) branch.name.pull.mergeoptions (defaults to pull.mergeoptions) branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions) As pull has two distinct phases fetch and merge/rebase, your mergeoptions/rebaseoptions is much better than mode, which does not tell which phase of pull the mode refers to. It is clear that they apply to the process to integrate the history obtained from the other side and your own history into one history. But it does not help Philip's case, if I understand correctly, where running git pull on some branches is always a mistake and the user wants it to stop at fetch the history and objects needed to complete the history from the other side phase without proceeding to the then integrate the history from the other side and the history of your branch into one step, which may be done with either merge or rebase. Even if we ignore that always fail, do not do anything use case, your two seemingly independent mergeoptions and rebaseoptions do not tell us which one is preferred between merge and rebase. A single pull.someoption = rebase | merge [| always-fail] makes that choice in a clear way, I think. Regarding the verb integrate. We used to explain pull is a fetch followed by a merge. With more people using git pull --rebase, the word merge used in that explanation of pull stopped being generic enough. Simplarily the upstream branch of local branch X is what you fetch and merge to update the branch X but that 'merge' can be 'rebase'. We needed a verb to call the process of integrate the two histories into one. git pull --help since 153d7265 (pull: change the description to integrate changes, 2013-07-07) uses that verb [*1*]. And that is where the name of the single configuration to pick how to integrate the history obtained by the first phase of pull came from. [Footnote] *1* I suspect that there may still be places in the documentation that have not been updated since the days back when the only valid way to integrate two lines of histories was to merge, and updating them may be a low-hanging fruit. Hint, hint. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On 2013-09-08 14:10, Junio C Hamano wrote: Richard Hansen rhan...@bbn.com writes: What about something like: pull.mergeoptions (defaults to --ff-only) pull.rebaseoptions (defaults to empty? --preserve-merges?) branch.name.pull.mergeoptions (defaults to pull.mergeoptions) branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions) [snip] But it does not help Philip's case, if I understand correctly, where running git pull on some branches is always a mistake and the user wants it to stop at fetch the history and objects needed to complete the history from the other side phase without proceeding to the then integrate the history from the other side and the history of your branch into one step, which may be done with either merge or rebase. How about: branch.name.pull.defaultIntegrationMode = merge | rebase | none If 'merge', pull acts like 'git pull --merge' by default, merging the other commits into this branch. If 'rebase', pull acts like 'git pull --rebase' by default, rebasing this branch onto the other commits. If 'none', pull acts like 'git fetch' by default. Default: whatever pull.defaultIntegrationMode is set to. branch.name.pull.mergeoptions Arguments to pass to 'git merge' during the merge phase of 'git pull --merge'. Default: whatever pull.mergeoptions is set to. branch.name.pull.rebaseoptions Arguments to pass to 'git rebase' during the rebase phase of 'git pull --rebase'. Default: whatever pull.rebaseoptions is set to. pull.defaultIntegrationMode = rebase | merge | none See branch.name.pull.defaultIntegrationMode. Default: merge pull.mergeoptions See branch.name.pull.mergeoptions. Default: empty, but warn that a future version will change this to --ff-only. pull.rebaseoptions See branch.name.pull.rebaseoptions. Default: empty, but warn that a future version will change this to --preserve-merges? There's probably a better alternative to the term 'defaultIntegrationMode'. We could even add a defaultIntegrationMode = merge-there that reverses the parent order (the other commits become the first parent, the current branch becomes the second parent). -Richard -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 12:26 PM, brian m. carlson sand...@crustytoothpaste.net wrote: On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote: On Sat, Sep 7, 2013 at 11:18 PM, Jeff King p...@peff.net wrote: $ hack hack hack $ svn commit ;# oops, somebody else committed in the meantime $ svn update $ svn commit Those people would now have to learn enough to choose between merge and rebase when running the git pull. But that's only if they don't care about the shape of history. In my experience the people that cling more to centralized VCS do not like merges, so they rebase everything to make it a straight line. That is much more svn-like. So chances are they are already doing 'git pull --rebase' (or similar), so their workflow wouldn't be affected. We end up squashing each project branch into one commit (usually using git reset --soft), so we don't care about the shape of history. Over the course of a project branch, in fact, there may be many merges from the main release branches (including other projects), so history is going to be very messy otherwise. Yeah, but the key question at hand in this discussion is; what happens when 'git pull' stops working for them, and they don't know what to do, will they choose 'git pull --rebase' by mistake? I say the answer is no, because: 1) As you say in your scenario, somebody is telling these guys what to do, so when 'git pull' fails, somebody will figure out that they were doing a merge, so 'git pull --merge' is what they want to type from now on. 2) Git itself would be warning them for months that a 'non fast-forward was found, and a merge will be done for them', so when the warning turns to an error, they'll know they want a merge, so they'll do 'git pull --merge', either because the warning told them that's git was doing all along, or because they figured that out by googling, or reading the man page, or whatever. Either way, it would not be a big deal for these people, their user-experience wouldn't be totally broken by this proposed change, and that is the important conclusion. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Junio C Hamano gits...@pobox.com Sent: Sunday, September 08, 2013 7:10 PM Richard Hansen rhan...@bbn.com writes: On 2013-09-07 22:41, Felipe Contreras wrote: On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Which can be solved by adding the above fail option, and then renaming them to pull.integrate and branch.name.integrate to clarify what these variables are about (it is no longer do you rebase or not---if you choose not to rebase, by definition you are going to merge, as there is a third choice to fail), while retaining pull.rebase and branch.name.rebase as a deprecated synonym. All these names are completely unintuitive. First of all, why integrate? Integrate what to what? And then, why fail? Fail on what circumstances? Always? My proposal that does: pull.mode = merge/rebase/merge-ff-only Is way more intuitive. +1 What about something like: pull.mergeoptions (defaults to --ff-only) pull.rebaseoptions (defaults to empty? --preserve-merges?) branch.name.pull.mergeoptions (defaults to pull.mergeoptions) branch.name.pull.rebaseoptions (defaults to pull.rebaseoptions) As pull has two distinct phases fetch and merge/rebase, your mergeoptions/rebaseoptions is much better than mode, which does not tell which phase of pull the mode refers to. It is clear that they apply to the process to integrate the history obtained from the other side and your own history into one history. But it does not help Philip's case, if I understand correctly, where running git pull on some branches is always a mistake Not quite always, it's when it won't fast forward and the user wants it to stop at fetch the history and objects needed to complete the history from the other side phase without proceeding to the then integrate the history from the other side and the history of your branch into one step, Yes, it/Git should stop and wait for instructions... which may be done with either merge or rebase. Here I would typically rebase onto an adjusted destination, e.g. onto pu, or maybe next, rather than master (or vice versa depending on expectations). That is its a feature branch that needs to decide what it's on top of (well, I need to decide ;-) Even if we ignore that always fail, do not do anything use case, your two seemingly independent mergeoptions and rebaseoptions do not tell us which one is preferred between merge and rebase. A single pull.someoption = rebase | merge [| always-fail] makes that choice in a clear way, I think. or 'fail on non-ff' (which may or may not be the users, or Git's default, as per the series title ;-) Regarding the verb integrate. We used to explain pull is a fetch followed by a merge. With more people using git pull --rebase, the word merge used in that explanation of pull stopped being generic enough. Simplarily the upstream branch of local branch X is what you fetch and merge to update the branch X but that 'merge' can be 'rebase'. We needed a verb to call the process of integrate the two histories into one. git pull --help since 153d7265 (pull: change the description to integrate changes, 2013-07-07) uses that verb [*1*]. And that is where the name of the single configuration to pick how to integrate the history obtained by the first phase of pull came from. [Footnote] *1* I suspect that there may still be places in the documentation that have not been updated since the days back when the only valid way to integrate two lines of histories was to merge, and updating them may be a low-hanging fruit. Hint, hint. -- Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 1:10 PM, Junio C Hamano gits...@pobox.com wrote: pull.someoption = rebase | merge [| always-fail] makes that choice in a clear way, I think. Regarding the verb integrate. I doubt anybody thinks of pull being an integration, and even if it is, it's still doesn't explain what 'integration = merge' means. To be human friendly you would need to say 'integration-type' or 'integration-kind', or 'integration-mode', then a human would understand, oh yeah, the mode I'm using to integrated is a merge, got ya'. But why bother with yet another useless concept the user has to learn? The user doesn't need to learn about this concept of integration, all the user wants is to map: git pull --rebase = pull.name = rebase git pull --merge pull.name = merge That's it. And my proposed name, 'mode' does the trick just fine. pull.mode = rebase | merge | merge-no-ff -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras wrote: On Sun, Sep 8, 2013 at 1:10 PM, Junio C Hamano gits...@pobox.com wrote: pull.someoption = rebase | merge [| always-fail] makes that choice in a clear way, I think. The core issue is that users rarely want to merge locally: that's the maintainer's job. Users simply want to rebase, and develop on different branches that they will rebase onto origin. I like Felipe's idea for using a pull.mode. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 05:38:50PM -0500, Felipe Contreras wrote: Yeah, but the key question at hand in this discussion is; what happens when 'git pull' stops working for them, and they don't know what to do, will they choose 'git pull --rebase' by mistake? I agree, they will not choose git pull --rebase by mistake. I say the answer is no, because: 1) As you say in your scenario, somebody is telling these guys what to do, so when 'git pull' fails, somebody will figure out that they were doing a merge, so 'git pull --merge' is what they want to type from now on. Yes, that would be me. My hesitance here is that as the one usually driving git updates (which so far have happened once a year), I will end up retraining forty developers. I don't think the current behavior is broken or really problematic at all: merging has always been the default, and people have come to expect that. People using workflows that don't want merge have always either needed to set a configuration option or use --rebase. As the man page says, --rebase is unsafe, and that's why it's not the default. I would be much less unhappy with your earlier change that did not affect uses with arguments. That would limit the number of use cases affected. 2) Git itself would be warning them for months that a 'non fast-forward was found, and a merge will be done for them', so when the warning turns to an error, they'll know they want a merge, so they'll do 'git pull --merge', either because the warning told them that's git was doing all along, or because they figured that out by googling, or reading the man page, or whatever. Again, you assume that git updates happen on a regular basis, and you assume that most developers really know what happens under the hood. I don't see a warning now; in fact, I see: vauxhall ok % git status # On branch master # Your branch and 'upstream/master' have diverged, # and have 1 and 128 different commits each, respectively. # (use git pull to merge the remote branch into yours) # The current behavior of git is to explicitly encourage this behavior, and now you want to make it not work. I think this change is a bad idea, and I think the number of changes required to the test suite indicates that. If there's going to be a change here, it should have a deprecation period with the above message changed and appropriate warnings, not a flag day; your patches don't do that. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 7:01 PM, brian m. carlson sand...@crustytoothpaste.net wrote: On Sun, Sep 08, 2013 at 05:38:50PM -0500, Felipe Contreras wrote: Yeah, but the key question at hand in this discussion is; what happens when 'git pull' stops working for them, and they don't know what to do, will they choose 'git pull --rebase' by mistake? I agree, they will not choose git pull --rebase by mistake. I say the answer is no, because: 1) As you say in your scenario, somebody is telling these guys what to do, so when 'git pull' fails, somebody will figure out that they were doing a merge, so 'git pull --merge' is what they want to type from now on. Yes, that would be me. My hesitance here is that as the one usually driving git updates (which so far have happened once a year), I will end up retraining forty developers. I don't think the current behavior is broken or really problematic at all: merging has always been the default, and people have come to expect that. It may not be broken for you, but it is for other people. Would you be so egocentric as to ignore everybody else because it works for you? People using workflows that don't want merge have always either needed to set a configuration option or use --rebase. As the man page says, --rebase is unsafe, and that's why it's not the default. Yes, but the problem is that people using other workflows end up avoiding 'git pull' at all, so at the end of the day we have one core command that the majority of users avoid, that's not good. I would be much less unhappy with your earlier change that did not affect uses with arguments. That would limit the number of use cases affected. I have no problem with: git pull $remote $branch Allowing non-fast-forward merges. And: git pull $remote git pull Not allowing them by default. But the problem is that it's not easy to implement. Either way, I'll venture that you don't want 'git pull $remote' to change, so it would be a waste of the time to try to get the above to work. 2) Git itself would be warning them for months that a 'non fast-forward was found, and a merge will be done for them', so when the warning turns to an error, they'll know they want a merge, so they'll do 'git pull --merge', either because the warning told them that's git was doing all along, or because they figured that out by googling, or reading the man page, or whatever. Again, you assume that git updates happen on a regular basis, and you assume that most developers really know what happens under the hood. No. The developers don't have to know what happens under the hood, Git would be telling them WARNING: we are doing a merge, what else is the developer to think, but that 'git pull' is doing a merge? As for the updates, yes, I assume updates happen at least each three months. If your company updates each year, I don't see what much more we can do to you help you. Doing a single change per year is certainly going to hold the project back. Fortunately this was only point 2), there's still point 1); you can tell them to use 'git pull --merge' from now on, and since you update once every year, you can do it while you give the training for the year. Or there's another option: 3) Distribute Git in your company with /etc/gitconfig having pull.mode = merge. This way nothing will change. I think we are being very accommodating to your company's use-case which is very far from the norm. Even in the absolute worst case scenario, you would have to tell people to use 'git pull --merge' instead, is that really so horrible? Should we really halt Git's progress because you would have to tell people to type nine extra characters or run one configuration command? I don't see a warning now; in fact, I see: vauxhall ok % git status # On branch master # Your branch and 'upstream/master' have diverged, # and have 1 and 128 different commits each, respectively. # (use git pull to merge the remote branch into yours) # The current behavior of git is to explicitly encourage this behavior, and now you want to make it not work. Yes, that's why it's a change. I think this change is a bad idea, and I think the number of changes required to the test suite indicates that. If there's going to be a change here, it should have a deprecation period with the above message changed and appropriate warnings, not a flag day; your patches don't do that. My patches pretty much do nothing else but introduce a warning. Nothing is broken, nothing is changed in the test suite: http://article.gmane.org/gmane.comp.version-control.git/233669 You are confusing my proposal with Junio's one. Also, my proposal was to enable this behavior (pull.mode = merge-ff-only) only for Git v2.0, which might happen probably way later than a year from now, so you your users might actually see the warning after all. But yeah, that's _my_ proposal. -- Felipe Contreras -- To unsubscribe from this list: send the line
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 8, 2013 at 7:29 PM, Felipe Contreras felipe.contre...@gmail.com wrote: My patches pretty much do nothing else but introduce a warning. Nothing is broken, nothing is changed in the test suite: http://article.gmane.org/gmane.comp.version-control.git/233669 You are confusing my proposal with Junio's one. Actually my mistake. My patches don't even add a warning, so nothing is changed at all (unless you manually configure pull.mode = merge-ff-only). I only suggested to add the warning, but didn't actually implement it. I'll do that soon. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 07:36:21PM -0500, Felipe Contreras wrote: On Sun, Sep 8, 2013 at 7:29 PM, Felipe Contreras felipe.contre...@gmail.com wrote: My patches pretty much do nothing else but introduce a warning. Nothing is broken, nothing is changed in the test suite: http://article.gmane.org/gmane.comp.version-control.git/233669 You are confusing my proposal with Junio's one. Actually my mistake. My patches don't even add a warning, so nothing is changed at all (unless you manually configure pull.mode = merge-ff-only). I only suggested to add the warning, but didn't actually implement it. I'll do that soon. I still wouldn't be crazy about the change, but if there's a warning, I could live with it. I think that's probably the best course of action if there's going to be a change here. -- brian m. carlson / brian with sandals: Houston, Texas, US +1 832 623 2791 | http://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: RSA v4 4096b: 88AC E9B2 9196 305B A994 7552 F1BA 225C 0223 B187 signature.asc Description: Digital signature
Re: [PATCH 0/3] Reject non-ff pulls by default
On Fri, Sep 06, 2013 at 03:14:25PM -0700, Junio C Hamano wrote:
Jonathan Nieder jrnie...@gmail.com writes:
John Keeping wrote:
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
upstream parameter to rebase.
You're right.
It makes me wonder why we did that one inside pull and not in
rebase, though.
I'd never realised pull --rebase does that - it's exactly what I want
sometimes and I normally do fetch followed by rebase to get more control
over the process.
Perhaps we should do something like this (with added tests and
documentation)?
-- 8 --
Subject: [PATCH] rebase: use reflog to find common base with upstream
Signed-off-by: John Keeping j...@keeping.me.uk
---
git-rebase.sh | 8
1 file changed, 8 insertions(+)
diff --git a/git-rebase.sh b/git-rebase.sh
index 8d7659a..5e3013d 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -428,6 +428,14 @@ then
error_on_missing_default_upstream rebase rebase \
against git rebase branch
fi
+ for reflog in $(git rev-list -g $upstream_name 2/dev/null)
+ do
+ if test $reflog = $(git merge-base $reflog HEAD)
+ then
+ upstream_name=$reflog
+ break
+ fi
+ done
;;
*) upstream_name=$1
shift
--
1.8.4.239.g2332621
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Thu, Sep 5, 2013 at 3:06 AM, John Keeping j...@keeping.me.uk wrote:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this may be
better off fetching and then updating their local branch, particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
That's why after recognizing the fact the you can't find the branch
point of a branch in Git, I decided to write patches to support the
@{tail} shorthand, which is basically the point where the branch was
created, or rebased to:
https://github.com/felipec/git/commits/fc/base
And if 'git rebase' was fixed to ignore the commits already in the
rebased onto branch, almost always what you would want to do is 'git
rebase @{tail} --onto @{upstream}'.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Fri, Sep 6, 2013 at 5:14 PM, Junio C Hamano gits...@pobox.com wrote:
Jonathan Nieder jrnie...@gmail.com writes:
John Keeping wrote:
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
upstream parameter to rebase.
You're right.
It makes me wonder why we did that one inside pull and not in
rebase, though.
Because there's a huge difference between:
git rebase @{u}@{1} --onto @{u}
And
git rebase @{u}
I was in the process of fixing that, but you stopped me.
--
Felipe Contreras
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 4, 2013 at 5:59 PM, Junio C Hamano gits...@pobox.com wrote: Which can be solved by adding the above fail option, and then renaming them to pull.integrate and branch.name.integrate to clarify what these variables are about (it is no longer do you rebase or not---if you choose not to rebase, by definition you are going to merge, as there is a third choice to fail), while retaining pull.rebase and branch.name.rebase as a deprecated synonym. All these names are completely unintuitive. First of all, why integrate? Integrate what to what? And then, why fail? Fail on what circumstances? Always? My proposal that does: pull.mode = merge/rebase/merge-ff-only Is way more intuitive. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 4, 2013 at 4:25 AM, Jeff King p...@peff.net wrote: The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. How? Subversion would complain if you have local changes when you do 'svn pull', there's no notion of remotes, branches and merges are rare, and forget about rebases. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. Imagine a workflow where each topic is in its own repository instead of in its own branch inside a repository. Or where each developer has his or her own repository, but everybody just works on the master branch of their repository (or perhaps uses branches, but keeps master as a stable base). Alice is the integration manager; Bob tells her that he has work ready to integrate. She runs git pull ~bob/project, which will merge Bob's HEAD. These integrators should know what they are doing, so they can do 'git pull --merge', or better 'git config pull.mode merge', as Linus himself suggested (or something like that). The defaults should care most about the clueless users. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sat, Sep 07, 2013 at 09:52:16PM -0500, Felipe Contreras wrote: On Wed, Sep 4, 2013 at 4:25 AM, Jeff King p...@peff.net wrote: The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. How? Subversion would complain if you have local changes when you do 'svn pull', there's no notion of remotes, branches and merges are rare, and forget about rebases. By svn-like, I mean the people whose workflow is: $ hack hack hack $ git commit $ git push ;# oops, somebody else pushed in the meantime $ git pull $ git push without using branches or worrying about the shape of history. I do not know what you mean by svn pull, since that command does not exist (unless you are talking about svk?). In subversion, that workflow would be: $ hack hack hack $ svn commit ;# oops, somebody else committed in the meantime $ svn update $ svn commit Those people would now have to learn enough to choose between merge and rebase when running the git pull. It may be OK to say we do not care about that case, and it is a good thing that they learn enough to make the choice consciously. But I do think they exist. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. Imagine a workflow where each topic is in its own repository instead of in its own branch inside a repository. Or where each developer has his or her own repository, but everybody just works on the master branch of their repository (or perhaps uses branches, but keeps master as a stable base). Alice is the integration manager; Bob tells her that he has work ready to integrate. She runs git pull ~bob/project, which will merge Bob's HEAD. These integrators should know what they are doing, so they can do 'git pull --merge', or better 'git config pull.mode merge', as Linus himself suggested (or something like that). The defaults should care most about the clueless users. In this part of the email you are quoting I was not intending to say anything about clueless users at all, nor even about what defaults there are. John indicated that he could not imagine a scenario of git pull $remote, so I gave an example. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sat, Sep 7, 2013 at 11:18 PM, Jeff King p...@peff.net wrote: On Sat, Sep 07, 2013 at 09:52:16PM -0500, Felipe Contreras wrote: On Wed, Sep 4, 2013 at 4:25 AM, Jeff King p...@peff.net wrote: The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. How? Subversion would complain if you have local changes when you do 'svn pull', there's no notion of remotes, branches and merges are rare, and forget about rebases. By svn-like, I mean the people whose workflow is: $ hack hack hack $ git commit $ git push ;# oops, somebody else pushed in the meantime $ git pull $ git push But that's not svn-like at all. without using branches or worrying about the shape of history. I do not know what you mean by svn pull, since that command does not exist (unless you are talking about svk?). In subversion, that workflow would be: $ hack hack hack $ svn commit ;# oops, somebody else committed in the meantime $ svn update $ svn commit Those people would now have to learn enough to choose between merge and rebase when running the git pull. But that's only if they don't care about the shape of history. In my experience the people that cling more to centralized VCS do not like merges, so they rebase everything to make it a straight line. That is much more svn-like. So chances are they are already doing 'git pull --rebase' (or similar), so their workflow wouldn't be affected. It may be OK to say we do not care about that case, and it is a good thing that they learn enough to make the choice consciously. But I do think they exist. Yeah, I'm sure they exist, but they are a tiny minority compared to the amount of people who don't actually understand what 'git pull' is doing and do merges by mistake. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote: By svn-like, I mean the people whose workflow is: $ hack hack hack $ git commit $ git push ;# oops, somebody else pushed in the meantime $ git pull $ git push But that's not svn-like at all. It's not if you understand the difference between merge-then-commit and commit-then-merge. But for a clueless user who has been told replace svn commit with git commit git push and replace svn update with git pull, it is quite similar. Those people would now have to learn enough to choose between merge and rebase when running the git pull. But that's only if they don't care about the shape of history. In my experience the people that cling more to centralized VCS do not like merges, so they rebase everything to make it a straight line. That is much more svn-like. So chances are they are already doing 'git pull --rebase' (or similar), so their workflow wouldn't be affected. I think we are talking about two classes of users. People who truly don't care about the shape of history will also not care about using git pull --rebase, because the only reason to use it is to impact the shape of history. I agree there is also a set of people coming from the centralized vcs world who want to keep a linear history. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sat, Sep 7, 2013 at 11:43 PM, Jeff King p...@peff.net wrote: On Sat, Sep 07, 2013 at 11:37:13PM -0500, Felipe Contreras wrote: By svn-like, I mean the people whose workflow is: $ hack hack hack $ git commit $ git push ;# oops, somebody else pushed in the meantime $ git pull $ git push But that's not svn-like at all. It's not if you understand the difference between merge-then-commit and commit-then-merge. But for a clueless user who has been told replace svn commit with git commit git push and replace svn update with git pull, it is quite similar. Well, yeah, but if they are so clueless they have to be told what to do, they can be told to do 'git pull --merge' instead, no? Those people would now have to learn enough to choose between merge and rebase when running the git pull. But that's only if they don't care about the shape of history. In my experience the people that cling more to centralized VCS do not like merges, so they rebase everything to make it a straight line. That is much more svn-like. So chances are they are already doing 'git pull --rebase' (or similar), so their workflow wouldn't be affected. I think we are talking about two classes of users. People who truly don't care about the shape of history will also not care about using git pull --rebase, because the only reason to use it is to impact the shape of history. I agree there is also a set of people coming from the centralized vcs world who want to keep a linear history. Yeah, and based on the evidence, one set of people is much much larger than the other; the people that care what the history look like. Either way, we can start by making it a warning, and then an error, and if more people complain that they have to do 'git pull --merge' now (I bet there won't be any), then you would be right, and we revert. No problem. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Sun, Sep 08, 2013 at 12:09:34AM -0500, Felipe Contreras wrote: It's not if you understand the difference between merge-then-commit and commit-then-merge. But for a clueless user who has been told replace svn commit with git commit git push and replace svn update with git pull, it is quite similar. Well, yeah, but if they are so clueless they have to be told what to do, they can be told to do 'git pull --merge' instead, no? I think it's fine to tell them to do git pull --merge. What I'd worry more about is somebody who is suddenly presented with the choice between --rebase and --merge and doesn't know which to choose. We've created a cognitive load on the user, and even more load if they choose --rebase and don't quite understand what it means. The current warning message in jc/pull-training-wheel is quite neutral between the two options. Perhaps we should lean more towards merging? I guess that works against John's case, though, which is clueless people working on a project that _does_ care about the shape of history. At least they would have to stop and think for a moment, though, which might help (and maybe convince them to ask more clueful project members). I don't know. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
John Keeping wrote:
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
Maybe Junio is thinking of 'git pull --rebase', which walks the reflog
until it finds an ancestor of the current branch and uses that as the
upstream parameter to rebase.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Although I agree with your side note below that people doing this may be
better off fetching and then updating their local branch, particularly
if @{1} is not the correct reflog entry for the upstream when they
created the branch.
Side note: a knee-jerk response to a yes answer to the
last question from me has always been then why are you
running 'git pull' in the first place. The next paragraph is
my attempt to extend my imagination a bit, stepping outside
that reaction.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Thu, Sep 05, 2013 at 07:01:03AM -0400, John Szakmeister wrote:
On Wed, Sep 4, 2013 at 6:59 PM, Junio C Hamano gits...@pobox.com wrote:
[snip]
When git pull stops because what was fetched in FETCH_HEAD does
not fast-forward, then what did _you_ do (and with the knowledge you
currently have, what would you do)? In a single project, would you
choose to sometimes rebase and sometimes merge, and if so, what is
the choice depend on? When I am on these selected branches, I want
to merge, but on other branches I want to rebase?
Our team isn't quite proficient enough yet to have a completely rebase
workflow... though we might have less of a problem if we did. So,
several interesting points. Most of the time, `git pull` would be a
fast-forward merge. We typically perform the merges of topic branches
server-side--we have a build server who checks to make sure the result
would be successful--and we just hit the big green button on the Merge
button for the pull request (we use GitHub Enterprise at the moment).
However, nearly as often, we just merge the branch locally because
someone on the team is doing some manual testing, and it's just
convenient to finish the process on the command line. What
occasionally happens is that you merge the topic locally, but someone
else has introduced a new commit to master. We try to preserve the
mainline ordering of commits, so `git pull` doing a merge underneath
the hood is undesirable (it moves the newly introduced commit off to
the side). Rebasing your current master branch is not the answer
either, because it picks up the commits introduced by the topic branch
and rebases those to--at least with the -p option, and without it, the
results are just as bad). Instead, we want to unfold our work,
fast-forward merge the upstream, and the replay our actions--namely
remerge the topic branch. It often ends up translating to this:
$ git reset --hard HEAD~1
$ git merge --ff-only @{u}
$ git merge topic
$ git push
So what I really want isn't quite rebase. I'm not sure any of the
proposed solutions would work. It'd be really nice to replay only the
mainline commits, without affecting commits introduced from a topic
branch.
Does git rebase --preserve-merges do what you want here?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Junio C Hamano gits...@pobox.com writes:
Peff already covered (1)---it is highly doubtful that a merge is
almost always wrong. In fact, if that _were_ the case, we should
simply be defaulting to rebase, not failing the command and asking
between merge and rebase like jc/pull-training-wheel topic did.
We simply do not know what the user wants, as it heavily depends on
the project, so we ask the user to choose one (and stick to it).
From my experience leading the first large project using git at BBN,
evolving a workflow (most work on topic branches, which are rebased,
banning 'git pull'-created merge commits, and explicit merge commits to
preserve --first-parent, basically), and seeing many people struggle to
learn all this, my take is that a user who does not understand non-ff
merge vs ff-merge vs rebase will end up doing the wrong thing. So two
thoughts:
In the glorious future, perhaps git could have a way to express a
machine-parseable representation of the workflow and rules for a repo,
so that these choices could be made accordingly.
In the current world, I think it makes sense to error out when there
are multiple reasonable choices depending on workflow.
One of my team members, Richard Hansen, has argued to us that 'git pull'
is harmful, essentially because it creates non-ff merges sometimes,
while our rules say those should be rebased out. So we use
[alias]
up = !git remote update -p git merge --ff-only @{u}
which acts like pull if ff merge works, and otherwise errors out.
I think the key question is: can a user who doesn't really understand
the implications of ff vs non-ff merges and the local workflow rules
actually function ok, or do they need to stop and go back and
understand. I'm in the you just have to take the time to understand
camp, which led to us having a semi-custom syllabus from github training
covering rebase, explicit vs ff merges and the consequences for
first-parent history, etc.
Therefore, I think git pull should do the update (perhaps of just the
remote corresponding to @{u}, perhaps without -p) and a --ff-only merge,
absent a configuration asking for non-ff merge or rebase. (Arguably, an
ff merge is a degenerate case of rebase and also of the ff/non-ff merge,
so it's safe with either policy.)
Greg
pgpT4Be5FrhuZ.pgp
Description: PGP signature
Re: [PATCH 0/3] Reject non-ff pulls by default
On 2013-09-04 18:59, Junio C Hamano wrote:
Philip Oakley philipoak...@iee.org writes:
From: Junio C Hamano gits...@pobox.com
John Keeping j...@keeping.me.uk writes:
I think there are two distinct uses for pull, which boil down to:
(1) git pull
...
Peff already covered (1)---it is highly doubtful that a merge is
almost always wrong. In fact, if that _were_ the case, we should
simply be defaulting to rebase, not failing the command and asking
between merge and rebase like jc/pull-training-wheel topic did.
We simply do not know what the user wants, as it heavily depends on
the project, so we ask the user to choose one (and stick to it).
We only offer a limited list. It won't be sufficient for all use
cases. It wasn't for me.
Very interesting. Tell us more.
I'm a bit late to the discussion, but I wanted to chime in. I detest
'git pull' and discourage everyone I meet from using it. See:
http://stackoverflow.com/questions/15316601/why-is-git-pull-considered-harmful
for my reasons.
Instead, I encourage people to do this:
git config --global alias.up '!git remote update -p; git merge
--ff-only @{u}'
and tell them to run 'git up' whenever they would be tempted to use a
plain 'git pull'.
I usually work with a central repository with topic branches. I follow
this rule of thumb:
* When merging a same-named branch (e.g., origin/foo into foo, foo
into origin/foo), it should always be a fast-forward. This may
require rebasing.
* When merging a differently-named branch (e.g., feature.xyz into
master), it should never be a fast-forward.
In distributed workflows, I think of 'git pull collaborator-repo
their-branch' as merging a differently-named branch (I wouldn't be
merging if they hadn't told me that a separate feature they were working
on is complete), so I generally want the merge commit. But when I do a
'git pull' without extra arguments, I'm updating a same-named branch so
I never want a merge.
When merging a differently-named branch, I prefer the merge --no-ff to
be preceded by a rebase to get a nice, pretty graph:
* merge feature.xyz - master
|\
| * xyz part 3/3
| * xyz part 2/3
| * xyz part 1/3
|/
* merge feature.foo
|\
| * foo part 2/2
| * foo part 1/2
|/
* merge feature.bar
|\
...
The explicit merge has several benefits:
* It clearly communicates to others that the feature is done.
* It makes it easier to revert the entire feature by reverting the
merge if necessary.
* It allows our continuous integration tool to skip over the
work-in-progress commits and test only complete features.
* It makes it easier to review the entire feature in one diff.
* 'git log --first-parent' shows a high-level summary of the changes
over time, while a normal 'git log' shows the details.
When git pull stops because what was fetched in FETCH_HEAD does
not fast-forward, then what did _you_ do (and with the knowledge you
currently have, what would you do)?
I stop and review what's going on, then make a decision:
* usually it's a rebase
* sometimes it's a rebase --onto (because the branch was
force-updated to undo a particularly bad commit)
* sometimes it's a rebase -p (because there's an explicit merge of a
different branch that I want to keep)
* sometimes it's a reset --hard (my changes were made obsolete by a
different upstream change)
* sometimes it's a merge
* sometimes I do nothing. This is a fairly regular pattern: I'm in
the middle of working on something that I know will conflict with
some changes that were just pushed upstream, and I want to finish
my changes before starting the rebase. My collaborator contacts me
and asks, Would you take a look at the changes I just pushed? If
I type 'git pull' out of habit to get the commits, then I'll make a
mess of my work-in-progress work tree. If I type 'git up' out of
habit, then the merge --ff-only will fail as expected and I can
quickly review the commits without messing with my work tree or
HEAD.
Even if I always rebase or always merge, I want to briefly review what
changed in the remote branch *before* I start the rebase. This helps me
understand the conflicts I might encounter.
Thus, ff-only always works for me. I might have to type a second merge
or rebase command, but that's OK -- it gives me an opportunity to think
about what I want first. Non-ff merges are rare enough that the
interruption isn't annoying at all.
In a single project, would you
choose to sometimes rebase and sometimes merge, and if so, what is
the choice depend on? When I am on these selected branches, I want
to merge, but on other branches I want to rebase?
My choice depends on the circumstances of the divergence. It's never as
simple as branch X always has this policy while branch Y has that policy.
Are there cases where you do not want to either
Re: [PATCH 0/3] Reject non-ff pulls by default
On Thu, Sep 05, 2013 at 12:18:45PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Sure, that would make sense.
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
It just does @{upstream} by default, which tends to get messy if the
upstream has been rewritten.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
John Keeping j...@keeping.me.uk writes:
On Wed, Sep 04, 2013 at 03:59:18PM -0700, Junio C Hamano wrote:
Are there cases where you do not want to either rebase nor merge?
If so what do you want to do after git pull fetches from the other
side? Nothing?
One other thing that I can see being useful occasionally is:
git rebase @{u}@{1} --onto @{u}
which allows local commits to be replayed onto a rewritten upstream
branch.
Sure, that would make sense.
I somehow thought that rebase by default looked in the reflog to do
exactly that. Perhaps I am not remembering correctly.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Junio C Hamano gits...@pobox.com Philip Oakley philipoak...@iee.org writes: From: Junio C Hamano gits...@pobox.com John Keeping j...@keeping.me.uk writes: I think there are two distinct uses for pull, which boil down to: (1) git pull ... Peff already covered (1)---it is highly doubtful that a merge is almost always wrong. In fact, if that _were_ the case, we should simply be defaulting to rebase, not failing the command and asking between merge and rebase like jc/pull-training-wheel topic did. We simply do not know what the user wants, as it heavily depends on the project, so we ask the user to choose one (and stick to it). We only offer a limited list. It won't be sufficient for all use cases. It wasn't for me. Very interesting. Tell us more. What I do now is avoid Pull because of the hassle of fixing anything that may have gone wrong. Instead I now use a 'git fetch', followed by a 'push . (+etc:etc)' once I understand what I've got, or what I need to do different if wasn't a simple fast forward 'pull'. When git pull stops because what was fetched in FETCH_HEAD does not fast-forward, then what did _you_ do (and with the knowledge you currently have, what would you do)? In a single project, would you choose to sometimes rebase and sometimes merge, and if so, what is the choice depend on? When I am on these selected branches, I want to merge, but on other branches I want to rebase? In my case I have two home machines (main Windows machine and an occasional Linux laptop, though not directly networked together) and github as my level group, and have MSysGit and git/git (on github) as true upstreams, though they haven't been named that way [Aside: we are short of a good name for one's 'across-stream server' that one uses for backup/transfer such as github]. I general now use a forced update to bring my local machine up to date relative to whatever is upstream or on my across stream server, such as when transferring development from one machine to the other (where overwrite is the desired action) - e.g. when testing on the Linux laptop and a few corrections, before patch preparation on the Windows machine (different levels of familiarity). I occasionally will need to rebase my topic onto an updated git/master or git/pu if it is to be submitted upstream (patches to the list) or if upstream has moved, though I want to choose where I will rebase the topic onto. I don't need merging in that scenario, as I see those via your git repo ;-) It's not clear to me that a single default that uses a merge or rebase, without a 'stop if' criteria would be of any help in my situation. My thoughts are that the options on a fetch-pull are for the branch to be: * Overwritte (--force) (i.e. a conflict scenario) * Stop if not-ff (conflict scenario, this patch series) * rebase existing onto tracked [not a conflict in terms of initiation] * merge existing into tracked [not a conflict in terms of initiation] * fast-forward (bring tracked onto existing) [desired] Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Junio C Hamano gits...@pobox.com writes:
I can imagine users might want to say when I am on these small
number of branches, I want to merge (or rebase), but when I am on
other, majority of my branches, because they are private, unfinished
and unpublished work, please stop me from accidentally messing their
histories with changes from upstream or anywhere else for that
matter. If that is the issue you are trying to raise, because
there is no
[pull]
rebase = fail
[branch master]
rebase = yes
to force git pull to fail by default on any branch while allowing
it to rebase (or merge, for that matter) only on a few selected
branches, we fall a bit short.
Which can be solved by adding the above fail option, and then
renaming them to pull.integrate and branch.name.integrate to
clarify what these variables are about (it is no longer do you
rebase or not---if you choose not to rebase, by definition you are
going to merge, as there is a third choice to fail), while
retaining pull.rebase and branch.name.rebase as a deprecated
synonym.
The first step of such an enhancement may look like this patch. It
introduces pull.integrate and branch.name.integrate that will
eventually deprecate *.rebase, but at this step only supports
values rebase and merge (i.e. no fail yet).
The steps after this change would be to
* Enhance addition to t5520 made by 949e0d8e (pull: require choice
between rebase/merge on non-fast-forward pull, 2013-06-27) to
make sure that setting pull.integrate and branch.name.integrate
will squelch the safety added by that patch;
* Teach branch.c to set branch.name.integrate either instead
of or in addition to branch.name.rebase, and adjust tests
that expect to see branch.name.rebase to expect to see that
branch.name.integrate is set to rebase;
* Add fail to the set of valid values for *.integrate, and teach
git pull honor it; and
* Update builtin/remote.c to show cases where branch.name.integrate
is set to fail in some different way.
I do not plan to do these follow-up steps myself soonish (hint,
hint).
builtin/remote.c | 12 ++--
git-pull.sh | 60 +---
2 files changed, 50 insertions(+), 22 deletions(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index 5e54d36..d3b6d0b5 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -274,7 +274,7 @@ static int config_read_branches(const char *key, const char
*value, void *cb)
char *name;
struct string_list_item *item;
struct branch_info *info;
- enum { REMOTE, MERGE, REBASE } type;
+ enum { REMOTE, MERGE, REBASE, INTEGRATE } type;
key += 7;
if (!postfixcmp(key, .remote)) {
@@ -286,6 +286,9 @@ static int config_read_branches(const char *key, const char
*value, void *cb)
} else if (!postfixcmp(key, .rebase)) {
name = xstrndup(key, strlen(key) - 7);
type = REBASE;
+ } else if (!postfixcmp(key, .integrate)) {
+ name = xstrndup(key, strlen(key) - 10);
+ type = INTEGRATE;
} else
return 0;
@@ -309,8 +312,13 @@ static int config_read_branches(const char *key, const
char *value, void *cb)
space = strchr(value, ' ');
}
string_list_append(info-merge, xstrdup(value));
- } else
+ } else if (type == REBASE) {
info-rebase = git_config_bool(orig_key, value);
+ } else if (type == INTEGRATE) {
+ if (!value)
+ return config_error_nonbool(orig_key);
+ info-rebase = !strcmp(value, rebase);
+ }
}
return 0;
}
diff --git a/git-pull.sh b/git-pull.sh
index 88c198f..5c557b7 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -45,16 +45,34 @@ merge_args= edit=
curr_branch=$(git symbolic-ref -q HEAD)
curr_branch_short=${curr_branch#refs/heads/}
-# See if we are configured to rebase by default.
-# The value $rebase is, throughout the main part of the code:
+# See what we are configured to do by default.
+# The value $integration is, throughout the main part of the code:
#(empty) - the user did not have any preference
-#true- the user told us to integrate by rebasing
-#false - the user told us to integrate by merging
-rebase=$(git config --bool branch.$curr_branch_short.rebase)
-if test -z $rebase
-then
- rebase=$(git config --bool pull.rebase)
-fi
+#rebase - the user told us to integrate by rebasing
+#merge - the user told us to integrate by merging
+
+integration=
+
+set_integration () {
+ integration=$(git config branch.$curr_branch_short.integrate)
+ test -n
Re: [PATCH 0/3] Reject non-ff pulls by default
Philip Oakley philipoak...@iee.org writes: It's not clear to me that a single default that uses a merge or rebase, without a 'stop if' criteria would be of any help in my situation. My thoughts are that the options on a fetch-pull are for the branch to be: * Overwritte (--force) (i.e. a conflict scenario) * Stop if not-ff (conflict scenario, this patch series) * rebase existing onto tracked [not a conflict in terms of initiation] * merge existing into tracked [not a conflict in terms of initiation] * fast-forward (bring tracked onto existing) [desired] In short, it sounds to me like that the answer to my question is what I do depends too much on what I did on my other machine that is not even directly connected to this matchine, so there is no way to formulate it as a concrete workflow---I need to inspect what I get from the central repository and decide the next step anyway, so I just want 'git pull' not to do anything. Among the things that were suggested so far (the 'pull' update that has been cooking in the 'next' branch, Felipe's tightening to apply the same logic to 'git pull $there $that' as well as 'git pull', and being able to set pull.rebase = fail and renaming the variable to something like pull.integrate = fail), only the last one seems to be the solution to your particular case. The other two would not help such an ad-hoc (non)workflow very much either way. Am I reading you correctly? If so, I sent out the first (or zeroth) step to add something like that separately. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Tue, Sep 03, 2013 at 03:38:58PM -0700, Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. We know what you said earlier. That does not make it right or wrong, but I do not think it is in line with the original discussion (that is why John Keeping is kept on the Cc: line). I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. But for (2) a merge is almost always the correct thing to do (in fact it may even be correct to create a merge commit even when this fast forwards) because this most likely comes for a pull request workflow. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. In the series currently in next, we treat this as (2) above but that's primarily because it is difficult to differentiate these in git-pull.sh without adding code to understand all of the options to git-fetch (or at least those that can accept unstuck arguments). Changing this so that git pull $remote is treated as (1) would be better, but I think it is more important to avoid catching case (1) in the same net which is why jc/pull-training-wheel simply checks if $# is zero; the cost of getting this completely right outweighed the benefit of getting code in that will catch 99% of users. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 04, 2013 at 09:10:47AM +0100, John Keeping wrote: I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. Is it always wrong? You are assuming a topic-branch workflow where --first-parent is actually meaningful. What about a centralized workflow where everyone works on master? The correct thing to do on a non-ff push in that case is git pull git push. Some people would argue that the pull should rebase there, but I think there are valid arguments either way. We can discuss in that direction if you want. I can perhaps buy the argument that it is better to help people who are using a topic branch workflow (which we generally want to encourage) to avoid making backwards merges, and the cost is that people with sloppy workflows will have to do more work / configuration. But we should be clear that this is a tradeoff we are making. The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. Imagine a workflow where each topic is in its own repository instead of in its own branch inside a repository. Or where each developer has his or her own repository, but everybody just works on the master branch of their repository (or perhaps uses branches, but keeps master as a stable base). Alice is the integration manager; Bob tells her that he has work ready to integrate. She runs git pull ~bob/project, which will merge Bob's HEAD. This is not very different from the kernel workflow, where Linus may do a git pull $remote to fetch a sub-system maintainer's work, except that these days people typically mark the to-be-integrated work in a for-linus branch or tag. However, you can find many Merge git:// entries even in recent kernel history. I think this kind of pull would fall into the same situation as your (2) above. -Peff -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Wed, Sep 04, 2013 at 05:25:27AM -0400, Jeff King wrote: On Wed, Sep 04, 2013 at 09:10:47AM +0100, John Keeping wrote: I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. Is it always wrong? You are assuming a topic-branch workflow where --first-parent is actually meaningful. What about a centralized workflow where everyone works on master? The correct thing to do on a non-ff push in that case is git pull git push. Some people would argue that the pull should rebase there, but I think there are valid arguments either way. We can discuss in that direction if you want. I'm one of the people who argues that it should rebase there ;-) The point of jc/pull-training-wheel is to help users think about that. I can perhaps buy the argument that it is better to help people who are using a topic branch workflow (which we generally want to encourage) to avoid making backwards merges, and the cost is that people with sloppy workflows will have to do more work / configuration. But we should be clear that this is a tradeoff we are making. The patch in jc/pull-training-wheel talks about annoying old timers, but I think you may also be annoying clueless new users who simply want an svn-like workflow without thinking too hard about it. The scenario I have is a central repository where some developers use a topic branch workflow but others are less familiar with Git and don't really think about what they're doing. I do not think we know what we want is to affect git pull origin. I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. Imagine a workflow where each topic is in its own repository instead of in its own branch inside a repository. Or where each developer has his or her own repository, but everybody just works on the master branch of their repository (or perhaps uses branches, but keeps master as a stable base). Alice is the integration manager; Bob tells her that he has work ready to integrate. She runs git pull ~bob/project, which will merge Bob's HEAD. This is not very different from the kernel workflow, where Linus may do a git pull $remote to fetch a sub-system maintainer's work, except that these days people typically mark the to-be-integrated work in a for-linus branch or tag. However, you can find many Merge git:// entries even in recent kernel history. I think this kind of pull would fall into the same situation as your (2) above. OK - so I was missing this. Given this, the jc/pull-training-wheel series is doing the right thing here. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
John Keeping j...@keeping.me.uk writes: On Tue, Sep 03, 2013 at 03:38:58PM -0700, Junio C Hamano wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. We know what you said earlier. That does not make it right or wrong, but I do not think it is in line with the original discussion (that is why John Keeping is kept on the Cc: line). I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. But for (2) a merge is almost always the correct thing to do (in fact it may even be correct to create a merge commit even when this fast forwards) because this most likely comes for a pull request workflow. I do not think we know what we want is to affect git pull origin. I didn't mean to limit this to with an explicit 'from where' without 'which branch', but it appears you took it that way. I should have added: I do not think we know what we want is to affect git pull origin master, either. to clarify. But it seems that Peff's later message in this thread already clarifies this point for me ;-) I consider git pull $remote to be an artifact of the way git-pull is implemented on top of git-fetch; perhaps I'm missing something but I can't see a scenario where this is useful. In the series currently in next, we treat this as (2) above but that's primarily because it is difficult to differentiate these in git-pull.sh without adding code to understand all of the options to git-fetch (or at least those that can accept unstuck arguments). Changing this so that git pull $remote is treated as (1) would be better, but I think it is more important to avoid catching case (1) in the same net which is why jc/pull-training-wheel simply checks if $# is zero; the cost of getting this completely right outweighed the benefit of getting code in that will catch 99% of users. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
From: Junio C Hamano gits...@pobox.com John Keeping j...@keeping.me.uk writes: I think there are two distinct uses for pull, which boil down to: (1) git pull (2) git pull $remote $branch For (1) a merge is almost always the wrong thing to do since it will be backwards and break --first-parent. But for (2) a merge is almost always the correct thing to do (in fact it may even be correct to create a merge commit even when this fast forwards) because this most likely comes for a pull request workflow. Peff already covered (1)---it is highly doubtful that a merge is almost always wrong. In fact, if that _were_ the case, we should simply be defaulting to rebase, not failing the command and asking between merge and rebase like jc/pull-training-wheel topic did. We simply do not know what the user wants, as it heavily depends on the project, so we ask the user to choose one (and stick to it). We only offer a limited list. It won't be sufficient for all use cases. It wasn't for me. The ability to say 'stop' if it doen't match expectations, as the --no-ff option would give, would be a help, as the user can then decide what to do (read the manual or `google` the problem perhaps ;-). the option of having a hook (if suggested), while suitable for advanced users won't help those that need that help, rather a few simple safe options are needed. I generally support the ability to set an option to reject non-ff pulls. I am not sure about (2), either. Is it really almost always the correct thing to do? I tend to think myself that (2) is a lot more likely to prefer merging than (1) would, but I certainly wouldn't say almost always. Again if almost always were the case, wouldn't it make sense for that mode of invocation of the command to even defeat pull.rebase configuration and default to merge, unless explicitly told to pull --rebase from the command line? (the last question is rhetoric, if anybody is wondering). -- Philip -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. The difference in changes needed to the test suite is illustrative; this series affects any use of git pull (with or without explicit what to integrate with and from where), unlike the other one that only affects the case where git pull was not given what to integrate with and from where. I think an earlier draft I did for the previous one did not special case only when no 'integrate with what from where' is given and had to touch all the places in the test in a similar way. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. The difference in changes needed to the test suite is illustrative; this series affects any use of git pull (with or without explicit what to integrate with and from where), unlike the other one that only affects the case where git pull was not given what to integrate with and from where. I think an earlier draft I did for the previous one did not special case only when no 'integrate with what from where' is given and had to touch all the places in the test in a similar way. Yeah, that version affects less, but it also doesn't achieve what we actually want. Either way, that's why I sent another version that doesn't need modifications on the tests. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. We know what you said earlier. That does not make it right or wrong, but I do not think it is in line with the original discussion (that is why John Keeping is kept on the Cc: line). The difference in changes needed to the test suite is illustrative; this series affects any use of git pull (with or without explicit what to integrate with and from where), unlike the other one that only affects the case where git pull was not given what to integrate with and from where. I think an earlier draft I did for the previous one did not special case only when no 'integrate with what from where' is given and had to touch all the places in the test in a similar way. Yeah, that version affects less, but it also doesn't achieve what we actually want. I do not think we know what we want is to affect git pull origin. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/3] Reject non-ff pulls by default
On Tue, Sep 3, 2013 at 5:38 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: On Tue, Sep 3, 2013 at 12:21 PM, Junio C Hamano gits...@pobox.com wrote: Felipe Contreras felipe.contre...@gmail.com writes: Junio already sent a similar patch, but I think this is simpler. I agree that this is simpler, but I am not sure if the behaviour is necessarily better (note that this is different from saying I think the behaviour of this patch is worse). The motivation I read from the original discussion was that new people did git pull (no other parameters) to sync my tree with the central repository as if it were SVN, and because we are not SVN, projects that prefer rebases were unhappy, and the other one was to address *only* that use case. I do not personally like that special casing (i.e. only when no 'integrate with what from where' is given), and applying the you must be explicit between rebase and merge like this series does uniformly might (or might not) be a good thing. I dunno. As I already said; there's is essentially no difference between git pull and git pull origin. We know what you said earlier. That does not make it right or wrong, but I do not think it is in line with the original discussion (that is why John Keeping is kept on the Cc: line). And nobody provided any argument against that claim. People staying silent doesn't make it wrong. The difference in changes needed to the test suite is illustrative; this series affects any use of git pull (with or without explicit what to integrate with and from where), unlike the other one that only affects the case where git pull was not given what to integrate with and from where. I think an earlier draft I did for the previous one did not special case only when no 'integrate with what from where' is given and had to touch all the places in the test in a similar way. Yeah, that version affects less, but it also doesn't achieve what we actually want. I do not think we know what we want is to affect git pull origin. Of course we do. What we want is to make git pull more user-friendly, specially to newcomers, and specially those that come from centralized VCS, where tool pull updates the checkout, and thus we want git pull not to create merges inadvertently, and the best way to do that is to warn the user that the merge is non-fast-forward, and he should do a merge or rebase. The fact that a particular user might have learned about remotes and did git pull origin instead is irrelevant, he would still want to be warned about non-fast-forward merges. Everybody would want to be warned about that by default, I know I would, I might even start using 'git pull' again, and so countless people that have stopped using 'git pull' precisely for this reason. -- Felipe Contreras -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
