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@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@
On Fri, Apr 21, 2017 at 7:44 PM, Martin Kelly mkelly@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
On 04/21/2017 01:33 PM, Sylvain Munaut wrote:
On Fri, Apr 21, 2017 at 7:44 PM, Martin Kelly mkelly@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.
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>"
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
On 04/21/2017 03:42 PM, Sylvain Munaut wrote:
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.
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/d9d49b6d0bb71ccc086968f262...
Certainly not ideal, but it may be the only solution.
(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.
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.
On 04/21/2017 04:39 PM, Martin Kelly wrote:
On 04/21/2017 03:42 PM, Sylvain Munaut wrote:
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.
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.
-
Martin Kelly -
Sylvain Munaut