Added a land bot script
AbandonedPublic
Actions

Authored by jasonbcox on Oct 2 2019, 00:25.

Details

Reviewers

deadalnix
Fabien

Group Reviewers

Restricted Project

Summary

This is the first step to enabling us to land changes more safely.
Currently, it's a toy script that rebases a diff on master, builds, and runs tests.
It's designed to error out immediately if any step of this process fails or prompts
for user input (as the intent is for this to be 100% automated in the near future).

Depends on D4213

Test Plan

This errors as expected for red diffs and completes (but doesn't push) for green diffs:

ABC_BUILD_NAME=build-without-wallet ./land-bot.sh --no-land -r Dxxxx

This was used to land D4196 successfully:

ABC_BUILD_NAME=build-default ./land-bot.sh -r D4196

Diff Detail

Repository

rABC Bitcoin ABC

Branch

land-bot

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7660
Build 13359: Bitcoin ABC Buildbot (legacy)
Build 13358: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Oct 2 2019, 00:25

Herald added a reviewer: deadalnix. · View Herald TranscriptOct 2 2019, 00:25

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox planned changes to this revision.Oct 2 2019, 00:25

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7660: Diff 13312.Oct 2 2019, 00:35

Minor fixes

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox edited the test plan for this revision. (Show Details)Oct 2 2019, 16:35

Harbormaster completed remote builds in B7669: Diff 13324.Oct 2 2019, 16:47

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Should anything fail the script errors out immediately, but the user working tree may have been modified in between.
I think you should trap the error and restore the previous state to avoid confusing the user.

This revision now requires changes to proceed.Oct 3 2019, 20:28

Rebase on D4213 and take advantage of it to make this script more robust.
Cleanup the repo state, especially in the case of failures.

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox edited the summary of this revision. (Show Details)Oct 4 2019, 21:04

jasonbcox added a parent revision: D4213: [build-configurations] Make BUILD_DIR configurable.

Harbormaster failed remote builds in B7692: Diff 13357!Oct 4 2019, 21:05

Build Bitcoin-ABC / Diffs / Diff Testing failed.

deadalnix requested changes to this revision.Oct 6 2019, 18:15

deadalnix added inline comments.

contrib/bots/land-bot.sh
6 ↗	(On Diff #13357)	Looks like this would benefit from some utf-8
19 ↗	(On Diff #13357)	This suggest this does more than one thing.
29 ↗	(On Diff #13357)	I'd suggest not building the tree at all. Also, it's a bit strange that this is the responsibility of this script to do sanity check.
69 ↗	(On Diff #13357)	The repository can be in all kind of fucked state. Semi applied patches, mid rebase or cherry-pick failure, etc... This is not sufficient. Also make sure that you do do not assume that scripts are good citizen and won't trash the repository. Having them do their best is okay - though that may in fact hide errors. You need to be able to pickup the repository in any shape or form and make it work.

This revision now requires changes to proceed.Oct 6 2019, 18:15

jasonbcox added inline comments.Oct 21 2019, 16:28

contrib/bots/land-bot.sh
29 ↗	(On Diff #13357)	This is an interesting point. Thinking about it some more, a rebasing bot could be separate from the land bot. The rebasing bot would need to build and run tests however, as it's modifying the source.

jasonbcox added inline comments.Oct 21 2019, 17:31

contrib/bots/land-bot.sh
29 ↗	(On Diff #13357)	Thinking more while actually writing the code I realize the last part isn't true. Separate scripts that are stringed together like: `rebase -> build -> land` have no dependencies on each other but complete the goal much more cleanly.