build: warn when generating man pages for binaries built from a dirty branch

[tylerchambers]

• Adjusted --version flag behavior in bitcoind and bitcoin-wallet to have the same behavior.
• Added --version flag to bitcoin-tx to match.
• Added functionality in gen-manpages.sh to error when attempting to generate man pages for binaries built from a dirty branch.

mitigates problem with issue #20412

[tylerchambers]

Just noticed the failing checks. On mobile, not sure how to mark as in progress, will investigate.

[laanwj]

Thank you! Need to test this but changes look good to me at first sight.

Implementing -version consistency across the tools makes sense too.

[laanwj]

Travis failure is boring (due to trailing whitespace)

This diff appears to have added new lines with trailing whitespace.
The following changes were suspected:
diff --git a/contrib/devtools/gen-manpages.sh b/contrib/devtools/gen-manpages.sh
@@ -20,0 +21,15 @@ BITCOINQT=${BITCOINQT:-$BINDIR/qt/bitcoin-qt}
+then
diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp
@@ -100,6 +101,9 @@ static int AppInitRawTx(int argc, char* argv[])
+
diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp
@@ -45,10 +46,12 @@ static bool WalletAppInit(int argc, char* argv[])
+                    "Usage:\n"
^---- failure generated from test/lint/lint-whitespace.sh

[tylerchambers]

fixed linting issues, we should be good to go now. thanks!

[luke-jr]

Concept NACK. No reason manpages shouldn't generate for any given checkout...

Maybe making sure the binaries match the current git describe would be okay.

Perhaps CI checks?

[tylerchambers]

@luke-jr ci checks would be good.

How does everyone feel about a warning/notification echoed out instead of returning early?

The warning to prevent accidental check-ins + the CI checks should get the desired effect while allowing man page generation at any point.

[luke-jr]

A warning sounds fine to me too.

[laanwj]

@luke-jr The problem is accidentally checking in manual pages with a -dirty tag. This happened to me a few times and more times almost.
This is why I'd like this behavior in. These scripts are part of our release process and should assist our release process by preventing common mistakes.

(I mean, the non-determinism implied by generating the pages from a state of the tree that was never checked in in the first place, isn't that wrong by default?)

[laanwj]

CI checks are way too late. And I don't really want to do anything complicated with git check-in hooks either. This script seems to be the best place to ensure it.

But sure, I'm okay with adding a way to bypass the check if you really want to. For say, testing the script it could make sense.

[luke-jr]

@laanwj The runtime warning from gen-manpages would hopefully be enough to alert the user not to commit it?

[tylerchambers]

updated to warn instead of outright fail, which should solve some of the problem in the original issue while still providing flexibility, should one want to generate man pages for bins built from a dirty branch.

[laanwj]

Tested ACK 6690adb

$ BUILDDIR="$PWD/build" contrib/devtools/gen-manpages.sh
$ cd build
$ make -j4
…
$ cd ..
$ BUILDDIR="$PWD/build" contrib/devtools/gen-manpages.sh

WARNING: the following binaries were built from a dirty tree:

/…/bitcoin/build/src/bitcoind
/…/bitcoin/build/src/bitcoin-cli
/…/bitcoin/build/src/bitcoin-tx
/…/bitcoin/build/src/bitcoin-wallet
/…/bitcoin/build/src/qt/bitcoin-qt

man pages generated from dirty binaries should NOT be committed.
To properly generate man pages, please commit your changes to the above binaries, rebuild them, then run this script again.

Yea seems strong enough warning.
Thanks for working on this!

[hebasto]

NACK, this change makes command line options help unavailable for bitcoind.

[hebasto]

I had a broken setup. Sorry for noise.