Opened 5 years ago

Last modified 8 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: Cc: ryandesign (Ryan 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 (22)

comment:1 in reply to:  description Changed 5 years ago by ryandesign (Ryan 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 5 years ago by cooljeanius (Eric Gallager)

Cc: egall@… added

Cc Me!

comment:3 Changed 5 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 5 years ago by anddam (Andrea D'Amore)

Cc: and.damore@… added

Cc Me!

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

Description: modified (diff)
Port: cmake removed

comment:6 Changed 5 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 5 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 5 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 5 years ago by ChristianFrisson (Christian Frisson)

Cc: christian.frisson@… added

Cc Me!

comment:10 in reply to:  7 Changed 5 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 5 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 5 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 5 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 5 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 5 years ago by mojca (Mojca Miklavec)

Port: clhep removed

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

Cc: michaelld@… added

Cc Me!

comment:17 Changed 5 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 5 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 5 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, r121164
  • 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
Last edited 5 years ago by mojca (Mojca Miklavec) (previous) (diff)

comment:20 Changed 5 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 5 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 8 months 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

Note: See TracTickets for help on using tickets.