From 6947dfbda6a72a9b3d4bcb651f28452e75870f9d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 29 Aug 2020 10:03:41 +0200 Subject: [PATCH 1/2] Merge #18817: doc: Document differences in bitcoind and bitcoin-qt locale handling ca185cf5a14b16d61814d7172284bc8efcd28b69 doc: Document differences in bitcoind and bitcoin-qt locale handling (practicalswift) Pull request description: Document differences in `bitcoind` and `bitcoin-qt` locale handling. Since this seems to be the root cause to the locale dependency issues we've seen over the years I thought it was worth documenting :) Note that 1.) `QLocale` (used by Qt), 2.) C locale (used by locale-sensitive C standard library functions/POSIX functions and some parts of the C++ standard library such as `std::to_string`) and 3.) C++ locale (used by the C++ input/output library) are three separate things. This comment is about the perhaps surprising interference with the C locale (2) that takes place as part of the Qt initialization. ACKs for top commit: hebasto: re-ACK ca185cf5a14b16d61814d7172284bc8efcd28b69 Tree-SHA512: e51c32f3072c506b0029a001d8b108125e1acb4f2b6a48a6be721ddadda9da0ae77a9b39ff33f9d9eebabe2244c1db09e8502e3e7012d7a5d40d98e96da0dc44 --- src/qt/bitcoin.cpp | 1 + test/lint/lint-locale-dependence.sh | 33 +++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 4ee2d11fb03d..fc3c246c2960 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -206,6 +206,7 @@ BitcoinApplication::BitcoinApplication(interfaces::Node& node): pollShutdownTimer(nullptr), returnValue(0) { + // Qt runs setlocale(LC_ALL, "") on initialization. setQuitOnLastWindowClosed(false); } diff --git a/test/lint/lint-locale-dependence.sh b/test/lint/lint-locale-dependence.sh index 7020c43917d7..3f1ef8daf1b5 100755 --- a/test/lint/lint-locale-dependence.sh +++ b/test/lint/lint-locale-dependence.sh @@ -8,6 +8,39 @@ export LC_ALL=C # TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent stoul/strtol with locale # independent ToIntegral(...). # TODO: Reduce KNOWN_VIOLATIONS by replacing uses of locale dependent snprintf with strprintf. + +# Be aware that bitcoind and bitcoin-qt differ in terms of localization: Qt +# opts in to POSIX localization by running setlocale(LC_ALL, "") on startup, +# whereas no such call is made in bitcoind. +# +# Qt runs setlocale(LC_ALL, "") on initialization. This installs the locale +# specified by the user's LC_ALL (or LC_*) environment variable as the new +# C locale. +# +# In contrast, bitcoind does not opt in to localization -- no call to +# setlocale(LC_ALL, "") is made and the environment variables LC_* are +# thus ignored. +# +# This results in situations where bitcoind is guaranteed to be running +# with the classic locale ("C") whereas the locale of bitcoin-qt will vary +# depending on the user's environment variables. +# +# An example: Assuming the environment variable LC_ALL=de_DE then the +# call std::to_string(1.23) will return "1.230000" in bitcoind but +# "1,230000" in bitcoin-qt. +# +# From the Qt documentation: +# "On Unix/Linux Qt is configured to use the system locale settings by default. +# This can cause a conflict when using POSIX functions, for instance, when +# converting between data types such as floats and strings, since the notation +# may differ between locales. To get around this problem, call the POSIX function +# setlocale(LC_NUMERIC,"C") right after initializing QApplication, QGuiApplication +# or QCoreApplication to reset the locale that is used for number formatting to +# "C"-locale." +# +# See https://doc.qt.io/qt-5/qcoreapplication.html#locale-settings and +# https://stackoverflow.com/a/34878283 for more details. + KNOWN_VIOLATIONS=( "src/bitcoin-tx.cpp.*stoul" "src/dbwrapper.cpp.*stoul" From b35557be971ebcd45bf1941d1fe3f8c1d845ef8b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 16 May 2020 06:16:45 -0400 Subject: [PATCH 2/2] Merge #18952: test: avoid os-dependant path 8a22fd01140bd957036fc00419b147e4268ae9b1 avoided os-dependant path (Ferdinando M. Ametrano) Pull request description: The current code fails on windows because of the forward slashes; using os.path.join solves the problem and it is in general more robust ACKs for top commit: MarcoFalke: ACK 8a22fd01140bd957036fc00419b147e4268ae9b1 Tree-SHA512: 813f27aea33f97c8afac52e716a55fc5d7fb69621023aba99d40df7e1d145e0ec8d1eee49ddd403b219bf0e0e168e0e987b05c78eaef611f744d99bf2fc8bc91 --- test/functional/test_framework/test_framework.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index abff8e31ee24..799142a619fa 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -238,8 +238,18 @@ def setup(self): config = self.config - self.options.bitcoind = os.getenv("BITCOIND", default=config["environment"]["BUILDDIR"] + '/src/dashd' + config["environment"]["EXEEXT"]) - self.options.bitcoincli = os.getenv("BITCOINCLI", default=config["environment"]["BUILDDIR"] + '/src/dash-cli' + config["environment"]["EXEEXT"]) + fname_bitcoind = os.path.join( + config["environment"]["BUILDDIR"], + "src", + "dashd" + config["environment"]["EXEEXT"] + ) + fname_bitcoincli = os.path.join( + config["environment"]["BUILDDIR"], + "src", + "dash-cli" + config["environment"]["EXEEXT"] + ) + self.options.bitcoind = os.getenv("BITCOIND", default=fname_bitcoind) + self.options.bitcoincli = os.getenv("BITCOINCLI", default=fname_bitcoincli) self.extra_args_from_options = self.options.dashd_extra_args