init: Clarify C and C++ locale assumptions. Add locale sanity checks to verify assumptions at run-time. #18124

pull practicalswift wants to merge 3 commits into bitcoin:master from practicalswift:LocaleSanityCheck changing 4 files +34 −2
  1. practicalswift commented at 10:32 pm on February 11, 2020: contributor

    Clarify locale assumptions.

    Add locale sanity check making sure the global bitcoind C and C++ locales are set to the classic "C" locale (the minimal locale).

    Add locale sanity check making sure the global bitcoin-qt C++ locale is set to the classic "C" locale (the minimal locale). (We do not assume any specific C locale in bitcoin-qt.)

    To summarize - assumed locales:

    Program C locale (setlocale) Global C++ locale (std::locale)
    bitcoind C (classic) C (classic)
    bitcoin-qt No assumed locale C (classic)
  2. DrahtBot added the label Tests on Feb 11, 2020
  3. bitcoind init: Add locale sanity check making sure the global bitcoind C and C++ locales are set to the classic "C" locale (the minimal locale) 7e7cd83caa
  4. practicalswift force-pushed on Feb 12, 2020
  5. practicalswift renamed this:
    init: Clarify C and C++ locale assumptions. Add locale sanity check to verify assumptions at run-time.
    init: Clarify C and C++ locale assumptions. Add locale sanity checks to verify assumptions at run-time.
    on Feb 12, 2020
  6. bitcoin-qt init: Add locale sanity check to making sure the global bitcoin-qt C++ locale is set to the classic "C" locale (the minimal locale) 83ba9b2513
  7. doc: Clarify locale assumptions e489fb9e5f
  8. practicalswift force-pushed on Feb 12, 2020
  9. laanwj commented at 3:36 pm on February 12, 2020: member

    I’m not sure whether it’s better to crash than start with a “wrong” locale. We’re already avoiding using locale-dependent functions in pretty much all important places.

    Edit: also, why are our assumptions different for bitcoin-qt and bitcoind?

  10. practicalswift commented at 3:45 pm on February 12, 2020: contributor

    @laanwj I think you misunderstand the patch. Note that currently there is no way for a user to make bitcoind or bitcoin-qt start with the “wrong” locale (“wrong” as in that the assertions in this PR does not hold).

    This is just a sanity check that makes sure that the assumption we know to be true today remain true also in the future. Like ECC_InitSanityCheck, Random_SanityCheck, etc it tests that our assumptions hold true also in practice at run-time. What risks do you see?

  11. laanwj commented at 3:53 pm on February 12, 2020: member

    This is just my personal opinion but I think this adds some quite ugly code into our already convoluted initialization process, using weird functions such as setlocale. I’d prefer to avoid it and focus on getting rid of locale dependent functions where it’s a problem.

    If we decide to do this, I’d prefer to add the common checks to the other sanity checks and not distinguish between bitcoind.cpp/qt/bitcoin.cpp.

  12. practicalswift commented at 3:58 pm on February 12, 2020: contributor

    @laanwj How would you suggest checking for the assumed locales in a less ugly way?

    How do you suggest reading the C locale without using setlocale? (Please note that the C locale is not the same thing as the C++ locale.)

  13. practicalswift commented at 4:04 pm on February 12, 2020: contributor

    If we decide to do this, I’d prefer to add the common checks to the other sanity checks and not distinguish between bitcoind.cpp/qt/bitcoin.cpp.

    But we’re making different assumptions for bitcoind and bitcoin-qt, no?

    Program C locale (setlocale) Global C++ locale (std::locale)
    bitcoind C (classic) C (classic)
    bitcoin-qt No assumed locale C (classic)

    As the code is written we apparently want the C locale to be user-defined in bitcoin-qt but not in bitcoind. That is how it is done now in practice at least.

  14. practicalswift commented at 4:14 pm on February 12, 2020: contributor

    Perhaps I should have included a few examples in the PR title. The gotcha list could be made very long, but this is a C locale gotcha example :)

    0$ cling
