Re: [PATCH v5 0/6] Reject non-ff pulls by default
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Apr 30, 2014, 18:44 UTC
- Message-ID
- <53614470489f9_f9b15012ecbc@nysa.notmuch>
- In-Reply-To
- <xmqqr44eg0s0.fsf@gitster.dls.corp.google.com>
Junio C Hamano wrote:
Show 35 quoted lines
> Felipe Contreras <felipe.contreras@gmail.com> writes: > > > These are the steps needed to achieve this: > > The overall progression (this comment is only about the design, not > the implementation) looks almost sensible, but I may have missed > some issues because the presentation was done in reverse. > > In the following comment, I'll flip the presentation order to better > show natural progression of what the users will see. > > > 1) Rename pull.rename to pull.mode and > > branch.<name>.rebase to branch.<name>.pullmode > > > > This way the configurations and options remain consistent: > > > > git pull --merge > > pull.mode = merge > > branch.<name>.pullmode = merge > > > > git pull --rebase > > pull.mode = rebase > > branch.<name>.pullmode = rebase > > > > git pull --rebase=preserve > > pull.mode = rebase-preserve > > branch.<name>.pullmode = rebase-preserve > > > > git pull > > pull.mode = merge-ff-only > > branch.<name>.pullmode = merge-ff-only > > Until the "--merge" option is added, "pull.mode = merge" cannot be > the same as "git pull --merge". I think you either need to squash > these two steps into one, or flip the order of them.
Yeah, but the documentation of --merge should mention `pull.mode` and `branch.<name>.pullmode`. If I do --merge first I would have to mention pull.rebase and branch.<name>.rebase, which is weird.
I think it's more sensible to do the less visible changes first.
Show 23 quoted lines
> > 2) Add --merge option > > > > Since we have a message that says "If unsure, run 'git pull --merge'", which is > > more friendly than 'git pull --no-rebase', we should add this option, and > > deprecate --no-rebase. > > Obviously s/have a/will have a/, but the intention is good. > > > However, the documentation would become confusing if --merge is configured in > > pull.rebase, instead, we want something like this: > > > > See `pull.mode`, `branch.<name>.pullmode` in linkgit:git-config[1] if you want > > to make `git pull` always use `--merge`. > > It gets unclear to me how the transition is planned around here. Is > this a correct paraphrasing of these four steps? > > - Add pull.mode (and its branch-specific friend) and "pull > --merge" so that people can set the former to "merge" or train > their fingers to type the latter to keep doing the > fetch-and-merge (your steps 1 and 2) > > - Add ff-only to pull.mode (your step 3)
Correct.
Show 13 quoted lines
> - With the endgame of "out of box Git without any configuration > refuses 'git pull' (without --merge/--rebase) that does not > fast forward" in mind, start warning "In the future you will > have to either set pull.mode (and/or its friends) or type > "pull --merge" (or "pull --rebase") when the endgame version > of 'git pull' would fail with the error message, but still do > as was asked to do as before. At this step, existing users > can set pull.mode to "merge" or "rebase" or whatever to > squelch the warning. > > - Flip the default. By the time this happens, thanks to the > previous step to warn beforehand, nobody needs to see the > warning. (your step 4)
This is what my last version of the series did[1]. However, my plan was to land this in 1.x so users could see the warning, and then flip the switch on 2.0.
This plan, however, fell off the cliff.
Show 12 quoted lines
> If that is the rough outline, I think it is sensible. > > > 3) Add merge-ff-only config > > > > This option would trigger a check inside `git pull` itself, and error out with > > the aforementioned message if it's not possible to do a fast-forward merge. > > > > However, this option conflicts with --rebase, and --no-rebase. Solution below. > > Am I reading you correctly that setting "pull.mode = ff-only" will > require you to explicitly say "git pull --merge/--rebase"? If that > is the case, I think the step makes sense.
pull.mode = merge-ff-only
[1] http://article.gmane.org/gmane.comp.version-control.git/235951
-- Felipe Contreras