# [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase 6 messages from 2009-02-18 to 2009-02-18. Participants: Jay Soffian, Johannes Schindelin. Thread: https://gitlist.dev/t/17869 ## Jay Soffian, 2009-02-18 04:44 Subject: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: <1234932262-63469-1-git-send-email-jaysoffian@gmail.com> URL: https://gitlist.dev/e/1234932262-63469-1-git-send-email-jaysoffian%40gmail.com ``` It does not make sense to provide multiple upstream branches to either git pull --rebase, or to git rebase, so disallow both. Signed-off-by: Jay Soffian --- git-pull.sh | 5 +++++ git-rebase.sh | 1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/git-pull.sh b/git-pull.sh index 2c7f432..25adddf 100755 --- a/git-pull.sh +++ b/git-pull.sh @@ -171,6 +171,11 @@ case "$merge_head" in echo >&2 "Cannot merge multiple branches into empty head" exit 1 fi + if test true = "$rebase" + then + echo >&2 "Cannot rebase onto multiple branches" + exit 1 + fi ;; esac diff --git a/git-rebase.sh b/git-rebase.sh index 5d9a393..ffb6027 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -319,6 +319,7 @@ do esac shift done +test $# -gt 1 && usage # Make sure we do not have $GIT_DIR/rebase-apply if test -z "$do_merge" -- 1.6.2.rc1.210.g159a ``` ## Johannes Schindelin, 2009-02-18 10:18 Subject: Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0902181118160.6274%40intel-tinevez-2-302 In-Reply-To: <1234932262-63469-1-git-send-email-jaysoffian@gmail.com> ``` Hi, On Tue, 17 Feb 2009, Jay Soffian wrote: > It does not make sense to provide multiple upstream branches to either > git pull --rebase, or to git rebase, so disallow both. > > Signed-off-by: Jay Soffian > --- > git-pull.sh | 5 +++++ > git-rebase.sh | 1 + > 2 files changed, 6 insertions(+), 0 deletions(-) > > diff --git a/git-pull.sh b/git-pull.sh > index 2c7f432..25adddf 100755 > --- a/git-pull.sh > +++ b/git-pull.sh > @@ -171,6 +171,11 @@ case "$merge_head" in > echo >&2 "Cannot merge multiple branches into empty head" > exit 1 > fi > + if test true = "$rebase" > + then > + echo >&2 "Cannot rebase onto multiple branches" > + exit 1 > + fi > ;; > esac > Good catch! > diff --git a/git-rebase.sh b/git-rebase.sh > index 5d9a393..ffb6027 100755 > --- a/git-rebase.sh > +++ b/git-rebase.sh > @@ -319,6 +319,7 @@ do > esac > shift > done > +test $# -gt 1 && usage Did you just break $ git rebase $UPSTREAM $BRANCH_TO_SWITCH_TO ? Ciao, Dscho ``` ## Jay Soffian, 2009-02-18 13:23 Subject: Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: <76718490902180523l6df2067bxc1274a105756d16b@mail.gmail.com> URL: https://gitlist.dev/e/76718490902180523l6df2067bxc1274a105756d16b%40mail.gmail.com In-Reply-To: ``` On Wed, Feb 18, 2009 at 5:18 AM, Johannes Schindelin wrote: >> diff --git a/git-rebase.sh b/git-rebase.sh >> index 5d9a393..ffb6027 100755 >> --- a/git-rebase.sh >> +++ b/git-rebase.sh >> @@ -319,6 +319,7 @@ do >> esac >> shift >> done >> +test $# -gt 1 && usage > > Did you just break > > $ git rebase $UPSTREAM $BRANCH_TO_SWITCH_TO Crap, I missed that usage somehow (and I guess the test suite doesn't rely on it either...). I think moving the "test $# -gt 1 && usage" below: if test -z "$rebase_root" then # The upstream head must be given. Make sure it is valid. upstream_name="$1" shift upstream=`git rev-parse --verify "${upstream_name}^0"` || die "invalid upstream $upstream_name" unset root_flag upstream_arg="$upstream_name" else test -z "$newbase" && die "--root must be used with --onto" unset upstream_name unset upstream root_flag="--root" upstream_arg="$root_flag" fi will do the trick, yes? j. ``` ## Johannes Schindelin, 2009-02-18 13:28 Subject: Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0902181427030.6274%40intel-tinevez-2-302 In-Reply-To: <76718490902180523l6df2067bxc1274a105756d16b@mail.gmail.com> ``` Hi, On Wed, 18 Feb 2009, Jay Soffian wrote: > On Wed, Feb 18, 2009 at 5:18 AM, Johannes Schindelin > wrote: > >> diff --git a/git-rebase.sh b/git-rebase.sh > >> index 5d9a393..ffb6027 100755 > >> --- a/git-rebase.sh > >> +++ b/git-rebase.sh > >> @@ -319,6 +319,7 @@ do > >> esac > >> shift > >> done > >> +test $# -gt 1 && usage > > > > Did you just break > > > > $ git rebase $UPSTREAM $BRANCH_TO_SWITCH_TO > > Crap, I missed that usage somehow (and I guess the test suite doesn't > rely on it either...). I think moving the "test $# -gt 1 && usage" > below: > > if test -z "$rebase_root" > then > # The upstream head must be given. Make sure it is valid. > upstream_name="$1" > shift > upstream=`git rev-parse --verify "${upstream_name}^0"` || > die "invalid upstream $upstream_name" > unset root_flag > upstream_arg="$upstream_name" > else > test -z "$newbase" && die "--root must be used with --onto" > unset upstream_name > unset upstream > root_flag="--root" > upstream_arg="$root_flag" > fi > > will do the trick, yes? Nope. Note the "shift" in the first arm? It is so that the code below can check for $#, and it indeed does, in a 'case' statement. (Note: I am writing all this from memory, it could be slightly different, but the essence is still valid.) Ciao, Dscho ``` ## Jay Soffian, 2009-02-18 13:32 Subject: Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: <76718490902180532l1de135ccyd4e58c713e6af2c@mail.gmail.com> URL: https://gitlist.dev/e/76718490902180532l1de135ccyd4e58c713e6af2c%40mail.gmail.com In-Reply-To: ``` On Wed, Feb 18, 2009 at 8:28 AM, Johannes Schindelin wrote: >> if test -z "$rebase_root" >> then >> # The upstream head must be given. Make sure it is valid. >> upstream_name="$1" >> shift >> upstream=`git rev-parse --verify "${upstream_name}^0"` || >> die "invalid upstream $upstream_name" >> unset root_flag >> upstream_arg="$upstream_name" >> else >> test -z "$newbase" && die "--root must be used with --onto" >> unset upstream_name >> unset upstream >> root_flag="--root" >> upstream_arg="$root_flag" >> fi >> >> will do the trick, yes? > > Nope. Note the "shift" in the first arm? It is so that the code below > can check for $#, and it indeed does, in a 'case' statement. The case statement checks $# against 1 and *, not 1 and 0. And I don't see how > 1 is valid at that point. So I can modify the case statement to check against 1, 0, and have * emit usage, or I think moving the "test $# -gt 1 && usage" to where I suggested in the last message would do the trick. The only difference would be whether a pre-rebase hook runs in the case of invalid arguments (the case statement is after that hook runs). j. ``` ## Johannes Schindelin, 2009-02-18 13:36 Subject: Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase Message-ID: URL: https://gitlist.dev/e/alpine.DEB.1.00.0902181436380.6274%40intel-tinevez-2-302 In-Reply-To: <76718490902180532l1de135ccyd4e58c713e6af2c@mail.gmail.com> ``` Hi, On Wed, 18 Feb 2009, Jay Soffian wrote: > On Wed, Feb 18, 2009 at 8:28 AM, Johannes Schindelin > wrote: > >> if test -z "$rebase_root" > >> then > >> # The upstream head must be given. Make sure it is valid. > >> upstream_name="$1" > >> shift > >> upstream=`git rev-parse --verify "${upstream_name}^0"` || > >> die "invalid upstream $upstream_name" > >> unset root_flag > >> upstream_arg="$upstream_name" > >> else > >> test -z "$newbase" && die "--root must be used with --onto" > >> unset upstream_name > >> unset upstream > >> root_flag="--root" > >> upstream_arg="$root_flag" > >> fi > >> > >> will do the trick, yes? > > > > Nope. Note the "shift" in the first arm? It is so that the code below > > can check for $#, and it indeed does, in a 'case' statement. > > The case statement checks $# against 1 and *, not 1 and 0. And I don't > see how > 1 is valid at that point. So I can modify the case statement > to check against 1, 0, and have * emit usage, or I think moving the > "test $# -gt 1 && usage" to where I suggested in the last message > would do the trick. The only difference would be whether a pre-rebase > hook runs in the case of invalid arguments (the case statement is > after that hook runs). Of course, you could also leave the test where it was, and just replace the 1 by a 2. Ciao, Dscho ```