git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Glen Choo <chooglen@google.com>
To: Chris Poucet <poucet@google.com>,
	Stefan Xenos via GitGitGadget <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "Jerry Zhang" <jerry@skydio.com>,
	"Phillip Wood" <phillip.wood123@gmail.com>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Christophe Poucet" <christophe.poucet@gmail.com>,
	vdye@github.com, "Junio C Hamano" <jch@google.com>,
	"Jonathan Tan" <jonathantanmy@google.com>
Subject: Re: [PATCH v2 01/10] technical doc: add a design doc for the evolve command
Date: Thu, 06 Oct 2022 13:53:16 -0700	[thread overview]
Message-ID: <kl6ltu4gwu6b.fsf@chooglen-macbookpro.roam.corp.google.com> (raw)
In-Reply-To: <CAN9+7XcYFa+Y9jsJSEmQhf29TUZADoz8=SzcNbjCH8ewqYriYg@mail.gmail.com>


Hi Chris!

Chris Poucet <poucet@google.com> writes:

> One thing that is not clear to me is whether this is the desired
> direction. I took at look at the git review notes but it was hard to
> get a sense of where people are at.

I'm really sorry, I meant to get back to this sooner with the takeaways
from Review Club. Hopefully this will still be useful.

You can find the Review Club notes here:

  https://docs.google.com/document/d/14L8BAumGTpsXpjDY8VzZ4rRtpAjuGrFSRqn3stCuS_w/edit?pli=1

> Would love input on the design.

Others have given a lot of input on the design, so instead, I'll focus
mostly on how to make the doc better on the mailing list.

>
> On Wed, Oct 5, 2022 at 4:59 PM Stefan Xenos via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
>>
>> From: Stefan Xenos <sxenos@google.com>
>>
>> This document describes what a change graph for
>> git would look like, the behavior of the evolve command,
>> and the changes planned for other commands.
>>
>> It was originally proposed in 2018, see
>> https://public-inbox.org/git/20181115005546.212538-1-sxenos@google.com/

This doc is quite well-thought-out and surprisingly readable despite its
length. That said, it is a lot to review in one sitting, and a reviewer
might get easily fatigued. I suspect that reviewers will find it hard to
keep up with the discussion if they have to review the entire doc on
every iteration.

As Victoria suggested in Review Club, it might be helpful to split up
the design over multiple patches to make feedback more focused. I think
this will make it easier for you (and others) to get a sense of how we
feel about each part of the design. e.g. here's one way to split up the
doc:

- Motivation, Background, High level idea of how a user would use this. 

  (Roughly corresponding to the sections "Objective", "Status",
  "Background", "Goals", "Similar technologies", "Semi-related work")

- Local change tracking, Changes to existing commands, Meta-commits

  (The parts about the data format and their implications for GC,
  negotiation, etc. Maybe include the `change` subcommand if it helps
  reviewers visualize the impact.)

- How evolve works, e.g. convergence, divergence, merge base finding.
  CLI

- Sharing changes

Besides the design, here other sections that I would find useful:

- Glossary. I thought that terms like "change", "change branch" and
  "change graph" were underdefined. This would also be a useful
  reference during the implementation phase.

- Implementation Plan (you can find examples in
  Documentation/technical/bundle-uri.txt and
  Documentation/technical/sparse-index.txt). Making the concrete next
  steps visible has numerous benefits:
  - Reviewers of future patches know what problem is being tackled and
    value is being delivered.
  - The list gains confidence that the author can deliver the work being
    promised.
  - The shared direction makes it easier for others to contribute
    patches.

- Open questions (e.g. "Implementation questions" in [1]). It would be
  useful to know what questions can be answered later instead of right
  now. Also, since you are not the original author, perhaps you also
  have questions about the design that you want answered by reviewers.
  I also wouldn't mind this being in the cover letter or "---" section.

[1] https://lore.kernel.org/git/pull.1367.git.1664064588846.gitgitgadget@gmail.com

As mentioned earlier, I'll comment only very lightly on the design.

>> +Similar technologies
>> +--------------------

I'd personally love to see "git evolve". If it helps to consider some
other tools, I use the following tools that implement similar workflows:

