gui-qml icon indicating copy to clipboard operation
gui-qml copied to clipboard

DRAFT: Custom Datadir Wiring

Open D33r-Gee opened this issue 11 months ago • 11 comments

This pull request introduces the functionality for users to specify a custom data directory (datadir) during the onboarding process.

Ubuntu Screenshots

Screenshot 2024-03-14 124000 Screenshot 2024-01-29 074759 Screenshot 2024-01-29 074734

Prerequisites

For testing this pull request, ensure you have the following Qt modules installed:

  • On Ubuntu 22.04:
sudo apt-get install qtdeclarative5-dev
sudo apt-get install qtquickcontrols2-5-dev
sudo apt-get install qml-module-qtquick-controls2
sudo apt install qml-module-qtquick-dialogs
  • For Android:

Make sure you delete the prior depends folder (i.e. depends/aarch64-linux-android) and rebuild them.

Implementation Details

This enhancement leverages FileDialog class for a user-friendly selection experience. Additionally, necessary code updates and adjustments were made to accommodate the delayed node creation until onboarding completion.

  • QML Backend
    • Introduced a new class within qml/bitcoin.h and cpp to manage custom data directory settings.
    • Integrated pull request #284 by Johny9.
    • Added onboardingmodel files to oversee the onboarding process, specifically custom data directory configuration and placeholders for optionsModel.
  • QML Frontend
    • Updated StorageLocations.qml to incorporate wiring for custom data directory selection and resetting to default.
    • Commented out settings code in OnboardingCover.qml due to the postponed node initialization until onboarding completion.
    • Modified Onboarding qml files to utilize onboardingModel instead of optionsModel. Additionally, arguments referencing chainModel were removed as node initialization now occurs at the end of the onboarding process.
  • The Make process was extended to include the onboardingmodel files.

TODO: will implement a way to display a read only path to the user's custom datadir, most likely tin the StorageOptions file.

D33r-Gee avatar Mar 08 '24 16:03 D33r-Gee

Thanks @johnny9 for commenting and reviewing!

During testing, I was unable to start the app after onboarding. The initial onboarding flow worked, however, It appeared to not find the custom data directory I set on the second time loading the app.

Was able to reproduce, it seems that if the custom datadir exist but hasn't been fully initialized, meaning the onboarding process didn't finished, it will trigger the error you encountered. Will look into how to mitigate that...

Ok found the problem, I had commented out the gArgs.ClearPathCache(); in qml/bitcoin.cpp, it works now... going to update the commits

D33r-Gee avatar Mar 12 '24 16:03 D33r-Gee

updated the commits from 87b4d75 to 76f83f2

D33r-Gee avatar Mar 13 '24 22:03 D33r-Gee

CI is failing, please fix

MarnixCroes avatar Mar 14 '24 09:03 MarnixCroes

At the end I found I needed also this one qml-module-qtquick-dialogs, so please update the section accordingly if you think this is correct.

Great catch @pablomartin4btc! Updated the description

D33r-Gee avatar Mar 18 '24 22:03 D33r-Gee

Another thing I noticed is that once I close the app if I delete the test chain subdirectory of the custom datadir (eg signet), which contains the settings.json file, when I start the app again still believes I'm on-boarded, don't get asked about the settings and that shouldn't be the case I think.

In this case the the custom directory needs to be deleted as well, since the code cached the top level directory and just the chainstate one (i.e. signet). In other words if you'd like to test onboarding again you'd have rm -r custom

D33r-Gee avatar Mar 18 '24 23:03 D33r-Gee

As part of this PR, shouldn't we show the custom datadir that has been selected by the user somewhere? Perhaps as a non editable field in the Storage settings page?

that's a good point... will update that along with @johnny9 android patches

D33r-Gee avatar Mar 18 '24 23:03 D33r-Gee

Let's make this draft as it still needs a good bit of work. It's a requirement that changes need to work with all builds (dynamic + static)

Great... I'll make it a draft...

BTW, @johnny9's patches for android to include the FileDialog functionality also work with the x86_64-pc-linux-gnu static build.

D33r-Gee avatar Mar 20 '24 23:03 D33r-Gee

it crashed once when clicking Custom at Storage Location. 951387487f0ad187d646f7c4b96f37c1e01d431d

terminal output

Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)

haven't been able to repro

MarnixCroes avatar Mar 25 '24 18:03 MarnixCroes

it crashed once when clicking Custom at Storage Location. 9513874

terminal output

Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)

haven't been able to repro

@MarnixCroes, thanks for testing and reviewing

Does this mean it the crashing is persisting or that it only happened once?

Also are you testing on Ubuntu? if so what version?

D33r-Gee avatar Mar 25 '24 21:03 D33r-Gee

it crashed once when clicking Custom at Storage Location. 9513874 terminal output

Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)

haven't been able to repro

@MarnixCroes, thanks for testing and reviewing

Does this mean it the crashing is persisting or that it only happened once?

Also are you testing on Ubuntu? if so what version?

it only happened once. can't reproduce yet This was indeed on ubuntu, 22.04.4 LTS

MarnixCroes avatar Mar 26 '24 07:03 MarnixCroes

Based on @pablomartin4btc and @johnny9 feedback:

Converted to draft for now and will post follow up PR with only the UI elements for more straight forward reviewing and merging.

D33r-Gee avatar Mar 27 '24 18:03 D33r-Gee

replaced by https://github.com/bitcoin-core/gui-qml/pull/408

jarolrod avatar Sep 25 '24 11:09 jarolrod