[PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Previous description of -f option was wrong as "git rebase" does not
require -f to perform rebase when "current branch is a descendant of
the commit you are rebasing onto", provided commit(s) to be rebased
contain merge(s).

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-rebase.txt | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..62dac31 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,10 +316,9 @@ which makes little sense.
 
 -f::
 --force-rebase::
-	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally non-interactive rebase will
-	exit with the message "Current branch is up to date" in such a
-	situation.
+	Force the rebase even if the result will only change commit
+	timestamps. Normally non-interactive rebase will exit with the
+	message "Current branch is up to date" in such a situation.
 	Incompatible with the --interactive option.
 +
 You may find this (or --no-ff with an interactive rebase) helpful after
-- 
1.9.3

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> Previous description of -f option was wrong as "git rebase" does not
> require -f to perform rebase when "current branch is a descendant of
> the commit you are rebasing onto", provided commit(s) to be rebased
> contain merge(s).

Both the above and the updated documentation are a bit hard to read
for me, so let me disect what you are and you are not saying to see
if I understood them correctly:

 - The plain-vanilla "git rebase" that flattens the history will be
   a no-op *only* when the current tip is a linear descendant of the
   "onto" commit without any merge in between.

 - Merge-preserving form of "git rebase -m/-p" is a no-op when the
   current tip is a descendant of the "onto" commit.

 - "rebase -f" is a way to force rebase when it would otherwise be a
   no-op.

 - When you force a rebase that would otherwise be a no-op, only the
   timestamps would change.

I think you are right that 'current branch is a descendant of the
"onto" commit' is not necessarily equal to 'rebase that would
otherwise be a no-op'.  I am not sure if a 'rebase that would
otherwise be a no-op' is equal to 'only timestamp would change',
though.  What happens if you do this, for example?

    $ GIT_COMMITTER_NAME='Somebody Else' git commit
    $ git rebase --force --onto HEAD^ HEAD^ HEAD^0

So I think the reasoning (i.e. "is a descendant" is not quite right)
is correct, but the updated text is not quite right.  Changing it
further to "only the committer timestamps and identities would
change" is probably not an improvement, either.  "Force the rebase
that would otherwise be a no-op" may be a better phrasing that does
not risk going stale even if we update what are preserved and what
are modified in the future.

Also I notice the sentence "Normally non-interactive...in such a
situation" is not helping the reader in this description very much.
I wonder if we should keep it if we are rewriting this paragraph.

Thanks.

> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-rebase.txt | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..62dac31 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,10 +316,9 @@ which makes little sense.
>  
>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> +	Force the rebase even if the result will only change commit
> +	timestamps. Normally non-interactive rebase will exit with the
> +	message "Current branch is up to date" in such a situation.
>  	Incompatible with the --interactive option.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> So I think the reasoning (i.e. "is a descendant" is not quite right)
> is correct, but the updated text is not quite right.  Changing it
> further to "only the committer timestamps and identities would
> change" is probably not an improvement, either.  "Force the rebase
> that would otherwise be a no-op" may be a better phrasing that does
> not risk going stale even if we update what are preserved and what
> are modified in the future.
>
> Also I notice the sentence "Normally non-interactive...in such a
> situation" is not helping the reader in this description very much.
> I wonder if we should keep it if we are rewriting this paragraph.

How about doing it this way, perhaps?

-- >8 --
From: Sergey Organov <sorganov@gmail.com>
Date: Tue, 12 Aug 2014 00:22:48 +0400
Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op

"Current branch is a descendant of the commit you are rebasing onto"
does not necessarily mean "rebase" requires "--force".  For a plain
vanilla "history flattening" rebase, the rebase can be done without
forcing if there is a merge between the tip of the branch being
rebased and the commit you are rebasing onto, even if the tip is
descendant of the other.

[jc: reworded both the text and the log description]

Signed-off-by: Sergey Organov <sorganov@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-rebase.txt | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..f14100a 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,11 +316,8 @@ which makes little sense.
 
 -f::
 --force-rebase::
-	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally non-interactive rebase will
-	exit with the message "Current branch is up to date" in such a
-	situation.
-	Incompatible with the --interactive option.
+	Force a rebase even if the current branch is up-to-date and
+	the command without `--force` would return without doing anything.
 +
 You may find this (or --no-ff with an interactive rebase) helpful after
 reverting a topic branch merge, as this option recreates the topic branch with
-- 
2.1.0-rc2-238-g2566d2d

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> So I think the reasoning (i.e. "is a descendant" is not quite right)
>> is correct, but the updated text is not quite right.  Changing it
>> further to "only the committer timestamps and identities would
>> change" is probably not an improvement, either.  "Force the rebase
>> that would otherwise be a no-op" may be a better phrasing that does
>> not risk going stale even if we update what are preserved and what
>> are modified in the future.
>>
>> Also I notice the sentence "Normally non-interactive...in such a
>> situation" is not helping the reader in this description very much.
>> I wonder if we should keep it if we are rewriting this paragraph.
>
> How about doing it this way, perhaps?

[...]

>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> -	Incompatible with the --interactive option.
> +	Force a rebase even if the current branch is up-to-date and
> +	the command without `--force` would return without doing anything.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after
>  reverting a topic branch merge, as this option recreates the topic branch with

It's OK with me, as in fact I think that there is no good explanation
for current git behavior, and thus it's git behavior that should have
been changed, not the documentation. I.e., git must not rebase anything
when "Current branch is a descendant of the commit you are rebasing
onto", unless -f is given. Simple, reasonable, straightforward.

The version you propose at least does not lie, so it's definiteley an
improvement. However,

"Force the rebase when the command exits with "Current branch is up to
date" message."

reads even more simple and clear for me.

-- 
Sergey.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

> ... I.e., git must not rebase anything
> when "Current branch is a descendant of the commit you are rebasing
> onto", unless -f is given. Simple, reasonable, straightforward.

It may be simple and straightforward, but breaks the use case the
plain vanilla rebase is used for, doesn't it?  You seem to ignore,
or perhaps you don't realize the existence of, the need of those who
want to flatten the history on top of the tip from the other side;
your statements in the "pull --rebase[=preserve]" thread may be
coming from the same place, I suspect.

For the "flatten the history on top of that commit" use case, two
conditions must be satisfied before the command can say "the history
is already in the desired shape and nothing needs to be done" to
allow it to make a short-cut.  It is not sufficient for the current
tip to be merely descendant of the tip from the other side (which is
the documentation bug we are fixing here).  The history between the
two commits need also to be linear, or it would not be flattening.

Punting when when only one of the two conditions is met and
requiring "--force" to perform what the user wanted to ask the
command to do does not fall into the "reasonable" category.

If your workflow does not involve the flattening plain vanilla
"rebase", not using it is perfectly fine.  But please do not break
it for other people only because you do not use it yourself.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> So I think the reasoning (i.e. "is a descendant" is not quite right)
>> is correct, but the updated text is not quite right.  Changing it
>> further to "only the committer timestamps and identities would
>> change" is probably not an improvement, either.  "Force the rebase
>> that would otherwise be a no-op" may be a better phrasing that does
>> not risk going stale even if we update what are preserved and what
>> are modified in the future.
>>
>> Also I notice the sentence "Normally non-interactive...in such a
>> situation" is not helping the reader in this description very much.
>> I wonder if we should keep it if we are rewriting this paragraph.
>
> How about doing it this way, perhaps?
>
> -- >8 --
> From: Sergey Organov <sorganov@gmail.com>
> Date: Tue, 12 Aug 2014 00:22:48 +0400
> Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op
>
> "Current branch is a descendant of the commit you are rebasing onto"
> does not necessarily mean "rebase" requires "--force".  For a plain
> vanilla "history flattening" rebase, the rebase can be done without
> forcing if there is a merge between the tip of the branch being
> rebased and the commit you are rebasing onto, even if the tip is
> descendant of the other.
>
> [jc: reworded both the text and the log description]
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-rebase.txt | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..f14100a 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,11 +316,8 @@ which makes little sense.
>  
>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> -	Incompatible with the --interactive option.
> +	Force a rebase even if the current branch is up-to-date and
> +	the command without `--force` would return without doing anything.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after
>  reverting a topic branch merge, as this option recreates the topic branch with


Junio C Hamano <gitster@pobox.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> So I think the reasoning (i.e. "is a descendant" is not quite right)
>> is correct, but the updated text is not quite right.  Changing it
>> further to "only the committer timestamps and identities would
>> change" is probably not an improvement, either.  "Force the rebase
>> that would otherwise be a no-op" may be a better phrasing that does
>> not risk going stale even if we update what are preserved and what
>> are modified in the future.
>>
>> Also I notice the sentence "Normally non-interactive...in such a
>> situation" is not helping the reader in this description very much.
>> I wonder if we should keep it if we are rewriting this paragraph.
>
> How about doing it this way, perhaps?
>
> -- >8 --
> From: Sergey Organov <sorganov@gmail.com>
> Date: Tue, 12 Aug 2014 00:22:48 +0400
> Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op
>
> "Current branch is a descendant of the commit you are rebasing onto"
> does not necessarily mean "rebase" requires "--force".  For a plain
> vanilla "history flattening" rebase, the rebase can be done without
> forcing if there is a merge between the tip of the branch being
> rebased and the commit you are rebasing onto, even if the tip is
> descendant of the other.
>
> [jc: reworded both the text and the log description]
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-rebase.txt | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..f14100a 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,11 +316,8 @@ which makes little sense.
>  
>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> -	Incompatible with the --interactive option.
> +	Force a rebase even if the current branch is up-to-date and
> +	the command without `--force` would return without doing anything.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after
>  reverting a topic branch merge, as this option recreates the topic branch with

I dig more into it, and that's what I came up with, using some of your
suggestions as well.

Please notice new text on essential interaction with --preserve-merges.

I also thought about "Force the rebase that would otherwise be a no-op",
and while it is future-changes-agnostic indeed, it doesn't actually
explain anything, so I put some explanation back.

-- >8 --

From: Sergey Organov <sorganov@gmail.com>
Date: Tue, 12 Aug 2014 00:10:19 +0400
Subject: [PATCH] Documentation/git-rebase.txt: fix -f description to match

"Current branch is a descendant of the commit you are rebasing onto"
does not necessarily mean "rebase" requires "--force". Presence of
merge commit(s) makes "rebase" perform its default flattening actions
anyway.

Signed-off-by: Sergey Organov <sorganov@gmail.com>
---
 Documentation/git-rebase.txt | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 2a93c64..9153369 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -316,11 +316,10 @@ which makes little sense.
 
 -f::
 --force-rebase::
-	Force the rebase even if the current branch is a descendant
-	of the commit you are rebasing onto.  Normally non-interactive rebase will
-	exit with the message "Current branch is up to date" in such a
-	situation.
-	Incompatible with the --interactive option.
+	If --preserve-merges is given, has no effect. Otherwise forces
+	rebase even if the current branch is a descendant of the commit
+	you are rebasing onto and there are no merge commits among
+	those to be rebased.
 +
 You may find this (or --no-ff with an interactive rebase) helpful after
 reverting a topic branch merge, as this option recreates the topic branch with
-- 
1.9.3

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>> ...
>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>> index 2a93c64..f14100a 100644
>> --- a/Documentation/git-rebase.txt
>> +++ b/Documentation/git-rebase.txt
>> @@ -316,11 +316,8 @@ which makes little sense.
>>  
>>  -f::
>>  --force-rebase::
>> -	Force the rebase even if the current branch is a descendant
>> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
>> -	exit with the message "Current branch is up to date" in such a
>> -	situation.
>> -	Incompatible with the --interactive option.
>> +	Force a rebase even if the current branch is up-to-date and
>> +	the command without `--force` would return without doing anything.
>>  +
>>  You may find this (or --no-ff with an interactive rebase) helpful after
>>  reverting a topic branch merge, as this option recreates the topic branch with
>
> I dig more into it, and that's what I came up with, using some of your
> suggestions as well.
>
> Please notice new text on essential interaction with --preserve-merges.
>
> I also thought about "Force the rebase that would otherwise be a no-op",
> and while it is future-changes-agnostic indeed, it doesn't actually
> explain anything, so I put some explanation back.

A sentence "--force has no effect under --preserve-merges mode" does
not tell the readers very much, either and leaves them wondering if
it means "--preserve-merges mode always rebases every time it is
asked, never noticing 'ah, the history is already in a good shape
and there is no need to do anything further'" or "--preserve-merges
mode ignores --force and refuses to recreate the history if the
history is in the shape the mode deems is already desirable."

I think the root cause of the issue we share in this thread, when
trying to come up with an improvement of this part, is that we are
trying to put more explanation to the description of --force, but if
we step back a bit, it may be that the explanation does not belong
there.  As far as the readers are concerned, --force is about
forcing a rebase that would not otherwise be a no-op, but the real
issue is that the condition under which a requested rebase becomes a
no-op, iow, "the history is already in the desired shape, nothing to
do", is different from mode to mode, because "the desired shape" is
what distinguishes the modes.  Preserve-merge rebase may think that
a history that is descendant of the "onto" commit is already in the
desired shape while plain-vanilla rebase does not if it has a merge
in between, for example.

The sentence that follows "Otherwise" in this version encourages the
readers to be in a wrong mind-set that rebase is only about "making
the branch a descendant of the 'onto' commit", which isn't the case.

The desired outcome depends on the mode (and that is why there are
modes), and not saying that explicitly will only help spread the
confusion, I am afraid.  Isn't it a better solution to explain what
that no-op condition is for the mode at the place in the document
where we describe each mode?

E.g. under "--preserve-merges" heading, we may say "make sure the
history is a descendant of the 'onto' commit; if it already is,
nothing is done because there is no need to do anything" or
something along that line.  The description for the plain-vanilla
rebase may say "flatten the history on top of the 'onto' commit by
replaying the changes in each non-merge commit; if the history is
already a descendant of the 'onto' commit without any merge in
between, nothing is done because there is no need to".

That would make description of the modes more understandable, too.
The users can read what kind of resulting history they can get out
of by using each mode in one place.

Hmm?

> -- >8 --
>
> From: Sergey Organov <sorganov@gmail.com>
> Date: Tue, 12 Aug 2014 00:10:19 +0400
> Subject: [PATCH] Documentation/git-rebase.txt: fix -f description to match
>
> "Current branch is a descendant of the commit you are rebasing onto"
> does not necessarily mean "rebase" requires "--force". Presence of
> merge commit(s) makes "rebase" perform its default flattening actions
> anyway.
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> ---
>  Documentation/git-rebase.txt | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..9153369 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,11 +316,10 @@ which makes little sense.
>  
>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> -	Incompatible with the --interactive option.
> +	If --preserve-merges is given, has no effect. Otherwise forces
> +	rebase even if the current branch is a descendant of the commit
> +	you are rebasing onto and there are no merge commits among
> +	those to be rebased.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after
>  reverting a topic branch merge, as this option recreates the topic branch with

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> ...
>>> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
>>> index 2a93c64..f14100a 100644
>>> --- a/Documentation/git-rebase.txt
>>> +++ b/Documentation/git-rebase.txt
>>> @@ -316,11 +316,8 @@ which makes little sense.
>>>  
>>>  -f::
>>>  --force-rebase::
>>> -	Force the rebase even if the current branch is a descendant
>>> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
>>> -	exit with the message "Current branch is up to date" in such a
>>> -	situation.
>>> -	Incompatible with the --interactive option.
>>> +	Force a rebase even if the current branch is up-to-date and
>>> +	the command without `--force` would return without doing anything.
>>>  +
>>>  You may find this (or --no-ff with an interactive rebase) helpful after
>>>  reverting a topic branch merge, as this option recreates the topic branch with
>>
>> I dig more into it, and that's what I came up with, using some of your
>> suggestions as well.
>>
>> Please notice new text on essential interaction with --preserve-merges.
>>
>> I also thought about "Force the rebase that would otherwise be a no-op",
>> and while it is future-changes-agnostic indeed, it doesn't actually
>> explain anything, so I put some explanation back.
>
> A sentence "--force has no effect under --preserve-merges mode" does
> not tell the readers very much, either and leaves them wondering if
> it means "--preserve-merges mode always rebases every time it is
> asked, never noticing 'ah, the history is already in a good shape
> and there is no need to do anything further'" or "--preserve-merges
> mode ignores --force and refuses to recreate the history if the
> history is in the shape the mode deems is already desirable."

In fact there is no way to force rebase when --preserve-merges is given.
Neither --force nor --no-ff has any effect.

Maybe some clarification could be given in --preserve-merges
description, provided it's not clear that "has no effect" for --force
means that one can't force the rebase in this case.

> I think the root cause of the issue we share in this thread, when
> trying to come up with an improvement of this part, is that we are
> trying to put more explanation to the description of --force, but if
> we step back a bit, it may be that the explanation does not belong
> there.  As far as the readers are concerned, --force is about
> forcing a rebase that would not otherwise be a no-op, but the real
> issue is that the condition under which a requested rebase becomes a
> no-op, iow, "the history is already in the desired shape, nothing to
> do", is different from mode to mode, because "the desired shape" is
> what distinguishes the modes.  Preserve-merge rebase may think that
> a history that is descendant of the "onto" commit is already in the
> desired shape while plain-vanilla rebase does not if it has a merge
> in between, for example.

I think the root cause is even deeper, in the current design of the
rebase interface, but those my opinion you already know and I'll leave
discussion for another post that I currently try to formulate.

> The sentence that follows "Otherwise" in this version encourages the
> readers to be in a wrong mind-set that rebase is only about "making
> the branch a descendant of the 'onto' commit", which isn't the case.

I'm not happy with the wording myself either.

> The desired outcome depends on the mode (and that is why there are
> modes), and not saying that explicitly will only help spread the
> confusion, I am afraid.  Isn't it a better solution to explain what
> that no-op condition is for the mode at the place in the document
> where we describe each mode?
>
> E.g. under "--preserve-merges" heading, we may say "make sure the
> history is a descendant of the 'onto' commit; if it already is,
> nothing is done because there is no need to do anything" or
> something along that line.  The description for the plain-vanilla
> rebase may say "flatten the history on top of the 'onto' commit by
> replaying the changes in each non-merge commit; if the history is
> already a descendant of the 'onto' commit without any merge in
> between, nothing is done because there is no need to".
>
> That would make description of the modes more understandable, too.
> The users can read what kind of resulting history they can get out
> of by using each mode in one place.

I think you've lost me here, though I think that all the suggested
variants are still better than what is there right now.

-- 
Sergey.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>> A sentence "--force has no effect under --preserve-merges mode" does
>> not tell the readers very much, either and leaves them wondering if
>> it means "--preserve-merges mode always rebases every time it is
>> asked, never noticing 'ah, the history is already in a good shape
>> and there is no need to do anything further'" or "--preserve-merges
>> mode ignores --force and refuses to recreate the history if the
>> history is in the shape the mode deems is already desirable."
>
> In fact there is no way to force rebase when --preserve-merges is given.
> Neither --force nor --no-ff has any effect.
>
> Maybe some clarification could be given in --preserve-merges
> description, provided it's not clear that "has no effect" for --force
> means that one can't force the rebase in this case.

I am not sure if that is an intended behaviour or simply a bug (I
rarely use preserve-merges myself, so I offhand do not know for
certain).

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> A sentence "--force has no effect under --preserve-merges mode" does
>>> not tell the readers very much, either and leaves them wondering if
>>> it means "--preserve-merges mode always rebases every time it is
>>> asked, never noticing 'ah, the history is already in a good shape
>>> and there is no need to do anything further'" or "--preserve-merges
>>> mode ignores --force and refuses to recreate the history if the
>>> history is in the shape the mode deems is already desirable."
>>
>> In fact there is no way to force rebase when --preserve-merges is given.
>> Neither --force nor --no-ff has any effect.
>>
>> Maybe some clarification could be given in --preserve-merges
>> description, provided it's not clear that "has no effect" for --force
>> means that one can't force the rebase in this case.
>
> I am not sure if that is an intended behaviour or simply a bug

I think nobody actually ever needed to make it work, even though
fundamentally it could have the same usage as in the case of flattening
rebase. Once again, it seems that most uses of rebase handle already
flat history and thus are served by vanilla invocation.

> (I rarely use preserve-merges myself, so I offhand do not know for
> certain).

I wonder, don't you yourself use preserve-merges because you don't care
and just use the default, or because you actually use vanilla
history-flattening feature?

-- 
Sergey.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>> ... I.e., git must not rebase anything
>> when "Current branch is a descendant of the commit you are rebasing
>> onto", unless -f is given. Simple, reasonable, straightforward.
>
> It may be simple and straightforward, but breaks the use case the
> plain vanilla rebase is used for, doesn't it?

I hope it does not. I don't think plain vanilla rebase's "flattening"
feature is usually used, as those who care about flat history mostly
don't merge. Either not at all, or do merges temporarily and undo them
later. At least that's the impression I've got from recent conversations
about the issue.

Moreover, I'm afraid rare person would correctly predict the result of
history flattening he will get from git rebase of even remotely complex
graph.

That said, I'm still to hear from somebody who would actually suffer. I
mean I see some use-case(s), but they: 1) are unlikely to be used in
practice. 2) are better handled by other means. 3) abuse bug/feature
that contradicts documentation. 4) won't be broken that hard anyway.

