diff mbox

pacakge/qt5/qt5base: fix build with ccache

Message ID 1440499298-39957-1-git-send-email-benoit@wsystem.com
State Superseded
Headers show

Commit Message

Benoît Thébaudeau Aug. 25, 2015, 10:41 a.m. UTC
Building with ccache failed with:

    Running configuration tests...
    Failed to process makespec for platform 'devices/linux-buildroot-g++'
    Project ERROR: Compiler <path_to_output_dir>/host/usr/bin/ccache <path_to_output_dir>/host/usr/bin/<cross_compile>-g++ not found. Check the value of CROSS_COMPILE -device-option
    Could not read qmake configuration file <path_to_output_dir>/build/qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf.
    Error processing project file: /dev/null

This was caused by Buildroot setting this in
qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf:

    QMAKE_CXX               = $${BR_CCACHE} $${CROSS_COMPILE}g++

But qt5base-5.5.0/mkspecs/features/device_config.prf expects QMAKE_CXX
to be a valid (absolute or QMAKE_PATH_ENV-relative) path to an existing
file, which is not possible if using ccache as above.

Add a patch removing these tests since the toolchain is already tested
by Buildroot, which allows to keep using ccache.

Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
---
 .../qt5/qt5base/0009-fix-build-with-ccache.patch   | 60 ++++++++++++++++++++++
 1 file changed, 60 insertions(+)
 create mode 100644 package/qt5/qt5base/0009-fix-build-with-ccache.patch

Comments

Thomas Petazzoni Aug. 26, 2015, 1:45 p.m. UTC | #1
Benoît,

On Tue, 25 Aug 2015 12:41:38 +0200, Benoît Thébaudeau wrote:
> Building with ccache failed with:
> 
>     Running configuration tests...
>     Failed to process makespec for platform 'devices/linux-buildroot-g++'
>     Project ERROR: Compiler <path_to_output_dir>/host/usr/bin/ccache <path_to_output_dir>/host/usr/bin/<cross_compile>-g++ not found. Check the value of CROSS_COMPILE -device-option
>     Could not read qmake configuration file <path_to_output_dir>/build/qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf.
>     Error processing project file: /dev/null
> 
> This was caused by Buildroot setting this in
> qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf:
> 
>     QMAKE_CXX               = $${BR_CCACHE} $${CROSS_COMPILE}g++
> 
> But qt5base-5.5.0/mkspecs/features/device_config.prf expects QMAKE_CXX
> to be a valid (absolute or QMAKE_PATH_ENV-relative) path to an existing
> file, which is not possible if using ccache as above.
> 
> Add a patch removing these tests since the toolchain is already tested
> by Buildroot, which allows to keep using ccache.
> 
> Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>

Thanks for the patch. I see the problem, but as usual, having a
solution that can be submitted upstream is better. See my suggestion
below.

