[PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Jay Soffian]

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 <jaysoffian@gmail.com>
---
 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

Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Johannes Schindelin]

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 <jaysoffian@gmail.com>
> ---
>  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

Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Jay Soffian]

On Wed, Feb 18, 2009 at 5:18 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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.

Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Johannes Schindelin]

Hi,

On Wed, 18 Feb 2009, Jay Soffian wrote:

> On Wed, Feb 18, 2009 at 5:18 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> 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

Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Jay Soffian]

On Wed, Feb 18, 2009 at 8:28 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> 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.

Re: [PATCH] disallow providing multiple upstream branches to rebase, pull --rebase

[Johannes Schindelin]

Hi,

On Wed, 18 Feb 2009, Jay Soffian wrote:

> On Wed, Feb 18, 2009 at 8:28 AM, Johannes Schindelin
> <Johannes.Schindelin@gmx.de> 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