[MacPorts] #40656: Use -isystem instead of -I in default configure.cppflags
#40656: Use -isystem instead of -I in default configure.cppflags --------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: defect | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Keywords: haspatch | Port: --------------------------+-------------------------------- We should [https://lists.macosforge.org/pipermail/macports- dev/2013-October/024536.html consider changing the default configure.cppflags] from "`-I`" to "`-isystem`". [http://gcc.gnu.org/onlinedocs/gcc/Directory-Options.html#Directory- Options -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. -- Ticket URL: <https://trac.macports.org/ticket/40656> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Changes (by ryandesign@…): * type: defect => enhancement -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:1> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): 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 [http://public.kitware.com/Bug/view.php?id=10837#c27206 -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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:2> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by raimue@…): 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.gnu.org/onlinedocs/gcc/Environment-Variables.html gcc manual]). However, older versions of clang do not honor this setting. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:4> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Right; MacPorts already sets `CPATH`. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:6> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: new Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Replying to [comment:2 ryandesign@…]:
However I found a mention that [http://public.kitware.com/Bug/view.php?id=10837#c27206 -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 [comment:4 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 ([http://gcc.gnu.org/onlinedocs/gcc/Environment-Variables.html 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 my any reason why it couldn't, 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). -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:10> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Changes (by ryandesign@…): * status: new => closed * resolution: => fixed Comment: r115978 -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:11> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by jmr@…): 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:12> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:13> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by raimue@…): 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:14> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Replying to [comment:14 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:15> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by 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? -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:16> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: closed Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: fixed | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by ryandesign@…): Replying to [comment:16 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:17> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: reopened Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Changes (by ryandesign@…): * status: closed => reopened * resolution: fixed => -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:18> MacPorts <http://www.macports.org/> Ports system for OS X
#40656: Use -isystem instead of -I in default configure.cppflags ---------------------------+-------------------------------- Reporter: ryandesign@… | Owner: macports-tickets@… Type: enhancement | Status: reopened Priority: Normal | Milestone: MacPorts Future Component: base | Version: 2.2.0 Resolution: | Keywords: haspatch Port: | ---------------------------+-------------------------------- Comment (by jmr@…): Replying to [comment:17 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. -- Ticket URL: <https://trac.macports.org/ticket/40656#comment:19> MacPorts <http://www.macports.org/> Ports system for OS X
participants (1)
-
MacPorts