1[cling]$ #include <clocale>
2[cling]$ #include <string>
3[cling]$ std::string current_locale = {setlocale(LC_ALL, nullptr)}
4(std::string &) "C"
5[cling]$ std::to_string(1.23)
6(std::string) "1.230000"
7[cling]$ setlocale(LC_ALL, "de_DE");
8[cling]$ std::to_string(1.23)
9(std::string) "1,230000"

    Perhaps surprisingly std::to_string cares about the C locale and not the global C++ locale.

  15. practicalswift commented at 4:22 pm on February 12, 2020: contributor

    And a C++ locale gotcha:

     0$ cling
 1[cling]$ #include <locale>
 2[cling]$ #include <sstream>
 3[cling]$ std::locale{}.name()
 4(std::string) "C"
 5[cling]$ std::ostringstream oss1;
 6[cling]$ oss1 << 1000000;
 7[cling]$ std::string s1 = oss1.str()
 8(std::string &) "1000000"
 9[cling]$ std::locale::global(std::locale("de_DE"));
10[cling]$ std::ostringstream oss2;
11[cling]$ oss2 << 1000000;
12[cling]$ std::string s2 = oss2.str()
13(std::string &) "1.000.000"
  16. practicalswift commented at 8:04 pm on February 12, 2020: contributor

    More C locale WTF:

     0$ uname -s
 1Darwin
 2$ cat poc.cpp
 3#include <iostream>
 4#include <locale>
 5
 6int main(void) {
 7    setlocale(LC_ALL, "");
 8    std::cout << std::isspace(133) << ' ' << std::isspace(154) << ' ' << std::isspace(160);
 9    std::cout << '\n';
10}
11$ clang++ -o poc poc.cpp
12$ ./poc
131 0 1
14$ LC_ALL=en_US ./poc
151 0 1
16$ LC_ALL=C ./poc
170 0 0
18$ LC_ALL=ru_RU.KOI8-R ./poc # an "interesting" locale
190 1 0

    Let me know when I’ve made my case sufficiently clear :)

  17. sipa commented at 8:11 pm on February 12, 2020: member

    @practicalswift You’re making the case that locales are annoying, but we already knew that.

    I think what @laanwj means is that he’d rather just avoid any locale-dependent functionality in the code (to make the locale settings irrelevant) as opposed to forcing the locale to a known value (correct me if I’m wrong of course).

  18. Empact commented at 8:11 pm on February 12, 2020: member
    -0 I agree with @laanwj let’s remove locale dependence rather than reduce the set of configurations we can run against.
  19. practicalswift commented at 9:09 pm on February 12, 2020: contributor

    @Empact

    -0 I agree with @laanwj let’s remove locale dependence rather than reduce the set of configurations we can run against.

    I don’t follow. What user configuration was possible before this PR that is not possible after this PR? Can you give a specific example?

  20. practicalswift commented at 9:42 pm on February 12, 2020: contributor

    @sipa

    I think what @laanwj means is that he’d rather just avoid any locale-dependent functionality in the code (to make the locale settings irrelevant) as opposed to forcing the locale to a known value (correct me if I’m wrong of course).

    Note that this PR simply documents and asserts restrictions we already have in place. More specifically it does not disallow any configuration that is currently allowed. (Please correct me if you think I’m wrong.)

    I’ve worked a lot on removing the use of locale-dependent functions and I fail to see how that effort would be in conflict with documenting and asserting the currently made locale assumptions.

    It is my understanding the following are the only possible effective locale combinations before and after this PR:

    Program C locale (setlocale) Global C++ locale (std::locale)
    bitcoind C (classic) C (classic)
    bitcoin-qt User configured (no assumed locale) C (classic)

    Is that your understanding as well?

    Perhaps the suggestion is that we might want to change the current locale logic and allow for bitcoind to run with a C locale (setlocale) or global C++ locale (std::locale) other than the classic locale in the future? That sounds really risky. The mere thought of an accidental fork/network split made possible by a locale dependency bug in combination with the fact that we would allow running bitcoind which an effective non-classic C or C++ locale (think zh_CN!) gives me nightmares :)

    Again, please correct me if my reasoning is wrong. I could be missing something :)

  21. practicalswift commented at 10:14 pm on February 12, 2020: contributor
    Recommended reading to better understand the motivation behind this PR: “Differences between the C Locale and the C++ Locales”
  22. sipa commented at 10:18 pm on February 12, 2020: member
    No, what I’m trying to say is simply that if we don’t have locale-dependent functions, this should be unnecessary complexity.
  23. practicalswift commented at 10:23 pm on February 12, 2020: contributor

    @sipa

    It is my understanding the following are the only possible effective locale combinations before and after this PR:

    Program C locale (setlocale) Global C++ locale (std::locale)
    bitcoind C (classic) C (classic)
    bitcoin-qt User configured (no assumed locale) C (classic)

    Is that your understanding as well?

  24. sipa commented at 10:28 pm on February 12, 2020: member
    I don’t understand what you mean by “effective locale”. The locale shouldn’t matter, so for all we care they’re both always effectively equal to “foobar”.
  25. practicalswift commented at 10:50 pm on February 12, 2020: contributor

    @sipa Sorry for my sloppy wording: with “effective locale” I simply meant the current locale in use (the C locale and the global C++ locale).

    More specifically the following:

    0const std::locale current_cpp_locale;