> You seem to ignore, or perhaps you don't realize the existence of, the
> need of those who want to flatten the history on top of the tip from
> the other side;

No, I do realize and I don't ignore the possibility. Please recall that
I didn't suggest to get rid of any functionality that is already there.

I didn't want to get into lengthy discussion about it, but here we are.

In fact there are 2 issues here (please notice that I don't cut any
existing functionality):

1. Better design of interface to "git rebase" features.

2. How far we are currently from those "better", is it worth the trouble
   to move in the direction of "better" at all, and how far and how fast
   we are going to move, considering backward compatibility issues.

If we don't agree on (1), i.e., where is point B, there is no reason to
discuss (2), i.e., the way from point A to point B. I'm trying to
discuss (1), and you are mostly objecting by pointing me to the fact
that changes could break some workflow, i.e., by discussing (2).

What about those better design? Here is my bet:

1. "git rebase" with no options takes a set of commits and rebases them
   on top of another commit, preserving precious history as much as
   possible.

   Reason: most common operation: "rebase my work on top of different
   commit". Nothing more, nothing less. That's what --preserve mode does
   right now. I.e., the result should be as close as possible to the
   case where I had started to do all the work on top of the new base
   commit from the beginning.

2. "git rebase" onto the same base should be no-op, unless --force
   option is given.

   Reason: if command changes base, it's natural to do nothing when base
   is not changed. Alternatively, rebase would have to be performed
   always, and commit dates etc. would be changed as a result. This
   would be just annoying most of times.

