Repository: bitcoin/bitcoin (language: cpp)

Issue:
Manual page generation on master broken
I'm not sure if this is related to other version-related issues, but the `-version` output (when building from git) changed from
```
Bitcoin Core version v0.21.2.0-89d148c8c65b3e6b6a8fb8b722efb4b6a7d0a375
```
to
```
Bitcoin Core version v22.0rc2
```
This means that the generated manual pages (using `contrib/devtools/gen-manpages.sh`) now contain the rc number in the embedded version tag. See d3bd5410f64e96d106486643e3d4300a9fb0ee71:
```
+.\" DO NOT MODIFY THIS FILE!  It was generated by help2man 1.47.13.
+.TH BITCOIN-CLI "1" "August 2021" "bitcoin-cli v22.0rc2" "User Commands"
```
No big problem, though it is slightly inconvenient. Up until now we didn't have to regenerate the manual pages between the last rc and -final as there is (by how the process is defined) no code difference, so option won't have changed.


Discussion / hints:
This is just an order of operations issue. Typically the manpages are generated on a commit that is not tagged as the RC. But for the 22.0rc2 manpage update, it was done on the 22.0rc2 tagged commit rather than prior to it. If I run `contrib/devtools/gen-manpages.sh` on the current head of 22.x (which is not tagged), then the manpages have the expected version.

Perhaps we should update the script to deal with this case, but I wouldn't say that this is an issue that needs to be solved for 22.0.
The thing is, it didn't use to be necessary to update the manual pages between rcs if there were no changes that affect options. at all. Nor was it needed to do it one final time before `-final`.

I think adding the rc number into the manual pages is of questionable value at the least. It just creates extra work. And requiring a very specific order of operations tends to make things fragile.
> And requiring a very specific order of operations tends to make things fragile.

Oh, so apparently #22685 made this worse, even the right order of operations doesn't solve it anymore.
There is a more serious problem with the generated manual pages. It looks like all `-`'s in the text have been purged; e.g.
```patch
-nblocks and maxtries arguments. Example: bitcoin-cli \fBgenerate\fR 4
+nblocks and maxtries arguments. Example: bitcoincli \fBgenerate\fR 4
```
The cause of this is the `sed -i` with `-${BTCVER[1]}` which ends up as empty. So all `-`'s are replaced with nothing.

Need to add a check whether `${BTCVER[1]}` is empty, and if so, skip the `sed -i`.
Also `footer.h2m` ends up empty, somehow, besides the `[COPYRIGHT]`. I think it's a leftover from then `--version` contained a copyright message?

(I don't think this one is critical; the copyright message still ends up in DESCRIPTION as it's printed by `--help`,  just not in the COPYRIGHT section)
Hmmm... No idea about the copyright footer, but for the versioning: what if we get rid of all our munging in the script and just change the process a bit? We'll just generate man pages after we bump the version. A little extra work between last `-rc` and `-final`, but we can get rid of code that looks a bit fragile to me.

Not sure if this diff is the right diff to release-process.md, but something like:
```diff
diff --git a/doc/release-process.md b/doc/release-process.md
index 1b6472e812..05fa2af378 100644
--- a/doc/release-process.md
+++ b/doc/release-process.md
@@ -6,13 +6,14 @@ Release Process
 ### Before every release candidate
 
 * Update translations see [translation_process.md](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md#synchronising-translations).
-* Update manpages, see [gen-manpages.sh](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#gen-manpagessh).
 * Update release candidate version in `configure.ac` (`CLIENT_VERSION_RC`).
+* Update manpages, see [gen-manpages.sh](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#gen-manpagessh).
 
 ### Before every major and minor release
 
 * Update [bips.md](bips.md) to account for changes since the last release (don't forget to bump the version number on the first line).
 * Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_RC` to `0`).
