From: Theodore Ts'o <email@example.com> To: Carl Baldwin <firstname.lastname@example.org> Cc: Git Mailing List <email@example.com> Subject: Re: Bring together merge and rebase Date: Sun, 24 Dec 2017 22:52:15 -0500 [thread overview] Message-ID: <20171225035215.GC1257@thunk.org> (raw) In-Reply-To: <CALiLy7pBvyqA+NjTZHOK9t0AFGYbwqwRVD3sZjUg0ZLx5y1h3A@mail.gmail.com> On Fri, Dec 22, 2017 at 11:10:19PM -0700, Carl Baldwin wrote: > I've been calling this proposal `git replay` or `git replace` but I'd > like to hear other suggestions for what to name it. It works like > rebase except with one very important difference. Instead of orphaning > the original commit, it keeps a pointer to it in the commit just like > a `parent` entry but calls it `replaces` instead to distinguish it > from regular history. In the resulting commit history, following > `parent` pointers shows exactly the same history as if the commit had > been rebased. Meanwhile, the history of iterating on the change itself > is available by following `replaces` pointers. The new commit replaces > the old one but keeps it around to record how the change evolved. As a suggestion, before diving into the technical details of your proposal, it might be useful consider the usage scenario you are targetting. Things like "git rebase" and "git merge" and your proposed "git replace/replay" are *mechanisms*. But how they fit into a particular workflow is much more important from a design perspective, and given that there are many different git workflows which are used by different projects, and by different developers within a particular project. For example, rebase gets used in many different ways, and many of the debates when people talk about "git rebase" being evil generally presuppose a particular workflow that that the advocate has in mind. If someone is using git rebase or git commit --amend before git commits have ever been pushed out to a public repository, or to anyone else, that's a very different case where it has been visible elsewhere. Even the the most strident, "you must never rewrite a commit and all history must be preserved" generally don't insist that every single edit must be preserved on the theory that "all history is valuable". > The git history now has two dimensions. The first shows a cleaned up > history where fix ups and code review feedback have been rolled into > the original changes and changes can possibly be ordered in a nice > linear progression that is much easier to understand. The second > drills into the history of a change. There is no loss and you don't > change history in a way that will cause problems for others who have > the older commits. If your goal is to preserve the history of the change, one of the problems with any git-centric solution is that you generally lose the code review feedback and the discussions that are involved with a commit. Just simply preserving the different versions of the commits is going to lose a huge amount of the context that makes the history valuable. So for example, I would claim that if *that* is your goal, a better solution is to use Gerrit, so that all of the different versions of the commits are preserved along with the line-by-line comments and discussions that were part of the code review. In that model, each commit has something like this in the commit trailer: Change-Id: I8d89b33683274451bcd6bfbaf75bce98 You can then cut and paste the Change-Id into the Gerrit user interface, and see the different commits, more important, the discussion surrounding each change. If the complaint about Gerrit is that it's not a core part of Git, the challenge is (a) how to carry the code review comments in the git repository, and (b) do so in a while that it doesn't bloat the core repository, since most of the time, you *don't* want or need to keep a local copy of all of the code review comments going back since the beginning of the project. ------------- Here's another potential use case. The stable kernels (e.g., 3.18.y, 4.4.y, 4.9.y, etc.) have cherry picks from the the upstream kernel, and this is handled by putting in the commit body something like this: [ Upstream commit 3a4b77cd47bb837b8557595ec7425f281f2ca1fe ] ---- And here's yet another use case. For internal Google kernel development, we maintain a kernel that has a large number of patches on top of a kernel version. When we backport an upstream fix (say, one that first appeared in the 4.12 version of the upstream kernel), we include a line in the commit body that looks like this: Upstream-4.12-SHA1: 5649645d725c73df4302428ee4e02c869248b4c5 This is useful, because when we switch to use a newer upstream kernel, we need make sure we can account for all patches that were built on top of the 3xx kernel (which might have been using 4.10, for the sake of argument), to the 4xx kernel series (which might be using 4.15 --- the version numbers have been changed to protect the innocent). This means going through each and every patch that was on top of the 3xx kernel, and if it has a line such as "Upstream 4.12-SHA1", we know that it will already be included in a 4.15 based kernel, so we don't need to worry about carrying that patch forward. In other cases, we might decide that the patch is no longer needed. It could be because the patch has already be included upstream, in which case we might check in a commit with an empty patch body, but whose header contains something like this in the 4xx kernel: Origin-3xx-SHA1: fe546bdfc46a92255ebbaa908dc3a942bc422faa Upstream-Dropped-4.11-SHA1: d90dc0ae7c264735bfc5ac354c44ce2e Or we could decide that the commit is no longer no longer needed --- perhaps because the relevant subsystem was completely rewritten and the functionality was added in a different way. Then we might have just have an empty commit with an explanation of why the commit is no longer needed and the commit body would have the metadata: Origin-Dropped-3xx-SHA1: 26f49fcbb45e4bc18ad5b52dc93c3afe Or perhaps the commit is still needed, and for various reasons the commit was never upstreamed; perhaps because it's only useful for Google-specific hardware, or the patch was rejected upstream. The we will have a cherry-pick that would include in the body: Origin-3xx-SHA1: 8f3b6df74b9b4ec3ab615effb984c1b5 (Note: all commits that are added in the rebase workflow, even the empty commits that just have the Origin-Dropped-3xx-SHA1 or Upstream-Droped-4.11-SHA1 headers, are patch reviewed through Gerrit, so we have an audited, second-engineer review to make sure each commit in the 3xx kernel that Google had been carrying had the correct disposition when rebasing to the 4xx kernel.) The point is that for this much more complex, real-world workflow, we need much *more* metadata than a simple "Replaces" metadata. (And we also have other metadata --- for example, we have a "Tested: " trailer that explains how to test the commit, or which unit test can and should be used to test this commit, combined with a link to the test log in our automated unit tester that has the test run, and a "Rebase-Tested-4xx: " trailer that might just have the URL to the test log when the commit was rebased since the testing instructions in the Tested: trailer is still relevant.) And since this metadata is not really needed by the core git machinery, we just use text trailers in the commit body; it's not hard to write code which parses this out of the git commit. > Various git commands will have to learn how to handle this kind of > history. For example, things like fetch, push, gc, and others that > move history around and clean out orphaned history should treat > anything reachable through `replaces` pointers as precious. Log and > related history commands may need new switches to traverse the history > differently in different situations. I'd encourage you to think very hard about how exactly "git log" and "gitk" might actually deal with these links. In the Google kernel development use cases, we use different repos for the 3xx and 4xx kernels. It would be possible to make hot links for the Original-3xx-SHA1: trailers, but you couldn't do it using gitk. It would actually have to be a completely new tool. (And we do have new tools, most especially a dashboard so we can keep track of how many commits in the 3xx kernel still have to be rebased to the 4xx kernel, or can be confirmed to be in the upstream kernel, or can be confirmed to be dropped. We have a *large* number of patches that we carry, so it's a multi-month effort involving a large number of engineers working together to do a kernel rebase operation from a 4.x upstream kernel to a 4.y upstream kernel. So having a dashboard is useful because we can see whether a particular subsystem team is ahead or behind the curve in terms of handling those commits which are their responsibility.) My experience, from seeing these much more complex use cases --- starting with something as simple as the Linux Kernel Stable Kernel Series, and extending to something much more complex such as the workflow that is used to support a Google Kernel Rebase, is that using just a simple extra "Replaces" pointer in the commit header is not nearly expressive enough. And, if you make it a core part of the commit data structure, there are all sorts of compatibility headaches with older versions of git that wouldn't know about it. And if it then turns out it's not sufficient more the more complex workflows *anyway*, maybe adding a new "replace" pointer in the core git data structures isn't worth it. It might be that just keeping such things as trailers in the commit body might be the better way to go. Cheers, - Ted
next prev parent reply other threads:[~2017-12-25 3:52 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 [this message] 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 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=20171225035215.GC1257@thunk.org \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.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).