Opened 10 years ago

Last modified 19 months ago

#42872 new defect

cmake PortGroup: don't add -I${prefix}/include to CXXFLAGS

Reported by: mojca (Mojca Miklavec) Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version:
Keywords: portgroup Cc: ryandesign (Ryan Carsten Schmidt), cssdev, cooljeanius (Eric Gallager), anddam (Andrea D'Amore), ChristianFrisson (Christian Frisson), michaelld (Michael Dickens)
Port:

Description (last modified by anddam (Andrea D'Amore))

The configure phase of a port using cmake portgroup is automatically fed with too many (too early) includes of $prefix/include:

CPPFLAGS='-I/opt/local/include'
CFLAGS='-pipe -Os -I/opt/local/include -arch x86_64'
CXXFLAGS='-pipe -Os -I/opt/local/include -arch x86_64'

with the following explanation in the PortGroup:

# The environment variable CPPFLAGS is not considered by CMake.
# (CMake upstream ticket #12928 "CMake silently ignores CPPFLAGS"
# <http://www.cmake.org/Bug/view.php?id=12928>).
# Thus, we have to add them manually to the CFLAGS and CXXFLAGS in the
# pre-configure phase.
if {${configure.cppflags} ne ""} {
    configure.cflags-append ${configure.cppflags}
    configure.cxxflags-append ${configure.cppflags}
}

As a consequence the CMake-based installations have problems when function definitions change in an upgrade. I don't know if I should blame the developers for that or not. (In case of CLHEP, the ./configure-based installation puts -I. in front of CPPFLAGS, so headers from sources are found first, but in CMake-based installation -I. comes later.)

We should check whether we could use CMAKE_INCLUDE_PATH (-DCMAKE_INCLUDE_PATH=${prefix}/include) instead of configure.cxxflags-append ${configure.cppflags} for example.

See also:

Change History (23)

comment:1 in reply to:  description Changed 10 years ago by ryandesign (Ryan Carsten Schmidt)

Replying to mojca@…:

According to that, we could just set CMAKE_PREFIX_PATH=${prefix}, to take care of libraries, includes and binaries. You could make that change to the portgroup locally and see whether it solves the problem, and also test some other cmake-portgroup-using ports to make sure they still build ok.

comment:2 Changed 10 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:3 Changed 10 years ago by ChristianFrisson (Christian Frisson)

Hi,

This affects Port vxl too, as explained in ticket #41241. Removing the cppflags appends fixes one of the issues, works in this case even without setting the CMAKE_PREFIX_PATH as proposed above.

Best regards, Christian

comment:4 Changed 10 years ago by anddam (Andrea D'Amore)

Cc: and.damore@… added

Cc Me!

comment:5 Changed 10 years ago by anddam (Andrea D'Amore)

Description: modified (diff)
Port: cmake removed

comment:6 Changed 10 years ago by anddam (Andrea D'Amore)

I'm building neovim using mp-provided libraries and I have -I$prefix before any other includes arg, this lead to error due to what clang interpret as implicit declaration. Is the position of -I$prefix in the makefile generated by cmake due to the issue described in this ticket?

comment:7 Changed 10 years ago by mojca (Mojca Miklavec)

Eeeeem. Why did you change the port from clhep cmake to clhep. Did you want to change it to cmake and removed the wrong one by accident? Or do you want to suggest that this isn't CMake's PortGroup problem at all and that ports should be fixed instead?

comment:8 Changed 10 years ago by mojca (Mojca Miklavec)

The position of -I$prefix comes so early in the Makefile because of $C(XX)FLAGS. The author of software has some influence on that order and could in principle make sure that other paths come in front. But on the other hand it's also somewhat true that one could add the include path to other variables instead of $CXXFLAGS.

comment:9 Changed 10 years ago by ChristianFrisson (Christian Frisson)

Cc: christian.frisson@… added

Cc Me!

comment:10 in reply to:  7 Changed 10 years ago by anddam (Andrea D'Amore)

Replying to mojca@…:

Eeeeem. Why did you change the port from clhep cmake to clhep. Did you want to change it to cmake and removed the wrong one by accident? Or do you want to suggest that this isn't CMake's PortGroup problem at all and that ports should be fixed instead?

No what I meant is that this ticket isn't about cmake port, but about cmake portgroup.

The cmake port is the one responsible for installing cmake while the cmake portgroup is a collection of functions to be used in other ports to leverage cmake features.

The issue you're mentioning happens in the latter ones so it occurs to me that putting cmake in the ticket's "port" field is wrong.

comment:11 Changed 10 years ago by mojca (Mojca Miklavec)

Yes, I'm aware of the difference between the port and the PortGroup, but cmake is the closest tag / description one can use. Unless we introduce a convention of how to tag the tickets related to PortGroups (say, cmake-1.0.tcl or group_cmake or maybe even changing the category from "ports" to "groups"), using 'cmake' in the port field is the most reasonable approach I can think of. After all, the maintainer of the port should to some extent also try to address tickets closely related to the PortGroup, so using the closest relative doesn't seem too problematic to me.

If we remove the tag cmake, please suggest another tag. Using clhep in the port field is just as wrong (or even more wrong).

comment:12 in reply to:  11 Changed 10 years ago by anddam (Andrea D'Amore)

Replying to mojca@…:

but cmake is the closest tag / description one can use.

The port field is not mandatory, just think of base-related tickets. I suggest to just leave it blank and remove clhep as well since the issue is not specific to that.

The ticket is already very well discoverable by its summary when searching cmake in ticket port & summary that is the most strict kind of query in generic search page.

After all, the maintainer of the port should to some extent also try to address tickets closely related to the PortGroup, so using the closest relative doesn't seem too problematic to me.

That's not necessarily true, the cmake maintainer could be not interested at all in following the ports that use cmake portgroup.

The problem with using a wrong port field value just because it's "the best guess" is that the information is misleading, I found this ticket while I was searching for an actual port:cmake issue so I tried to make the ticket more correct.

comment:13 Changed 10 years ago by mojca (Mojca Miklavec)

Another victim requiring

configure.cppflags-delete -I${prefix}/include

seems to be wxLua.

Maybe we can continue passing other cppflags to cxxflags, but it seems that -I${prefix}/include really needs to go.

comment:14 Changed 10 years ago by neverpanic (Clemens Lang)

r118762 is another instance of this. Can we please get the offending line removed ASAP? CMAKE_SYSTEM_PREFIX_PATH should already be enough to specify the required includes.

comment:15 Changed 10 years ago by mojca (Mojca Miklavec)

Port: clhep removed

comment:16 Changed 10 years ago by michaelld (Michael Dickens)

Cc: michaelld@… added

Cc Me!

comment:17 Changed 10 years ago by mojca (Mojca Miklavec)

Is the following OK?

  • cmake-1.0.tcl

     
    8181    # The environment variable CPPFLAGS is not considered by CMake.
    8282    # (CMake upstream ticket #12928 "CMake silently ignores CPPFLAGS"
    8383    # <http://www.cmake.org/Bug/view.php?id=12928>).
    84     # Thus, we have to add them manually to the CFLAGS and CXXFLAGS in the
    85     # pre-configure phase.
    86     if {${configure.cppflags} ne ""} {
    87         configure.cflags-append ${configure.cppflags}
    88         configure.cxxflags-append ${configure.cppflags}
    89     }
     84    #
     85    # But adding -I${prefix}/include to CFLAGS/CXXFLAGS is a bad idea.
     86    # If any other flags are needed, we need to add them.
    9087
    9188    # In addition, CMake provides build-type-specific flags for
    9289    # Release (-O3 -DNDEBUG), Debug (-g), MinSizeRel (-Os -DNDEBUG), and

comment:18 Changed 10 years ago by mojca (Mojca Miklavec)

I committed the patch in r121112. I'll go ahead and remove workarounds from ports that tried to overcome the problem by removing the flag manually.

comment:19 Changed 10 years ago by mojca (Mojca Miklavec)

List of ports that likely removed cppflags because of this problem:

  • _resources/port1.0/group/kde4-1.1.tcl
  • databases/mysql*
  • devel/libssh
  • devel/LucenePlusPlus
  • graphics/glfw, r121154
  • graphics/graphite2
  • graphics/hugin-app, r121163
  • graphics/openjpeg
  • graphics/vtk5
  • graphics/wxLua
  • print/libLASi
  • science/bladeRF, r121144
  • science/clhep, r121114
  • science/gnuradio, r121146
  • science/gr-baz, r121150
  • science/gr-fcdproplus, r121148
  • science/gr-fosphor, r121147
  • science/gr-iqbalance, r121149
  • science/gr-osmosdr, r121143
  • science/gr-rds, r121146
  • science/hackrf, r121151
  • science/root6, r121122
  • science/rtl-sdr, r121153
  • science/uhd, r121152
Version 5, edited 10 years ago by mojca (Mojca Miklavec) (previous) (next) (diff)

comment:20 Changed 10 years ago by michaelld (Michael Dickens)

I'll remove them from my ports: graphics/glfw and science/{gnuradio, gr-*, hackrf, rtl-sdr, uhd}.

comment:21 Changed 10 years ago by michaelld (Michael Dickens)

Done: gr-osmosdr in r121143. bladeRF in r121144. gnuradio* in r121146. gr-rds in r121146. gr-fosphor in r121147. gr-fcdproplus in r121148. gr-iqbalance in r121149. gr-baz in r121150. hackrf in r121151. uhd* in r121152. rtl-sdr in r121153. glfw in r121154. I think that's all of my ports.

comment:22 Changed 5 years ago by dgilman (David Gilman)

In 88eb4637a61a79a8bfc27d2d019179aa67d50293/macports-ports (master):

graphite2: Update to 1.3.12

  • Use github 1.0 portgroup
  • Use cxx11 1.1 portgroup; 1.3.12 and later require C++11
  • Update to cmake 1.1 portgroup
  • Remove patch-src-Face.cpp.diff as it builds without this
  • Re-enable the featuremap test as it works now
  • Add fonttools and python36 test dependencies
  • Remove test.env as tests pass without it
  • Remove cppflags-replace no longer needed after cmake portgroup fix
  • Remove removal of -fno-stack-protector on Darwin 8 as upstream no longer uses this flag
  • Remove disabling segment caching code on PowerPC as upstream has removed the segment caching code as of 1.3.12
  • Remove muniversal 1.0 portgroup no longer needed due to above

See: #42872

Closes: #54141

comment:23 Changed 19 months ago by mascguy (Christopher Nielsen)

Keywords: portgroup added

Add keyword portgroup, to pg-related tickets

Note: See TracTickets for help on using tickets.