3. "git rebase" may have an option to flatten the history instead of
   preserving its shape (--flatten or some such). This mode may imply
   some relaxing of (2) by always performing the rebase when there are
   merge commits among those to be rebased.

   Reason: somebody somewhere could probably find a real use for such a
   feature. BTW, there could be different modes of flattening, so this
   option may take corresponding values (e.g., squash every merged set
   of commits to a single commit). Please also notice that function of
   (3) could be achieved by other means, so it's not a requirement,
   rather a convenience (that's why I said "may have an option").

We can discuss this if somebody interested. If not, let's just fix
documentation to match current historical design, please.

> your statements in the "pull --rebase[=preserve]" thread may be
> coming from the same place, I suspect.

Yes, it's the same place. It's all started from a big trouble I ran into
by simply following current git documentation for "git pull" and "git
rebase" and from assumption that defaults are usually safe (and sane,
yes, in my understanding of "sane" ;-) ).

And yes, they share the same reasoning. That is, by default a command
must perform its primary job, and only it. Everything else should be an
option. For "git pull" it'd mean to fetch the changes from upstream, and
then either merge or rebase to integrate with them. For "git pull", my
plan would be to introduce new "pull.rebase=flatten" value and deprecate
"pull.rebase=true". Bare "pull --rebase" should be deprecated as well
in favor of explicit "pull --rebase=flatten", then later "pull
--rebase" could be given saner and safer meaning of "pull
--rebase=preserve", or rather removed.

