[PATCH] rebase: rebasing can also be done when HEAD is detached

[PATCH] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

In a repository when attempting to rebase when the HEAD is detached
and it is already up to date with upstream (so there's nothing to do),
the following message is shown

        Current branch HEAD is up to date.

which is clearly wrong as HEAD is not a branch.

Handle the special case of HEAD correctly to give a more precise
error message.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 In this patch, I basically assumed that there would be no
 branch named "HEAD". To the cotrary if it did, it would make
 'git' throw spurious ambiguity messages, in general. So, I
 guess it's not worth trying to check if HEAD is a branch or
 not and handle that specially.

 git-rebase.sh | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 6344e8d5e..933df832a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -598,11 +598,21 @@ then
 		test -z "$switch_to" ||
 		GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
 			git checkout -q "$switch_to" --
-		say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+		if test "$branch_name" = "HEAD"
+		then
+			say "$(eval_gettext "HEAD is up to date.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+		fi
 		finish_rebase
 		exit 0
 	else
-		say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+		if test "$branch_name" = "HEAD"
+		then
+			say "$(eval_gettext "HEAD is up to date, rebase forced.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+		fi
 	fi
 fi
 
-- 
2.15.0.345.gf926f18f3

Re: [PATCH] rebase: rebasing can also be done when HEAD is detached

[Junio C Hamano]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> In a repository when attempting to rebase when the HEAD is detached
> and it is already up to date with upstream (so there's nothing to do),
> the following message is shown
>
>         Current branch HEAD is up to date.
>
> which is clearly wrong as HEAD is not a branch.
>
> Handle the special case of HEAD correctly to give a more precise
> error message.
>
> Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
> ---
>  In this patch, I basically assumed that there would be no
>  branch named "HEAD".

Perhaps time to learn "git symbolic-ref HEAD" and use it instead of
depending on the name?

[PATCH v2 0/3] rebase: give precise error message

[Kaartic Sivaraam]

Junio C Hamano <gitster@pobox.com> writes:
> Perhaps time to learn "git symbolic-ref HEAD" and use it instead of
> depending on the name?

Good point. Helped remove the assumption that there's no branch named HEAD.
(and indirectly led to 2 additional patches and the 3 questions found below ;-)


This started as a small fix to make 'git rebase' give a precise
error message when a rebase was done with a detached HEAD. Now it
includes a few cleanups that I caught while analysing the code.

There were a few weird observations when I was anlaysing the code.
They are listed below. Please give your thoughts about them.

The commands I use below were run on my local clone of 'git' and 'origin'
in this context refers to the git mirror at GitHub.

1. "git rebase <upstream> <remote_branch>" does nothing

I tried to play around with rebase by trying out various combinations while
analysing and found the following to have not effect even though the output
doesn't say anything about that,

$ git rebase origin/next origin/maint 
First, rewinding head to replay your work on top of it...
Fast-forwarded origin/maint to origin/next.

IOW, updating a remote branch with a remote upstream had no effect.
Though trying to update a remote branch with a remote upstream doesn't
seem to be very meaningful, the output says it HAS updated the remote
which seems to be misleading. What should be done about this?

2. It's possible to do "git rebase <upstream> <commit>"

$ git origin/next f926f18f3dda0c52e794b2de0911f1b046c7dadf"

This checks out the commit(detaches HEAD) tries to rebase origin/next
from there.

This behaviour doesn't seems to be documented. It says that only a 'branch'
can be specified. (The error message updated in 1/3 previously reported that
the 'branch' name is invalid rather than stating the 'ref (branch/commit) is
invlid')

 git rebase [...] [<upstream> [<branch>]]
 git rebase [...] --root [<branch>]
 ...

Shouldn't it have said that we can give any <ref> apart from <branch> instead of
saying we could give only a <branch>. If intentional, why?

3. "git rebase <upstream> <commit>" shows misleading message

$ git origin/next f926f18f3dda0c52e794b2de0911f1b046c7dadf"
Current branch f926f18f3dda0c52e794b2de0911f1b046c7dadf is up to date.

As it's clear the commit is not a branch. What should be done to fix this?


Kaartic Sivaraam (3):
  rebase: use a more appropriate variable name
  rebase: distinguish user input by quoting it
  rebase: rebasing can also be done when HEAD is detached

 git-rebase.sh | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

-- 
2.15.0.345.gf926f18f3

[PATCH v2 1/3] rebase: use a more appropriate variable name

[Kaartic Sivaraam]

The variable name "branch_name" used to indicate the <branch> parameter
in "git rebase <upstream> <branch>" is misleading as it seems to indicate
that it holds only a "branch name" although at times it might actually hold
any valid <ref> (branch/commit).

So, use a more suitable name for that variable.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 git-rebase.sh | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 6344e8d5e..a675cf691 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -518,36 +518,40 @@ case "$onto_name" in
 esac
 
 # If the branch to rebase is given, that is the branch we will rebase
-# $branch_name -- branch being rebased, or HEAD (already detached)
+# $branch_or_commit -- branch/commit being rebased, or HEAD (already detached)
 # $orig_head -- commit object name of tip of the branch before rebasing
 # $head_name -- refs/heads/<that-branch> or "detached HEAD"
 switch_to=
 case "$#" in
 1)
 	# Is it "rebase other $branchname" or "rebase other $commit"?
-	branch_name="$1"
+	branch_or_commit="$1"
 	switch_to="$1"
 
-	if git show-ref --verify --quiet -- "refs/heads/$1" &&
-	   orig_head=$(git rev-parse -q --verify "refs/heads/$1")
+	# Is it a local branch?
+	if git show-ref --verify --quiet -- "refs/heads/$branch_or_commit" &&
+	   orig_head=$(git rev-parse -q --verify "refs/heads/$branch_or_commit")
 	then
-		head_name="refs/heads/$1"
-	elif orig_head=$(git rev-parse -q --verify "$1")
+		head_name="refs/heads/$branch_or_commit"
+
+	# If not is it a valid ref (branch or commit)?
+	elif orig_head=$(git rev-parse -q --verify "$branch_or_commit")
 	then
 		head_name="detached HEAD"
+
 	else
-		die "$(eval_gettext "fatal: no such branch: \$branch_name")"
+		die "$(eval_gettext "fatal: no such branch/commit: \$branch_or_commit")"
 	fi
 	;;
 0)
 	# Do not need to switch branches, we are already on it.
-	if branch_name=$(git symbolic-ref -q HEAD)
+	if branch_or_commit=$(git symbolic-ref -q HEAD)
 	then
-		head_name=$branch_name
-		branch_name=$(expr "z$branch_name" : 'zrefs/heads/\(.*\)')
+		head_name=$branch_or_commit
+		branch_or_commit=$(expr "z$branch_or_commit" : 'zrefs/heads/\(.*\)')
 	else
 		head_name="detached HEAD"
-		branch_name=HEAD ;# detached
+		branch_or_commit="HEAD"
 	fi
 	orig_head=$(git rev-parse --verify HEAD) || exit
 	;;
@@ -598,11 +602,11 @@ then
 		test -z "$switch_to" ||
 		GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
 			git checkout -q "$switch_to" --
-		say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+		say "$(eval_gettext "Current branch \$branch_or_commit is up to date.")"
 		finish_rebase
 		exit 0
 	else
-		say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+		say "$(eval_gettext "Current branch \$branch_or_commit is up to date, rebase forced.")"
 	fi
 fi
 
@@ -632,7 +636,7 @@ git update-ref ORIG_HEAD $orig_head
 # we just fast-forwarded.
 if test "$mb" = "$orig_head"
 then
-	say "$(eval_gettext "Fast-forwarded \$branch_name to \$onto_name.")"
+	say "$(eval_gettext "Fast-forwarded \$branch_or_commit to \$onto_name.")"
 	move_to_original_branch
 	finish_rebase
 	exit 0
-- 
2.15.0.345.gf926f18f3

[PATCH v2 2/3] rebase: distinguish user input by quoting it

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 git-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index a675cf691..3f8d99e99 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -477,7 +477,7 @@ then
 		;;
 	esac
 	upstream=$(peel_committish "${upstream_name}") ||
-	die "$(eval_gettext "invalid upstream \$upstream_name")"
+	die "$(eval_gettext "invalid upstream '\$upstream_name'")"
 	upstream_arg="$upstream_name"
 else
 	if test -z "$onto"
@@ -540,7 +540,7 @@ case "$#" in
 		head_name="detached HEAD"
 
 	else
-		die "$(eval_gettext "fatal: no such branch/commit: \$branch_or_commit")"
+		die "$(eval_gettext "fatal: no such branch/commit '\$branch_or_commit'")"
 	fi
 	;;
 0)
