21 Apr
2017
21 Apr
'17
1:44 p.m.
This shouldn't be necessary as it's part of the default compiler include
paths anyway. Morever, it can cause GCC 6 C++ build failures in
downstream packages when combined with QMake (such as
qtmultimedia-rtlfm-radio-plugin).
Fix these issues by removing it.
Signed-off-by: Martin Kelly <mkelly(a)xevo.com>
---
librtlsdr.pc.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/librtlsdr.pc.in b/librtlsdr.pc.in
index 5e55049..84b6d0c 100644
--- a/librtlsdr.pc.in
+++ b/librtlsdr.pc.in
@@ -6,6 +6,6 @@ includedir=@includedir@
Name: RTL-SDR Library
Description: C Utility Library
Version: @VERSION@
-Cflags: -I${includedir}/ @RTLSDR_PC_CFLAGS@
+Cflags: @RTLSDR_PC_CFLAGS@
Libs: -L${libdir} -lrtlsdr -lusb-1.0
Libs.private: @RTLSDR_PC_LIBS@
--
2.1.4
21 Apr
21 Apr
4:33 p.m.
On Fri, Apr 21, 2017 at 7:44 PM, Martin Kelly <mkelly(a)xevo.com> wrote:
This shouldn't be necessary as it's part of
the default compiler include
paths anyway.
Huh ... no.
It's only part of the default compiler include if you installed in
/usr ... but if you installed in a custom prefix, this will be
required and that's the whole point of the .pc file.
Cheers,
Sylvain
4:56 p.m.
On 04/21/2017 01:33 PM, Sylvain Munaut wrote:
On Fri, Apr 21, 2017 at 7:44 PM, Martin Kelly
<mkelly(a)xevo.com> wrote:
Let me provide a bit more context. I created this patch because a
package in Openembedded (qtmultimedia-rtlfm-radio-plugin) relies on the
.pc file for rtl-sdr, which injects -I /usr/include. That package uses
QMake, when then translates the include into -isystem /usr/include. This
breaks GCC 6's #include_next directive, as documented by GCC and Qt:
GCC bug report:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70129
Qt bug report:
https://bugreports.qt.io/browse/QTBUG-53367
In both cases, the maintainers state that the right solution is to not
do -I /usr/include. AFAICT, this means there are two options:
(a) Don't export -I /usr/include in the .pc file.
(b) Manually remove -I /usr/include for anyone who compiles with GCC 6
and needs to use rtl-sdr.
Solution (a) seems cleaner to me. If you have another way that avoids
breaking packages that include rtl-sdr and use GCC 6, I'm happy to hear it.
In my case, when I ask GCC for the default include path, I'm seeing
/usr/include listed. In the case of Openembedded, the cross-compile
toolchain already takes care of prefix issues by using --sysroot.
GCC default include paths:
martin@columbia:~$ echo | gcc -E -Wp,-v -
ignoring nonexistent directory "/usr/local/include/x86_64-linux-gnu"
ignoring nonexistent directory
"/usr/lib/gcc/x86_64-linux-gnu/4.9/../../../../x86_64-linux-gnu/include"
#include "..." search starts here:
#include <...> search starts here:
/usr/lib/gcc/x86_64-linux-gnu/4.9/include
/usr/local/include
/usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed
/usr/include/x86_64-linux-gnu
/usr/include
End of search list.
# 1 "<stdin>"
# 1 "<built-in>"
# 1 "<command-line>"
# 1 "/usr/include/stdc-predef.h" 1 3 4
# 1 "<command-line>" 2
# 1 "<stdin>"
This shouldn't be necessary as it's part
of the default compiler include
paths anyway.
Huh ... no.
It's only part of the default compiler include if you installed in
/usr ... but if you installed in a custom prefix, this will be
required and that's the whole point of the .pc file.
6:42 p.m.
In both cases, the maintainers state that the right
solution is to not do -I
/usr/include. AFAICT, this means there are two options:
Not really no ...
They say to not use the -isystem /usr/include
My opinion there is that qmake is broken doing the translation from
-Ixxx to -isystem xxx blindly when both don't have _exactly_ the same
semantic. QMake should check the system include list when doing the
translation and omit the system ones to compensate for the difference
of behavior of -I vs -isystem.
(a) Don't export -I /usr/include in the .pc file.
(b) Manually remove -I /usr/include for anyone who compiles with GCC 6 and
needs to use rtl-sdr.
Solution (a) seems cleaner to me. If you have another way that avoids
breaking packages that include rtl-sdr and use GCC 6, I'm happy to hear it.
Well (a) is fine ... but you can only remove /usr/include ... not ${includedir}
So you need to test is includedir is indeed /usr/include and then you
can omit it but if not, then you need to leave it.
Cheers,
Sylvain
7:39 p.m.
On 04/21/2017 03:42 PM, Sylvain Munaut wrote:
I see your point and agree that QMake's behavior is undesirable. I also
agree that removing includedir will break compiling with --prefix
without --sysroot; from a quick survey of a bunch of .pc files, they all
include either -I${includedir} or -I${includedir}/project. The second
category will not hit these issues while the first will.
Bugs have been filed on QMake regarding this in the past:
https://bugreports.qt.io/browse/QTBUG-53375
https://bugreports.qt.io/browse/QTBUG-53367
A patch was even drafted to fix it:
https://codereview.qt-project.org/#/c/159215
The patch was abandoned with this reasoning:
"The idea is that -isystem also suppresses warnings from files found in
those paths. We'd like to keep that functionality.
The problem is that -isystem /usr/include, specifically, reorders that
path."
The workaround in qtBittorrent was to explicitly remove the include/libs
that QMake had added:
https://github.com/qbittorrent/qBittorrent/commit/d9d49b6d0bb71ccc086968f26…
Certainly not ideal, but it may be the only solution.
This could work, but it still would break some machine that uses
somewhere else for system compiler path. Perhaps doing what qtbittorrent
did and just removing what QMake adds is the best solution, absent QMake
fixing it.
In both cases,
the maintainers state that the right solution is to not do -I
/usr/include. AFAICT, this means there are two options:
Not really no ...
They say to not use the -isystem /usr/include
My opinion there is that qmake is broken doing the translation from
-Ixxx to -isystem xxx blindly when both don't have _exactly_ the same
semantic. QMake should check the system include list when doing the
translation and omit the system ones to compensate for the difference
of behavior of -I vs -isystem.
(a) Don't export -I /usr/include in the .pc
file.
(b) Manually remove -I /usr/include for anyone who compiles with GCC 6 and
needs to use rtl-sdr.
Solution (a) seems cleaner to me. If you have another way that avoids
breaking packages that include rtl-sdr and use GCC 6, I'm happy to hear it.
Well (a) is fine ... but you can only remove /usr/include ... not ${includedir}
So you need to test is includedir is indeed /usr/include and then you
can omit it but if not, then you need to leave it.
8:54 p.m.
On 04/21/2017 04:39 PM, Martin Kelly wrote:
On 04/21/2017 03:42 PM, Sylvain Munaut wrote:
OK, I have fixed it in the manner that qtbittorrent did (manually
removing -isystem /usr/include flags from what QMake provides). It's not
pretty, but if QMake's behaviors remains as it is, I'm not sure there's
much else to do. Feel free to drop this patch.
Well (a) is fine ... but you can only remove /usr/include ... not
${includedir}
So you need to test is includedir is indeed /usr/include and then you
can omit it but if not, then you need to leave it.
This could work, but it still would break some machine that uses
somewhere else for system compiler path. Perhaps doing what qtbittorrent
did and just removing what QMake adds is the best solution, absent QMake
fixing it.
3005
days inactive
3006
days old
5 comments
2 participants
participants (2)
-
Martin Kelly -
Sylvain Munaut