> For the "flatten the history on top of that commit" use case, two
> conditions must be satisfied before the command can say "the history
> is already in the desired shape and nothing needs to be done" to
> allow it to make a short-cut.  It is not sufficient for the current
> tip to be merely descendant of the tip from the other side (which is
> the documentation bug we are fixing here).  The history between the
> two commits need also to be linear, or it would not be flattening.
>
> Punting when when only one of the two conditions is met and
> requiring "--force" to perform what the user wanted to ask the
> command to do does not fall into the "reasonable" category.

Current behavior you described above would be "reasonable" for "git
flatten" command. If it were reasonable for "git rebase", why is it
that difficult to correctly document it? Good design is usually easily
documentable.

> If your workflow does not involve the flattening plain vanilla
> "rebase", not using it is perfectly fine.  But please do not break
> it for other people only because you do not use it yourself.

I only aim at making "git rebase" better. Backward compatibility
shouldn't be forgotten, but you seem to disagree that my suggestion(s)
do make it better in the first place, so no need to discuss any breakage
at this point.

Anyway, those imaginary people, if any, who would suffer from the change
that would require --force to rebase *on the same commit* currently
silently rely on the feature that contradicted documentation, and that
sometimes needs to be punished ;-)

-- 
Sergey.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Junio C Hamano]

