git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Carl Baldwin <carl@ecbaldwin.net>
To: Martin Fick <mfick@codeaurora.org>
Cc: Theodore Ts'o <tytso@mit.edu>, Git Mailing List <git@vger.kernel.org>
Subject: Re: Bring together merge and rebase
Date: Thu, 4 Jan 2018 22:06:50 -0700	[thread overview]
Message-ID: <20180105050647.GB12861@Carl-MBP.local> (raw)
In-Reply-To: <45915667.ye9M3CbBo5@mfick-lnx>

On Thu, Jan 04, 2018 at 01:06:27PM -0700, Martin Fick wrote:
> On Tuesday, December 26, 2017 01:31:55 PM Carl Baldwin 
> wrote:
> ...
> > What I propose is that gerrit and github could end up more
> > robust, featureful, and interoperable if they had this
> > feature to build from.
> 
> I agree (assuming we come up with a well defined feature)
> 
> > With gerrit specifically, adopting this feature would make
> > the "change" concept richer than it is now because it
> > could supersede the change-id in the commit message and
> > allow a change to evolve in a distributed non-linear way
> > with protection against clobbering work.
> 
> We (the Gerrit maintainers) would like changes to be able to 
> evolve non-linearly so that we can eventually support 
> distributed Gerrit reviews, and the amended-commit pointer 
> is one way I have thought to resolve this.

I really think that keeping these references is the key to doing this.

> > I have no intention to disparage either tool. I love them
> > both. They've both made my career better in different
> > ways. I know there is no guarantee that github, gerrit,
> > or any other tool will do anything to adopt this. But,
> > I'm hoping they are reading this thread and that they
> > recognize how this feature can make them a little bit
> > better and jump in and help. I know it is a lot to hope
> > for but I think it could be great if it happened.
> 
> We (the Gerrit maintainers) do recognize it, and I am glad 
> that someone is pushing for solutions in this space.  I am 
> not sure what the right solution is, and how to modify 
> workflows to deal better with this.  I do think that starting 
> by making your local repo track pointers to amended-commits, 
> likely with various git hooks and notes (as also proposed by 
> Johannes Schindelin), would be a good start.   With that in 
> place, then you can attack various specific workflows.

I have started a prototype that I will use to demonstrate this. I hope
to have something in a couple of weeks. I do have a day job also, so it
will be slow going. One idea that I had was to put my own server with
special hooks in it in front of gerrit to illustrate how collaboration
on a gerrit change, or even a chain of them, can be made safe. It would
act as a middle man between my client and the gerrit server. I'd just
have to change remote reference on my client to demonstrate.

> If you want to then attack the Gerrit workflow, it would be 
> good if you could prevent pushing new patchests that are 
> amended versions of patchsets that are out of date.  While 
> it would be great if Gerrit could reject such pushes, I 
> wonder if to start, git could detect and it prevent the push 
> in this situation?  Could a git push hook analyze the ref 
> advertisements and figure this out (all the patchsets are in 
> the advertisement)?  Can a git hook look at the ref 
> advertisement?

I'll think about this. At the least, the hook would have to look at the
server to see if there are new revisions. It would be difficult to close
race conditions that occur because the client will always be using
potentially out of date information even if it just went and pulled down
the latest stuff. I think I still like my middle man idea better as a
short term proof of concept.

Preventing pushing amended/rebased versions of out of date changes is
simple. Follow the "predecessor" references until you hit a known
commit. If that commit is the latest revision of the change then it is
up to date. If that commit not the latest revision, then it is out of
date. Reject it. This is what I plan to illustrate in my middle man
server.

If you traverse the entire graph of predecessors without finding a known
commit, then you have a new change. (In fact, the changeset id in the
commit message in a gerrit change seems unnecessary at this point). It
gets a little more complicated when you think about combining/squashing
changes (resulting in two or more "predecessor" references from a single
commit) or dividing a change into multiple but it works.

