"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: got backout/cherrypick metadata (for commit log messages)
To:
Mark Jamsek <mark@jamsek.com>
Cc:
Game of Trees <gameoftrees@openbsd.org>
Date:
Thu, 26 Jan 2023 09:50:23 +0100

Download raw body.

Thread
On Thu, Jan 26, 2023 at 06:04:43PM +1100, Mark Jamsek wrote:
> As a quick recap, when the user runs 'got {backout,cherrypick} commit',
> we create a "logmsg" reference pointing to the tree's base commit. What
> it points at is largely irrelevant for now, but the reference path takes
> the form:
> 
>   refs/got/worktree/cherrypick-${WORKTREE_UUID}-$cherry-picked-commit-id}
>   refs/got/worktree/backout-${WORKTREE_UUID}-${backed-out-commit-id}
> 
> As such, when the user next runs 'got commit', we parse the log
> message(s) from the ${backed-out,cherry-picked}-commit-id refs with
> which we prepopulate the editor. This way, if the user invokes N 'got
> {bo,cy}' commands, all N log messages will appear in the editor. This is
> the primary reason I think this idea of stsp's is better than the first.
> The other being what op observed: no change is required from the user;
> just cherrypick and commit!

Thanks for taking the time to work on this!

Yes, I think we should try this. If it becomes a burden somehow we can
keep refining the feature, or even rip it out again in a later release
and implement the -a option approach instead.

I think it would be nice to have this feature in-tree, plus some tests.

> Second, when 'got commit' is run, we loop through each logmsg ref and if
> at least one changed path in the referenced bo/cy commit has changes in
> the work tree that are now being committed, we append its log message to
> the temp file opened in the editor.
> 
> Upon a successful commit, we remove all references created by previous
> backout/cherrypick operations in the current work tree. If a commit is
> aborted or otherwise fails, the refs are kept on disk.

Reading this, I am wondering if we should tweak the behaviour regarding
removal after commit: Would it be better to keep a ref around if there
is no overlap between the cherrypicked commit's list of changed paths
and the newly created commit's list of changed paths?

> This is not foolproof, there are cases where we will inevitably have
> messages that aren't quite relevant, but as stsp remarked, it's not
> a big deal and the benefit outweights the cost of having to tidy things
> up in the editor on the rare occasion this happens.

Yes, and we will see how this works out in general usage.
And our documentation should be clear about this limitation.

Speaking of which:

> +.It Fl l
> +Display a list of references created by previous cherrypick operations
> +If a
> +.Ar commit

I think the manual page should talk about this at a higher level
of abstraction.

Users won't know/care whether cherrypick commands created any references.
What matters to users is that Got will try to remember log messages of
cherrypicked commits and append any relevant log messages to the commit
message template.

We could say something like:
[[[
.It Fl l
Display a list of commit log messages recorded by cherrypick operations,
represented by references in the "refs/got/worktree/" reference namespace.
]]]

Similar to how documentation of -X options for rebase/histedit just talks
about "backups" rather than references, and how the corresponding -l option
mentions how this is implemented, but otherwise keeps the high-level concepts
in focus.