1const std::string current_c_locale{setlocale(LC_ALL, nullptr)};

    I claim the following is guaranteed to hold true no matter user configuration:

    Program C locale (setlocale) Global C++ locale (std::locale)
    bitcoind C (classic) C (classic)
    bitcoin-qt User configured (no assumed locale) C (classic)

    To make it even more explicit - it is my understanding that the following assertions are guaranteed to hold for the entire lifetime of the processes regardless of user configuration and/or what the user has in his/hers LC_ALL:

    For bitcoind:

    0assert(std::locale{}.name() == "C");
1const std::string current_c_locale = {setlocale(LC_ALL, nullptr)};
2assert(current_c_locale == "C");
3
4std::ostringstream oss1;
5oss1 << 1000000;
6assert(oss1.str() == "1000000");
7assert(std::to_string(1.23) == "1.230000");

    For bitcoin-qt:

    0assert(std::locale{}.name() == "C");
1
2std::ostringstream oss1;
3oss1 << 1000000;
4assert(oss1.str() == "1000000");

    Is that your understanding as well? :)

  26. sipa commented at 11:03 pm on February 12, 2020: member
    Before this PR, those assertions obviously don’t hold? I’m very confused.
  27. DrahtBot commented at 11:03 pm on February 12, 2020: member

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

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #18539 (Avoid using locale-dependent boost trim functions in RPCAuthorized(…) and bitcoin-tx by practicalswift)
    • #18130 (Replace uses of boost::trim* with locale-independent alternatives by Empact)

    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.

  28. practicalswift commented at 11:21 pm on February 12, 2020: contributor

    Before this PR, those assertions obviously don’t hold? I’m very confused.

    Then I’m confused by your confusion :)

    What assertion are you claiming not to hold before this PR? The bitcoind or the bitcoin-qt assertions or all of them? A single counter-example would settle this. What user configuration should I run to make the assertions fail?

    Very interesting: it seems like one of us will walk away from here with a “TIL moment” :) Very exciting regardless of outcome! :)

    FWIW:

     0$ git diff
 1diff --git a/src/logging.h b/src/logging.h
 2index b2fde1b9e..6b3bc5032 100644
 3--- a/src/logging.h
 4+++ b/src/logging.h
 5@@ -10,9 +10,12 @@
 6 #include <tinyformat.h>
 7 
 8 #include <atomic>
 9+#include <clocale>