- git-branchless [2] features anonymous heads, obsolescence tracking, 
  history manipulations and "git evolve". Having used this for a while,
  I'm of the opnion that having any of these features without the
  others is still very useful, and implementing them in phases 
  will still deliver value without having to complete all of the work
  (granted, each of these features is incrementally dependent on the
  others).

  Case in point: I don't use the "evolve" equivalent of git-branchless
  (IIRC "restack); being able to see obsolescence and manually
  manipulating history is good enough for me.

- Jujutsu [3] also features anonymous heads, obsolescence tracking and
  advanced history manipulations. Instead of "evolve", descendents of an
  obsolete commit are automatically rebased on the obsoleting commit.

[2] https://github.com/arxanas/git-branchless
[3] https://github.com/martinvonz/jj

>> +Changes
>> +-------
>> +A branch of meta-commits describes how a commit was produced and what previous
>> +commits it is based on. It is also an identifier for a thing the user is
>> +currently working on. We refer to such a meta-branch as a change.
>> +
>> +Local changes are stored in the new refs/metas namespace. Remote changes are
>> +stored in the refs/remote/<remotename>/metas namespace.

I find this terminology of "changes" and "metas" more confusing than
necessary. A glossary would help, but it might be even better to also
use an appropriate ref namespace. "refs/changes/" is an obvious
candidate, though I assume this wasn't mentioned because Gerrit uses
that namespace extensively.

Maybe `refs/changelists`, `refs/change-requests`, `refs/proposals`? Idk.

>> +Sharing changes
>> +---------------
>> +Change histories are shared by pushing or fetching meta-commits and change
>> +branches. This provides users with a lot of control of what to share and
>> +repository implementations with control over what to retain.
>> +
>> +Users that only want to share the content of a commit can do so by pushing the
>> +commit itself as they currently would. Users that want to share an edit history
>> +for the commit can push its change, which would point to a meta-commit rather
>> +than the commit itself if there is any history to share. Note that multiple
>> +changes can refer to the same commits, so it’s possible to construct and push a
>> +different history for the same commit in order to remove sensitive or irrelevant
>> +intermediate states.

I would not like to see the ability to share all intermediate states
with the server because this increases the risk of unintentional
disclosure by a lot.

How exactly we could tweak this can be an open discussion for later.
Some examples I can think of:
  - Asking the user to go through the obsolescence log and manually
    prune revisions (sounds too onerous for users IMO).
  - Push a truncated history consisting of only the latest version and
    commits that the server already knows (somewhat similar to Gerrit).

>> +Evolve
>> +------
>> +The evolve command performs the correct sequence of rebases such that no change
>> +has an obsolete parent. The syntax looks like this:
>> +
>> +git evolve [upstream…]
>> +
>> +It takes an optional list of upstream branches. All changes whose parent shows
>> +up in the history of one of the upstream branches will be rebased onto the
>> +upstream branch before resolving obsolete parents.
>> +

This CLI is an example of something that can be reviewed largely
independently of the implementing data structures.

>> +Merge
>> +-----
>> +
>> +To select between these two behaviors, merge gets new “--amend” and “--noamend”
>> +options which select between the “create” and “modify” behaviors respectively,
>> +with noamend being the default.

Ditto.


  reply	other threads:[~2022-10-06 20:53 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-09-23 18:55 [PATCH 00/10] Add the Git Change command Christophe Poucet via GitGitGadget
2022-09-23 18:55 ` [PATCH 01/10] technical doc: add a design doc for the evolve command Stefan Xenos via GitGitGadget
2022-09-23 19:59   ` Jerry Zhang
2022-09-28 21:26   ` Junio C Hamano
2022-09-28 22:20   ` Junio C Hamano
2022-09-29  9:17     ` Phillip Wood
2022-09-29 19:57   ` Jonathan Tan
2022-09-23 18:55 ` [PATCH 02/10] sha1-array: implement oid_array_readonly_contains Chris Poucet via GitGitGadget
2022-09-26 13:08   ` Phillip Wood
2022-09-23 18:55 ` [PATCH 03/10] ref-filter: add the metas namespace to ref-filter Chris Poucet via GitGitGadget
2022-09-26 13:13   ` Phillip Wood
2022-10-04  9:50     ` Chris P
2022-09-23 18:55 ` [PATCH 04/10] evolve: add support for parsing metacommits Stefan Xenos via GitGitGadget
2022-09-26 13:27   ` Phillip Wood
2022-10-04 11:21     ` Chris P
2022-10-04 14:10       ` Phillip Wood
2022-09-23 18:55 ` [PATCH 05/10] evolve: add the change-table structure Stefan Xenos via GitGitGadget
2022-09-27 13:27   ` Phillip Wood
2022-09-27 13:50     ` Ævar Arnfjörð Bjarmason
2022-09-27 14:13       ` Phillip Wood
2022-09-27 15:28         ` Ævar Arnfjörð Bjarmason
2022-09-28 14:33           ` Phillip Wood
2022-09-28 15:14             ` Ævar Arnfjörð Bjarmason
2022-09-28 15:59             ` Junio C Hamano
2022-09-27 14:18     ` Phillip Wood
2022-10-04 14:48     ` Chris P
2022-09-23 18:55 ` [PATCH 06/10] evolve: add support for writing metacommits Stefan Xenos via GitGitGadget
2022-09-28 14:27   ` Phillip Wood
2022-10-05  9:40     ` Chris P
2022-10-05 11:09       ` Phillip Wood
2022-09-23 18:55 ` [PATCH 07/10] evolve: implement the git change command Stefan Xenos via GitGitGadget
2022-09-25  9:10   ` Phillip Wood
2022-09-26  8:23     ` Ævar Arnfjörð Bjarmason
2022-09-26  8:25   ` Ævar Arnfjörð Bjarmason
2022-10-05 12:30     ` Chris P
2022-09-23 18:55 ` [PATCH 08/10] evolve: add the git change list command Stefan Xenos via GitGitGadget
2022-09-23 18:55 ` [PATCH 09/10] evolve: add delete command Chris Poucet via GitGitGadget
2022-09-26  8:38   ` Ævar Arnfjörð Bjarmason
2022-09-26  9:10     ` Chris Poucet
2022-09-23 18:55 ` [PATCH 10/10] evolve: add documentation for `git change` Chris Poucet via GitGitGadget
2022-09-25  8:41   ` Phillip Wood
2022-09-25  8:39 ` [PATCH 00/10] Add the Git Change command Phillip Wood
2022-10-04  9:33   ` Chris P
2022-10-04 14:24 ` Phillip Wood
2022-10-04 15:19   ` Chris P
2022-10-04 15:55     ` Chris P
2022-10-04 16:00       ` Phillip Wood
2022-10-04 15:57     ` Phillip Wood
2022-10-05 14:59 ` [PATCH v2 00/10] RFC: Git Evolve / Change Christophe Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 01/10] technical doc: add a design doc for the evolve command Stefan Xenos via GitGitGadget
2022-10-05 15:16     ` Chris Poucet
2022-10-06 20:53       ` Glen Choo [this message]
2022-10-10 19:35     ` Victoria Dye
2022-10-11  8:59       ` Phillip Wood
2022-10-11 16:59         ` Victoria Dye
2022-10-12 19:19           ` Phillip Wood
2022-10-05 14:59   ` [PATCH v2 02/10] sha1-array: implement oid_array_readonly_contains Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 03/10] ref-filter: add the metas namespace to ref-filter Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 04/10] evolve: add support for parsing metacommits Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 05/10] evolve: add the change-table structure Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 06/10] evolve: add support for writing metacommits Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 07/10] evolve: implement the git change command Stefan Xenos via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 08/10] evolve: add delete command Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 09/10] evolve: add documentation for `git change` Chris Poucet via GitGitGadget
2022-10-05 14:59   ` [PATCH v2 10/10] evolve: add tests for the git-change command Chris Poucet via GitGitGadget
2022-10-10  9:23   ` [PATCH v2 00/10] RFC: Git Evolve / Change Phillip Wood

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=kl6ltu4gwu6b.fsf@chooglen-macbookpro.roam.corp.google.com \
    --to=chooglen@google.com \
    --cc=avarab@gmail.com \
    --cc=christophe.poucet@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jch@google.com \
    --cc=jerry@skydio.com \
    --cc=jonathantanmy@google.com \
    --cc=phillip.wood123@gmail.com \
    --cc=poucet@google.com \
    --cc=vdye@github.com \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).