Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull
Has this patch been released yet? -- View this message in context: http://git.661346.n2.nabble.com/first-parent-commit-graph-layout-and-pull-merge-direction-tp7586671p7602383.html Sent from the git mailing list archive at Nabble.com. -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
Eric Sunshine writes: >> With no repository or branch on the command line, `git pull` needs >> to be told how to integrate the changes with your history. >> >> This can be done via either `--merge` or `--rebase` option, but most >> people would want to decide which method matches the workflow of the >> project once, and set the configuration variable `pull.rebase` or >> `branch..rebase` to stick to it; see linkgit:git-config[1]. > > At this point, I'm probably just bike-shedding. Perhaps? > > With no repository or branch on the command line, `git pull` > needs to be told how to integrate the changes with your history, > via either `--merge` or `--rebase`. > > To match a project's workflow and make the choice of merge or > rebase permanent, set configuration variable `pull.rebase` or > `branch..rebase` (see linkgit:git-config[1]). I agree with the bike-shedding aspect of your comment, and actually I like my version better. It makes it clear that a single-shot --merge or --rebase from the command line is not recommended. "To match the project's workflow" is not optional in most projects, and it is preferrable to decide once and set the choice in stone. -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jul 19, 2013 at 6:20 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >> Dropping the parenthetical comment might improve flow slightly: >> >> Without repository or branch on the command line, `git pull` >> needs to be told how to integrate the changes with your history, >> via either `--merge` or `--rebase`. >> >> With or without mention of the configuration options, either phrasing >> seems pretty easy to digest. > > Yeah, that reads much better, but I do prefer to see something that > explains this is often "just make sure you use the one that suits > your project and always use that". How about something like this? > > With no repository or branch on the command line, `git pull` needs > to be told how to integrate the changes with your history. > > This can be done via either `--merge` or `--rebase` option, but most > people would want to decide which method matches the workflow of the > project once, and set the configuration variable `pull.rebase` or > `branch..rebase` to stick to it; see linkgit:git-config[1]. At this point, I'm probably just bike-shedding. Perhaps? With no repository or branch on the command line, `git pull` needs to be told how to integrate the changes with your history, via either `--merge` or `--rebase`. To match a project's workflow and make the choice of merge or rebase permanent, set configuration variable `pull.rebase` or `branch..rebase` (see linkgit:git-config[1]). -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
Eric Sunshine writes: > Dropping the parenthetical comment might improve flow slightly: > > Without repository or branch on the command line, `git pull` > needs to be told how to integrate the changes with your history, > via either `--merge` or `--rebase`. > > With or without mention of the configuration options, either phrasing > seems pretty easy to digest. Yeah, that reads much better, but I do prefer to see something that explains this is often "just make sure you use the one that suits your project and always use that". How about something like this? With no repository or branch on the command line, `git pull` needs to be told how to integrate the changes with your history. This can be done via either `--merge` or `--rebase` option, but most people would want to decide which method matches the workflow of the project once, and set the configuration variable `pull.rebase` or `branch..rebase` to stick to it; see linkgit:git-config[1]. -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
On Fri, Jul 19, 2013 at 12:22 PM, Junio C Hamano wrote: > Eric Sunshine writes: > >>> +When `git pull` that does not explicitly specify what branch from >>> +which repository is to be integrated with your history on the >>> +command line, recent Git will refuse to work until you specify how >>> +that integration should happen, either with a command line option >>> +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase` >>> +or `branch..rebase`, which is the same as `--merge` >>> +(`--rebase`) when set to `false` (`true`) respectively. >> >> This paragraph-long single sentence may be intimidating. Perhaps some >> simplification is possible: >> >> As a safety measure, bare `git pull` (without repository or >> branch) needs to be told how to integrate pulled changes with >> your history; either via `--merge` or `--rebase`. Also see >> configuration variables `pull.rebase` and `branch..rebase` >> in linkgit:git-config[1]. >> >> I intentionally omitted the true/false explanation of the >> configuration variables since the user can follow the link and read >> about them. It also may make sense to drop mention of those variables >> altogether since they are already described (including link) in the >> description of --rebase. >> >> I also intentionally omitted "recent Git" since it's rather nebulous. > > Looks much better than the original. I would further suggest > dropping the "As a safety measure, bare " at the beginning. > > `git pull` (without repository or branch on the command line) > needs to be told how to integrate the changes with your > history via either `--merge` or `--rebase` (see configuration > variables `pull.rebase` and `branch..rebase` in > linkgit:git-config[1]). > > perhaps? That works; or without the mentioning the configuration variables at all (assuming the reader will discover them from reading --rebase description): `git pull` (without repository or branch on the command line) needs to be told how to integrate the changes with your history via either `--merge` or `--rebase`. Dropping the parenthetical comment might improve flow slightly: Without repository or branch on the command line, `git pull` needs to be told how to integrate the changes with your history, via either `--merge` or `--rebase`. With or without mention of the configuration options, either phrasing seems pretty easy to digest. -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
Eric Sunshine writes: >> +When `git pull` that does not explicitly specify what branch from >> +which repository is to be integrated with your history on the >> +command line, recent Git will refuse to work until you specify how >> +that integration should happen, either with a command line option >> +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase` >> +or `branch..rebase`, which is the same as `--merge` >> +(`--rebase`) when set to `false` (`true`) respectively. > > This paragraph-long single sentence may be intimidating. Perhaps some > simplification is possible: > > As a safety measure, bare `git pull` (without repository or > branch) needs to be told how to integrate pulled changes with > your history; either via `--merge` or `--rebase`. Also see > configuration variables `pull.rebase` and `branch..rebase` > in linkgit:git-config[1]. > > I intentionally omitted the true/false explanation of the > configuration variables since the user can follow the link and read > about them. It also may make sense to drop mention of those variables > altogether since they are already described (including link) in the > description of --rebase. > > I also intentionally omitted "recent Git" since it's rather nebulous. Looks much better than the original. I would further suggest dropping the "As a safety measure, bare " at the beginning. `git pull` (without repository or branch on the command line) needs to be told how to integrate the changes with your history via either `--merge` or `--rebase` (see configuration variables `pull.rebase` and `branch..rebase` in linkgit:git-config[1]). perhaps? -- 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 v2] pull: require choice between rebase/merge on non-fast-forward pull
On Thu, Jul 18, 2013 at 3:35 PM, Junio C Hamano wrote: > Add a safety valve to fail "git pull" that does not explicitly > specify what branch from which repository to integrate your history > with, when it is neither a fast-forward or "already up-to-date", > until/unless the user expressed her preference between the two ways > of integration. > --- > diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt > index 24ab07a..86f5170 100644 > --- a/Documentation/git-pull.txt > +++ b/Documentation/git-pull.txt > @@ -97,6 +97,14 @@ must be given before the options meant for 'git fetch'. > Options related to merging > ~~ > > +When `git pull` that does not explicitly specify what branch from > +which repository is to be integrated with your history on the > +command line, recent Git will refuse to work until you specify how > +that integration should happen, either with a command line option > +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase` > +or `branch..rebase`, which is the same as `--merge` > +(`--rebase`) when set to `false` (`true`) respectively. This paragraph-long single sentence may be intimidating. Perhaps some simplification is possible: As a safety measure, bare `git pull` (without repository or branch) needs to be told how to integrate pulled changes with your history; either via `--merge` or `--rebase`. Also see configuration variables `pull.rebase` and `branch..rebase` in linkgit:git-config[1]. I intentionally omitted the true/false explanation of the configuration variables since the user can follow the link and read about them. It also may make sense to drop mention of those variables altogether since they are already described (including link) in the description of --rebase. I also intentionally omitted "recent Git" since it's rather nebulous. -- 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
[PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull
Because it is so easy to let Git handle automatically a trivial merge with "git pull", a person who is new to Git may not realize that the project s/he is interacting with may prefer a "rebase" workflow. Add a safety valve to fail "git pull" that does not explicitly specify what branch from which repository to integrate your history with, when it is neither a fast-forward or "already up-to-date", until/unless the user expressed her preference between the two ways of integration. This can be an irritating backward incompatible change for old timers, but it can be a one time irritation by doing: git config --global pull.rebase false once to say "I will always --merge", and they'd not even notice. http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326 for a full discussion. Helped-by: John Keeping Signed-off-by: Junio C Hamano --- * This time with updates to the documentation and the test suite. Documentation/git-pull.txt | 9 git-pull.sh| 40 +++- t/t5520-pull.sh| 51 ++ t/t5524-pull-msg.sh| 2 +- 4 files changed, 100 insertions(+), 2 deletions(-) diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt index 24ab07a..86f5170 100644 --- a/Documentation/git-pull.txt +++ b/Documentation/git-pull.txt @@ -97,6 +97,14 @@ must be given before the options meant for 'git fetch'. Options related to merging ~~ +When `git pull` that does not explicitly specify what branch from +which repository is to be integrated with your history on the +command line, recent Git will refuse to work until you specify how +that integration should happen, either with a command line option +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase` +or `branch..rebase`, which is the same as `--merge` +(`--rebase`) when set to `false` (`true`) respectively. + include::merge-options.txt[] :git-pull: 1 @@ -119,6 +127,7 @@ It rewrites history, which does not bode well when you published that history already. Do *not* use this option unless you have read linkgit:git-rebase[1] carefully. +--merge:: --no-rebase:: Override earlier --rebase. diff --git a/git-pull.sh b/git-pull.sh index 638aabb..88c198f 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -41,13 +41,21 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge strategy_args= diffstat= no_commit= squash= no_ff= ff_only= log_arg= verbosity= progress= recurse_submodules= verify_signatures= 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: +#(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 + dry_run= while : do @@ -113,7 +121,8 @@ do -r|--r|--re|--reb|--reba|--rebas|--rebase) rebase=true ;; - --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase) + --no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\ + -m|--m|--me|--mer|--merg|--merge) rebase=false ;; --recurse-submodules) @@ -219,6 +228,7 @@ test true = "$rebase" && { fi done } + orig_head=$(git rev-parse -q --verify HEAD) git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1 test -z "$dry_run" || exit 0 @@ -264,6 +274,34 @@ case "$merge_head" in die "$(gettext "Cannot rebase onto multiple branches")" fi ;; +*) + # integrating with a single other history; be careful not to + # trigger this check when we will say "fast-forward" or "already + # up-to-date". + merge_head=${merge_head% } + if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" && + test -n "$orig_head" && + test $# = 0 && + ! git merge-base --is-ancestor "$orig_head" "$merge_head" && + ! git merge-base --is-ancestor "$merge_head" "$orig_head" + then +echo >&2 "orig-head was $orig_head" +echo >&2 "merge-head is $merge_head" +git show >&2 --oneline -s "$orig_head" "$merge_head" + + die "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." + fi + ;; esac if test -z "$orig_head" diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh index 6af6c63..1