10 #include <cstdint>
11 #include <list>
12+#include <locale>
13 #include <mutex>
14+#include <sstream>
15 #include <string>
16 #include <vector>
17 
18@@ -162,6 +165,15 @@ bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str);
19 template <typename... Args>
20 static inline void LogPrintf(const char* fmt, const Args&... args)
21 {
22+    std::ostringstream oss1;
23+    oss1 << 1000000;
24+    assert(oss1.str() == "1000000");
25+    assert(std::to_string(1.23) == "1.230000");
26+
27+    assert(std::locale{}.name() == "C");
28+    std::string current_locale = {setlocale(LC_ALL, nullptr)};
29+    assert(current_locale == "C");
30+
31     if (LogInstance().Enabled()) {
32         std::string log_msg;
33         try {
34$ make
35$ LC_ALL=de_DE src/bitcoind
36… note the absence of assertion failures …
  29. sipa commented at 11:32 pm on February 12, 2020: member
    std::to_string(1.23) won’t always be "1.230000" with the current code, if LC_ALL isn’t set to C?
  30. practicalswift commented at 11:45 pm on February 12, 2020: contributor

    @sipa

    As you probably know the equivalent of std::setlocale(LC_ALL, "C"); is always executed before main is run.

    I think you’re incorrectly assuming that we are doing std::setlocale(LC_ALL, ""); in the bitcoind code (which would set the locale to whatever the user has set the environment variable LC_ALL to) .

    Could that be the case?

  31. sipa commented at 11:52 pm on February 12, 2020: member
    No, I’m not assuming any setlocale call. I’m saying that before this PR, there is no such call, and thus std::to_string(1.23) right now won’t be guaranteed to be “1.230000”.
  32. practicalswift commented at 0:03 am on February 13, 2020: contributor

    @sipa

    Sounds like you’re incorrectly assuming that setlocale(LC_ALL, nullptr); sets the locale. Could that be the case? Note that setlocale(LC_ALL, nullptr); is not equivalent to setlocale(LC_ALL, "");.

    I claim that std::to_string(1.23) right now (prior to this PR) is guaranteed to be "1.230000" during the entire lifetime of the bitcoind process regardless of the user’s configuration and/or his/her LC_ALL environment variable.

    If you still don’t think that is the case then I kindly ask you to provide a counter-example in the form of a configuration and/or diff. I would love to be proved wrong and walk away with a TIL experience :)

    FWIW:

     0SYNOPSIS
 1       #include <locale.h>
 2
 3       char *setlocale(int category, const char *locale);
 4
 5DESCRIPTION
 6       The setlocale() function is used to set or query the program's current locale.
 7
 8       If  locale is not NULL, the program's current locale is modified according
 9       to the arguments.
10
11       […]
12
13       If locale is NULL, the current locale is only queried, not modified.
14
15       […]
16
17       On startup of the main program, the portable "C" locale is selected as default.
18       A program may be made portable to all locales by calling:
19
20           setlocale(LC_ALL, "");
21
22       after program initialization […]
  33. sipa commented at 0:52 am on February 13, 2020: member

    Aha, I was indeed making the assumption that setlocale(?, nullptr) was changing the locale.

    In that case, I have another concern: doesn’t this break running bitcoind with locale env variables set to anything but LC_ALL?

  34. practicalswift commented at 4:56 pm on February 13, 2020: contributor

    @sipa

    In that case, I have another concern: doesn’t this break running bitcoind with locale env variables set to anything but LC_ALL?

    It sounds like you’re incorrectly assuming that any of the LC_* environment variables will somehow control the C and C++ locales used without any explicit call to setlocale(LC_*, "") being made in the user code. Could that be the case?

    As you probably know the equivalent of setlocale(LC_ALL, "C") is called automatically in all applications before main.

    Thus when the execution of main begins the C locale and the global C++ locale are both guaranteed to equal to the classic locale (a.k.a. the “C” locale).

    Without any explicit call to setlocale(LC_*, "") in the code the default “C” locale will be in use throughout the process lifetime no matter what the LC_* environment variables are set to.

    Let’s demonstrate this with a small test program:

     0#include <clocale>
 1#include <cstdlib>
 2#include <iostream>
 3#include <locale>
 4#include <sstream>
 5#include <string>
 6
 7int main(void)
 8{
 9    std::cout << "C locale at beginning of main: " << std::string{setlocale(LC_ALL, nullptr)} << "\n";
10    std::cout << "C++ locale at beginning of main: " << std::locale{}.name() << "\n\n";
11#if OPT_IN_TO_LOCALIZATION_USING_SET_LOCALE
12    setlocale(LC_ALL, "");
13#endif
14#if OPT_IN_TO_LOCALIZATION_USING_STD_LOCALE_GLOBAL
15    std::locale::global(std::locale(""));
16#endif
17    std::ostringstream oss;
18    oss << 1000000;
19    std::cout << "std::ostringstream oss; oss << 1000000; oss.str() equals " << oss.str() << "\n";
20    std::cout << "std::to_string(1.23) equals " << std::to_string(1.23) << "\n\n";
21    std::cout << "C locale at end of main: " << std::string{setlocale(LC_ALL, nullptr)} << "\n";
22    std::cout << "C++ locale at end of main: " << std::locale{}.name() << "\n\n";
23    std::cout << "setlocale(LC_COLLATE,  nullptr) (read-only operation) = " << std::string{setlocale(LC_COLLATE, nullptr)} << "\n";
24    std::cout << "setlocale(LC_CTYPE,    nullptr) (read-only operation) = " << std::string{setlocale(LC_CTYPE, nullptr)} << "\n";
25    std::cout << "setlocale(LC_MESSAGES, nullptr) (read-only operation) = " << std::string{setlocale(LC_MESSAGES, nullptr)} << "\n";
26    std::cout << "setlocale(LC_MONETARY, nullptr) (read-only operation) = " << std::string{setlocale(LC_MONETARY, nullptr)} << "\n";
27    std::cout << "setlocale(LC_NUMERIC,  nullptr) (read-only operation) = " << std::string{setlocale(LC_NUMERIC, nullptr)} << "\n";
28    std::cout << "setlocale(LC_TIME,     nullptr) (read-only operation) = " << std::string{setlocale(LC_TIME, nullptr)} << "\n";
29}

    Let’s try with LC_ALL=de_DE ./l8n without any opt-in to locale dependence:

     0$ clang++ -o l8n l8n.cpp
 1$ LC_ALL=de_DE ./l8n
 2C locale at beginning of main: C
 3C++ locale at beginning of main: C
 4
 5std::ostringstream oss; oss << 1000000; oss.str() equals 1000000
 6std::to_string(1.23) equals 1.230000
 7
 8C locale at end of main: C
 9C++ locale at end of main: C
10
11setlocale(LC_COLLATE,  nullptr) (read-only operation) = C
12setlocale(LC_CTYPE,    nullptr) (read-only operation) = C
13setlocale(LC_MESSAGES, nullptr) (read-only operation) = C
14setlocale(LC_MONETARY, nullptr) (read-only operation) = C
15setlocale(LC_NUMERIC,  nullptr) (read-only operation) = C
16setlocale(LC_TIME,     nullptr) (read-only operation) = C

    Let’s try with LC_NUMERIC=de_DE ./l8n without any opt-in to locale dependence:

     0$ clang++ -o l8n l8n.cpp
 1$ LC_NUMERIC=de_DE ./l8n
 2C locale at beginning of main: C
 3C++ locale at beginning of main: C
 4
 5std::ostringstream oss; oss << 1000000; oss.str() equals 1000000
 6std::to_string(1.23) equals 1.230000
 7
 8C locale at end of main: C
 9C++ locale at end of main: C
10
11setlocale(LC_COLLATE,  nullptr) (read-only operation) = C
12setlocale(LC_CTYPE,    nullptr) (read-only operation) = C
13setlocale(LC_MESSAGES, nullptr) (read-only operation) = C
14setlocale(LC_MONETARY, nullptr) (read-only operation) = C
15setlocale(LC_NUMERIC,  nullptr) (read-only operation) = C
16setlocale(LC_TIME,     nullptr) (read-only operation) = C

    Let’s try LC_ALL=de_DE ./l8n with opt-in to locale dependence using set_locale:

     0$ clang++ -DOPT_IN_TO_LOCALIZATION_USING_SET_LOCALE -o l8n l8n.cpp
 1$ LC_ALL=de_DE ./l8n
 2C locale at beginning of main: C
 3C++ locale at beginning of main: C
 4
 5std::ostringstream oss; oss << 1000000; oss.str() equals 1000000
 6std::to_string(1.23) equals 1,230000
 7
 8C locale at end of main: de_DE
 9C++ locale at end of main: C
10
11setlocale(LC_COLLATE,  nullptr) (read-only operation) = de_DE
12setlocale(LC_CTYPE,    nullptr) (read-only operation) = de_DE
13setlocale(LC_MESSAGES, nullptr) (read-only operation) = de_DE
14setlocale(LC_MONETARY, nullptr) (read-only operation) = de_DE
15setlocale(LC_NUMERIC,  nullptr) (read-only operation) = de_DE
16setlocale(LC_TIME,     nullptr) (read-only operation) = de_DE

    Let’s try LC_ALL=de_DE ./l8n with opt-in to locale dependence using std::locale::global:

     0$ clang++ -DOPT_IN_TO_LOCALIZATION_USING_STD_LOCALE_GLOBAL -o l8n l8n.cpp
 1$ LC_ALL=de_DE ./l8n
 2C locale at beginning of main: C
 3C++ locale at beginning of main: C
 4
 5std::ostringstream oss; oss << 1000000; oss.str() equals 1.000.000
 6std::to_string(1.23) equals 1,230000
 7
 8C locale at end of main: de_DE
 9C++ locale at end of main: de_DE
10
11setlocale(LC_COLLATE,  nullptr) (read-only operation) = de_DE
12setlocale(LC_CTYPE,    nullptr) (read-only operation) = de_DE
13setlocale(LC_MESSAGES, nullptr) (read-only operation) = de_DE
14setlocale(LC_MONETARY, nullptr) (read-only operation) = de_DE
15setlocale(LC_NUMERIC,  nullptr) (read-only operation) = de_DE
16setlocale(LC_TIME,     nullptr) (read-only operation) = de_DE

    QED? :)

  35. sipa commented at 6:11 pm on February 13, 2020: member

    Oh, I’m indeed learning. I assumed that the LC_ environment variables automatically affect all programs.

    So what is this testing? It seems to verify things that are guaranteed by the language?

  36. practicalswift commented at 3:29 pm on February 14, 2020: contributor

    @sipa The only thing guaranteed by the language is that a C++ program has the C and C++ locales set to the classic locale when entering main. After that the user code is free to mess with the locale as it seen fit: hence the need to verify that the assumption we make hold true :)

    There are quite a few gotchas that are worth guarding against in my opinion:

    A common gotcha is that setlocale(LC_ALL, "") (write) where setlocale(LC_ALL, nullptr) (read) was intended.

    Another common gotcha is libraries calling setlocale(LC_ALL, "") without the application author being aware of it which of course messes with the locale of the entire application in unexpected ways.

    I think we have that problem today in the form of Qt which runs std::setlocale(LC_ALL, "") as part of its setup process. That is why the reported locale issues historically have popped up in bitcoin-qt and not in bitcoind AFAICT.

  37. practicalswift commented at 3:55 pm on February 14, 2020: contributor
    To make things even more unpredictable some versions of Qt messes with all locale categories whereas other versions only messes with a subset of locale categories. I believe a major locale handling change was made in Qt 4.4 for example.
  38. luke-jr commented at 5:15 pm on February 14, 2020: member
    Some of our stuff, like bitcoin-qt and bitcoin-wallet should use locales…
  39. practicalswift commented at 4:48 pm on February 15, 2020: contributor

    @luke-jr

    We produce the following five binaries:

    • bitcoin-cli
    • bitcoin-qt
    • bitcoin-tx
    • bitcoin-wallet
    • bitcoind

    Which of them should be localized and which should not be localized?

  40. practicalswift commented at 10:27 am on February 23, 2020: contributor
    @luke-jr Could you clarify your comment by answering the last question? :)
  41. MarcoFalke commented at 6:25 pm on February 24, 2020: member
    @practicalswift Purely user-facing graphical displays should be localized
  42. practicalswift commented at 9:07 pm on February 24, 2020: contributor
    @MarcoFalke bitcoin-qt is the only of the five binaries producing purely user-facing graphical displays AFAICT, so bitcoin-qt is thus the only application that should be localized. Correct? :)
  43. sipa commented at 9:15 pm on February 24, 2020: member

    bitcoin-qt is the only of the five binaries producing purely user-facing graphical displays AFAICT, so bitcoin-qt is thus the only application that should be localized. Correct? :)

    I think it’s wrong to think about binaries being localized or not.

    The user-interfacing code in bitcoin-qt should/may be localized, but other code in bitcoin-qt (e.g. the RPC code in it) still shouldn’t.

  44. practicalswift commented at 3:46 pm on March 6, 2020: contributor

    bitcoin-qt is the only of the five binaries producing purely user-facing graphical displays AFAICT, so bitcoin-qt is thus the only application that should be localized. Correct? :)

    I think it’s wrong to think about binaries being localized or not.

    The user-interfacing code in bitcoin-qt should/may be localized, but other code in bitcoin-qt (e.g. the RPC code in it) still shouldn’t.

    I think our statements can be combined to a meaningful policy:

    • Code in bitcoin-cli, bitcoin-tx, bitcoin-wallet and bitcoind must not be localized.
    • Purely user-facing graphical displays in bitcoin-qt should/may be localized, but all other code in bitcoin-qt must not be localized.

    Or alternatively:

    • Code in the project must not be localized with the exception of purely user-facing graphical displays in Qt related code which should/may be localized.

    Sounds good? :)

  45. MarcoFalke referenced this in commit 45cdcd47d9 on Mar 6, 2020
  46. sipa commented at 3:10 am on March 13, 2020: member
    @practicalswift That sounds right; I don’t think any of that is related to the changes in this PR.
  47. practicalswift commented at 2:42 pm on April 29, 2020: contributor
    Closing this PR: consensus opinion seems to be that the locale assumptions we’re are making implicitly are not worth asserting explicitly :)
  48. practicalswift closed this on Apr 29, 2020
  49. practicalswift deleted the branch on Apr 10, 2021
  50. DrahtBot locked this on Aug 18, 2022


practicalswift laanwj sipa Empact DrahtBot luke-jr MarcoFalke

Labels
Tests

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-18 18:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me