Page MenuHomePhabricator

[land-bot] Add a sanity rebase after running smoke tests
ClosedPublic

Authored by jasonbcox on Sep 17 2020, 00:24.

Details

Summary

This ensures that any non-colliding changes pushed upstream within the few minute
window that smoke tests were running will not break the git push. While we do make
the assumption that tests still pass after this, breaking changes from automated
diffs should be exceedingly rare. For human-authored changes, we have a land queue to
help mitigate this.

This is copied from automated-commits.sh to bring this script up to
par with the sanity checking we already do there. The next step is to cleanup
automated-commits.sh so that it calls land-patch.sh directly.

Depends on D7467 and D7911

Test Plan
cmake -GNinja -DENABLE_SOURCE_CONTROL_TOOLS_TESTS=ON ..
ninja check-source-control-tools

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

deadalnix requested changes to this revision.Sep 17 2020, 13:03
deadalnix added a subscriber: deadalnix.

I don't see what problem this solves. The description doesn't explains this.

This revision now requires changes to proceed.Sep 17 2020, 13:03
jasonbcox edited the summary of this revision. (Show Details)
deadalnix requested changes to this revision.Sep 17 2020, 15:49

This does not solve the problem mentioned, because there might be more changes in between the rebase and the push. This is fundamentally broken and cannot be fixed with this approach.

If git push cannot handle this, then you are bound to rebase in a loop for as long as push is non fast forward - but still fail if the push fails for other reasons, or if the rebase fails. However I'd be very surprised if git did not include a way to do this properly already.

This revision now requires changes to proceed.Sep 17 2020, 15:49

You want something like:

do {
  PUSH_OUTPUT=$(git push --porcelain)
  PUSH_SUCCESS=$?
  if ($PUSH_SUCCESS) {
    exit(SUCCESS)
  }
} while (echo "$PUSH_OUTPUT" | grep -qF "To gitRepoURL ! refs/heads/master:refs/heads/master [rejected] (non-fast-forward) Done");

exit(FAILURE)

Make it more robust and add tests (now depends on D7911)

jasonbcox edited the test plan for this revision. (Show Details)
This revision is now accepted and ready to land.Oct 16 2020, 16:29