From owner-freebsd-hackers@freebsd.org Thu Jan 28 19:53:49 2021 Return-Path: Delivered-To: freebsd-hackers@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 69C274EB7B2 for ; Thu, 28 Jan 2021 19:53:49 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from smtp.freebsd.org (smtp.freebsd.org [96.47.72.83]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4DRWPT2cTPz4cf0 for ; Thu, 28 Jan 2021 19:53:49 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from John-Baldwins-MacBook-Pro.local (c-98-35-218-221.hsd1.ca.comcast.net [98.35.218.221]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: jhb) by smtp.freebsd.org (Postfix) with ESMTPSA id 0465742DB for ; Thu, 28 Jan 2021 19:53:48 +0000 (UTC) (envelope-from jhb@FreeBSD.org) To: freebsd-hackers@freebsd.org References: From: John Baldwin Subject: Re: arc wrapper for tracking phabricator reviews Message-ID: <3c372d16-3aea-5291-c23a-f49e9e9a8edc@FreeBSD.org> Date: Thu, 28 Jan 2021 11:53:48 -0800 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:78.0) Gecko/20100101 Thunderbird/78.6.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Technical discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Jan 2021 19:53:49 -0000 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 2) When you get feedback, amend the original commit, then do git arc update 3) Once the change is ready: git arc stage 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.. When you get review feedback, you can do fixup commits (e.g. git commit --fixup ) and then do a 'rebase -i --autosquash' to squash them back down. You can then do 'git arc update ' listing the hashes of the commits you've changed. You might even be able to do 'git arc update main..' but I haven't tried that. 3) Once the series is ready: git arc stage main.. 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..' 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 ' 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..' 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