Sergey Organov <sorganov@gmail.com> writes:

>> (I rarely use preserve-merges myself, so I offhand do not know for
>> certain).
>
> I wonder, don't you yourself use preserve-merges because you don't care
> and just use the default, or because you actually use vanilla
> history-flattening feature?

The latter.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:

> Sergey Organov <sorganov@gmail.com> writes:
>
>>> A sentence "--force has no effect under --preserve-merges mode" does
>>> not tell the readers very much, either and leaves them wondering if
>>> it means "--preserve-merges mode always rebases every time it is
>>> asked, never noticing 'ah, the history is already in a good shape
>>> and there is no need to do anything further'" or "--preserve-merges
>>> mode ignores --force and refuses to recreate the history if the
>>> history is in the shape the mode deems is already desirable."
>>
>> In fact there is no way to force rebase when --preserve-merges is given.
>> Neither --force nor --no-ff has any effect.
>>
>> Maybe some clarification could be given in --preserve-merges
>> description, provided it's not clear that "has no effect" for --force
>> means that one can't force the rebase in this case.
>
> I am not sure if that is an intended behaviour or simply a bug (I
> rarely use preserve-merges myself, so I offhand do not know for
> certain).

It seems that there is some problem there anyway. Probably a slight
misfeature rather than a bug, as --preserve-merges always pretends it
does something, even when it does not:

