Page MenuHomePhabricator

Add a land bot script for running smoke tests before landing patches
ClosedPublic

Authored by jasonbcox on Apr 22 2020, 01:40.

Details

Summary

This patch provides a way for the CI to make sure a given patch is
rebased on master, smoke tested, and still landed as the submitter as one
would normally expect from a landing workflow.

The land workflow, once complete, will be roughly this:

  1. User runs arc land
  2. A custom ABC Arcanist workflow intercepts the land command and diverts it to abc-land workflow rather than the default. The default workflow will still be available under another name, if needed.
  3. The abc-land workflow sends the REVISION and encrypted CONDUIT_TOKEN to abcbot.
  4. abcbot queues the build.
  5. The build decrypts CONDUIT_TOKEN and runs land-patch.sh

Some parts are obviously missing, but these will come later:

  • Applying autogen onto a patch in-flight, such as updating manpages
  • Being able to land unreviewed diffs (reserved for automated commits such as updating chainparams).

Regarding the CONDUIT_TOKEN, note that the script needs it unencrypted in order to land the patch
as you. This is safe to run locally. For the complete land-bot pipeline, your Conduit token will
be encrypted before passing along via arc land. This prevents the token from accidentally
getting logged while your request is on its journey through abcbot and the patch queue. It will only
be unencrypted once the land-bot begins working on your patch, immediately before calling this script.

While this infra is being worked on, feel free to run this script directly and provide additional feedback.

Test Plan
./land-patch.sh -h
./land-patch.sh           # errors as expected
./land-patch.sh -r D5780  # errors as expected

CONDUIT_TOKEN=<token> ./land-patch.sh -r D5780 --dry-run
CONDUIT_TOKEN=<token> ./land-patch.sh -r D5780  # was used to land https://reviews.bitcoinabc.org/D5780

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien added inline comments.
contrib/source-control-tools/land-patch.sh
91 ↗(On Diff #19009)

Either this has a meaning that I don't know, or arc land will never be called ?

jasonbcox added inline comments.
contrib/source-control-tools/land-patch.sh
91 ↗(On Diff #19009)

It's a short way of telling arc land to die if it requests user input. This isn't expected and we don't want it to hang waiting for input.

Fabien requested changes to this revision.Apr 23 2020, 08:17

Could you please add the full expected land workflow to the summary ?
I'm lacking a global view of the various actors and steps.

contrib/source-control-tools/land-patch.sh
79 ↗(On Diff #19009)

Could be tested once to avoid boilerplate:

if [ -z "${REVISION}" ]; then
  echo "Error: Landing unreviewed patches is not supported yet."
  exit 20
fi
This revision now requires changes to proceed.Apr 23 2020, 08:17
jasonbcox edited the summary of this revision. (Show Details)
jasonbcox marked an inline comment as done.

Added the full workflow to the summary. Hopefully it's more clear now.

contrib/source-control-tools/land-patch.sh
79 ↗(On Diff #19009)

I will need to add it back later. This scaffolding provides for easier to review patches later on.

This revision is now accepted and ready to land.Apr 28 2020, 20:47