Use -isystem instead of -I in default configure.cppflags

[ryandesign (Ryan Carsten Schmidt)]

We should ​consider changing the default configure.cppflags from "-I" to "-isystem". ​-isystem is similar to -I except that the given directory is checked after all "-I" directories are checked. In this way, using "-isystem" instead of "-I" would fix a whole class of problems where some ports sometimes fail to upgrade when a previous version is installed. Several ports (including freetype, ghostscript, pianobar) already make this change, so making this change in base would allow us to simplify those ports. The attached patch changes "-I" to "-isystem" everywhere I could find it.

[ryandesign (Ryan Carsten Schmidt)]

proposed patch

[ryandesign (Ryan Carsten Schmidt)]

I've been using this fix locally on Mountain Lion and Mavericks with no problems that I could see.

However I found a mention that ​-isystem causes problems for C++ code on Tiger; if I can reproduce that, then maybe we have to make this change only on Leopard and later.

[larryv (Lawrence Velázquez)]

Cc Me!

[raimue (Rainer Müller)]

When building MacPorts itself, we should stick to the usual -I. I don't see any advantage of this change there.

Be aware that using -isystem hides all warnings from the header files as they are treated as if they were system headers. I worry this could mask problems during building and make it harder to identify the cause.

There is also the environment variable CPATH for additional include paths as if they were given using -I on the command line (​gcc manual). However, older versions of clang do not honor this setting.

[michaelld (Michael Dickens)]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

Right; MacPorts already sets CPATH.

[mkae (Marko Käning)]

Cc Me!

[drkp (Dan Ports)]

Cc Me!

[cooljeanius (Eric Gallager)]

Cc Me!

[ryandesign (Ryan Carsten Schmidt)]

Replying to ryandesign@…:

However I found a mention that ​-isystem causes problems for C++ code on Tiger; if I can reproduce that, then maybe we have to make this change only on Leopard and later.

I have indeed seen this problem, e.g. in texlive-bin: #41528. But switching Tiger's default compiler from gcc-4.0 to apple-gcc-4.2 should be the end of that problem: #41782.

Replying to raimue@…:

Be aware that using -isystem hides all warnings from the header files as they are treated as if they were system headers. I worry this could mask problems during building and make it harder to identify the cause.

I didn't know this would hide warnings, but that's an acceptable tradeoff for me. Every week or so, I encounter another ticket about a build problem that I did not see on my system because I was using the patch in this ticket. It fixes so many problems with one simple change that we should definitely make this change.

As for it treating headers as if they were system headers: they are system headers—that is, they are headers installed by the MacPorts system. No need, IMHO, to differentiate that from headers installed by the operating system; they're all just headers provided by software other than the one being compiled. Anytime "#include <foo>" is written, it's telling the preprocessor "please include the system header named 'foo'"; by using -isystem, we're telling the preprocessor (an additional place) where those headers may be found.

There is also the environment variable CPATH for additional include paths as if they were given using -I on the command line (​gcc manual). However, older versions of clang do not honor this setting.

Yes, with the difference that paths specified in CPATH are interpreted as if they were given "after any paths given with -I options on the command line" which would indeed be what we want.

MacPorts already sets CPATH and the corresponding LIBRARY_PATH, since several years. I'm not sure if this could completely replace our global -I / -L flags in CPPFLAGS and LDFLAGS. The documentation doesn't show me any reason why it couldn't, but then why haven't we stopped setting -I / -L? A couple years ago lack of support from older clang versions concerned me, but by this point in time we could afford to just blacklist them globally.

One advantage to -isystem is that any path specified with both -I and -isystem is interpreted as if it were specified only with -isystem. This would probably fix a few additional build failures (i.e. from ports that use pkg-config or other -config script which drags -I/opt/local/include back into the CPPFLAGS which could introduce a build failure if the order of the -I flags is wrong).

[ryandesign (Ryan Carsten Schmidt)]

r115978

[jmroot (Joshua Root)]

This changes the include order in subtle ways: -I${prefix}/include -I${prefix}/include/some_subdir is different to -isystem${prefix}/include -I${prefix}/include/some_subdir. Definite potential for confusing breakage.

[ryandesign (Ryan Carsten Schmidt)]

I've been using this patch since I opened this ticket 3 months ago with only positive results, as far as I can tell. Of course I have not tested all ports, and the scenario you propose is possible. However I think the number of issues this change will fix will far outnumber the new problems it creates.

[raimue (Rainer Müller)]

As I said earlier the behavior we want would be the environemnt variable CPATH:

