Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Phillip Wood]

On 12/04/18 10:30, Johannes Schindelin wrote:

Hi Phillip,

On Wed, 11 Apr 2018, Phillip Wood wrote:


On 10/04/18 13:30, Johannes Schindelin wrote:

Firstly let me say that I think expanding the documentation and having an
example is an excellent idea.


Thanks! At first, I meant to leave this for others to contribute, but I
think it makes sense for me to describe it, as I do have a little bit of
experience with rebasing merges.


+
+
+label onto
+
+# Branch: refactor-button
+reset onto
+pick 123456 Extract a generic Button class from the DownloadButton one
+pick 654321 Use the Button class for all buttons
+label refactor-button
+
+# Branch: report-a-bug
+reset refactor-button # Use the Button class for all buttons
+pick abcdef Add the feedback button
+label report-a-bug
+
+reset onto
+merge -C a1b2c3 refactor-button # Merge 'refactor-button'
+merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
+
+
+In contrast to a regular interactive rebase, there are `label`, `reset` and
+`merge` commands in addition to `pick` ones.
+
+The `label` command puts a label to whatever will be the current


s/puts a label to/associates a label with/ would be clearer I think. Maybe
s/whatever will be the current revision/the current HEAD/ an well?


Thanks, I incorporated both changes here.


+revision when that command is executed. Internally, these labels are
+worktree-local refs that will be deleted when the rebase finishes or
+when it is aborted.


I agree they should be deleted when the rebase is aborted but I cannot see any
changes to git-rebase.sh to make that happen. I think they should also be
deleted by 'rebase --quit'.


Oh right! For some reason I thought I already hooked up rebase--helper
--abort when rebase was called with --abort or quit, but I had not managed
yet. I think I will leave this for later, or for GSoC, or something.

In the meantime, I'll just drop the "or when it is aborted.".


That way, rebase operations in multiple worktrees
+linked to the same repository do not interfere with one another.
+
+The `reset` command is essentially a `git reset --hard` to the specified
+revision (typically a previously-labeled one).


s/labeled/labelled/


As Eric pointed out, I am using 'murricane spelling here (or is it
speling? Ya never know these days).


:-)


I think it would be worthwhile to point out that unlike the other commands
this will not preserve untracked files. Maybe something like
"Note that unlike the `pick` or `merge` commands or initial checkout when the
rebase starts the `reset` command will overwrite any untracked files."


You know what? You just pointed out a bug in my thinking. Previously, I
thought that this is impossible, that you cannot overwrite untracked files
because we labeled this revision previously, so the only new files to
write by `reset` were tracked files previous. But that forgets `exec` and
`reset` with unlabeled revisions (e.g. for cousins).

So I changed the `reset` command to refuse overwriting untracked files...


That sounds like the safest plan

Thanks

Phillip


Thank you for improving this patch series!
Dscho

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Phillip Wood]

On 11/04/18 20:10, Eric Sunshine wrote:

On Wed, Apr 11, 2018 at 11:35 AM, Phillip Wood
 wrote:

On 10/04/18 13:30, Johannes Schindelin wrote:

+The `reset` command is essentially a `git reset --hard` to the specified
+revision (typically a previously-labeled one).


s/labeled/labelled/


American vs. British English spelling.


Ah, I'd forgotten that the American version only had one 'l'

Thanks

Phillip


CodingGuidelines and SubmittingPatches talk about this. Junio
summarizes the issue well in [1]. The TL;DR is to lean toward the
American English spelling.

[1]: https://public-inbox.org/git/xmqq4m9gpebm@gitster.mtv.corp.google.com/

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Jacob Keller]

On Thu, Apr 12, 2018 at 2:30 AM, Johannes Schindelin
 wrote:
