Bug#958479: qmake passes unsupported -isystem to gcc

2020-05-28 Thread Dmitry Shachnev
Control: forwarded -1 https://codereview.qt-project.org/c/qt/qtbase/+/302175

Hi Helmut,

On Wed, May 27, 2020 at 09:35:10PM +0200, Helmut Grohne wrote:
> I looked and the process is waaay to beyond what I can do. I'm not up to
> consenting to a 10-page TOS for a 5-line patch. Sorry.
>
> The 5-page CLA doesn't make this any better. Not a friendly project to
> contribute to.

OK, that is understandable.

> Your suggestion to explicitly license it under BSD seems much more
> reasonable to me. Let's do that. However, given that I find you much
> more trustworthy than QT upstream, I think that there is an even simpler
> option: Please treat the patch I sent to this bug as yours. You may
> claim authorship. I think legally that means I transfer whatever
> copyright I may have on this particular patch to you after licensing it
> under BSD.

I have now submitted this patch to Gerrit, see forwarded link. Let's see
what upstream thinks about it.

Also I simplified the unixmake2.cpp part of the patch a bit. The logic
should stay the same as in your patch.

--
Dmitry Shachnev


signature.asc
Description: PGP signature


Bug#958479: qmake passes unsupported -isystem to gcc

2020-05-27 Thread Helmut Grohne
Hi Dmitry,

On Wed, May 27, 2020 at 06:17:56PM +0300, Dmitry Shachnev wrote:
> Can I ask you to forward it to upstream Gerrit [1] (see [2] for the
> instruction)? Unfortunately, upstream requires the patch author to sign a
> CLA so I cannot easily forward it under my account. Alternatively, you may
> publicly state that you license your patch under a permissive license
> (e.g. BSD), which should also work (but upstream does not like this much).

I looked and the process is waaay to beyond what I can do. I'm not up to
consenting to a 10-page TOS for a 5-line patch. Sorry.

The 5-page CLA doesn't make this any better. Not a friendly project to
contribute to.

Your suggestion to explicitly license it under BSD seems much more
reasonable to me. Let's do that. However, given that I find you much
more trustworthy than QT upstream, I think that there is an even simpler
option: Please treat the patch I sent to this bug as yours. You may
claim authorship. I think legally that means I transfer whatever
copyright I may have on this particular patch to you after licensing it
under BSD.

Helmut



Bug#958479: qmake passes unsupported -isystem to gcc

2020-05-27 Thread Dmitry Shachnev
Hi Helmut!