CPATH specifies a list of directories to be searched as if specified with -I, but after any paths given with -I options on the command line.

The only catch is that old clang versions do not support it and I can't remember which (Apple) clang version introduced it. I propose we switch to CPATH when using Xcode >= 5. That version would definitely be new enough and required for both our currently supported platforms. Same for LIBRARY_PATH.

[ryandesign (Ryan Carsten Schmidt)]

Replying to raimue@…:

I propose we switch to CPATH when using Xcode >= 5. That version would definitely be new enough and required for both our currently supported platforms. Same for LIBRARY_PATH.

MacPorts already sets CPATH and LIBRARY_PATH in all phases and has done so for years.

[jmroot (Joshua Root)]

So this breaks some things that parse flags and don’t know about -isystem, right? Specifically swig, which is used kind of a lot.

​https://lists.macosforge.org/pipermail/macports-dev/2014-January/025753.html

Can we please revert this and clear or change configure.cppflags in only the ports that need it, or if you don’t care about old clang as per comment:10 just don’t set CPPFLAGS to anything by default, and reply on CPATH?

[ryandesign (Ryan Carsten Schmidt)]

Replying to jmr@…:

So this breaks some things that parse flags and don’t know about -isystem, right? Specifically swig, which is used kind of a lot.

​https://lists.macosforge.org/pipermail/macports-dev/2014-January/025753.html

Can we please revert this and clear or change configure.cppflags in only the ports that need it, or if you don’t care about old clang as per comment:10 just don’t set CPPFLAGS to anything by default, and reply on CPATH?

I see you reverted this in r118016. I would like to reconsider that reversion because I stand by the change.

It fixes a whole host of build failures due to incorrect -I flag ordering that are often not found during the normal course of updating a port, because they may be caused by having an older version of the same port installed, or by having another port installed that coincidentally installs headers whose files have the same names as private headers this port builds with. These problems either go unfixed, or it takes a MacPorts developer significant time and effort to track down the cause and then learn enough about the project's build system to integrate the fix. Our time and effort can be best spent elsewhere, and we can increase the happiness of our users by solving this whole class of problem instantly, centrally, without needing to bother the user with build failures and filing tickets.

I have been using this patch for months without problems. This problem you've mentioned with swig is the first problem I've heard of. Could you be more specific? Could we fix that problem instead?

I do not believe that the suggestion to rely on CPATH entirely solves the problem, because many ports will use pkg-config or other -config scripts, most of which will reintroduce -I/opt/local/include into CPPFLAGS and thus reintroduce the problem. Using -isystem/opt/local/include in CPPFLAGS, as my patch did, supersedes any -I/opt/local/include that may also be specified.

[jmroot (Joshua Root)]

Replying to ryandesign@…:

I have been using this patch for months without problems. This problem you've mentioned with swig is the first problem I've heard of. Could you be more specific? Could we fix that problem instead?

As per the link, lal fails to build.

Let’s be clear: it is a bug when a build system passes externally-supplied CPPFLAGS to the compiler before its own internal include paths. IMO we should be working with upstream to fix such bugs, not applying a workaround globally. Even in cases where there is no upstream, just having a patch is more valuable to others in the larger community who encounter the same problem building the software, compared to having an invisible workaround.

But it appears that I’m outvoted, so I’ll go along with reintroducing the change into trunk provided the problem with swig is fixed, and no other problems turn up.

[DanEble]

Greetings. I'm speaking as a developer of GNU LilyPond investigating our ​issue 6135, which is an error while compiling the MacPorts port.

From reading the build log provided to me, I believe that the main issue is that MacPorts sets up the build to search /opt/local/include with -I rather than -isystem. (Note that I am not currently a MacPorts user and I have not tried to reproduce the issue myself.)

Using -I makes headers in that directory subject to the same compiler warnings as project-specific headers, which is undesirable in my view. There are warnings (one of which is currently treated as an error) that we benefit from enabling for LilyPond headers, but which we do not want to to enable for external headers. In our own build scripts, we distinguish external headers with -isystem.

I would like to find out whether MacPorts developers are willing to address this issue in MacPorts the near future. Seeing that the previous comment is seven years old, I guess that this issue has either been forgotten or that there is no real interest in ever doing it. Which one is it?

Thanks and regards.

[kencu (Ken)]

Josh adding you for comment.

Do you think the entire tree of MacPorts headers should all be declared as "system" headers?

IIRC, that would change them all to being included last rather than first, which could be interesting given 25,000 ports that haven't been built that way...