>> I think it would be worthwhile to point out that unlike the other commands
>> this will not preserve untracked files. Maybe something like
>> "Note that unlike the `pick` or `merge` commands or initial checkout when the
>> rebase starts the `reset` command will overwrite any untracked files."
>
> You know what? You just pointed out a bug in my thinking. Previously, I
> thought that this is impossible, that you cannot overwrite untracked files
> because we labeled this revision previously, so the only new files to
> write by `reset` were tracked files previous. But that forgets `exec` and
> `reset` with unlabeled revisions (e.g. for cousins).
>
> So I changed the `reset` command to refuse overwriting untracked files...
>
> Thank you for improving this patch series!
> Dscho

Good catch! This could possibly have bitten someone badly.

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Sergey Organov]

Johannes Schindelin  writes:
> +
> +*   Merge branch 'report-a-bug'
> +|\
> +| * Add the feedback button
> +* | Merge branch 'refactor-button'
> +|\ \
> +| |/
> +| * Use the Button class for all buttons
> +| * Extract a generic Button class from the DownloadButton one
> +

Consider to put SHA1s into the diagram, as they are then used in
explanaitions. Hopefully I got them right here:


*   6f5e4d Merge branch 'report-a-bug'
|\
| * abcdef Add the feedback button
* | a1b2c3 Merge branch 'refactor-button'
|\ \
| |/
| * 654321 Use the Button class for all buttons
| * 123456 Extract a generic Button class from the DownloadButton one


Original explanation, just for reference, unchanged:

> +
> +label onto
> +
> +# Branch: refactor-button
> +reset onto
> +pick 123456 Extract a generic Button class from the DownloadButton one
> +pick 654321 Use the Button class for all buttons
> +label refactor-button
> +
> +# Branch: report-a-bug
> +reset refactor-button # Use the Button class for all buttons
> +pick abcdef Add the feedback button
> +label report-a-bug
> +
> +reset onto
> +merge -C a1b2c3 refactor-button # Merge 'refactor-button'
> +merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
> +

-- Sergey

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Johannes Schindelin]

Hi Phillip,

On Wed, 11 Apr 2018, Phillip Wood wrote:

> On 10/04/18 13:30, Johannes Schindelin wrote:
> 
> Firstly let me say that I think expanding the documentation and having an
> example is an excellent idea.

Thanks! At first, I meant to leave this for others to contribute, but I
think it makes sense for me to describe it, as I do have a little bit of
experience with rebasing merges.

> > +
> > +
> > +label onto
> > +
> > +# Branch: refactor-button
> > +reset onto
> > +pick 123456 Extract a generic Button class from the DownloadButton one
> > +pick 654321 Use the Button class for all buttons
> > +label refactor-button
> > +
> > +# Branch: report-a-bug
> > +reset refactor-button # Use the Button class for all buttons
> > +pick abcdef Add the feedback button
> > +label report-a-bug
> > +
> > +reset onto
> > +merge -C a1b2c3 refactor-button # Merge 'refactor-button'
> > +merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
> > +
> > +
> > +In contrast to a regular interactive rebase, there are `label`, `reset` and
> > +`merge` commands in addition to `pick` ones.
> > +
> > +The `label` command puts a label to whatever will be the current
> 
> s/puts a label to/associates a label with/ would be clearer I think. Maybe
> s/whatever will be the current revision/the current HEAD/ an well?

Thanks, I incorporated both changes here.

> > +revision when that command is executed. Internally, these labels are
> > +worktree-local refs that will be deleted when the rebase finishes or
> > +when it is aborted.
> 
> I agree they should be deleted when the rebase is aborted but I cannot see any
> changes to git-rebase.sh to make that happen. I think they should also be
> deleted by 'rebase --quit'.

Oh right! For some reason I thought I already hooked up rebase--helper
--abort when rebase was called with --abort or quit, but I had not managed
yet. I think I will leave this for later, or for GSoC, or something.

In the meantime, I'll just drop the "or when it is aborted.".

> > That way, rebase operations in multiple worktrees
> > +linked to the same repository do not interfere with one another.
> > +
> > +The `reset` command is essentially a `git reset --hard` to the specified
> > +revision (typically a previously-labeled one).
> 
> s/labeled/labelled/

As Eric pointed out, I am using 'murricane spelling here (or is it
speling? Ya never know these days).

