Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

qt, build: Fix QFileDialog for static builds #19536

Merged
merged 1 commit into from Jul 17, 2020
Merged

qt, build: Fix QFileDialog for static builds #19536

merged 1 commit into from Jul 17, 2020

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Jul 16, 2020

This change partially reverts 248e22b (#16386) and makes QFileDialogs work again for static builds.

Fixes bitcoin-core/gui#32.

@hebasto
qt: Fix QFileDialog for static builds
6457361 
This change partially reverts 248e22b.
This was referenced Jul 16, 2020
Closed
Open
Merged
@hebasto
Copy link
Member Author

hebasto commented Jul 16, 2020

@MarcoFalke @fanquake label for backporting?

theuni
theuni approved these changes Jul 16, 2020
View reviewed changes
Copy link
Member

@theuni theuni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 6457361

@maflcko maflcko added this to the 0.20.1 milestone Jul 17, 2020
@Sjors
Copy link
Member

Sjors commented Jul 17, 2020 •
edited

Concept ACK. It might make sense to maintain a list of QT components we do use, or least mention somewhere that filesystemwatcher really shouldn't be removed.

I assume this can be tested with the Load PSBT menu as well, once the Gitian build is ready.

@fanquake
Copy link
Member

ACK 6457361. Although it would be good to know exactly why this fixes the issue. At this stage I also don't think this should be a blocker for 0.20.1.

After discussing with @hebasto I gather it's restricted to Linux-only, and is somehow related to the xcb QPA plugin. I had a quick look through the qtbase 5.9.8 source, and my assumption is there maybe a dependency like QFileDialog -> QFileSystemModel -> QFileSystemWatcher which may not be completely #ifndef QT_NO_FILESYSTEMWATCHER'd out, or which is not possible to break, as it seems that QFileSystemModel may try and use QFileSystemWatcher for caching.

@fanquake fanquake merged commit c044858 into bitcoin:master Jul 17, 2020
@hebasto
Copy link
Member Author

hebasto commented Jul 17, 2020

@Sjors

Concept ACK. It might make sense to maintain a list of QT components we do use, or least mention somewhere that filesystemwatcher really shouldn't be removed.

Nice suggestion!

I assume this can be tested with the Load PSBT menu as well, once the Gitian build is ready.

For testing builds with depends could be used as well.

@hebasto
Copy link
Member Author

hebasto commented Jul 17, 2020

@MarcoFalke @fanquake label for backporting?

As the bug was introduced in v0.19 maybe also label this PR with "Needs backport (0.19)" ?

@hebasto hebasto deleted the 200716-fqd branch July 17, 2020 09:28
laanwj pushed a commit that referenced this pull request Jul 17, 2020
@hebasto @laanwj
qt: Fix QFileDialog for static builds
8b40937 
This change partially reverts 248e22b.

Github-Pull: #19536
Rebased-From: 6457361
Tree-SHA512: 3b48c64f59068d70b3aec4514ebe4a091813c77518a52a81bd8a36b44d0854b3f5e187f52c0f8e4527506087cb7e2cafb7c39cb7d28e4b8f620a77980e8a4697
@maflcko maflcko modified the milestones: 0.20.1, 0.19.2 Jul 17, 2020
@alexstrbonn
Copy link

@MarcoFalke MarcoFalke
Will this be only fixed once 0.20.1 will be available?
Thanks!

@hebasto
Copy link
Member Author

hebasto commented Jul 19, 2020

@alexstrbonn

Will this be only fixed once 0.20.1 will be available?

You already could test v0.20.1rc1 :)

@maflcko
Copy link
Member

maflcko commented Jul 19, 2020

rc1 binaries will be downloadable early this week. If nothing goes wrong, the final release will be out early next week.

backpacker69 referenced this pull request in peercoin/peercoin Sep 8, 2020
@hebasto @backpacker69
qt: Fix QFileDialog for static builds
2d03b59 
This change partially reverts 248e22b.

Github-Pull: #19536
Rebased-From: 6457361
Tree-SHA512: 3b48c64f59068d70b3aec4514ebe4a091813c77518a52a81bd8a36b44d0854b3f5e187f52c0f8e4527506087cb7e2cafb7c39cb7d28e4b8f620a77980e8a4697
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Oct 5, 2020
@hebasto @Fabcien
qt: Fix QFileDialog for static builds
0bd424b 
Summary:
```
This change partially reverts 248e22bbc0d7bc40ae3584d53a18507c46b0e553.
```

Backport of core [[bitcoin/bitcoin#19536 | PR19536]].

Test Plan:
  cmake -GNinja .. \
    -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Linux64.cmake
  ninja
With no existing data directory:
  ./src/qt/bitcoin-qt
In the Welcome dialog, select "Use a custom data directory" and browse
for a path. Before this patch browsing was broken.

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Differential Revision: https://reviews.bitcoinabc.org/D7754
fanquake pushed a commit to fanquake/bitcoin that referenced this pull request Oct 14, 2020
@hebasto @fanquake
qt: Fix QFileDialog for static builds
a8411b3 
This change partially reverts 248e22b.

Github-Pull: bitcoin#19536
Rebased-From: 6457361
@fanquake fanquake mentioned this pull request Oct 15, 2020
Merged
@fanquake
Copy link
Member

Being backported to 0.19 in #20150.

Bushstar pushed a commit to Bushstar/omnicore that referenced this pull request Oct 21, 2020
@hebasto @Bushstar
qt: Fix QFileDialog for static builds
4e29ae2 
This change partially reverts 248e22b.

Github-Pull: bitcoin#19536
Rebased-From: 6457361
Tree-SHA512: 3b48c64f59068d70b3aec4514ebe4a091813c77518a52a81bd8a36b44d0854b3f5e187f52c0f8e4527506087cb7e2cafb7c39cb7d28e4b8f620a77980e8a4697
Platinumwrist referenced this pull request in syscoin/syscoin Oct 25, 2020
@laanwj
doc: Add historical release notes for 0.20.1
05e2740
MarkLTZ added a commit to litecoinz-core/litecoinz that referenced this pull request Nov 7, 2020
@MarkLTZ
[build] Fix QFileDialog for static builds (bitcoin#19536)
7ba0dab
maflcko pushed a commit that referenced this pull request Dec 2, 2020
Merge #20150: [0.19] Backports
5baaea4 
9c71499 rpc: Adjust witness-tx deserialize error message (MarcoFalke)
a7bdf5c rpc: Properly deserialize txs with witness before signing (MarcoFalke)
0b64310 Avoid the use of abs64 in timedata (Pieter Wuille)
5b2de04 Bump vcpkg commit ID to get new msys mirror list (Aaron Clauson)
6957419 build: set minimum required Boost to 1.48.0 (fanquake)
27bb2cc util: Don't reference errno when pthread fails. (MIZUTA Takeshi)
8bd2ab1 docs: Correct description for getblockstats's txs field (Nadav Ivgi)
a8411b3 qt: Fix QFileDialog for static builds (Hennadii Stepanov)

Pull request description:

  Backports the following to the 0.19 branch:
  * #19194 - util: Don't reference errno when pthread fails. - not clean.
  * #19536 - qt, build: Fix QFileDialog for static builds
  * #19777 - docs: Correct description for getblockstats's txs field
  * #19836 - rpc: Properly deserialize txs with witness before signing
  * #20095 - CI: Bump vcpkg commit ID to get new msys mirror list
  * #20141 - Avoid the use of abs64 in timedata
  * #20142 - [0.20] build: set minimum required Boost to 1.48.0

ACKs for top commit:
  jnewbery:
    utACK 9c71499
  dergoegge:
    utACK 9c71499
  MarcoFalke:
    ACK 9c71499

Tree-SHA512: 2151f22bc37a6a2f51a8f36c27376622016b51ff99b570e95354356fce1f1761cf19cb4f8ebfa26d38485a0bff6ff6ee834d2798fb383e2ae2abb175548b8fe6
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@theuni theuni theuni approved these changes

Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
0.19.2
Development

Successfully merging this pull request may close these issues.

Bitcoin Core on Ubuntu 18.04 not allowing to chose data directory
7 participants
@hebasto @Sjors @fanquake @alexstrbonn @maflcko @theuni @DrahtBot