git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Igor Djordjevic <igor.d.djordjevic@gmail.com>
To: Johannes Sixt <j6t@kdbg.org>
Cc: git@vger.kernel.org, Nikolay Shustov <nikolay.shustov@gmail.com>,
	Johannes Schneider <mailings@cedarsoft.com>,
	Patrik Gornicz <patrik-git@mail.pgornicz.com>,
	Martin Waitz <tali@admingilde.org>,
	Shawn Pearce <spearce@spearce.org>, Sam Vilain <sam@vilain.net>,
	Jakub Narebski <jnareb@gmail.com>
Subject: Re: [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes)
Date: Thu, 30 Nov 2017 00:10:21 +0100	[thread overview]
Message-ID: <741dfedc-07f8-24fb-ebe2-940f8b2639d4@gmail.com> (raw)
In-Reply-To: <ea156b8b-29d8-7501-b5a5-a29cfbd7d1d6@kdbg.org>

Hi Hannes,

On 29/11/2017 20:11, Johannes Sixt wrote:
> 
> Ok, then please explain, how this process should work in my workflow
> and with the `commit --onto-parent` feature that you have in mind. I
> have an integration branch (which is a throw-away type, so you can
> mangle it in any way you want); it is a series of merges:
> 
>  ...A    ...C            <- topics A, C
>      \       \ E
>    ---o---o---o---o I    <- integration
>          /       /
>      ...B    ...D        <- topics B, D
> 
> Now I find a bug in topic B. Assume that the merges of C and D have
> textual conflicts with the integration branch (but not with B) and/or
> may be evil. What should I do?
> 
> With git-post, I make a fixup commit commit on the integration
> branch, then `git post B && git merge B`:
> 
>  ...A    ...C                  <- topics A, C
>      \       \
>    ---o---o---o---o---f---F    <- integration
>          /       /       /
>      ...B    ...D       /      <- topic D
>          \             /
>           f'----------'        <- topic B
> 
> The merge F does not introduce any changes on the integration branch,
> so I do not need it, but it helps keep topic B off radar when I ask
> `git branch --no-merged` later.

But you`re not committing (posting) on your HEAD`s (direct) parent in 
the first place (topic B), so `commit --onto-parent` isn`t right tool 
for the job... yet :)

To make it easier to explain, I marked your integration branch 
initial head with "I" in the quote above (commit merging-in branch 
D), and marked commit merging-in branch C with "E".

HEAD being currently on commit "I", you can only use `--onto-parent` 
option to commit onto "E" or "D", being parents of "I".

To work with `--onto-parent` and be able to commit on top of any of 
the topic branches, you would need a situation like this instead:

 (1)  ...C      <- topic C
         |
    ...A |      <- topic A
        \|
      ...o I    <- integration
        /|
    ...B |      <- topic B
         |
      ...D      <- topic D

With `commit --onto-parent` you would skip `git post B && git merge 
B` steps, where "fixup commit" would be done with `--onto-parent B`, 
So you end up in situation like this:

 (2)      ...C      <- topic C
             |
        ...A |      <- topic A
            \|
          ...o I'   <- integration
            /|
    ...B---f |      <- topic B
             |
          ...D      <- topic D

State of index and working tree files in your F and my I' commit is 
exactly the same (I' = I + f), where in my case (2) history looks 
like "f" was part of topic B from the start, before integration 
merge happened.

BUT, all this said, I understand that your starting position, where 
not all topic branches are merged at the same time (possibly to keep 
conflict resolution sane), is probably more often to be encountered 
in real-life work... and as existing `--onto-parent` machinery should 
be able to support it already, I`m looking forward to make it happen :)

Once there, starting from your initial position:

>    ...A    ...C            <- topics A, C
>        \       \ E
>      ---o---o---o---o I    <- integration <- HEAD
>            /       /
>        ...B    ...D        <- topics B, D

... and doing something like `git commit --onto B --merge` would yield:
 
 (3) ...A    ...C            <- topics A, C
         \       \ E
       ---o---o---o---o I'   <- integration
             /       /|
         ...B    ...D |      <- topic D
             \        |
              f-------'      <- topic B

... where (I' = I + f) is still true. If that`s preferred in some 
cases, it could even look like this instead:

 (4) ...A    ...C             <- topics A, C
         \       \ E  I
       ---o---o---o---o---F   <- integration
             /       /   /
         ...B    ...D   /     <- topic D
             \         /
              f-------'       <- topic B

... where F(4) = I'(3), so similar situation, just that we don`t 
discard I but post F on top of it.

Good thing is all necessary logic should already be in place, I just 
need to think a bit about the most sane user interface, and get back 
to you. Thanks for invaluable input so far :)

Of course, do feel free to drop any ideas you come up with as well, 
on how `git commit` user interface/options leading to (3) or (4) 
should look like (they could both be supported).

> > Like working on multiple branches at the same time, in the manner
> > similar to what `git add --patch` allows in regards to working on
> > multiple commits at the same time. This just takes it on yet another
> > level... hopefully :)
> 
> 'kay, I'm not eagerly waiting for this particular next level (I
> prefer to keep things plain and simple), but I would never say this
> were a broken workflow. ;)

Hehe, thanks, I guess :) Simplicity, but user-oriented, is the major 
point here, where you can work on unrelated patch series at the same 
time (patch queues?), without a need to switch branches, while you 
still make "per series" history as you go, without a need to do it 
after the fact.

