Add update-manpages to automated-commits
ClosedPublic
Actions

Authored by jasonbcox on Feb 25 2020, 23:40.

Details

Reviewers

Fabien

Group Reviewers

Restricted Project

Commits

rSTAGING9224e2c142c5: Add update-manpages to automated-commits
rABC9224e2c142c5: Add update-manpages to automated-commits

Summary

This enables us to automatically update manpages immediately prior to cutting a release.

Depends on D5248

Test Plan

COMMIT_TYPE=update-manpages ./automated-commits.sh

Diff Detail

Repository

rABC Bitcoin ABC

Branch

automanpages

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 9577
Build 17058: Default Diff Build & Tests
Build 17057: arc lint + arc unit

Event Timeline

jasonbcox created this revision.Feb 25 2020, 23:40

Herald added a reviewer: Restricted Project. · View Herald TranscriptFeb 25 2020, 23:40

Build Bitcoin-ABC / Diffs / Diff Testing started.

jasonbcox planned changes to this revision.Feb 25 2020, 23:45

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9574: Diff 16513.Feb 25 2020, 23:50

Rebase

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B9577: Diff 16516.Feb 26 2020, 00:53

Fabien requested changes to this revision.Feb 26 2020, 15:20

Fabien added a subscriber: Fabien.

Fabien added inline comments.

contrib/source-control-tools/automated-commits.sh
109	That seems unnecessary.
111	Dito, already done by `build_cmake.sh`
128	That would be a good idea to check against `bitcoind -version`.
133	Use a wildcard here, something like `*.1`. At some point there will be `bitcoin-wallet` and maybe others, that would limit the amount of maintenance.

This revision now requires changes to proceed.Feb 26 2020, 15:20

jasonbcox added inline comments.Feb 26 2020, 17:37

contrib/source-control-tools/automated-commits.sh
109	This is necessary to run ninja in the right place. But it looks like running ninja explicitly isn't needed (see other comment).
111	`build_cmake.sh` runs `ninja`, not `ninja all`. I'm not sure if it's an illusion on my end, but I was certain at one time that `ninja all` was necessary to build all of the binaries. However I've double checked in a clean build dir and this doesn't seem to be the case.
128	This is a good idea.
133	I thought about this, but was taking a very conservative stance on which files would be added for the other automated commits. I think it's safe-ish to use a wildcard here only because these files are documentation and very unlikely to break the build.

Addressed feedback

Build Bitcoin-ABC / Diffs / Diff Testing started.

Harbormaster completed remote builds in B9583: Diff 16526.Feb 26 2020, 18:35

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Fabien added inline comments.Feb 26 2020, 20:53

contrib/source-control-tools/automated-commits.sh
111	Without any argument, `ninja` runs the default target which is `all`. Unless these defaults are changed (not sure how to do so), `ninja` and `ninja all` are the same. We use `ninja all` where there is another target as an argument, like `ninja all check` to ensure that default targets are built in addition to the ones required by the check target (e.g. `bitcoind` is not needed to run `check` and won't be built). Note that `all` doesn't mean that all the targets are built ! e.g. test_bitcoin is not part of the `all` target (but is a dependency for the `check` target).

jasonbcox added inline comments.Feb 26 2020, 22:33

contrib/source-control-tools/automated-commits.sh
111	Got it. Thanks for clearing that up.

Fabien accepted this revision.Feb 27 2020, 07:12

Fabien added inline comments.

contrib/source-control-tools/automated-commits.sh
133	If you're really concerned about this, there is another solution: you can run `ninja install` to some tmp dir and use the installed man files as a filter. Not sure it's worth it.