> I think it would be worthwhile to point out that unlike the other commands
> this will not preserve untracked files. Maybe something like
> "Note that unlike the `pick` or `merge` commands or initial checkout when the
> rebase starts the `reset` command will overwrite any untracked files."

You know what? You just pointed out a bug in my thinking. Previously, I
thought that this is impossible, that you cannot overwrite untracked files
because we labeled this revision previously, so the only new files to
write by `reset` were tracked files previous. But that forgets `exec` and
`reset` with unlabeled revisions (e.g. for cousins).

So I changed the `reset` command to refuse overwriting untracked files...

Thank you for improving this patch series!
Dscho

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Johannes Schindelin]

Hi Eric & Phillip,

On Wed, 11 Apr 2018, Eric Sunshine wrote:

> On Wed, Apr 11, 2018 at 11:35 AM, Phillip Wood
>  wrote:
> > On 10/04/18 13:30, Johannes Schindelin wrote:
> >> +The `reset` command is essentially a `git reset --hard` to the specified
> >> +revision (typically a previously-labeled one).
> >
> > s/labeled/labelled/
> 
> American vs. British English spelling.
> 
> CodingGuidelines and SubmittingPatches talk about this. Junio
> summarizes the issue well in [1]. The TL;DR is to lean toward the
> American English spelling.
> 
> [1]: 
> https://public-inbox.org/git/xmqq4m9gpebm@gitster.mtv.corp.google.com/

Thanks, I meant to look that up because I was not sure, and now I do not
have to ;-)

No worries, Phillip, I will keep spelling your name with two `l`s. :0)

Ciao,
Dscho

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Eric Sunshine]

On Wed, Apr 11, 2018 at 11:35 AM, Phillip Wood
 wrote:
> On 10/04/18 13:30, Johannes Schindelin wrote:
>> +The `reset` command is essentially a `git reset --hard` to the specified
>> +revision (typically a previously-labeled one).
>
> s/labeled/labelled/

American vs. British English spelling.

CodingGuidelines and SubmittingPatches talk about this. Junio
summarizes the issue well in [1]. The TL;DR is to lean toward the
American English spelling.

[1]: https://public-inbox.org/git/xmqq4m9gpebm@gitster.mtv.corp.google.com/

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Phillip Wood]

On 10/04/18 13:30, Johannes Schindelin wrote:

Firstly let me say that I think expanding the documentation and having 
an example is an excellent idea.



+
+
+label onto
+
+# Branch: refactor-button
+reset onto
+pick 123456 Extract a generic Button class from the DownloadButton one
+pick 654321 Use the Button class for all buttons
+label refactor-button
+
+# Branch: report-a-bug
+reset refactor-button # Use the Button class for all buttons
+pick abcdef Add the feedback button
+label report-a-bug
+
+reset onto
+merge -C a1b2c3 refactor-button # Merge 'refactor-button'
+merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
+
+
+In contrast to a regular interactive rebase, there are `label`, `reset` and
+`merge` commands in addition to `pick` ones.
+
+The `label` command puts a label to whatever will be the current


s/puts a label to/associates a label with/ would be clearer I think. 
Maybe s/whatever will be the current revision/the current HEAD/ an well?



+revision when that command is executed. Internally, these labels are
+worktree-local refs that will be deleted when the rebase finishes or
+when it is aborted.


I agree they should be deleted when the rebase is aborted but I cannot 
see any changes to git-rebase.sh to make that happen. I think they 
should also be deleted by 'rebase --quit'.



That way, rebase operations in multiple worktrees
+linked to the same repository do not interfere with one another.
+
+The `reset` command is essentially a `git reset --hard` to the specified
+revision (typically a previously-labeled one).


s/labeled/labelled/

I think it would be worthwhile to point out that unlike the other 
commands this will not preserve untracked files. Maybe something like
"Note that unlike the `pick` or `merge` commands or initial checkout 
when the rebase starts the `reset` command will overwrite any untracked 
files."



Best Wishes

Phillip

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Johannes Schindelin]

Hi Martin,

On Tue, 10 Apr 2018, Martin Ågren wrote:

