arc wrapper for tracking phabricator reviews

[John Baldwin]

On 1/25/21 9:58 AM, Mark Johnston wrote:
> Hi,
> 
> For the past while I've been using a script, git arc, to automate some
> of my interactions with Phabricator.  A few other developers have found
> it useful so it seems time to make it more accessible by committing it
> to the src tree.
> 
> The script is here: https://reviews.freebsd.org/D28334
> 
> The basic idea is to make it simple to create and update Phabricator
> reviews from git commits, without adding any metadata to your git
> repository so as to stay as flexible as possible.  In particular, I tend
> to rebase sets of commits regularly so things like notes and branch
> pointers end up being invalidated.
> 
> git arc lets you
> - Create reviews from a series of commits.  It automatically creates
>    parent/child pointers between consecutive Differential revisions.
> - Update reviews after amending commits based on reviewer feedback.
> - Stage commits in preparation for pushing to the upstream repository.
>    This automatically adds "Reviewed by:" and "Differential Revision:"
>    tags.
> See the rather verbose usage message for more details.
> 
> To map git commits to phabricator revisions it uses commit summaries.
> This is kind of gross but I don't see a better way to do it that doesn't
> involve adding metadata to my git tree, and it seems to work fine in
> practice.  In general git arc wraps the functionality of arc, though in
> some cases it uses jq and arc's "call-conduit" functionality to call
> into the lower-level Phabricator APIs.
> 
> There exists a script with similar goals, arcgit, in the tools/ dir.  I
> used it for a while but found that with its workflow I still had to do a
> fair bit of manual work to keep phabricator in sync with my commits.
> I'm not sure how best to reconcile the two scripts.  It would be better
> for new developers to not have to choose between them.  The main
> advantage of arcgit is that it helps maintain large patch stacks by
> making it easier to edit intermediate commits in response to review
> feedback.  In particular, it adds a branch for each review posted to
> phabricator, so it's a bit easier to go back and edit a particular
> commit, but I believe this assumes that you aren't rebasing your branch.
> 
> I'd appreciate any feedback/suggestions/complaints regarding the new
> script, especially if you use arcgit today.
> 
> Thanks to jhb for submitting a number of improvements and suggesting a
> few others that I implemented recently.

Some quick recipes from my experience.  Note that you need to run these in
a git checkout (or a worktree):

If you have a single commit you want to eventually commit, the workflow would
be something like this:

1) git arc create <hash>

2) When you get feedback, amend the original commit, then do

    git arc update <newhash>

3) Once the change is ready:

    git arc stage <finalhash>

This will checkout 'main' and cherry-pick the commit on to 'main'.
It will also amend the commit log to include the 'Reviewed by:' line
from phab along with the Phab URL and open your editor with the updated
commit log so you can clean it up if needed.

You will be left checked out on your main and can now do

git push freebsd

To push the change.

If you are working with a branch, you can use git arc to upload
the entire branch as a patch series of related reviews (which show
up as a "Stack" in phabricator).  For this the workflow is somewhat
similar:

1) git arc create main..<branch>

When you get review feedback, you can do fixup commits
(e.g. git commit --fixup <hash>) and then do a 'rebase -i --autosquash'
to squash them back down.  You can then do 'git arc update <hashes>'
listing the hashes of the commits you've changed.  You might even be
able to do 'git arc update main..<branch>' but I haven't tried that.

3) Once the series is ready:

    git arc stage main..<branch>

This will let you edit each commit log appending the phab metadata.

You can then 'git push freebsd'.

A few other notes:

create -l)

If you have a branch with several commits, you might not want to review
all the diffs of each change when creating the reviews, but just the
list of commits.  The '-l' flag to 'git arc create' will do that, i.e.
'git arc create -l main..<branch>'

list)

You can determine the status of commits via 'git arc list'.  This can
be handy to figure out the state of commits in a branch, e.g.:

> git arc list main..chacha20_poly1035_aead
f0de84910927 Accepted       D27836: Add an OCF algorithm for ChaCha20-Poly1035 AEAD.
9b55e0d60438 Needs Review   D27837: Add an implementation of CHACHA20_POLY1035 to cryptosoft.
09035ad3b7d2 Needs Review   D27838: Add Chacha20-Poly1305 AEAD coverage.
011529f781c7 Accepted       D27839: Add Chacha20-Poly1305 as a KTLS cipher suite.
5f513f0e8e66 Needs Review   D27841: Add Chacha20-Poly1305 support in the OCF backend for KTLS.
d8cb9e648fbe No Review      : cryptosoft: Support per-op keys for AES-GCM and AES-CCM.

Here you can see that this branch has 2 accepted commits, 3 still waiting
for review, and 1 commit I've added since the initial upload that I haven't
yet uploaded for review.

adding/removing patches)

If you add a new patch to a series, you will need to 'git arc create <hash>'
to upload the new patch and then use the web UI to hook it into the stack.

If you remove a patch from a series, you will need to abandon the associated
review via the web UI.

subject lines)

git arc finds the matching review first by checking for a Phab URL in the
commit log.  If it doesn't find one, it compares each commit's subject line
to the commits listed by 'arc list' to find a match.  If you reword a commit's
subject line, you will either need to add the phab URL to the commit log by
hand or update the commit log in phab.

staging branches)

If you already have 'main' checked out somewhere else, you will want to either
run 'git arc stage' in that work tree, or ask 'git arc stage' to create a new
branch off of main to stage the commits to.  You can do this via '-b', e.g.
'git arc stage -b staging main..<branch>' will create a new 'staging' branch
and stage all of the commits from 'branch' into it.  You can then push
that via 'git push freebsd HEAD:main'.

stable branches)

Currently 'git arc stage' hardcodes main.  It probably should take a new option
(-B?) to set the "base" branch to work with patches that are direct commits to
stable and releng branches.

-- 
John Baldwin