-- 
2.15.0.345.gf926f18f3

[PATCH v2 3/3] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

Attempting to rebase when the HEAD is detached and is already
up to date with upstream (so there's nothing to do), the
following message is shown

        Current branch HEAD is up to date.

which is clearly wrong as HEAD is not a branch.

Handle the special case of HEAD correctly to give a more precise
error message.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---

 git-rebase.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 3f8d99e99..9cce1483a 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -602,11 +602,23 @@ then
 		test -z "$switch_to" ||
 		GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
 			git checkout -q "$switch_to" --
-		say "$(eval_gettext "Current branch \$branch_or_commit is up to date.")"
+		if test "$branch_or_commit" = "HEAD" &&
+			 !(git symbolic-ref -q HEAD)
+		then
+			say "$(eval_gettext "HEAD is up to date.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_or_commit is up to date.")"
+		fi
 		finish_rebase
 		exit 0
 	else
-		say "$(eval_gettext "Current branch \$branch_or_commit is up to date, rebase forced.")"
+		if test "$branch_or_commit" = "HEAD" &&
+			 !(git symbolic-ref -q HEAD)
+		then
+			say "$(eval_gettext "HEAD is up to date, rebase forced.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_or_commit is up to date, rebase forced.")"
+		fi
 	fi
 fi
 
-- 
2.15.0.345.gf926f18f3

Re: [PATCH v2 3/3] rebase: rebasing can also be done when HEAD is detached

[Junio C Hamano]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> +		if test "$branch_or_commit" = "HEAD" &&
> +			 !(git symbolic-ref -q HEAD)

Did you need a subshell here?  Now with a proper test with
"symbolic-ref -q HEAD", I wonder if you'd need to check if the
original was named HEAD in the first place (I do not feel strongly
enough to say that checking is wrong, at least not yet, but the
above does make me wonder), and instead something like

	if ! git symbolic-ref -q HEAD
	then
		...

might be sufficient.  I dunno.

I find what 2/3 and 3/3 want to do quite sensible.  They depend on
1/3, which I find is more a needless churn than a clean-up, which is
unfortunate, though.

Thanks.

Re: [PATCH v2 3/3] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

On Tue, 2017-11-28 at 11:31 +0900, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> > +		if test "$branch_or_commit" = "HEAD" &&
> > +			 !(git symbolic-ref -q HEAD)
> 
> Did you need a subshell here? 

No. That's a consequence of me not remembering that I would span a sub-
shell with a simple '()' when I was doing that part. (partial
transition from C to shell)


>  Now with a proper test with
> "symbolic-ref -q HEAD", I wonder if you'd need to check if the
> original was named HEAD in the first place (I do not feel strongly
> enough to say that checking is wrong, at least not yet, but the
> above does make me wonder), and instead something like
> 
> 	if ! git symbolic-ref -q HEAD
> 	then
> 		...
> 
> might be sufficient.  I dunno.
> 

It does  seem you're right. The only thing we would be losing is the
short-circuiting when $branch_or_commit is not HEAD (which I suspect to
be the case most of the time). So, I'm not sure if I should remove the
check (of course, I'll change the check to avoid spawning a sub-shell).


Thanks, 
Kaartic

[PATCH v3 3/3] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

Attempting to rebase when the HEAD is detached and is already
up to date with upstream (so there's nothing to do), the
following message is shown

        Current branch HEAD is up to date.

which is clearly wrong as HEAD is not a branch.

Handle the special case of HEAD correctly to give a more precise
error message.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---

Changes in v2:

	- avoided unnecesarily spawning a subshell in a conditional


 git-rebase.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 3f8d99e99..1886167e0 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -602,11 +602,23 @@ then
 		test -z "$switch_to" ||
 		GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
 			git checkout -q "$switch_to" --
-		say "$(eval_gettext "Current branch \$branch_or_commit is up to date.")"
+		if test "$branch_or_commit" = "HEAD" &&
+			 ! git symbolic-ref -q HEAD
+		then
+			say "$(eval_gettext "HEAD is up to date.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_or_commit is up to date.")"
+		fi
 		finish_rebase
 		exit 0
 	else
-		say "$(eval_gettext "Current branch \$branch_or_commit is up to date, rebase forced.")"
+		if test "$branch_or_commit" = "HEAD" &&
+			 ! git symbolic-ref -q HEAD
+		then
+			say "$(eval_gettext "HEAD is up to date, rebase forced.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_or_commit is up to date, rebase forced.")"
+		fi
 	fi
 fi
 
-- 
2.15.0.531.g2ccb3012c

Re: [PATCH v2 0/3] rebase: give precise error message

[Junio C Hamano]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> 1. "git rebase <upstream> <remote_branch>" does nothing

Not limited to "rebase", you do not muck with remote-tracking branch
in your local repository, so it would be a bug if the above updated
where the remote-tracking branch points at.

The form of "git rebase" with one extra argument (i.e. not rebasing
the history that leads to the current checkout) is mere shorthand of
checking that extra thing out before doing the rebase, i.e.

	$ git rebase origin/next origin/maint

first checks out origin/maint (you'd get on a detached HEAD) and
rebase the history leading to the detached HEAD on top of
origin/next.  If it fast-forwards (and it should if you are talking
about 'maint' and 'next' I publish), you'll end up sitting on a
detached HEAD that points at origin/next.

There is nothing to see here.

> 2. It's possible to do "git rebase <upstream> <commit>"

Again, that's designed behaviour you can trigger by giving <commit>
(not <branch>).  Very handy when you do not trust your upstream or
yourself's ability to resolve potential conflicts as a trial run
before really committing to perform the rebase, e.g.

	$ git rebase origin master^0

Re: [PATCH v2 0/3] rebase: give precise error message

[Kaartic Sivaraam]

On Tue, 2017-11-28 at 11:25 +0900, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> > 1. "git rebase <upstream> <remote_branch>" does nothing
> 
> Not limited to "rebase", you do not muck with remote-tracking branch
> in your local repository, so it would be a bug if the above updated
> where the remote-tracking branch points at.
> 
> The form of "git rebase" with one extra argument (i.e. not rebasing
> the history that leads to the current checkout) is mere shorthand of
> checking that extra thing out before doing the rebase, i.e.
> 
> 	$ git rebase origin/next origin/maint
> 
> first checks out origin/maint (you'd get on a detached HEAD) and
> rebase the history leading to the detached HEAD on top of
> origin/next.  If it fast-forwards (and it should if you are talking
> about 'maint' and 'next' I publish), you'll end up sitting on a
> detached HEAD that points at origin/next.
> 
> There is nothing to see here.
> 

You're right. It was my mistake. It seems I didn't notice that I was
already on 'origin/next' before I did,

 	$ git rebase origin/next origin/maint

So (obviously) I thought it did nothing, sorry.


> > 2. It's possible to do "git rebase <upstream> <commit>"
> 
> Again, that's designed behaviour you can trigger by giving <commit>
> (not <branch>).  Very handy when you do not trust your upstream or
> yourself's ability to resolve potential conflicts as a trial run
> before really committing to perform the rebase, e.g.
> 
> 	$ git rebase origin master^0
> 

I can't comment about usefulness as I haven't used rebase in this way
but I'm pretty sure that this should be mentioned in the
"Documentation" to help those might be in bare need of syntax like this
to discover it.

Something like the following diff with additional changes to other
places that refer to <branch>,

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 67d48e688..ba4a545bf 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -9,9 +9,9 @@ SYNOPSIS
 --------
 [verse]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
-       [<upstream> [<branch>]]
+       [<upstream> [<ref>]]
 'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
-       --root [<branch>]
+       --root [<ref>]
 'git rebase' --continue | --skip | --abort | --quit | --edit-todo
 
 DESCRIPTION


If <ref> is the correct substitute <branch>, I could try to send a
patch that fixes this.


-- 
Kaartic

Re: [PATCH v2 0/3] rebase: give precise error message

[Junio C Hamano]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

> Something like the following diff with additional changes to other
> places that refer to <branch>,
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 67d48e688..ba4a545bf 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -9,9 +9,9 @@ SYNOPSIS
>  --------
>  [verse]
>  'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
> -       [<upstream> [<branch>]]
> +       [<upstream> [<ref>]]
>  'git rebase' [-i | --interactive] [options] [--exec <cmd>] [--onto <newbase>]
> -       --root [<branch>]
> +       --root [<ref>]
>  'git rebase' --continue | --skip | --abort | --quit | --edit-todo
>  
>  DESCRIPTION
>
>
> If <ref> is the correct substitute <branch>, I could try to send a
> patch that fixes this.

I do not think the above is a good change in the first place for at
least two reasons.  By saying <ref>, the updated text says "not just
branches but you can also give tags and remote-tracking branches".
In reality, however, it can take any commit-ish, as the "no we are
not rebasing the current checkout" form of the command is merely a
short-hand to check it out first [*1*].  It gives appearance that
the change is making it more broad, but not making it broad enough.
At the same time, more than 90% of the time, people give a branch
name there.  Saying "branch-or-commit" for a short description of
the command line (which is what synopsis section is) does not add
that much value.  The body text of the description where we refer to
the <branch> parameter of course need to be updated to say "in
addition, instead of a branch name, you can give a commit-ish that
is *not* a branch name.  When you do so, instead of checking out the
branch, the HEAD is detached at that commit before the history
leading to it is rebased."

And because we have to say "it can be a non-branch commit-ish and
here is what happens when you do so" anyway, I think the current
synopsis as-is would be better than making it less clear and more
scary by replacing <branch> with other things like <commit-ish> or
[<branch> | <commit-ish>].


[Footnote]

*1* As my "log --first-parent --oneline master..pu" is a strand of
    pearls each of which is a merge of a topic branch,

    $ git rebase -i master pu~$n^2

    can be a handy way to make a trial rebase (after double checking
    the result of "git tbdiff", I may do "git checkout -B topic" to
    overwrite the original or I may discard the result of rebasing).

Re: [PATCH v2 0/3] rebase: give precise error message

[Kaartic Sivaraam]

On Wed, 2017-11-29 at 09:10 +0900, Junio C Hamano wrote:
> Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:
> 
> > If <ref> is the correct substitute <branch>, I could try to send a
> > patch that fixes this.
> 
> I do not think the above is a good change in the first place for at
> least two reasons.  By saying <ref>, the updated text says "not just
> branches but you can also give tags and remote-tracking branches".

I used <ref> as you could actually use tags, remote-tracking branches
and even "notes" (i.e.) any kind of <ref> for the <branch> part. That's
how the code for rebasing works currently (specifically the elif part),

	if git show-ref --verify --quiet -- "refs/heads/$1" &&
	   orig_head=$(git rev-parse -q --verify "refs/heads/$1")
	then
		head_name="refs/heads/$1"
	elif orig_head=$(git rev-parse -q --verify "$1")
	then
		head_name="detached HEAD"
	else
		die "$(eval_gettext "fatal: no such branch: \$branch_name")"
	fi

Which means that you could not only do,

	$ git rebase origin/next refs/remotes/origin/maint

You could even get creative and do,

	$ git rebase origin/next refs/notes/$something

It just works (of course, I couldn't understand what it did)! I didn't
want to lie to the user. So, I used <ref>. So, should we just update
the <branch> part of the doc or should we update the code for 'rebase'?
I'm unsure.

> In reality, however, it can take any commit-ish, as the "no we are
> not rebasing the current checkout" form of the command is merely a
> short-hand to check it out first [*1*].  It gives appearance that
> the change is making it more broad, but not making it broad enough.
> At the same time, more than 90% of the time, people give a branch
> name there.  Saying "branch-or-commit" for a short description of
> the command line (which is what synopsis section is) does not add
> that much value.  The body text of the description where we refer to
> the <branch> parameter of course need to be updated to say "in
> addition, instead of a branch name, you can give a commit-ish that
> is *not* a branch name.  When you do so, instead of checking out the
> branch, the HEAD is detached at that commit before the history
> leading to it is rebased."
> 
> And because we have to say "it can be a non-branch commit-ish and
> here is what happens when you do so" anyway, I think the current
> synopsis as-is would be better than making it less clear and more
> scary by replacing <branch> with other things like <commit-ish> or
> [<branch> | <commit-ish>].
> 
> 
> [Footnote]
> 
> *1* As my "log --first-parent --oneline master..pu" is a strand of
>     pearls each of which is a merge of a topic branch,
> 
>     $ git rebase -i master pu~$n^2
> 
>     can be a handy way to make a trial rebase (after double checking
>     the result of "git tbdiff", I may do "git checkout -B topic" to
>     overwrite the original or I may discard the result of rebasing).

Re: [PATCH v2 0/3] rebase: give precise error message

[Junio C Hamano]

Kaartic Sivaraam <kaartic.sivaraam@gmail.com> writes:

>> I do not think the above is a good change in the first place for at
>> least two reasons.  By saying <ref>, the updated text says "not just
>> branches but you can also give tags and remote-tracking branches".
>
> I used <ref> as you could actually use tags, remote-tracking branches
> and even "notes" ...
> ...
> It just works (of course, I couldn't understand what it did)! I didn't
> want to lie to the user. So, I used <ref>. So, should we just update
> the <branch> part of the doc or should we update the code for 'rebase'?
> I'm unsure.

By saying <ref>, you are not covering these cases

	git rebase master HEAD^0
	git rebase master pu^2

where the command gets non refs.

Most of the time, people use a <branch>, and rare cases like these
what a user can give is not restricted to a <ref>, so there is *no*
value in replacing <branch> with <ref>.  If we needed to replace it
with something, replacing <branch> with [<branch> | <commit-ish>] is
not wrong per-se, but I do not think it is an improvement.

As <branch> is merely a kind of <commit-ish>, it may be tempting to
instead replace <branch> with <commit-ish>, but I do not think it is
a good idea, either.  No matter what you write there in the synopsis
(and let's call it X), the description would have to say "when X is
the name of a branch, that branch is checked out, its history gets
rebased, and at the end, the tip of that branch points at the
result.  When X is not a branch but just a commit-ish, the HEAD is
detached at that commit, its history gets rebased and you'll be left
in that state".  Having <branch> in that sentence is clear enough
and any intelligent reader would understand what we mean by that
notation: we are showing there can be various things that can come
on the command line depicted in the SYNOPSIS section at the point
where we have a placeholder called <branch>, and the argument does
not necessarily have to be the name of a branch.

[PATCH v5 0/3] rebase: give precise error messages

[Kaartic Sivaraam]

The tip of the v4 of this patch can be found at [1]. It was a revamp
sent by Junio mostly touching [PATCH v2 1/3] of the series. I've updated
it a little to add in something of my taste ;-)

There's only one concern that still bothers me a little. With the current
code you would see the following,

    $ git rebase origin/maint 3013dff86
    Current branch 3013dff86 is up to date.

That doesn't look good, does it? How about we overcome the issue of
handling this case and the HEAD case done in 3/3 by simplifying the
message as shown in the following diff,

diff --git a/git-rebase.sh b/git-rebase.sh
index 0f379ba2b..4d5400034 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -601,11 +601,11 @@ then
                test -z "$switch_to" ||
                GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
                        git checkout -q "$switch_to" --
-               say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+               say "$(eval_gettext "\$branch_name is up to date.")"
                finish_rebase
                exit 0
        else
-               say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+               say "$(eval_gettext "\$branch_name is up to date, rebase forced.")"
        fi
 fi

I guess this one is much better than 3/3 of this series as it handles
any kind of case by making no assumptions.

Thoughts ??


Note: In case you're wondering where's v3 of this series, there wasn't
a v3 series but there was a v3 PATCH of 3/3 [2].

References:

[1]: <xmqq1sjxt3tz.fsf@gitster.mtv.corp.google.com>

[2]: <20171201060935.19749-1-kaartic.sivaraam@gmail.com>

Here's the interdiff between v4 and v5,

diff --git a/git-rebase.sh b/git-rebase.sh
index f3dd86443..fd72a35c6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -518,7 +518,7 @@ case "$onto_name" in
 esac
 
 # If the branch to rebase is given, that is the branch we will rebase
-# $branch_name -- branch being rebased, or HEAD (already detached)
+# $branch_name -- branch/commit being rebased, or HEAD (already detached)
 # $orig_head -- commit object name of tip of the branch before rebasing
 # $head_name -- refs/heads/<that-branch> or "detached HEAD"
 switch_to=
@@ -602,7 +602,7 @@ then
                GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
                        git checkout -q "$switch_to" --
                if test "$branch_name" = "HEAD" &&
-                        !(git symbolic-ref -q HEAD)
+                        ! git symbolic-ref -q HEAD
                then
                        say "$(eval_gettext "HEAD is up to date.")"
                else
@@ -612,7 +612,7 @@ then
                exit 0
        else
                if test "$branch_name" = "HEAD" &&
-                        !(git symbolic-ref -q HEAD)
+                        ! git symbolic-ref -q HEAD
                then
                        say "$(eval_gettext "HEAD is up to date, rebase forced.")"
                else



Kaartic Sivaraam (3):
  rebase: consistently use branch_name variable
  rebase: distinguish user input by quoting it
  rebase: rebasing can also be done when HEAD is detached

 git-rebase.sh | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

-- 
2.15.0.531.g2ccb3012c

[PATCH v5 1/3] rebase: consistently use branch_name variable

[Kaartic Sivaraam]

The variable "branch_name" holds the <branch> parameter in "git
rebase <upstream> <branch>", but one codepath did not use it after
assigning $1 to it (instead it kept using $1).  Make it use the
variable consistently.

Also, update an error message to say there is no such branch or
commit, as we are expecting either of them, and not limiting
ourselves to a branch name.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 git-rebase.sh | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 60b70f3de..a299bcc52 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -518,7 +518,7 @@ case "$onto_name" in
 esac
 
 # If the branch to rebase is given, that is the branch we will rebase
-# $branch_name -- branch being rebased, or HEAD (already detached)
+# $branch_name -- branch/commit being rebased, or HEAD (already detached)
 # $orig_head -- commit object name of tip of the branch before rebasing
 # $head_name -- refs/heads/<that-branch> or "detached HEAD"
 switch_to=
@@ -528,15 +528,18 @@ case "$#" in
 	branch_name="$1"
 	switch_to="$1"
 
-	if git show-ref --verify --quiet -- "refs/heads/$1" &&
-	   orig_head=$(git rev-parse -q --verify "refs/heads/$1")
+	# Is it a local branch?
+	if git show-ref --verify --quiet -- "refs/heads/$branch_name" &&
+	   orig_head=$(git rev-parse -q --verify "refs/heads/$branch_name")
 	then
-		head_name="refs/heads/$1"
-	elif orig_head=$(git rev-parse -q --verify "$1")
+		head_name="refs/heads/$branch_name"
+	# If not is it a valid ref (branch or commit)?
+	elif orig_head=$(git rev-parse -q --verify "$branch_name")
 	then
 		head_name="detached HEAD"
+
 	else
-		die "$(eval_gettext "fatal: no such branch: \$branch_name")"
+		die "$(eval_gettext "fatal: no such branch/commit: \$branch_name")"
 	fi
 	;;
 0)
@@ -547,7 +550,7 @@ case "$#" in
 		branch_name=$(expr "z$branch_name" : 'zrefs/heads/\(.*\)')
 	else
 		head_name="detached HEAD"
-		branch_name=HEAD ;# detached
+		branch_name=HEAD
 	fi
 	orig_head=$(git rev-parse --verify HEAD) || exit
 	;;
-- 
2.15.0.531.g2ccb3012c

[PATCH v5 2/3] rebase: distinguish user input by quoting it

[Kaartic Sivaraam]

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 git-rebase.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index a299bcc52..0f379ba2b 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -477,7 +477,7 @@ then
 		;;
 	esac
 	upstream=$(peel_committish "${upstream_name}") ||
-	die "$(eval_gettext "invalid upstream \$upstream_name")"
+	die "$(eval_gettext "invalid upstream '\$upstream_name'")"
 	upstream_arg="$upstream_name"
 else
 	if test -z "$onto"
@@ -539,7 +539,7 @@ case "$#" in
 		head_name="detached HEAD"
 
 	else
-		die "$(eval_gettext "fatal: no such branch/commit: \$branch_name")"
+		die "$(eval_gettext "fatal: no such branch/commit '\$branch_name'")"
 	fi
 	;;
 0)
-- 
2.15.0.531.g2ccb3012c

[PATCH v5 3/3] rebase: rebasing can also be done when HEAD is detached

[Kaartic Sivaraam]

Attempting to rebase when the HEAD is detached and is already
up to date with upstream (so there's nothing to do), the
following message is shown

        Current branch HEAD is up to date.

which is clearly wrong as HEAD is not a branch.

Handle the special case of HEAD correctly to give a more precise
error message.

Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
---
 git-rebase.sh | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/git-rebase.sh b/git-rebase.sh
index 0f379ba2b..fd72a35c6 100755
--- a/git-rebase.sh
+++ b/git-rebase.sh
@@ -601,11 +601,23 @@ then
 		test -z "$switch_to" ||
 		GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $switch_to" \
 			git checkout -q "$switch_to" --
-		say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+		if test "$branch_name" = "HEAD" &&
+			 ! git symbolic-ref -q HEAD
+		then
+			say "$(eval_gettext "HEAD is up to date.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_name is up to date.")"
+		fi
 		finish_rebase
 		exit 0
 	else
-		say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+		if test "$branch_name" = "HEAD" &&
+			 ! git symbolic-ref -q HEAD
+		then
+			say "$(eval_gettext "HEAD is up to date, rebase forced.")"
+		else
+			say "$(eval_gettext "Current branch \$branch_name is up to date, rebase forced.")"
+		fi
 	fi
 fi
 
-- 
2.15.0.531.g2ccb3012c