> On 10 April 2018 at 14:30, Johannes Schindelin
>  wrote:
> > The --rebase-merges mode is probably not half as intuitive to use as
> > its inventor hopes, so let's document it some.
> 
> I quite like this documentation. Well-structured and well-paced.
> Already after the first reading, I believe I understand how to use this.

Thanks!

> > +The `label` command puts a label to whatever will be the current
> > +revision when that command is executed. Internally, these labels are
> > +worktree-local refs that will be deleted when the rebase finishes or
> > +when it is aborted. That way, rebase operations in multiple worktrees
> > +linked to the same repository do not interfere with one another.
> 
> In the above paragraph, you say "internally".

I guess that I should reword this to say "These labels are created as
worktree-local refs (`refs/rewritten/`) that will be ..."

I'll do that, thanks for the sanity check!

> > +At this time, the `merge` command will *always* use the `recursive`
> > +merge strategy, with no way to choose a different one. To work around
> > +this, an `exec` command can be used to call `git merge` explicitly,
> > +using the fact that the labels are worktree-local refs (the ref
> > +`refs/rewritten/onto` would correspond to the label `onto`).
> 
> This sort of encourages use of that "internal" detail, which made me a
> little bit surprised at first. But if we can't come up with a reason why
> we would want to change the "refs/rewritten/"-concept later (I
> can't) and if we think the gain this paragraph gives is significant (it
> basically gives access to `git merge` in its entirety), then providing
> this hint might be the correct thing to do.

You are right. I made it sound as if this was an implementation detail
that you should not rely on, when I wanted to say that this is how it is
implemented and you are free to use it in your scripts.

> > +Note: the first command (`reset onto`) labels the revision onto which
> > +the commits are rebased; The name `onto` is just a convention, as a nod
> > +to the `--onto` option.
> 
> s/reset onto/label onto/

D'oh!

Thanks, fixed. Current state is in `sequencer-shears` in
https://github.com/dscho/git (I will update the `recreate-merges` branch,
which needs to keep its name so that my scripts will connect the mail
threads for the patch submissions, once I called `git rebase -kir @{u}`).

Ciao,
Dscho

Re: [PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Martin Ågren]

On 10 April 2018 at 14:30, Johannes Schindelin
 wrote:
> The --rebase-merges mode is probably not half as intuitive to use as
> its inventor hopes, so let's document it some.

I quite like this documentation. Well-structured and well-paced.
Already after the first reading, I believe I understand how to use this.

> +The `label` command puts a label to whatever will be the current
> +revision when that command is executed. Internally, these labels are
> +worktree-local refs that will be deleted when the rebase finishes or
> +when it is aborted. That way, rebase operations in multiple worktrees
> +linked to the same repository do not interfere with one another.

In the above paragraph, you say "internally".

> +At this time, the `merge` command will *always* use the `recursive`
> +merge strategy, with no way to choose a different one. To work around
> +this, an `exec` command can be used to call `git merge` explicitly,
> +using the fact that the labels are worktree-local refs (the ref
> +`refs/rewritten/onto` would correspond to the label `onto`).

This sort of encourages use of that "internal" detail, which made me a
little bit surprised at first. But if we can't come up with a reason why
we would want to change the "refs/rewritten/"-concept later (I
can't) and if we think the gain this paragraph gives is significant (it
basically gives access to `git merge` in its entirety), then providing
this hint might be the correct thing to do.

> +Note: the first command (`reset onto`) labels the revision onto which
> +the commits are rebased; The name `onto` is just a convention, as a nod
> +to the `--onto` option.

s/reset onto/label onto/

Martin

[PATCH v6 15/15] rebase -i --rebase-merges: add a section to the man page

[Johannes Schindelin]

The --rebase-merges mode is probably not half as intuitive to use as
its inventor hopes, so let's document it some.

Signed-off-by: Johannes Schindelin 
---
 Documentation/git-rebase.txt | 125 +++
 1 file changed, 125 insertions(+)

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index 8feadf6e663..be946de2efb 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -389,6 +389,8 @@ By default, or when `no-rebase-cousins` was specified, 
commits which do not
 have `` as direct ancestor will keep their original branch point.
 If the `rebase-cousins` mode is turned on, such commits are rebased onto
 `` (or ``, if specified).