On Wed, Apr 22, 2020 at 08:39:16PM +0200, Helmut Grohne wrote:
> The context of this bug is #798955 aka moving glibc headers from
> /usr/include to /usr/include/. When doing this, some packages
> FTBFS with the symptom that  or  fail to find
> #include_next  or #include_next  respectively. The
> cause is passing -isystem /usr/include/ to the compiler. I
> asked libstdc++ upstream to fix this, but the reply was that passing
> this flag is unsupported (see bug log of #798955). The takeaway is:
> Pass -isystem /usr/include or -isystem /usr/include/ is broken.
>
> Unfortunately, qmake does exactly that.
> [...]
>
> I can also propose a solution. When constructing INCPATH, any directory
> that directly is a compiler search directory (not a subdirectory
> thereof), the directory should be skipped. Doing so ensures that the
> order of compiler search directories is not changed. The possible harm
> seems quite limited given that the compiler will already search these
> directories (just in the correct order).
>
> What do you think?

I think your patch makes sense.

Can I ask you to forward it to upstream Gerrit [1] (see [2] for the
instruction)? Unfortunately, upstream requires the patch author to sign a
CLA so I cannot easily forward it under my account. Alternatively, you may
publicly state that you license your patch under a permissive license
(e.g. BSD), which should also work (but upstream does not like this much).

In case you agree to forward it, please also fix the indentation, it should
not contain tabs.

[1]: https://codereview.qt-project.org/
[2]: https://wiki.qt.io/Gerrit_Introduction

--
Dmitry Shachnev


signature.asc
Description: PGP signature


Bug#958479: qmake passes unsupported -isystem to gcc

2020-04-22 Thread Helmut Grohne
Package: qt5-qmake
Version: 5.12.5+dfsg-10
Severity: important
Tags: ftbfs patch upstream
Control: affects -1 + src:deepin-music
Control: block 798955 by -1

The context of this bug is #798955 aka moving glibc headers from
/usr/include to /usr/include/. When doing this, some packages
FTBFS with the symptom that  or  fail to find
#include_next  or #include_next  respectively. The
cause is passing -isystem /usr/include/ to the compiler. I
asked libstdc++ upstream to fix this, but the reply was that passing
this flag is unsupported (see bug log of #798955). The takeaway is:
Pass -isystem /usr/include or -isystem /usr/include/ is broken.

Unfortunately, qmake does exactly that. It collects include directories
(e.g. from pkg-config file). For instance libavutil.pc contains
includedir=/usr/include/. The collected directories reside in a
variable INCLUDEPATH. It then passes -isystem for every path that
resides below a set of compiler include directories and -I for
everything else. Since /usr/include/ resides below
/usr/include, qmake turns the flag into -isystem. Doing so changes the
order of -isystem directories and breaks gcc. I think this is a bug in
qmake. The resuling flags are assigned to a variable INCPATH. The
relevant source can be found at:
https://sources.debian.org/src/qtbase-opensource-src/5.12.5+dfsg-10/qmake/generators/unix/unixmake2.cpp/#L196

An example of an affected package is deepin-music. I think around 10 (an
upper bound seems to be 30, but that includes other causes). You can
quickly check whether a package is affected by grepping a build log for
"-isystem /usr/include/".

I can also propose a solution. When constructing INCPATH, any directory
that directly is a compiler search directory (not a subdirectory
thereof), the directory should be skipped. Doing so ensures that the
order of compiler search directories is not changed. The possible harm
seems quite limited given that the compiler will already search these
directories (just in the correct order).

What do you think?

Please expect that this bug becomes a regular ftbfs bug eventually.

Helmut
--- qtbase-opensource-src-5.12.5+dfsg.orig/qmake/generators/makefiledeps.cpp
+++ qtbase-opensource-src-5.12.5+dfsg/qmake/generators/makefiledeps.cpp
@@ -359,6 +359,11 @@
 return false;
 }
 
+bool QMakeSourceFileInfo::isCompilerInclude(const QString )
+{
+return systemIncludes.contains(name);
+}
+
 char *QMakeSourceFileInfo::getBuffer(int s) {
 if(!spare_buffer || spare_buffer_size < s)
 spare_buffer = (char *)realloc(spare_buffer, spare_buffer_size=s);
--- qtbase-opensource-src-5.12.5+dfsg.orig/qmake/generators/makefiledeps.h
+++ qtbase-opensource-src-5.12.5+dfsg/qmake/generators/makefiledeps.h
@@ -106,6 +106,7 @@
 void addSourceFile(const QString &, uchar seek, SourceFileType type=TYPE_C);
 bool containsSourceFile(const QString &, SourceFileType type=TYPE_C);
 bool isSystemInclude(const QString &);
+bool isCompilerInclude(const QString &);
 
 int included(const QString );
 QStringList dependencies(const QString );
--- qtbase-opensource-src-5.12.5+dfsg.orig/qmake/generators/unix/unixmake2.cpp
+++ qtbase-opensource-src-5.12.5+dfsg/qmake/generators/unix/unixmake2.cpp
@@ -202,11 +202,12 @@
 if (inc.isEmpty())
 continue;
 
-if (!isystem.isEmpty() && isSystemInclude(inc.toQString()))
-t << ' ' << isystem << ' ';
+	if (isCompilerInclude(inc.toQString()))
+		;
+	else if (!isystem.isEmpty() && isSystemInclude(inc.toQString()))
+t << ' ' << isystem << ' ' << escapeFilePath(inc);
 else
-t << " -I";
-t << escapeFilePath(inc);
+t << " -I" << escapeFilePath(inc);
 }
 }
 if(!project->isEmpty("QMAKE_FRAMEWORKPATH_FLAGS"))