While investigating #31730 I noticed that
dev-mode preset already contained -DWITH_BDB=ON and -DBUILD_GUI=ON;USDT, and MULTIPROCESS and on Linux also without MULTIPROCESS.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31731.
<!--021abf342d371248e50ceaed478a90ca-->
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.
<!--174a7506f384e20aa4161008e828411d-->
Reviewers, this pull request conflicts with the following ones:
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.
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
Couldn't disabling lead to missed translations?
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?
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.
We should add a Mac section in that case, because for me
cmake --preset dev-mode
fails with:
CMake Error at /opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:233 (message):
Could NOT find USDT (missing: HAVE_USDT_H)
Call Stack (most recent call first):
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:603 (_FPHSA_FAILURE_MESSAGE)
cmake/module/FindUSDT.cmake:57 (find_package_handle_standard_args)
CMakeLists.txt:131 (find_package)
and
cmake --preset dev-mode -DWITH_USDT=OFF
fails with
CMake Error at CMakeLists.txt:146 (find_package):
By not providing "FindLibmultiprocess.cmake" in CMAKE_MODULE_PATH this
project has asked CMake to find a package configuration file provided by
"Libmultiprocess", but CMake did not find one.
Could not find a package configuration file provided by "Libmultiprocess"
with any of the following names:
LibmultiprocessConfig.cmake
libmultiprocess-config.cmake
Add the installation prefix of "Libmultiprocess" to CMAKE_PREFIX_PATH or
set "Libmultiprocess_DIR" to a directory containing one of the above files.
If "Libmultiprocess" provides a separate development package or SDK, be
sure it has been installed.
Or should we remove multiprocess from dev-mode after #31709?
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).
Thank you! What about the MULTIPROCESS option? I cannot yet run this on my Mac or on Linux, unless I disable it.
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`.
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?
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
This PR was meant to fix the example only, I don't have enough context to update the docs as well.
Added back -DWITH_USDT=OFF
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).
The command only runs on my Mac if we disable USDT which won't affect translation
Thanks @pablomartin4btc for the review, I've added back -DWITH_USDT=OFF
ACK 758a93d6215c2fa4799741d721e610a8a7214c34.
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."