Hi, On Mon, Apr 20, 2020 at 10:12:37AM +0100, Jonathan Wakely wrote: > > Now you are probably going to say that "-isystem /usr/include" is a bad > > idea and that you shouldn't do that. > > Right. > > > I'm inclined to agree. This isn't a > > problem just yet. Debian wants to move /usr/include/stdlib.h to > > /usr/include/<multiarch>/stdlib.h. After that move, the problematic flag > > becomes "-isystem /usr/include/<multiarch>". Unfortunately, around 30 > > Debian packages[1] do pass exactly that flag. Regardless whether doing > > so is a bad idea, I guess we will have to support that. > > Or Debian should fix what they're going to break.
This is not quite precise. The offending -isystem /usr/include/<multiarch> flag is already being passed. According to what you write later, doing so is broken today. It just happens to work by accident. So all we do is making the present breakage visible. > > I am proposing to replace those two #include_next with plain #include. > > That'll solve the problem described above, but it is not entirely > > obvious that doing so doesn't break something else. > > > > After switching those #include_next to #include, > > libstdc++-v3/include/c_global/cstdlib will continue to temporarily > > will #include <stdlib.h>. Now, it'll search all include directories. It > > may find libstdc++-v3/include/c_comaptibility/stdlib.h or the libc's > > version. We cannot tell which. If it finds the one from libstdc++-v3, > > the header will notice the _GLIBCXX_INCLUDE_NEXT_C_HEADERS macro and > > immediately #include_next <stdlib.h> skipping the rest of the header. > > That in turn will find the libc version. So in both cases, it ends up > > using the right one. Precisely what we wanted. > > As Marc said, this doesn't work. That is not very precise either. Marc said that it won't fix all cases. In practice, it would make those work that don't #include <stdlib.h> but use #include <cstdlib> instead. Marc also indicated that using include_next for a header of a different name is wrong. So this is a bug in libstdc++ regardless of whether it breaks or unbreaks other pieces of software. > If a program tries to include <stdlib.h> it needs to get the libstdc++ > version, otherwise only the libc versions of certain functions are > defined. That means the additional C++ overloads such as ::abs(long) > and ::abs(long long) won't be defined. That is the reason why > libstdc++ provides its own <stdlib.h>. > > And if you do -isystem /usr/include (or any other option that causes > libstdc++'s <stdlib.h> to be skipped) that doesn't work. Only > ::abs(int) gets defined. > > So -isystem /usr/include breaks code, with or without your patch. It is very difficult to disagree with -isystem /usr/include or -isystem /usr/include/<triplet> being broken and unsupported. Having you state it that clearly does help with communicating to other upstreams. For this reason, I've looked into the remaining cases. It turns out that there aren't that many left. In particular chromium, opencv and vtk got fixed in the mean time. Basically all remaining failures could be attributed to qmake, which passes all directories below /usr/include (including /usr/include and /usr/include/<triplet> if a .pc file mentions them) using -isystem. I've sent a patch https://bugs.debian.org/958479 to make qmake stop doing that. I therefore agree with you that the patch I sent for libstdc++ is not necessary to make packages build on Debian. Removing the offending -isystem flags from the respective builds is a manageable option and has already happened to a large extend. We can conclude that the motivation for my patch is not a good one, because it embraces broken behaviour. However, the use of include_next remains a bug, because the name of the including and the name of the included header differ, and it should be fixed on that ground. Helmut