+* Update manpages, see [gen-manpages.sh](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#gen-manpagessh).
 * Write release notes (see "Write the release notes" below).
 
 ### Before every major release
```
Don't forget to also do a build in between. Just updating `configure.ac` is not enough. Also commit after updating `configure.ac` (but before the build!), otherwise you'll get `-dirty` in the version in the manpages (or maybe this is no longer the case now? i don't know).
(this is why the whole "run it for every RC" feels fragile to me apart from the risk of forgetting to run it)
Ideally we'd have *some* flexibility on when to run the steps and in what order, also to allow distributing the work.
> There is a more serious problem with the generated manual pages. It looks like all -'s in the text have been purged; e.g.

No idea how we want to fix this eventually, but I hacked around this in c1c79f4c816fa1224ad593f2af90752160fabb09 on the 22.x branch.  86de56776aaef9fc043c22200a88f3a9513234fb brings back correct hypen-ization.

Removing the milestone as this was fixed (well…) on the 22.x branch and is not a blocker. Needs a proper fix for the next major release. Probably consisting of deleting most of the script as suggested by @dongcarl .

Fix patch:
diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md
index afbad096c4b97..79b0134adc25d 100644
--- a/contrib/devtools/README.md
+++ b/contrib/devtools/README.md
@@ -76,7 +76,7 @@ year rather than two hyphenated years.
 If the file already has a copyright for `The Bitcoin Core developers`, the
 script will exit.
 
-gen-manpages.sh
+gen-manpages.py
 ===============
 
 A small script to automatically create manpages in ../../doc/man by running the release binaries with the -help option.
@@ -87,7 +87,7 @@ repostitory. To use this tool with out-of-tree builds set `BUILDDIR`. For
 example:
 
 ```bash
-BUILDDIR=$PWD/build contrib/devtools/gen-manpages.sh
+BUILDDIR=$PWD/build contrib/devtools/gen-manpages.py
 ```
 
 security-check.py and test-security-check.py
diff --git a/contrib/devtools/gen-manpages.py b/contrib/devtools/gen-manpages.py
new file mode 100755
index 0000000000000..05f613da38d39
--- /dev/null
+++ b/contrib/devtools/gen-manpages.py
@@ -0,0 +1,73 @@
+#!/usr/bin/env python3
+# Copyright (c) 2022 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+import os
+import subprocess
+import sys
+import tempfile
+
+BINARIES = [
+'src/bitcoind',
+'src/bitcoin-cli',
+'src/bitcoin-tx',
+'src/bitcoin-wallet',
+'src/bitcoin-util',
+'src/qt/bitcoin-qt',
+]
+
+# Paths to external utilities.
+git = os.getenv('GIT', 'git')
+help2man = os.getenv('HELP2MAN', 'help2man')
+
+# If not otherwise specified, get top directory from git.
+topdir = os.getenv('TOPDIR')
+if not topdir:
+    r = subprocess.run([git, 'rev-parse', '--show-toplevel'], stdout=subprocess.PIPE, check=True, universal_newlines=True)
+    topdir = r.stdout.rstrip()
+
+# Get input and output directories.
+builddir = os.getenv('BUILDDIR', topdir)
+mandir = os.getenv('MANDIR', os.path.join(topdir, 'doc/man'))
+
+# Verify that all the required binaries are usable, and extract copyright
+# message in a first pass.
+copyright = None
+versions = []
+for relpath in BINARIES:
+    abspath = os.path.join(builddir, relpath)
+    try:
+        r = subprocess.run([abspath, '--version'], stdout=subprocess.PIPE, universal_newlines=True)
+    except IOError:
+        print(f'{abspath} not found or not an executable', file=sys.stderr)
+        sys.exit(1)
+    # take first line (which must contain version)
+    verstr = r.stdout.split('\n')[0]
+    # last word of line is the actual version e.g. v22.99.0-5c6b3d5b3508
+    verstr = verstr.split()[-1]
+    assert verstr.startswith('v')
+
+    # Only bitcoin-qt prints the copyright message on --version, so store it specifically.
+    if relpath == 'src/qt/bitcoin-qt':
+        copyright = r.stdout.split('\n')[1:]
+
+    versions.append((abspath, verstr))
+
+if any(verstr.endswith('-dirty') for (_, verstr) in versions):
+    print("WARNING: Binaries were built from a dirty tree.")
+    print('man pages generated from dirty binaries should NOT be committed.')
+    print('To properly generate man pages, please commit your changes (or discard them), rebuild, then run this script again.')
+    print()
+
+with tempfile.NamedTemporaryFile('w', suffix='.h2m') as footer:
+    # Create copyright footer, and write it to a temporary include file.
+    assert copyright
+    footer.write('[COPYRIGHT]\n')
+    footer.write('\n'.join(copyright).strip())
+    footer.flush()
+
+    # Call the binaries through help2man to produce a manual page for each of them.
+    for (abspath, verstr) in versions:
+        outname = os.path.join(mandir, os.path.basename(abspath) + '.1')
+        print(f'Generating {outname}…')
+        subprocess.run([help2man, '-N', '--version-string=' + verstr, '--include=' + footer.name, '-o', outname, abspath], check=True)
diff --git a/contrib/devtools/gen-manpages.sh b/contrib/devtools/gen-manpages.sh
deleted file mode 100755
index 8da6ff120477a..0000000000000
--- a/contrib/devtools/gen-manpages.sh
+++ /dev/null
@@ -1,53 +0,0 @@
-#!/usr/bin/env bash
-# Copyright (c) 2016-2021 The Bitcoin Core developers
-# Distributed under the MIT software license, see the accompanying
-# file COPYING or http://www.opensource.org/licenses/mit-license.php.
-
-export LC_ALL=C
-TOPDIR=${TOPDIR:-$(git rev-parse --show-toplevel)}
-BUILDDIR=${BUILDDIR:-$TOPDIR}
-
-BINDIR=${BINDIR:-$BUILDDIR/src}
-MANDIR=${MANDIR:-$TOPDIR/doc/man}
-
-BITCOIND=${BITCOIND:-$BINDIR/bitcoind}
-BITCOINCLI=${BITCOINCLI:-$BINDIR/bitcoin-cli}
-BITCOINTX=${BITCOINTX:-$BINDIR/bitcoin-tx}
-WALLET_TOOL=${WALLET_TOOL:-$BINDIR/bitcoin-wallet}
-BITCOINUTIL=${BITCOINQT:-$BINDIR/bitcoin-util}
-BITCOINQT=${BITCOINQT:-$BINDIR/qt/bitcoin-qt}
-
-[ ! -x "$BITCOIND" ] && echo "$BITCOIND not found or not executable." && exit 1
-
-# Don't allow man pages to be generated for binaries built from a dirty tree
-DIRTY=""
-for cmd in $BITCOIND $BITCOINCLI $BITCOINTX $WALLET_TOOL $BITCOINUTIL $BITCOINQT; do
-  VERSION_OUTPUT=$($cmd --version)
-  if [[ $VERSION_OUTPUT == *"dirty"* ]]; then
-    DIRTY="${DIRTY}${cmd}\n"
-  fi
-done
-if [ -n "$DIRTY" ]
-then
-  echo -e "WARNING: the following binaries were built from a dirty tree:\n"
-  echo -e "$DIRTY"
-  echo "man pages generated from dirty binaries should NOT be committed."
-  echo "To properly generate man pages, please commit your changes to the above binaries, rebuild them, then run this script again."
-fi
-
-# The autodetected version git tag can screw up manpage output a little bit
-read -r -a BTCVER <<< "$($BITCOINCLI --version | head -n1 | awk -F'[ -]' '{ print $6, $7 }')"
-
-# Create a footer file with copyright content.
-# This gets autodetected fine for bitcoind if --version-string is not set,
-# but has different outcomes for bitcoin-qt and bitcoin-cli.
-echo "[COPYRIGHT]" > footer.h2m
-$BITCOIND --version | sed -n '1!p' >> footer.h2m
-
-for cmd in $BITCOIND $BITCOINCLI $BITCOINTX $WALLET_TOOL $BITCOINUTIL $BITCOINQT; do
-  cmdname="${cmd##*/}"
-  help2man -N --version-string="${BTCVER[0]}" --include=footer.h2m -o "${MANDIR}/${cmdname}.1" "${cmd}"
-  sed -i "s/\\\-${BTCVER[1]}//g" "${MANDIR}/${cmdname}.1"
-done
-
-rm -f footer.h2m
diff --git a/doc/release-process.md b/doc/release-process.md
index f786b345b1a94..86fd366c1fe03 100644
--- a/doc/release-process.md
+++ b/doc/release-process.md
@@ -6,13 +6,14 @@ Release Process
 ### Before every release candidate
 
 * Update translations see [translation_process.md](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md#synchronising-translations).
-* Update manpages, see [gen-manpages.sh](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#gen-manpagessh).
 * Update release candidate version in `configure.ac` (`CLIENT_VERSION_RC`).
+* Update manpages (after rebuilding the binaries), see [gen-manpages.py](https://github.com/bitcoin/bitcoin/blob/master/contrib/devtools/README.md#gen-manpagespy).
 
 ### Before every major and minor release
 
 * Update [bips.md](bips.md) to account for changes since the last release (don't forget to bump the version number on the first line).
 * Update version in `configure.ac` (don't forget to set `CLIENT_VERSION_RC` to `0`).
+* Update manpages (see previous section)
 * Write release notes (see "Write the release notes" below).
 
 ### Before every major release


Test patch: