Skip to content

depends: Do not force Precompiled Headers (PCH) for building Qt on Linux #20520

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.

Sign up for GitHub

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
Merged
merged 1 commit into from
Dec 3, 2020
Merged

depends: Do not force Precompiled Headers (PCH) for building Qt on Linux #20520

merged 1 commit into from
Dec 3, 2020

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Nov 27, 2020 •
edited
Loading

On CentOS 8 (Cirrus CI job) the forced -pch option breaks Qt build.

Removing -pch option does not affect build time for other systems:

$ time make -j 9 -C depends/ qt
...
Caching qt...
make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'

real	4m22,359s
user	18m3,719s
sys     1m24,769s
  • this PR:
$ time make -j 9 -C depends/ qt
...
Caching qt...
make: Leaving directory '/home/hebasto/guix/GitHub/bitcoin/depends'

real	4m14,862s
user	18m3,355s
sys 	1m24,506s

Qt docs: https://doc.qt.io/qt-5/qmake-precompiledheaders.html

Fixes #20423

@DrahtBot
Copy link
Contributor

DrahtBot commented Nov 28, 2020 •
edited
Loading

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

Conflicts

No conflicts as of last run.

@DrahtBot DrahtBot mentioned this pull request Nov 28, 2020
Closed
@maflcko
Copy link
Member

maflcko commented Nov 28, 2020

This effectively reverts commit 9267a47 by @theuni

review ACK b1f3587 because this enables the GUI build again for i686, haven't checked the performance

@fanquake
Copy link
Member

@hebasto can you post the log of the qt build failure when using -pch? Do you know what has changed between CentOS 7 & 8 that is causing the issue?

Is using -pch still a problem when building newer Qt? i.e 5.15.x
If the problem is just with CentOS, can we disable pch for Linux Qt builds, rather than wholesale?

@hebasto
Copy link
Member Author

hebasto commented Nov 30, 2020 •
edited
Loading

can you post the log of the qt build failure when using -pch?

$ make -C depends qt
...
Running configuration tests...
...
Checking for precompiled header support... 
+ cd /home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/config.tests/common/pch && PKG_CONFIG_SYSROOT_DIR=/ PKG_CONFIG_LIBDIR=/home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/lib/pkgconfig /home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/bin/qmake "CONFIG -= qt debug_and_release app_bundle lib_bundle" "CONFIG += static warn_off console single_arch" -early "CONFIG += cross_compile" /home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/config.tests/common/pch
+ cd /home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/config.tests/common/pch && MAKEFLAGS= /usr/bin/gmake
> gmake[1]: Entering directory '/home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/config.tests/common/pch'
> g++ -m64 -pipe -pipe -O2 -I/home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include -O2 -w -fPIC  -I. -I/home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/mkspecs/linux-g++-64 -x c++-header -c header.h -o .pch/pch.gch/c++
> g++ -c -include .pch/pch -m64 -pipe -pipe -O2 -I/home/hebasto/bitcoin/depends/x86_64-pc-linux-gnu/include -O2 -w -fPIC  -I. -I/home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/mkspecs/linux-g++-64 -o source.o source.cpp
> cc1plus: error: one or more PCH files were found, but they were invalid
> cc1plus: error: use -Winvalid-pch for more information
> cc1plus: fatal error: .pch/pch: No such file or directory
> compilation terminated.
> gmake[1]: *** [Makefile:183: source.o] Error 1
> gmake[1]: Leaving directory '/home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/config.tests/common/pch'
test config.qtbase.tests.precompile_header FAILED
...
ERROR: Feature 'precompile_header' was enabled, but the pre-condition 'config.msvc || tests.precompile_header' failed.
make: *** [funcs.mk:268: /home/hebasto/bitcoin/depends/work/build/x86_64-pc-linux-gnu/qt/5.9.8-991caec5731/qtbase/.stamp_configured] Error 3
make: Leaving directory '/home/hebasto/bitcoin/depends'

Do you know what has changed between CentOS 7 & 8 that is causing the issue?

Compiler? CentOS 8 has gcc 8.3.1.

@maflcko
Copy link
Member

maflcko commented Dec 2, 2020

Anything left do do here? Disabling the gui cross compilation was only a temporary hack and it should be enabled again at the earliest convenience

@hebasto
depends: Do not force Precompiled Headers (PCH) for building Qt on Linux
c82d15b 
On CentOS 8 the forced '-pch' option breaks Qt build.
Removing '-pch' option does not affect build time for other Linux
systems.
@hebasto hebasto changed the title ci, depends: Do not force Precompiled Headers (PCH) when building Qt depends: Do not force Precompiled Headers (PCH) when building Qt Dec 2, 2020
@hebasto hebasto changed the title depends: Do not force Precompiled Headers (PCH) when building Qt depends: Do not force Precompiled Headers (PCH) for building Qt on Linux Dec 2, 2020
@hebasto
Copy link
Member Author

hebasto commented Dec 2, 2020

Updated b1f3587 -> c82d15b (pr20520.01 -> pr20520.02, diff):

If the problem is just with CentOS, can we disable pch for Linux Qt builds, rather than wholesale?

@maflcko
Copy link
Member

maflcko commented Dec 2, 2020

review ACK c82d15b

I liked the simplicity of the first patch, but this looks fine, too

@laanwj
Copy link
Member

laanwj commented Dec 3, 2020

This effectively reverts commit 9267a47 by @theuni

I'm confused as to why this is suddenly broken. This patch is from 2016 !

That said, merging this even as a temporary workaround makes sense.

@laanwj laanwj merged commit a3186b6 into bitcoin:master Dec 3, 2020
@hebasto hebasto deleted the 201127-pch branch December 3, 2020 12:30
@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
No reviews
Assignees
No one assigned
Labels
Build system
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

build: Re-enable i686 qt build on centos 8
5 participants
@hebasto @DrahtBot @maflcko @fanquake @laanwj