qt: allow user to choose data directory #2670

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2013_05_datadir changing 6 files +671 −44
  1. laanwj commented at 4:14 PM on May 19, 2013: member

    This adds an introduction screen that is shown when the client is first started in which the user can choose a data directory.

    It is also possible to force the intro screen to appear using command line argument -choosedatadir.

    The data directory is remembered in the QSettings.

    The user is warned that the client will download and store 10Gb of data. The intro screen shows how much space is available on the device that contains the chosen directory and warns if this is less than the 10Gb.

    To make it possible to translate the introduction dialog, the initialization sequence is changed so that translations are loaded before the data directory. This has the by-effect that it is no longer possible to specify a language in bitcoin.conf inside the data directory.

    Please help testing!

  2. Diapolo commented at 7:46 PM on May 19, 2013: none

    I'm going to take a look next week, can you post a screen until I do :)?

  3. laanwj commented at 10:08 AM on May 20, 2013: member

    OK selection

    x01

    Warning when directory already exists, user can still click OK

    x02

    Invalid or unreachable path

    x03

    Too little free space on device

    x04

  4. laanwj commented at 1:41 PM on May 20, 2013: member

    Changed Gb -> GB, checking now happens in separate thread to prevent blocking the GUI thread.

  5. Diapolo commented at 3:32 PM on May 20, 2013: none

    Just some small comments:

    • Do you allow OK even when low disk space was detected, looks like OK is available there.
    • Do you intend to show this even for existing installations or just new ones (and for existing ones when supplying -choosedatadir)?
    • What happens when a user supplies -choosedatadir, will you copy old to new or just switch from old to new?
    • Do we want to place a clickable URI to official Bitcoin site here or anywhere else in the client? I think that would be a good addition.
    • Perhaps display none for available space when case "Invalid or unreachable path"
    • Perhaps we could also add a clickable button for launching -choosedatadir into the options dialog, when I finish my work there?
  6. laanwj commented at 3:43 PM on May 20, 2013: member

    <i>&gt; Do you allow OK even when low disk space was detected, looks like OK is available there.</i>

    Yes. The user can choose OK in all cases where it's possible to create the directory. They may plain on cleaning up the drive immediately (ie, deleting one blueray image should be enough to make space). In the end it's their own responsibility.

    <i>&gt; Do you intend to show this even for existing installations or just new ones</i>

    Only for new installations (when the default data directory doesn't exist). Otherwise it will confuse users into making a new data directory and they will wonder where their block chain / wallet will have gone.

    <i>&gt; Do we want to place a clickable URI to official Bitcoin site here or anywhere else in the client? I think that would be a good addition.</i>

    I'm not sure why, unless it links to an explanation about data directories. But feel free to add more flair to the window later. I'm just focusing on the functionality for now.

    <i>&gt; Perhaps display none for available space when case "Invalid or unreachable path"</i>

    Yeah... or N/A

    <i>&gt; Perhaps we could also add a clickable button for launching -choosedatadir into the options dialog, when I finish my work there?</i>

    I don't want to support changing the data directory while the client is running.

    If the user wants to move the data directory they can do so themselves while the client is not running, and the "choose data directory" dialog will automatically pop up on next run as the software notices that the data directory does not exist.

  7. in src/util.cpp:None in 3515b04638 outdated 
     601 | @@ -602,6 +602,12 @@ bool SoftSetArg(const std::string& strArg, const std::string& strValue)
 602 |      return true;
 603 |  }
 604 |  
 605 | +bool SetArg(const std::string& strArg, const std::string& strValue)
    Diapolo commented at 3:46 PM on May 20, 2013:

    I guess Gavin would love to see some unit test for this :-P. Btw. I also thought about such a thing a few days ago ^^.

    laanwj commented at 4:09 PM on May 20, 2013:

    I didn't like adding this function, but I didn't see another way to do this (well ok it's possible to assign to mapArgs directly, as it is exported outside util.cpp, this is done in rpcmining.cpp for example... but that wouldn't be better would it). SetSoftArg is not good enough as the directory selected in the dialog should override everything, even the data directory passed on the command line, otherwise it'd be confusing. Another solution would be to not show the dialog at all when the datadir is overridden on the command line. Hmm that may be better.

  8. gmaxwell commented at 5:20 PM on May 20, 2013: contributor

    ObUIshedpaint (feel free to ignore): The first "Warning:" is perhaps a bit intense and should probably be more reserved for things with risk of irreparable harm (encrypting wallets, deleting wallet directories, sending a 1000 BTC fee, etc.). Using space is just a fact of normal software operation and is nothing to be concerned about... unless you don't have enough— so I think it's okay in a low/insufficient space case...

  9. laanwj commented at 5:49 PM on May 20, 2013: member

    @gmaxwell: fixed that, all the warnings was indeed a bit over the top

  10. BitcoinPullTester commented at 6:48 PM on May 20, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/ff78dd81170c15f7fed00f803dde109283e78c72 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  11. rebroad commented at 3:40 AM on May 21, 2013: contributor

    "official bitcoin site"?!

    I suspect you mean "original satoshi client site"...

    Do we want to place a clickable URI to official Bitcoin site here or anywhere else in the client? I think that would be a good addition.

  12. jbreher commented at 5:58 AM on May 21, 2013: none

    Just a question. When you write 'GB', do you really mean 'GB' - i.e. 10^9, or do you instead mean 'GiB' - i.e. 2^30 ?

    On May 20, 2013, at 7:41 AM, Wladimir J. van der Laan wrote:

    Changed Gb -> GB, checking now happens in separate thread to prevent blocking the GUI thread.

    — Reply to this email directly or view it on GitHub.

  13. laanwj commented at 6:00 AM on May 21, 2013: member

    @rebroad No, he means the Armory site obviously... @jbreher GB as in 10^9. This is Bitcoin, we use SI units.

  14. Diapolo commented at 6:07 AM on May 21, 2013: none

    I really hate that GiB stuff, for me all units are 1024 based but I would never use GiB, KiB or such nonsense ^^.

  15. BitcoinPullTester commented at 6:31 AM on May 21, 2013: none

    Automatic sanity-testing: FAILED BUILD/TEST, see http://jenkins.bluematt.me/pull-tester/55605706a21e2325720c989ad690f83aa2934f78 for binaries and test log.

    This could happen for one of several reasons:

    1. It chanages paths in makefile.linux-mingw or otherwise changes build scripts in a way that made them incompatible with the automated testing scripts (please tweak those patches in contrib/test-scripts)
    2. It adds/modifies tests which test network rules (thanks for doing that), which conflicts with a patch applied at test time
    3. It does not build on either Linux i386 or Win32 (via MinGW cross compile)
    4. The test suite fails on either Linux i386 or Win32
    5. The block test-cases failed (lookup the first bNN identifier which failed in https://github.com/TheBlueMatt/test-scripts/blob/master/FullBlockTestGenerator.java)

    If you believe this to be in error, please ping BlueMatt on freenode or TheBlueMatt here.

    This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  16. laanwj commented at 6:59 AM on May 21, 2013: member

    Ugh, I'm trying to find out the OS path separator using make_preferred() for a message but this doesn't exist on the ancient boost used by pulltester.

  17. laanwj commented at 7:02 AM on May 21, 2013: member

    Should be solved now.

  18. Diapolo commented at 7:02 AM on May 21, 2013: none

    @laanwj I also think we should drop support for ancient Boost version. I never understood, why it is a problem to do so anyway.

  19. BitcoinPullTester commented at 7:27 AM on May 21, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e024910520ab3868240beebfacec04de15c929ca for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  20. jbreher commented at 10:57 PM on May 21, 2013: none

    I understand that many hate it. However, your insistence on casually using units which never had any standards organization bless them in any way leaves you perpetuating a needless ambiguity, and in conflict with SI, ANSI, NIST, ISO, IEC, ITC, IETF, BSA, and just about any accredited standards organization one might care to name. All of whom have issued clear guidance that (e.g.) GB always refers to 10^9, and GiB always refers to 2^30.

    https://en.wikipedia.org/wiki/IEEE_1541

    http://physics.nist.gov/cuu/Units/binary.html

    On May 21, 2013, at 12:07 AM, Philip Kaufmann wrote:

    I really hate that GiB stuff, for me all units are 1024 based but I would never use GiB, KiB or such nonsense ^^.

    — Reply to this email directly or view it on GitHub.

  21. rebroad commented at 1:51 AM on May 22, 2013: contributor

    As someone who has been using computers since 1979, I can confirm that KB always referred to 1024 and only recently did I notice that people confusingly started using KB to refer to 1000, and this confusion has been further compounded by the addition of the term KiB. On May 22, 2013 5:58 AM, "Joe Breher" notifications@github.com wrote:

    I understand that many hate it. However, your insistence on casually using units which never had any standards organization bless them in any way leaves you perpetuating a needless ambiguity, and in conflict with SI, ANSI, NIST, ISO, IEC, ITC, IETF, BSA, and just about any accredited standards organization one might care to name. All of whom have issued clear guidance that (e.g.) GB always refers to 10^9, and GiB always refers to 2^30.

    https://en.wikipedia.org/wiki/IEEE_1541

    http://physics.nist.gov/cuu/Units/binary.html

    On May 21, 2013, at 12:07 AM, Philip Kaufmann wrote:

    I really hate that GiB stuff, for me all units are 1024 based but I would never use GiB, KiB or such nonsense ^^.

    — Reply to this email directly or view it on GitHub.

    — Reply to this email directly or view it on GitHubhttps://github.com/bitcoin/bitcoin/pull/2670#issuecomment-18247220 .

  22. laanwj commented at 3:48 AM on May 22, 2013: member

    Can you take the units discussion elsewhere please? These kinds of discussions can go on for centuries (let's settle imperial versus metric first when we're at it) and still go nowhere. We don't have time for that.

  23. Suffice commented at 5:30 AM on May 31, 2013: none

    Perhaps have two choices when the client starts up for the first time, the first to download the block chain with all the defaults, and the second being to change the data directory or whatever. This will alleviate the technicality of it for non-technical people, and make the first dialogue screen more welcoming.

    Something maybe like:

    Start Block Chain | Change Directory

  24. sipa commented at 5:36 AM on May 31, 2013: member

    @Suffice As a follow-up, maybe there can be some menu option "Move datadir", which requires a restart immediately afterwards (or not, but that's definitely a lot more work). I think that's independent from this pullreq, which already improves 99% of use cases.

  25. laanwj commented at 5:51 AM on May 31, 2013: member

    @suffice yeah maybe... but I do want to make sure people see the warning about downloading 10Gb, as one of the reasons for this is that people don't get freaked out later when they see their harddisk being filled up. In any case they already accept the default by clicking "OK" immediately. On a fresh install, it will initially show the default data directory. @sipa Could be useful option. But I feel better about that once block chain dir != wallet dir. No problem with moving the block chain, it can be redownloaded if the user makes a mistake, but as moving implies a delete I don't really want to "move" the wallet.dat. Copy at most. Let people that know what they're doing do this manually for now.

  26. sipa commented at 5:59 AM on May 31, 2013: member

    @laanwj ACK, separating datadir from wallets must happen first.

  27. laanwj commented at 8:02 AM on May 31, 2013: member

    default @suffice this is better I think. It makes clear that the initially selected directory is the default, and requires an extra radio button click to change it to prevent it from being changed cluelessly. And it still shows what the default data directory is (though disabled), and whether there is enough space there.

  28. Suffice commented at 6:51 PM on May 31, 2013: none

    I don't know anymore. . Pulling this off in best way possible is tricky. This is how most program installs look:

    capture

    Perhaps something like this:

    Some concise text here for an explanation of something

The block chain and your wallet files will go here...
[ C:\Program Files (x86)\Bitcoin           ]   [ Browse... ]


Space required:   10.3 GB's
Space available:  32.4 GB's
The space required for the block chain will increase over time as the chain grows.
(The block chain is essentially a ledger of every bitcoin transaction ever.)

                                               Some buttons here

    I noticed that the block chain is currently stored in the AppData directory on Windows, will that change?

    It would be nice if the wallet auto purged parts of the block chain that are no longer needed, and perhaps requests parts of the chain that it needs again, like in the event of importing addresses. (Tricky)

    New clients could start from the near end of this chain, as they won't likely need the whole chain.

  29. laanwj commented at 8:39 PM on May 31, 2013: member

    <b>I noticed that the block chain is currently stored in the AppData directory on Windows, will that change?</b>

    After this pull it will be possible to change that, at least on new installs, that's the point of it.

  30. BitcoinPullTester commented at 3:13 AM on June 2, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/e8d6ef18ac4d38ca93827f739aa560345c5fcff1 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  31. jonasschnelli commented at 10:57 AM on June 3, 2013: contributor

    Thats how it looks on mac with Qt 4.8.4:

    bildschirmfoto 2013-06-03 um 12 55 00 bildschirmfoto 2013-06-03 um 12 54 31 bildschirmfoto 2013-06-03 um 12 54 16

    Maybe the button for opening the file dialog must be rearranged a little bit?

  32. Diapolo commented at 11:56 AM on June 3, 2013: none

    Can you display the path for the default data directory behind option one or as tooltip :)?

  33. laanwj commented at 12:17 PM on June 3, 2013: member

    @jonasschnelli eh I'll take a look @diapolo when you click the first option, it shows the default directory greyed out in the input field.

  34. Diapolo commented at 1:46 PM on June 3, 2013: none

    I'm currently trying to build and got 2 conflicts while rebasing to current master, can you take a look :).

  35. in src/qt/guiutil.cpp:None in e8d6ef18ac outdated 
     430 | @@ -431,7 +431,8 @@ bool SetStartOnSystemStartup(bool fAutoStart)
 431 |      uiOptions = tr("UI options") + ":\n" +
 432 |          "  -lang=<lang>           " + tr("Set language, for example \"de_DE\" (default: system locale)") + "\n" +
 433 |          "  -min                   " + tr("Start minimized") + "\n" +
 434 | -        "  -splash                " + tr("Show splash screen on startup (default: 1)") + "\n";
 435 | +        "  -splash                " + tr("Show splash screen on startup (default: 1)") + "\n" +
 436 | +        "  -choosedatadir         " + tr("Choose data directory on startup (default: 0)") + "\n";
    Diapolo commented at 1:54 PM on June 3, 2013:

    Perhaps extend (default: 0) to (default: 0, 1 on first client startup only)?

    laanwj commented at 3:56 PM on June 3, 2013:

    I think that'd only add clutter. Yes, it displays the dialog also if the configured (or default) data directory does not exist, but the setting still defaults to 0. -choosedatadir is just a way to force this.

  36. in src/qt/forms/intro.ui:None in e8d6ef18ac outdated 
     110 | +        <layout class="QHBoxLayout" name="horizontalLayout_2">
 111 | +         <item>
 112 | +          <widget class="QLineEdit" name="dataDirectory"/>
 113 | +         </item>
 114 | +         <item>
 115 | +          <widget class="QPushButton" name="ellipsisButton">
    Diapolo commented at 1:59 PM on June 3, 2013:

    Can you make this non-translatable, as currentlty this generates ... to be translated :).

    laanwj commented at 3:56 PM on June 3, 2013:

    Yeah, good point

  37. in src/qt/bitcoin.cpp:None in e8d6ef18ac outdated 
     168 | @@ -128,6 +169,21 @@ int main(int argc, char *argv[])
 169 |      // Register meta types used for QMetaObject::invokeMethod
 170 |      qRegisterMetaType< bool* >();
 171 |  
 172 | +    // Application identification (must be set before OptionsModel is initialized,
 173 | +    // as it is used to locate QSettings)
 174 | +    QApplication::setOrganizationName("Bitcoin");
 175 | +    QApplication::setOrganizationDomain("bitcoin.org");
 176 | +    if(GetBoolArg("-testnet")) // Separate UI settings for testnet
    Diapolo commented at 2:03 PM on June 3, 2013:

    This needs the false parameter now.

  38. laanwj commented at 4:16 PM on June 3, 2013: member

    Rebased, and ... changed to untranslatable.

    I still don't understand the macosx problem. The button is simply in a horizontal box layout (with no special settings) with the input field.

  39. BitcoinPullTester commented at 5:13 PM on June 3, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/aaa6b5e87b69f6cbcca3d1fc430d8bc9f73ac609 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  40. BitcoinPullTester commented at 6:26 AM on June 4, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4c7685061d2437b9c3a1c61e490dd673d9a7ed6e for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  41. in src/qt/intro.cpp:None in 4c7685061d outdated 
       7 | +#include <QMessageBox>
   8 | +
   9 | +#include <boost/filesystem.hpp>
  10 | +
  11 | +/* Minimum free space (in bytes) needed for data directory */
  12 | +static const uint64 BLOCK_CHAIN_SIZE = 10LL * 1000000000LL;
    Diapolo commented at 6:43 AM on June 6, 2013:

    Can you add a small comment that this is 10GB and below 1GB currently :)?

    laanwj commented at 7:46 AM on June 6, 2013:

    Indeed could be clarified. But as such comments tend to go out of date, making it clear in the code through multiplying 10 with a constant GB may be better.

    Diapolo commented at 7:49 AM on June 6, 2013:

    Yeah, so perhaps just switch the 2 and use GB_BYTES * 10 then :).

  42. Diapolo commented at 6:55 AM on June 6, 2013: none

    It behaves a little weird, let me explain:

    • default selected (C:\Users\Diapolo\AppData\Roaming\Bitcoin) -- message 9GB of free space available (of 10GB needed). displayed
    • switching to custom, message is the same -- it doesn't show that this directory already exists -- adding a \ leads to Directory already exists... -- adding a \test\ leads toWarning: Low disk space on device``

    Edit: Sorry, I didn't have the recent rebase changes in my local branch... will test again!

    Edit 2: Behaviour is the same, I would await, that C:\Users\Diapolo\AppData\Roaming\Bitcoin is detected as existing path already, even without the \ at the end. @laanwj Did you take a look at this comments yet?

  43. in src/qt/intro.cpp:None in 4c7685061d outdated 
      58 | +    fs::path dataDir = fs::path(dataDirStr.toStdString());
  59 | +    uint64 freeBytesAvailable = 0;
  60 | +    int replyStatus = ST_OK;
  61 | +    QString replyMessage = tr("A new data directory will be created.");
  62 | +
  63 | +    /* Find first parent that exists */
    Diapolo commented at 7:36 AM on June 6, 2013:

    Can you explain to me what this is for?

    laanwj commented at 7:41 AM on June 6, 2013:

    When you enter a path that does not exist, it has to find the first parent path up the tree that does exist to be able to determine the free space on that file system. Boost will not accept non existing paths.

  44. in src/qt/intro.cpp:None in 4c7685061d outdated 
      70 | +    try {
  71 | +        freeBytesAvailable = fs::space(parentDir).available;
  72 | +        if(freeBytesAvailable < BLOCK_CHAIN_SIZE)
  73 | +        {
  74 | +            replyStatus = ST_WARNING;
  75 | +            replyMessage = tr("Low disk space on device.");
    Diapolo commented at 7:38 AM on June 6, 2013:

    Should we perhaps allow replyMessage to contain more strings, so that Low disk space can co-exist with others via +=?

    laanwj commented at 7:39 AM on June 6, 2013:

    If there is a necessity to, sure.

    Diapolo commented at 7:47 AM on June 6, 2013:

    Was just a hint, because Low disk space should always be shown, even if there are other messages available IMHO. Edit: Do you intent to change that one before a merge or consider it useless?

    Diapolo commented at 11:09 AM on June 14, 2013:

    @laanwj Do we need this really? I mean we have freeSpace-label, which is rather clear IMHO.

  45. in src/qt/intro.cpp:None in 4c7685061d outdated 
      87 | +            } else {
  88 | +                replyStatus = ST_ERROR;
  89 | +                replyMessage = tr("Path already exists, and is not a directory.");
  90 | +            }
  91 | +        }
  92 | +    } catch(fs::filesystem_error &e)
    Diapolo commented at 7:50 AM on June 6, 2013:

    Just to understand, fs::space and fs::exists() throws a fs error, when the path doesn't exist or there is a problem?

    laanwj commented at 8:51 AM on June 6, 2013:

    fs::exists? No, only when something goes wrong, when it simply doesn't exist it returns false. The free disk space function on the other hand throws in all cases where something goes wrong. The catch is a catch-all for all problems that can happen during probing.

  46. Diapolo commented at 6:30 PM on June 10, 2013: none

    @laanwj Can you test if you are able to use a non-default language (no english) and that this is loaded? Either there is a bug in this pull or I have a problem with my local build related to this in combination with my QSettings work (or a problem with this and #2700?).

  47. in src/qt/bitcoin.cpp:None in 4c7685061d outdated 
     125 | +    // 3) -lang command line argument
 126 | +    lang_territory = QString::fromStdString(GetArg("-lang", lang_territory.toStdString()));
 127 | +
 128 | +    // Convert to "de" only by truncating "_DE"
 129 | +    QString lang = lang_territory;
 130 | +    lang.truncate(lang_territory.lastIndexOf('_'));
    Diapolo commented at 6:50 PM on June 10, 2013:

    You've got some nasty bug in this code part.

    Step 1: de_DE Step 2 (because of a QSetting): de Step 3 (still from the QSetting): de

    lang = de and after this line it is just empty, which leads to the default language getting activated!

    laanwj commented at 7:36 AM on June 11, 2013:

    Thanks for testing and reporting, I'll take a look.

    Diapolo commented at 3:12 PM on June 11, 2013:

    After your recent changes the thing here is like this:

    1. lang_territory = de_DE
    2. lang_territory = de (as that is my stored QSetting)
    3. lang_territory = de (as I didn't pass -lang)

    So lang = de before doing the truncate() and is empty afterwards. So you pass an empty variable lang into the further calls, where it should be de and de (via lang_territory) where it should be de_DE, right? It seems to work okay, but I'm not sure if this is intended :).

    laanwj commented at 5:31 PM on June 11, 2013:

    That's how it always worked -- if lang_territory contains a string such as "de_DE", lang will be "de" and both will be loaded. If lang_territory contains a string such as "de", lang will be "" and just "de" will be loaded. It will indeed pass an empty string, but as it cannot find a translation with an empty name that's a no-op.

  48. laanwj commented at 2:14 PM on June 11, 2013: member

    Rebased, and language problem solved (the QTranslators were getting freed).

  49. BitcoinPullTester commented at 3:28 AM on June 13, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/6aad2cacef1af45185773622548ba74c755bbe79 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  50. laanwj commented at 8:49 AM on June 14, 2013: member

    @Diapolo does it still show the strange behavior on Windows? If so, any idea what causes that?

  51. Diapolo commented at 10:59 AM on June 14, 2013: none

    Sugestion: Can you make the info/error messages selectable and show the beam-cursor? I also think the dialog could be a little smaller in height.

    Translations are working again!

    Anyway, I'll report what is happening, my default datadir is: C:\Users\Diapolo\AppData\Roaming\Bitcoin.

    Start with -choosedatadir and default selected: 8GB free of 10GB needed message shown.

    Change from default to custom: 8GB free of 10GB needed message shown.

    Change path into C:\Users\Diapolo\AppData\Roaming\Bitcoi: 8GB free of 10GB needed message shown. Warning: Low disk space on device shown.

    Change path into C:\Users\Diapolo\AppData\Roaming\Bitcoin\ 8GB free of 10GB needed message shown. Directory already exists. Add \name... and so on, shown.

    I think the default detected path C:\Users\Diapolo\AppData\Roaming\Bitcoin should show the Directory already exists message but the current implementation does this only after applying the \, which I don't expect.

  52. qt: allow user to choose data directory
    This adds an introduction screen that is shown when the client is first
started in which the user can choose a data directory.

It is also possible to force the intro screen to appear using command
line argument `-choosedatadir`.

The user is warned that the client will download and store 10Gb of data.
The intro screen shows how much space is available on the device that
contains the chosen directory and warns if this is less than the 10Gb.

To make it possible to translate the introduction dialog, the initialization
sequence is changed so that translations are
loaded before the data directory. This has the by-effect that it is
no longer possible to specify a language in bitcoin.conf inside the data
directory.
    be77b637fc
  53. in src/qt/intro.cpp:None in 6aad2cacef outdated 
     192 | +        ui->errorMessage->setText(message);
 193 | +        ui->errorMessage->setStyleSheet("");
 194 | +        break;
 195 | +    case FreespaceChecker::ST_ERROR:
 196 | +        ui->errorMessage->setText(tr("Error") + ": " + message);
 197 | +        ui->errorMessage->setStyleSheet("QLabel { color: #800000 }");
    Diapolo commented at 11:10 AM on June 14, 2013:

    Can you change these style-sheets to color: [#800000](/bitcoin-bitcoin/800000/); to be CSS standard-conformant?

    laanwj commented at 8:13 AM on June 16, 2013:

    I'm pretty sure this is not required by the standard (stack overflow agrees, see top answer here http://stackoverflow.com/questions/11939595/leaving-out-the-last-semicolon-of-a-css-block ). Can you quote me the part of the standard where it says that the last declaration needs a ; separator?

    Diapolo commented at 3:35 PM on June 16, 2013:

    Seems you are correct here, I checked via: http://jigsaw.w3.org/css-validator/#validate_by_input and both are correct, with and without ;.

  54. in src/qt/intro.cpp:None in 6aad2cacef outdated 
     200 | +        ui->errorMessage->setText(tr("Warning") + ": " + message);
 201 | +        ui->errorMessage->setStyleSheet("QLabel { color: #800000 }");
 202 | +        break;
 203 | +    }
 204 | +    /* Indicate number of bytes available */
 205 | +    if(bytesAvailable == 0)
    Diapolo commented at 11:12 AM on June 14, 2013:

    Why is bytesAvailable == 0 no problem so that you choose to cover it here?

  55. in src/qt/intro.cpp:None in 6aad2cacef outdated 
     204 | +    /* Indicate number of bytes available */
 205 | +    if(bytesAvailable == 0)
 206 | +    {
 207 | +        ui->freeSpace->setText("");
 208 | +    } else {
 209 | +        QString freeString = QString::number(bytesAvailable/1000000000) + tr("GB of free space available");
    Diapolo commented at 11:14 AM on June 14, 2013:

    Perhaps exchange the number with GB_BYTES?

  56. laanwj commented at 8:30 AM on June 16, 2013: member

    Should be solved now.

  57. Diapolo commented at 4:04 PM on June 16, 2013: none

    The directory things and disk-space stuff is fixed, great :). Only thing left is the layout / size, I still think the dialogs height is too large and I would perhaps align the input field for the path and the message labels with the left border (like the selection boxes).

  58. laanwj commented at 5:49 PM on June 16, 2013: member

    Feel free to play with the layout a bit...

  59. Diapolo commented at 7:03 PM on June 16, 2013: none

    GUI can be improved later :-).

    ACK

  60. BitcoinPullTester commented at 12:04 AM on June 19, 2013: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/be77b637fcf5983286382a9b9677fb97b026abe2 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  61. in src/qt/bitcoin.cpp:None in be77b637fc 
     171 | @@ -130,6 +172,22 @@ int main(int argc, char *argv[])
 172 |      // Register meta types used for QMetaObject::invokeMethod
 173 |      qRegisterMetaType< bool* >();
 174 |  
 175 | +    // Application identification (must be set before OptionsModel is initialized,
 176 | +    // as it is used to locate QSettings)
 177 | +    QApplication::setOrganizationName("Bitcoin");
 178 | +    QApplication::setOrganizationDomain("bitcoin.org");
 179 | +    if (GetBoolArg("-testnet", false)) // Separate UI settings for testnet
    Diapolo commented at 3:48 PM on June 23, 2013:

    Perhaps you can update this to use TestNet() from chainparams.h, which you need to include here then? Otherwise I can do this after it's merged :).

    Edit: We don't need chainparams.h as it's coming from init.h alreay.

    laanwj commented at 7:33 AM on June 24, 2013:

    Are you sure that will work? The reason for looking for testnet arg here directly is that it is called before Init.

    Diapolo commented at 8:11 AM on June 24, 2013:

    I tried it and it doesn't work, as we are before init here. I think it would be nice, if we could achieve to use TestNet() with re-ordering the code, but my try doesn't yet do this work ^^. See #2788

    Anyway, I would suggest you merge this pull and we can look into this afterwards.

  62. Diapolo commented at 11:13 AM on July 13, 2013: none

    @laanwj Can we perhaps merge this now :)?

  63. sipa commented at 11:36 AM on July 14, 2013: member

    ACK - seems to work fine.

  64. laanwj referenced this in commit 4eab2dcc81 on Jul 19, 2013
  65. laanwj merged this on Jul 19, 2013
  66. laanwj closed this on Jul 19, 2013
  67. laanwj deleted the branch on Apr 9, 2014
  68. DrahtBot locked this on Sep 8, 2021
Contributors
laanwjDiapologmaxwellBitcoinPullTesterrebroadjbreherSufficesipajonasschnelli
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: 2026-04-13 15:16 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me