++
+See also REBASING MERGES below.
 
 -p::
 --preserve-merges::
@@ -787,6 +789,129 @@ The ripple effect of a "hard case" recovery is especially 
bad:
 'everyone' downstream from 'topic' will now have to perform a "hard
 case" recovery too!
 
+REBASING MERGES
+-
+
+The interactive rebase command was originally designed to handle
+individual patch series. As such, it makes sense to exclude merge
+commits from the todo list, as the developer may have merged the
+current `master` while working on the branch, only to eventually
+rebase all the commits onto `master` (skipping the merge commits).
+
+However, there are legitimate reasons why a developer may want to
+recreate merge commits: to keep the branch structure (or "commit
+topology") when working on multiple, inter-related branches.
+
+In the following example, the developer works on a topic branch that
+refactors the way buttons are defined, and on another topic branch
+that uses that refactoring to implement a "Report a bug" button. The
+output of `git log --graph --format=%s -5` may look like this:
+
+
+*   Merge branch 'report-a-bug'
+|\
+| * Add the feedback button
+* | Merge branch 'refactor-button'
+|\ \
+| |/
+| * Use the Button class for all buttons
+| * Extract a generic Button class from the DownloadButton one
+
+
+The developer might want to rebase those commits to a newer `master`
+while keeping the branch topology, for example when the first topic
+branch is expected to be integrated into `master` much earlier than the
+second one, say, to resolve merge conflicts with changes to the
+DownloadButton class that made it into `master`.
+
+This rebase can be performed using the `--rebase-merges` option.
+It will generate a todo list looking like this:
+
+
+label onto
+
+# Branch: refactor-button
+reset onto
+pick 123456 Extract a generic Button class from the DownloadButton one
+pick 654321 Use the Button class for all buttons
+label refactor-button
+
+# Branch: report-a-bug
+reset refactor-button # Use the Button class for all buttons
+pick abcdef Add the feedback button
+label report-a-bug
+
+reset onto
+merge -C a1b2c3 refactor-button # Merge 'refactor-button'
+merge -C 6f5e4d report-a-bug # Merge 'report-a-bug'
+
+
+In contrast to a regular interactive rebase, there are `label`, `reset` and
+`merge` commands in addition to `pick` ones.
+
+The `label` command puts a label to whatever will be the current
+revision when that command is executed. Internally, these labels are
+worktree-local refs that will be deleted when the rebase finishes or
+when it is aborted. That way, rebase operations in multiple worktrees
+linked to the same repository do not interfere with one another.
+
+The `reset` command is essentially a `git reset --hard` to the specified
+revision (typically a previously-labeled one).
+
+The `merge` command will merge the specified revision into whatever is
+HEAD at that time. With `-C `, the commit message of
+the specified merge commit will be used. When the `-C` is changed to
+a lower-case `-c`, the message will be opened in an editor after a
+successful merge so that the user can edit the message.
+
+At this time, the `merge` command will *always* use the `recursive`
+merge strategy, with no way to choose a different one. To work around
+this, an `exec` command can be used to call `git merge` explicitly,
+using the fact that the labels are worktree-local refs (the ref
+`refs/rewritten/onto` would correspond to the label `onto`).
+
+Note: the first command (`reset onto`) labels the revision onto which
+the commits are rebased; The name `onto` is just a convention, as a nod
+to the `--onto` option.
+
+It is also possible to introduce completely new merge commits from scratch
+by adding a command of the form `merge `. This form will
+generate a tentative commit message and always open an editor to let the
+user edit it. This can be useful e.g. when a topic branch turns out to
+address more than a single concern and wants to be split into two or
+even more topic branches. Consider this todo list:
+
+
+pick 192837 Switch from GNU Makefiles to CMake
+pick 5a6c7e Document the switch to CMake
+pick 918273 Fix detection of OpenSSL in