The harder part is the push/pull interaction between client and server.
When you go to push your amended update to a patchset, you want git to
send along any other new commits to complete the predecessor graph on
the server side. For example, you might rebase your commit and then
amend it to fix something. Personally, I'd like the rebase and the amend
to both be kept separately.

Similarly, when you've just had a push rejected because you're out of
date, you want to be able to easily pull down the commits you're missing
so that you can merge locally and try to push again.

You also don't want gc to garbage collect the intermediate commits. I
think gerrit uses many references internally in the git repo to "pin"
older revisions in the repository so that they don't appear orphaned. I
think I'm going to have to do something similar in my prototype.

If you think about it, this is all very much like what git already does
with its commit history and branches. If you stick to a strict
branch/merge model and don't rewrite commits, then it is unnecessary.
However, for those that do rewrite commits (such as anyone using the
gerrit workflow), this is a way to bring that power to them.

I'd like to point out the RECOVERING FROM UPSTREAM REBASE section of the
git-rebase man page. If we have the graph of "predecessor" references
for a change, it could be used to automatically recover from the cases
described in this section much like regular branching and merging.
Rewriting changes would no longer be something to consider "a bad idea"
for these reasons.

Carl

  reply	other threads:[~2018-01-05  5:06 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-23  6:10 Bring together merge and rebase Carl Baldwin
2017-12-23 18:59 ` Ævar Arnfjörð Bjarmason
2017-12-23 21:01   ` Carl Baldwin
2017-12-23 22:09     ` Ævar Arnfjörð Bjarmason
2017-12-26  0:16       ` Carl Baldwin
2017-12-26  1:28         ` Jacob Keller
2017-12-26 23:30           ` Igor Djordjevic
2017-12-26 17:49         ` Ævar Arnfjörð Bjarmason
2017-12-26 19:44           ` Carl Baldwin
2017-12-26 20:19             ` Paul Smith
2017-12-26 21:07               ` Carl Baldwin
2017-12-23 22:19     ` Randall S. Becker
2017-12-25 20:05       ` Carl Baldwin
2017-12-23 23:01     ` Johannes Schindelin
2017-12-24 14:13       ` Alexei Lozovsky
2018-01-04 15:44         ` Johannes Schindelin
2017-12-25 23:43       ` Carl Baldwin
2017-12-26  0:01         ` Randall S. Becker
2018-01-04 19:49       ` Martin Fick
2017-12-23 22:30   ` Johannes Schindelin
2017-12-25  3:52 ` Theodore Ts'o
2017-12-26  1:16   ` Carl Baldwin
2017-12-26  1:47     ` Jacob Keller
2017-12-26  6:02       ` Carl Baldwin
2017-12-26  8:40         ` Jacob Keller
2018-01-04 19:19           ` Martin Fick
2018-01-05  0:31             ` Martin Fick
2018-01-05  5:09             ` Carl Baldwin
2018-01-05  5:20               ` Carl Baldwin
2017-12-26 18:04     ` Theodore Ts'o
2017-12-26 20:31       ` Carl Baldwin
2018-01-04 20:06         ` Martin Fick
2018-01-05  5:06           ` Carl Baldwin [this message]
2018-01-04 19:54     ` Martin Fick
2018-01-05  4:08       ` Carl Baldwin
2018-01-05 20:14       ` Junio C Hamano
2018-01-06 17:29         ` Carl Baldwin
2018-01-06 17:32           ` Carl Baldwin
2018-01-06 21:38           ` Theodore Ts'o
2017-12-27  4:35   ` Carl Baldwin
2017-12-27 13:35     ` Alexei Lozovsky
2017-12-28  5:23       ` Carl Baldwin
2017-12-26  4:08 ` Mike Hommey
2017-12-27  2:44   ` Carl Baldwin

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=20180105050647.GB12861@Carl-MBP.local \
    --to=carl@ecbaldwin.net \
    --cc=git@vger.kernel.org \
    --cc=mfick@codeaurora.org \
    --cc=tytso@mit.edu \
    /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).