$ git log --oneline --graph --decorate
*   6f25bd5 (HEAD, topic) M
|\  
| * c5a4a43 C
* | 3c6ab7a (master) N
* | 99ff328 B
|/  
* 130ae2d A
$ git rebase --preserve-merges master
Successfully rebased and updated refs/heads/topic.
$ git log --oneline --graph --decorate
*   6f25bd5 (HEAD, topic) M
|\  
| * c5a4a43 C
* | 3c6ab7a (master) N
* | 99ff328 B
|/  
* 130ae2d A
$

"git reflog topic" doesn't show any changes either.

-- 
Sergey.

Re: [PATCH] Documentation/git-rebase.txt: fix -f description to match actual git behavior.

[Sergey Organov]

Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
[...]
> How about doing it this way, perhaps?

Could you please apply this your suggestion, as we seem not to agree
on anything better?

> -- >8 --
> From: Sergey Organov <sorganov@gmail.com>
> Date: Tue, 12 Aug 2014 00:22:48 +0400
> Subject: [PATCH] Documentation/git-rebase.txt: -f forces a rebase that would otherwise be a no-op
>
> "Current branch is a descendant of the commit you are rebasing onto"
> does not necessarily mean "rebase" requires "--force".  For a plain
> vanilla "history flattening" rebase, the rebase can be done without
> forcing if there is a merge between the tip of the branch being
> rebased and the commit you are rebasing onto, even if the tip is
> descendant of the other.
>
> [jc: reworded both the text and the log description]
>
> Signed-off-by: Sergey Organov <sorganov@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-rebase.txt | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 2a93c64..f14100a 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -316,11 +316,8 @@ which makes little sense.
>  
>  -f::
>  --force-rebase::
> -	Force the rebase even if the current branch is a descendant
> -	of the commit you are rebasing onto.  Normally non-interactive rebase will
> -	exit with the message "Current branch is up to date" in such a
> -	situation.
> -	Incompatible with the --interactive option.
> +	Force a rebase even if the current branch is up-to-date and
> +	the command without `--force` would return without doing anything.
>  +
>  You may find this (or --no-ff with an interactive rebase) helpful after
>  reverting a topic branch merge, as this option recreates the topic branch with

--  
Sergey.