It`s a matter of possibilities (where Git usually excels), so one can 
choose for himself, depending on the situation at hand.

> > [SCRIPT/RFC 3/3] git-commit--onto-parent.sh[1] (I guess you didn`t
> > have time to checked that one yet?):
> 
> I did have a brief look, but I stopped when I saw
> 
>     # Remove entry from HEAD reflog, not to pollute it with
>     # uninteresting in-between steps we take, leaking implementation
>     # details to end user.
> 
> It's a clear sign for me that's something wrong. It is not just
> reflogs that can become stale, but all operations that follow the
> `git commit` can fail. How do you clean up such a mess?

I would respectfully disagree about it being wrong per se, but that 
said, "cleaning up" reflogs was a last minute design decision so we 
don`t show uninteresting states - from user`s point of view, all that 
happened is commit on top of --onto-parent, and merging that commit 
into where he already was (instead of the commit he was on). 

Implementation details on how we got there should be irrelevant, even 
more if we failed to do so, cluttering reflog with unimportant 
leftovers.

Commands are chained, and in case _anything_ goes wrong, we just bail 
out and restore original index and HEAD position. As working tree is 
not touched at all, no cleanup needed there, so everything is pretty 
simple. Of course, valid tests still should/need to be made, to make 
sure everything is covered, but there really isn`t much to cover in 
the first place.

But this is open for discussion, of course, and code for ref-logging 
all steps is still inside the script, just commented out. Heck, 
there`s even get_checkout_reflog_message() function at the end of the 
script, used to mimic real checkout reflog message, that`s currently 
unused ;)

> The "complex situation" I had in mind was not so much about the
> user's view, but how the implementation has to work when there are
> errors.

Simple - abort. Restore backed up index and HEAD state is all that`s 
to it, and it looks like we never issued the command to begin with.

> > From what I understand, your `git-post` makes a commit where you
> > are _and copies_ it over to destination, so you end up with same
> > changes in two places (two commits). More, you seem to keep working
> > on top of your previous commit(s), which seems logical, later
> > "posting" it where desired - so after the fact. It also means you
> > do still have all your "fix" commits in HEAD while you work on them
> > (and until you git-post and abandon them, discarding the
> > integration branch).
> 
> Just to clarify: git-post only copies existing commits, but does not
> make the original itself.

Yes, I understand that (hopefully communicated from the second part 
of the quote), but I see my wording of that first sentence was a bit 
unfortunate there (to say the least, lol), thanks for clarifying.

> > Could it be that you`re looking at `git commit --onto-parent` from
> > an unexpected perspective (through your `git-post`, being
> > conceptually different)...?
> 
> That may be the case. I'm very interested in your answer to my
> challenge above.

From everything said above, it seems so - at the moment, but I`m 
looking forward making it work for you, too, as it does make sense 
"out in the wild", in real-life scenarios.

Thank you again, for inspiring discussion.

Regards, Buga

  reply	other threads:[~2017-11-29 23:10 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-26 22:35 [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes) Igor Djordjevic
2017-11-26 22:36 ` [SCRIPT/RFC 1/3] setup.sh Igor Djordjevic
2017-11-26 22:36 ` [SCRIPT/RFC 2/3] git-merge-one-file--cached Igor Djordjevic
2017-11-26 22:45 ` [SCRIPT/RFC 3/3] git-commit--onto-parent.sh Igor Djordjevic
2017-11-27 21:54 ` [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working tree file changes) Johannes Sixt
2017-11-28  1:15   ` Igor Djordjevic
2017-11-29 19:11     ` Johannes Sixt
2017-11-29 23:10       ` Igor Djordjevic [this message]
2017-12-01 17:23         ` Johannes Sixt
2017-12-04  2:33           ` Igor Djordjevic
2017-12-06 18:34             ` Johannes Sixt
2017-12-06 18:40               ` Junio C Hamano
2017-12-08  0:15                 ` Igor Djordjevic
2017-12-08 16:24                   ` Junio C Hamano
2017-12-08 23:54                     ` Igor Djordjevic
2017-12-09  2:18                       ` Alexei Lozovsky
2017-12-09  3:03                         ` Igor Djordjevic
2017-12-09 19:00                           ` [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking " Phillip Wood
2017-12-09 19:01                           ` [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, noworking " Phillip Wood
2017-12-10  1:20                             ` Igor Djordjevic
2017-12-10 12:22                               ` [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge,noworking " Phillip Wood
2017-12-10 23:17                                 ` Igor Djordjevic
2017-12-11  1:13                                   ` Alexei Lozovsky
2017-12-11  1:00                                 ` Alexei Lozovsky
2017-11-30 22:40 ` [SCRIPT/RFC 0/3] git-commit --onto-parent (three-way merge, no working " Chris Nerwert
2017-12-03 23:01   ` Igor Djordjevic

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=741dfedc-07f8-24fb-ebe2-940f8b2639d4@gmail.com \
    --to=igor.d.djordjevic@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=jnareb@gmail.com \
    --cc=mailings@cedarsoft.com \
    --cc=nikolay.shustov@gmail.com \
    --cc=patrik-git@mail.pgornicz.com \
    --cc=sam@vilain.net \
    --cc=spearce@spearce.org \
    --cc=tali@admingilde.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).