doc: update translation generation cmake example

l0rinc commented at 12:45 pm on January 24, 2025: contributor

While investigating #31730 I noticed that

the dev-mode preset already contained -DWITH_BDB=ON and -DBUILD_GUI=ON;
the preset already contained a default binary dir which we could use;
the command only runs on my Mac if we disable USDT, and MULTIPROCESS and on Linux also without MULTIPROCESS.

DrahtBot commented at 12:45 pm on January 24, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31731.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto
Stale ACK	pablomartin4btc

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Docs on Jan 24, 2025

in doc/translation_process.md:21 in c2f375641a outdated

17@@ -18,8 +18,8 @@ We use automated scripts to help extract translations in both Qt, and non-Qt sou
18 
19 To automatically regenerate the `bitcoin_en.ts` file, run the following commands:
20 ```sh
21-cmake -B build --preset dev-mode -DWITH_BDB=ON -DBUILD_GUI=ON
22-cmake --build build --target translate
23+cmake --preset dev-mode -DWITH_USDT=OFF -DWITH_MULTIPROCESS=OFF

maflcko commented at 1:07 pm on January 24, 2025:

Couldn’t disabling lead to missed translations?

l0rinc commented at 1:35 pm on January 24, 2025:

Didn’t expect USDT to be necessary here (which I understood only works on Linux, right?) So I’ll remove the OFFs and add a comment that this should be run from Linux only - would that work?

fanquake commented at 2:30 pm on January 24, 2025:

and add a comment that this should be run from Linux only

I don’t think there’s a requirement that updating/verifying the translations requires a Linux machine.

l0rinc commented at 3:43 pm on January 24, 2025:

We should add a Mac section in that case, because for me

cmake –preset dev-mode

fails with:

0CMake Error at /opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:233 (message):
1  Could NOT find USDT (missing: HAVE_USDT_H)
2Call Stack (most recent call first):
3  /opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:603 (_FPHSA_FAILURE_MESSAGE)
4  cmake/module/FindUSDT.cmake:57 (find_package_handle_standard_args)
5  CMakeLists.txt:131 (find_package)

and

cmake –preset dev-mode -DWITH_USDT=OFF

fails with

 0CMake Error at CMakeLists.txt:146 (find_package):
 1  By not providing "FindLibmultiprocess.cmake" in CMAKE_MODULE_PATH this
 2  project has asked CMake to find a package configuration file provided by
 3  "Libmultiprocess", but CMake did not find one.
 4
 5  Could not find a package configuration file provided by "Libmultiprocess"
 6  with any of the following names:
 7
 8    LibmultiprocessConfig.cmake
 9    libmultiprocess-config.cmake
10
11  Add the installation prefix of "Libmultiprocess" to CMAKE_PREFIX_PATH or
12  set "Libmultiprocess_DIR" to a directory containing one of the above files.
13  If "Libmultiprocess" provides a separate development package or SDK, be
14  sure it has been installed.

l0rinc commented at 4:38 pm on January 24, 2025:

Or should we remove multiprocess from dev-mode after #31709?

hebasto commented at 2:59 pm on January 30, 2025:

Couldn’t disabling lead to missed translations?

A build option might affect the resulting translations if it changes the list of source files and headers containing translatable strings. However, this is not the case for WITH_USDT. It only controls the ENABLE_TRACING macro. Additionally, both the extract_strings_qt.py script and Qt’s lupdate tool process raw sources (not preprocessed ones).

l0rinc commented at 3:16 pm on January 30, 2025:

Thank you! What about the MULTIPROCESS option? I cannot yet run this on my Mac or on Linux, unless I disable it.

hebasto commented at 3:41 pm on January 30, 2025:

What about the MULTIPROCESS option?

WITH_MULTIPROCESS does modify source list, for example:https://github.com/bitcoin/bitcoin/blob/809d7e763cc9bdfff3288860a1c530460c76ffff/src/qt/CMakeLists.txt#L244-L248

So, disabling it could potentially result in missed translations.

I cannot yet run this on my Mac or on Linux, unless I disable it.

I always build depends with MULTIPROCESS=1 before configuring and building the translate target`.

l0rinc commented at 3:44 pm on January 30, 2025:

Thanks, I’ll figure out how to make it work with MULTIPROCESS next time I need to do the translations :). So is the PR correct as it is now?

hebasto commented at 10:56 am on February 6, 2025:

You might consider referencing #31809 for the actual process of regenerating a translation source file.

Another important point to amend in this doc is that the translate target generates the src/qt/locale/bitcoin_en.xlf file, which is the translation source for the Transifex:https://github.com/bitcoin/bitcoin/blob/d6c229d8bd4a6203a7255c140aa35c59fb20378b/.tx/config#L6

l0rinc commented at 8:48 am on February 7, 2025:

This PR was meant to fix the example only, I don’t have enough context to update the docs as well.

l0rinc commented at 7:47 pm on February 19, 2025:

Added back -DWITH_USDT=OFF

l0rinc force-pushed on Jan 24, 2025

DrahtBot added the label CI failed on Jan 26, 2025

l0rinc force-pushed on Jan 26, 2025

DrahtBot removed the label CI failed on Jan 26, 2025

i-am-yuvi commented at 2:12 pm on January 27, 2025: contributor

I don’t think we need to keep USDT OFF as we’ll have support for macOS soon. But we can definitely remove -DWITH_BDB=ON -DBUILD_GUI=ON from the doc

see here

pablomartin4btc commented at 6:44 pm on February 19, 2025: member

ACK 79eb14003f42e1329fe3cdc17a42023c8b2d14b9

If we want to use the dev-mode preset perhaps we need to build /depends with MULTIPROCESS=1, as the preset has "WITH_MULTIPROCESS": "ON",. Please consider #31899 as it fixes the existent problem with multiprocess built.

We should add -DWITH_USDT=OFF (fails on mac otherwise) as @hebasto confirmed it won’t affect the translation.

Just for reference, translation generation instructions were updated with cmake (#30875) roughly around the same time the preset was updated with more options (#30871).

doc: update translation generation cmake example

The command only runs on my Mac if we disable USDT which won't affect translation

758a93d621

l0rinc force-pushed on Feb 19, 2025

l0rinc commented at 7:47 pm on February 19, 2025: contributor

Thanks @pablomartin4btc for the review, I’ve added back -DWITH_USDT=OFF

hebasto approved

hebasto commented at 5:12 pm on February 20, 2025: member

ACK 758a93d6215c2fa4799741d721e610a8a7214c34.

DrahtBot requested review from pablomartin4btc on Feb 20, 2025

fanquake merged this on Feb 20, 2025

fanquake closed this on Feb 20, 2025

pablomartin4btc commented at 6:31 pm on February 20, 2025: member

I think this would require a follow-up, we need to build depends with multiprocess otherwise the command would fail.

We could use -DWITH_MULTIPROCESS=OFF, but I do agree with @hebasto here:

“At present, multiprocess-specific sources do not contain any translatable strings. Nonetheless, it is prudent to maintain a general approach.”

pablomartin4btc referenced this in commit 02964624a6 on Feb 21, 2025

pablomartin4btc referenced this in commit ba3bb09a12 on Feb 21, 2025

l0rinc deleted the branch on Feb 22, 2025

doc: update translation generation cmake example #31731

Code Coverage & Benchmarks

Reviews

Conflicts