> ---
>  .../qt5/qt5base/0009-fix-build-with-ccache.patch   | 60 ++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
>  create mode 100644 package/qt5/qt5base/0009-fix-build-with-ccache.patch
> 
> diff --git a/package/qt5/qt5base/0009-fix-build-with-ccache.patch b/package/qt5/qt5base/0009-fix-build-with-ccache.patch
> new file mode 100644
> index 0000000..04d41a4
> --- /dev/null
> +++ b/package/qt5/qt5base/0009-fix-build-with-ccache.patch
> @@ -0,0 +1,60 @@
> +Fix build with ccache
> +
> +Building with Buildroot and ccache failed with:
> +
> +    Running configuration tests...
> +    Failed to process makespec for platform 'devices/linux-buildroot-g++'
> +    Project ERROR: Compiler <path_to_output_dir>/host/usr/bin/ccache <path_to_output_dir>/host/usr/bin/<cross_compile>-g++ not found. Check the value of CROSS_COMPILE -device-option
> +    Could not read qmake configuration file <path_to_output_dir>/build/qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf.
> +    Error processing project file: /dev/null
> +
> +This was caused by Buildroot setting this in
> +qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf:
> +
> +    QMAKE_CXX               = $${BR_CCACHE} $${CROSS_COMPILE}g++
> +
> +But qt5base-5.5.0/mkspecs/features/device_config.prf expects QMAKE_CXX
> +to be a valid (absolute or QMAKE_PATH_ENV-relative) path to an existing
> +file, which is not possible if using ccache as above.
> +
> +Remove these tests since the toolchain is already tested by Buildroot,
> +which allows to keep using ccache.
> +
> +Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
> +---
> + mkspecs/features/device_config.prf | 23 -----------------------
> + 1 file changed, 23 deletions(-)
> +
> +diff --git a/mkspecs/features/device_config.prf b/mkspecs/features/device_config.prf
> +index cd3a0cf..ef16540 100644
> +--- a/mkspecs/features/device_config.prf
> ++++ b/mkspecs/features/device_config.prf
> +@@ -14,28 +14,5 @@ host_build {
> + 
> + # Provide a function to be used by mkspecs
> + defineTest(deviceSanityCheckCompiler) {
> +-    equals(QMAKE_HOST.os, Windows): \
> +-        sfx = .exe
> +-    else: \
> +-        sfx =
> +-
> +-    # Check if the binary exists with an absolute path. Do this check
> +-    # before the CROSS_COMPILE empty check below to allow the mkspec
> +-    # to derive the compiler path from other device options.
> +-    exists($$QMAKE_CXX$$sfx):return()
> +-
> +-    # Check for possible reasons of failure
> +-    # check if CROSS_COMPILE device-option is set
> +-    isEmpty(CROSS_COMPILE):error("CROSS_COMPILE needs to be set via -device-option CROSS_COMPILE=<path>")
> +-
> +-    # Check if QMAKE_CXX points to an executable.
> +-    ensurePathEnv()
> +-    for (dir, QMAKE_PATH_ENV) {
> +-        exists($$dir/$${QMAKE_CXX}$$sfx): \
> +-            return()
> +-    }

Instead of removing this entire piece of code, can you try to replace
just this last part by something like:

	system($${QMAKE_CXX}$$sfx --version): return()

Instead of trying to look for a file, it will actually try to run the
compiler.

Also, can you report the bug and submit the patch upstream to Qt? See
https://bugreports.qt.io/secure/Dashboard.jspa.

Thanks!

Thomas
Benoît Thébaudeau Aug. 26, 2015, 10:10 p.m. UTC | #2
Hi Thomas,

On Wed, Aug 26, 2015 at 3:45 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> On Tue, 25 Aug 2015 12:41:38 +0200, Benoît Thébaudeau wrote:
>> +-    # Check if QMAKE_CXX points to an executable.
>> +-    ensurePathEnv()
>> +-    for (dir, QMAKE_PATH_ENV) {
>> +-        exists($$dir/$${QMAKE_CXX}$$sfx): \
>> +-            return()
>> +-    }
>
> Instead of removing this entire piece of code, can you try to replace
> just this last part by something like:
>
>         system($${QMAKE_CXX}$$sfx --version): return()
>
> Instead of trying to look for a file, it will actually try to run the
> compiler.
>
> Also, can you report the bug and submit the patch upstream to Qt? See
> https://bugreports.qt.io/secure/Dashboard.jspa.

I have opened a bug report upstream as you suggested:
https://bugreports.qt.io/browse/QTBUG-47951

As explained in this report, I think that your suggestion would work
for Buildroot (not yet tested), but not for upstream Qt.

My point with this patch was that upstream could consider this
Buildroot use case as illegal because the definition of QMAKE_CXX
(http://doc.qt.io/qt-5/qmake-variable-reference.html#qmake-cxx) only
mentions a filename and a path as possibilities. In that case, the bug
is in Buildroot, and since this sanity check of the compiler is not
needed for Buildroot, we can just remove this test. If we don't want a
local patch for this, maybe we could use a wrapper above ccache, or
put ccache into QMAKE_CXX and the compiler into QMAKE_CXXFLAGS (not
tested, and a bit ugly).

Wait and see what upstream says.

Best regards,
Benoît
Benoît Thébaudeau Aug. 27, 2015, 8:12 a.m. UTC | #3
Hi Thomas,

On 27/08/2015 00:10, Benoît Thébaudeau wrote:
> On Wed, Aug 26, 2015 at 3:45 PM, Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com> wrote:
>> On Tue, 25 Aug 2015 12:41:38 +0200, Benoît Thébaudeau wrote:
>>> +-    # Check if QMAKE_CXX points to an executable.
>>> +-    ensurePathEnv()
>>> +-    for (dir, QMAKE_PATH_ENV) {
>>> +-        exists($$dir/$${QMAKE_CXX}$$sfx): \
>>> +-            return()
>>> +-    }
>>
>> Instead of removing this entire piece of code, can you try to replace
>> just this last part by something like:
>>
>>         system($${QMAKE_CXX}$$sfx --version): return()
>>
>> Instead of trying to look for a file, it will actually try to run the
>> compiler.
>>
>> Also, can you report the bug and submit the patch upstream to Qt? See
>> https://bugreports.qt.io/secure/Dashboard.jspa.
> 
> I have opened a bug report upstream as you suggested:
> https://bugreports.qt.io/browse/QTBUG-47951
> 
> As explained in this report, I think that your suggestion would work
> for Buildroot (not yet tested), but not for upstream Qt.
> 
> My point with this patch was that upstream could consider this
> Buildroot use case as illegal because the definition of QMAKE_CXX
> (http://doc.qt.io/qt-5/qmake-variable-reference.html#qmake-cxx) only
> mentions a filename and a path as possibilities. In that case, the bug
> is in Buildroot, and since this sanity check of the compiler is not
> needed for Buildroot, we can just remove this test. If we don't want a
> local patch for this, maybe we could use a wrapper above ccache, or
> put ccache into QMAKE_CXX and the compiler into QMAKE_CXXFLAGS (not
> tested, and a bit ugly).
> 
> Wait and see what upstream says.

Upstream recommends this approach:
http://lists.qt-project.org/pipermail/qt-creator/2014-January/003148.html

What do you think?

Best regards,
Benoît
Thomas Petazzoni Aug. 27, 2015, 8:54 a.m. UTC | #4
Hello,

On Thu, 27 Aug 2015 10:12:44 +0200, Benoît Thébaudeau wrote:

> > As explained in this report, I think that your suggestion would work
> > for Buildroot (not yet tested), but not for upstream Qt.
> > 
> > My point with this patch was that upstream could consider this
> > Buildroot use case as illegal because the definition of QMAKE_CXX
> > (http://doc.qt.io/qt-5/qmake-variable-reference.html#qmake-cxx) only
> > mentions a filename and a path as possibilities. In that case, the bug
> > is in Buildroot, and since this sanity check of the compiler is not
> > needed for Buildroot, we can just remove this test. If we don't want a
> > local patch for this, maybe we could use a wrapper above ccache, or
> > put ccache into QMAKE_CXX and the compiler into QMAKE_CXXFLAGS (not
> > tested, and a bit ugly).
> > 
> > Wait and see what upstream says.
> 
> Upstream recommends this approach:
> http://lists.qt-project.org/pipermail/qt-creator/2014-January/003148.html
> 
> What do you think?

Well, we already have a wrapper for the compiler in the case of the
external toolchain. And we will probably also extend the wrapper in the
future to the internal toolchain case, in order to support per-package
staging directory. In this case, we could take advantage of this
wrapper to include the ccache functionality, and avoid having CC and
CXX values that have two words.

I really would like to have the wrapper suggested in this e-mail
calling our wrapper, then calling the real compiler.

Best regards,

Thomas
Benoît Thébaudeau Aug. 27, 2015, 9:12 a.m. UTC | #5
Hi,

On 27/08/2015 10:54, Thomas Petazzoni wrote:
> On Thu, 27 Aug 2015 10:12:44 +0200, Benoît Thébaudeau wrote:
> 
>>> As explained in this report, I think that your suggestion would work
>>> for Buildroot (not yet tested), but not for upstream Qt.
>>>
>>> My point with this patch was that upstream could consider this
>>> Buildroot use case as illegal because the definition of QMAKE_CXX
>>> (http://doc.qt.io/qt-5/qmake-variable-reference.html#qmake-cxx) only
>>> mentions a filename and a path as possibilities. In that case, the bug
>>> is in Buildroot, and since this sanity check of the compiler is not
>>> needed for Buildroot, we can just remove this test. If we don't want a
>>> local patch for this, maybe we could use a wrapper above ccache, or
>>> put ccache into QMAKE_CXX and the compiler into QMAKE_CXXFLAGS (not
>>> tested, and a bit ugly).
>>>
>>> Wait and see what upstream says.
>>
>> Upstream recommends this approach:
>> http://lists.qt-project.org/pipermail/qt-creator/2014-January/003148.html
>>
>> What do you think?
> 
> Well, we already have a wrapper for the compiler in the case of the
> external toolchain. And we will probably also extend the wrapper in the
> future to the internal toolchain case, in order to support per-package
> staging directory. In this case, we could take advantage of this
> wrapper to include the ccache functionality, and avoid having CC and
> CXX values that have two words.
> 
> I really would like to have the wrapper suggested in this e-mail
> calling our wrapper, then calling the real compiler.

OK, but in the meantime, until we have a wrapper for the internal toolchain too,
what do you want to do to fix the current build failure?
 - Forbid the use of ccache with Qt 5?
 - Use the current patch?
 - Use the wrapper from the URL above? In that case, where would you put it in
   Buildroot (qt5base, new package, toolchain, support)?

Best regards,
Benoît
Thomas Petazzoni Aug. 27, 2015, 12:39 p.m. UTC | #6
Benoît,

On Thu, 27 Aug 2015 11:12:47 +0200, Benoît Thébaudeau wrote:

 > OK, but in the meantime, until we have a wrapper for the internal toolchain too,
> what do you want to do to fix the current build failure?
>  - Forbid the use of ccache with Qt 5?
>  - Use the current patch?
>  - Use the wrapper from the URL above? In that case, where would you put it in
>    Buildroot (qt5base, new package, toolchain, support)?

For 2015.08, your patch is certainly the simplest solution.

Thanks!

Thomas
Jaap Crezee Aug. 27, 2015, 12:49 p.m. UTC | #7
Hi all,


On 08/27/15 11:12, Benoît Thébaudeau wrote:
> OK, but in the meantime, until we have a wrapper for the internal toolchain too,
> what do you want to do to fix the current build failure?
>  - Forbid the use of ccache with Qt 5?

I would not recommend that. Qt5base is *huge* and recompiling it over and over again (as I
do when I am developing with/against buildroot) is just too time consuming.

>  - Use the current patch?

You can also look at one of my previous patches (see list archive). I simply don't have
the time to respond to all correction requests unfortunately.


Regards,


Jaap Crezee
Benoît Thébaudeau Aug. 27, 2015, 7:03 p.m. UTC | #8
Dear Jaap Crezee,

On Thu, Aug 27, 2015 at 2:49 PM, Jaap Crezee <jaap@jcz.nl> wrote:
> On 08/27/15 11:12, Benoît Thébaudeau wrote:
>> OK, but in the meantime, until we have a wrapper for the internal toolchain too,
>> what do you want to do to fix the current build failure?
>>  - Forbid the use of ccache with Qt 5?
>
> I would not recommend that. Qt5base is *huge* and recompiling it over and over again (as I
> do when I am developing with/against buildroot) is just too time consuming.

Agreed.

>>  - Use the current patch?
>
> You can also look at one of my previous patches (see list archive). I simply don't have
> the time to respond to all correction requests unfortunately.

I have added your suggestion here:
https://bugreports.qt.io/browse/QTBUG-47951?focusedCommentId=291119&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-291119

Best regards,
Benoît
Benoît Thébaudeau Aug. 28, 2015, 9:50 a.m. UTC | #9
Hi Thomas,

On 27/08/2015 14:39, Thomas Petazzoni wrote:
> On Thu, 27 Aug 2015 11:12:47 +0200, Benoît Thébaudeau wrote:
> 
>  > OK, but in the meantime, until we have a wrapper for the internal toolchain too,
>> what do you want to do to fix the current build failure?
>>  - Forbid the use of ccache with Qt 5?
>>  - Use the current patch?
>>  - Use the wrapper from the URL above? In that case, where would you put it in
>>    Buildroot (qt5base, new package, toolchain, support)?
> 
> For 2015.08, your patch is certainly the simplest solution.

I've just posted a v2:
http://patchwork.ozlabs.org/patch/511841/

I will submit it upstream. According to the last comment in the upstream bug
report, this patch could possibly be accepted upstream.

Best regards,
Benoît
diff mbox

Patch

diff --git a/package/qt5/qt5base/0009-fix-build-with-ccache.patch b/package/qt5/qt5base/0009-fix-build-with-ccache.patch
new file mode 100644
index 0000000..04d41a4
--- /dev/null
+++ b/package/qt5/qt5base/0009-fix-build-with-ccache.patch
@@ -0,0 +1,60 @@ 
+Fix build with ccache
+
+Building with Buildroot and ccache failed with:
+
+    Running configuration tests...
+    Failed to process makespec for platform 'devices/linux-buildroot-g++'
+    Project ERROR: Compiler <path_to_output_dir>/host/usr/bin/ccache <path_to_output_dir>/host/usr/bin/<cross_compile>-g++ not found. Check the value of CROSS_COMPILE -device-option
+    Could not read qmake configuration file <path_to_output_dir>/build/qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf.
+    Error processing project file: /dev/null
+
+This was caused by Buildroot setting this in
+qt5base-5.5.0/mkspecs/devices/linux-buildroot-g++/qmake.conf:
+
+    QMAKE_CXX               = $${BR_CCACHE} $${CROSS_COMPILE}g++
+
+But qt5base-5.5.0/mkspecs/features/device_config.prf expects QMAKE_CXX
+to be a valid (absolute or QMAKE_PATH_ENV-relative) path to an existing
+file, which is not possible if using ccache as above.
+
+Remove these tests since the toolchain is already tested by Buildroot,
+which allows to keep using ccache.
+
+Signed-off-by: Benoît Thébaudeau <benoit@wsystem.com>
+---
+ mkspecs/features/device_config.prf | 23 -----------------------
+ 1 file changed, 23 deletions(-)
+
+diff --git a/mkspecs/features/device_config.prf b/mkspecs/features/device_config.prf
+index cd3a0cf..ef16540 100644
+--- a/mkspecs/features/device_config.prf
++++ b/mkspecs/features/device_config.prf
+@@ -14,28 +14,5 @@ host_build {
+ 
+ # Provide a function to be used by mkspecs
+ defineTest(deviceSanityCheckCompiler) {
+-    equals(QMAKE_HOST.os, Windows): \
+-        sfx = .exe
+-    else: \
+-        sfx =
+-
+-    # Check if the binary exists with an absolute path. Do this check
+-    # before the CROSS_COMPILE empty check below to allow the mkspec
+-    # to derive the compiler path from other device options.
+-    exists($$QMAKE_CXX$$sfx):return()
+-
+-    # Check for possible reasons of failure
+-    # check if CROSS_COMPILE device-option is set
+-    isEmpty(CROSS_COMPILE):error("CROSS_COMPILE needs to be set via -device-option CROSS_COMPILE=<path>")
+-
+-    # Check if QMAKE_CXX points to an executable.
+-    ensurePathEnv()
+-    for (dir, QMAKE_PATH_ENV) {
+-        exists($$dir/$${QMAKE_CXX}$$sfx): \
+-            return()
+-    }
+-
+-    # QMAKE_CXX does not point to a compiler.
+-    error("Compiler $$QMAKE_CXX not found. Check the value of CROSS_COMPILE -device-option")
+ }
+