cmake portgroup version 1.1

[mkae (Marko Käning)]

... as well as Ryan, of course.

[RJVB (René Bertin)]

Most of these changes have already been presented elsewhere (and for a large part, ratified as useful, just never incorporated). I can't find the tickets with a few quick searches, so here's a bit of background.

First off, I've decided to give my variant a higher version, so both Michael's original and my more elaborate portgroup can co-exist (for the time being).

changes include:

• use CMAKE_BUILD_TYPE=MacPorts. Inspired by Debian's own build type, this allows us to specify all compiler settings via the well-known configure.* commands and exported via the environment. If one of CMake's predefined types is used the corresponding standard options will be *appended* to our options, which will override notably the optimisation options. Some parsing of configure.cppflags, configure.cflags and configure.cxxflags is done to ensure this works as expected, in lines 145-200 .
• add CMAKE_EXPORT_COMPILE_COMMANDS so that each compiler command is saved to an additional compile_commands.json file. Completely transparent for most purposes, but very useful if you want to open the source and existing build directory in KDevelop (which then won't rerun cmake with default settings).
• cmake.install_rpath : a convenience function to construct CMAKE_INSTALL_RPATH in steps, working around the fact that cmake doesn't support incremental commandline variables.
• lines 204-243 : ensure there's a compile_commands.json file, and generate a .macports.${subport}.configure.cmd file that summarises the configure details, as a more permanent reference than the information available through the logfile. There's been some discussion of adding this to "base", if that happens this feature will evidently disappear or be refactored.

[mkae (Marko Käning)]

Adding two cmake'ish devs for review.

[mkae (Marko Käning)]

As well as Ryan, of course.

[michaelld (Michael Dickens)]

Does this fix address ticket #37732 ("provide option to disable CMAKE_OSX_ARCHITECTURES")? That would be really useful ...

[RJVB (René Bertin)]

I don't think so, no, because I never had to deal with this issue myself.

It's unclear to me if there has been a proposal how to fix this in #37732. It seems the fix would have to be in CMake. But if it is a sufficient and acceptable workaround NOT to set CMAKE_OSX_ARCHITECTURES in the portgroup then that shouldn't be hard to implement.

Edit: as long as it's a flag to turn off setting the thing altogether, without compiler lookups

[mojca (Mojca Miklavec)]

That ticket is only asking for an option in the PortGroup, not for a fix in CMake. Sure, it would be nice if the bug in CMake was fixed, so that the flags would not proliferate to Fortran, but that's a different issue.

[RJVB (René Bertin)]

Yeah, well, we usually aim for fixes, not workarounds, no? :)

As I said, it's easy enough to introduce such an option, in a way that nothing changes for most ports.

I remembered there was a project of making out-of-source builds the default, so I applied that too.

Edit: maybe we should NOT disable setting CMAKE_OSX_ARCHITECTURES in non-universal builds?

[michaelld (Michael Dickens)]

Cool. I'm all for creating a new cmake portgroup; this looks OK to me. Why don't we go ahead & get it in place, to allow cmake ports to test it out -- maybe add that this is a "work in progress for testing only" or something like that. Leave this ticket open & get feedback to tweak / correct any issues that come up. I like the changes implemented in this version, and if it works around a cmake issue, even better (thx, btw; I wasn't asking for you to do this fix, just enquiring if it did so).

[RJVB (René Bertin)]

Great :)

Did you see my edit? Should we leave the CMAKE_OSX_ARCHITECTURES when a non-universal build is being done, or is it fine if the option disables all occasions? If you don't know the answer either that's fine too, it will be trivial to undo this part of the change should the need arise.

Marko, it seems you can go ahead and commit this. Once that's done we'll send out a message on macports-devel telling everyone about the immediate availability of this new portgroup version.

[mojca (Mojca Miklavec)]

On the contrary. Unless I'm mistaken (I didn't check) CMAKE_OSX_ARCHITECTURES is actually needed to build universally, so it should stay.

I would only disable CMAKE_OSX_ARCHITECTURES on request. From what I know this is only a problem when Fortran is involved.

[michaelld (Michael Dickens)]

Ditto what Mojca said: disable only on specific request.

[RJVB (René Bertin)]

Which is what the patch does (cmake.set_osx_architectures is true by default; set it to false to disable CMAKE_OSX_ARCHITECTURES).

Let's just leave it at my current implementation then, and thus leave it up to port developers to add the appropriate arch flag(s) for their ports to build correctly. For a moment I had a doubt whether we needed to do something different in the case a port want to build, say, in 32bit on a 64bit host, but there's something to say for making that the port's responsibility.

[mkae (Marko Käning)]

OK, folks, can I commit the current state then?

[mkae (Marko Käning)]

Clicking on the URL underlying the 'Port' data field "cmake" in the ticket header reveals a substantial ticket line for this port.

So, will we ignore all that for now and simply commit this new port group, so that devs can test this new version 1.1?

[larryv (Lawrence Velázquez)]

Please remove the added copyright claim.

[larryv (Lawrence Velázquez)]

Why the weird cmake.install_rpath function? cmake_install_rpath should just be an option so that it has the appending/prepending functionality that all other portfile options have. Then set

default configure.args {-DCMAKE_INSTALL_RPATH="[join ${cmake_install_rpath} ;]"}

to construct the path on the fly.

[mkae (Marko Käning)]

Replying to larryv:

Please remove the added copyright claim.

Hmm, that’s interesting. Why would one not be allowed to append one's copyright, if one were the one who worked it out?

[larryv (Lawrence Velázquez)]

It’s uncommon for individual contributors to add copyright attribution for additions this small.

[larryv (Lawrence Velázquez)]

Replying to RJVB:

changes include:

• use CMAKE_BUILD_TYPE=MacPorts. Inspired by Debian's own build type, this allows us to specify all compiler settings via the well-known configure.* commands and exported via the environment. If one of CMake's predefined types is used the corresponding standard options will be *appended* to our options, which will override notably the optimisation options. Some parsing of configure.cppflags, configure.cflags and configure.cxxflags is done to ensure this works as expected, in lines 145-200 .

This parsing code is very fragile. There’s almost certainly a better approach. Can you explain again why it is necessary at all? I am not very familiar with CMake.

• lines 204-243 : ensure there's a compile_commands.json file, and generate a .macports.${subport}.configure.cmd file that summarises the configure details, as a more permanent reference than the information available through the logfile. There's been some discussion of adding this to "base", if that happens this feature will evidently disappear or be refactored.

Note that base augments several options before setting up the environment variables. For example, this:

puts ${fd} "CXXFLAGS=\"${configure.cxxflags}\""

will not include any -isysroot or -march flags that are added by base.

[RJVB (René Bertin)]

This parsing code is very fragile. There’s almost certainly a better approach. Can you explain again why it is necessary at all? I am not very familiar with CMake.

I've been using this code for months now, and never had issues with it. You're probably right it's fragile but in practice it seems to work well enough, and it shouldn't drop any options which is probably the most important part.

You have much of the explanation right in the text you quoted. CMake defines 4 build types that each come with their own set of compiler options. Options set in the environment (CFLAGS, CXXFLAGS, etc) are not ignored, but they are prepended to the options CMake imposes. So if a port or user wants to build with -O3 but the build type wants -O2, you get -O3 -O2 which equals -O2. It's tricky to override this, unless you specify a custom build type which evidently doesn't come with predefined compiler options. Then you get only the options you set in the environment.

The brunt of the parsing however concerns configure.cppflags. This is done because CMake doesn't have an equivalent, it only has an option to define the include path (which has to be done in one go, not in incremental steps). So to be sure that all specified include paths are known to CMake this parser rewrites configure.cppflags so that it contains only the -I/foo and -I /foo options. All other options are moved to configure.*cflags and configure.*cxxflags.

Note that base augments several options before setting up the environment variables.

Meaning I should output $::env(CXXFLAGS), or does it not yet have its final value in the pre-configure? Not that this is crucial, but it'd be nice to get it right.

[RJVB (René Bertin)]

(I'm having difficulties finding the right button to submit comments, so here's a reply that was still not posted :-/)

I found my additions significant enough to bother signing them.

Replying to larryv:

Why the weird cmake.install_rpath function?

Define weird?! I have never found any documentation how to create custom variables with -append, -prepend etc. functionality, so I rolled my own implementation. I'm sorry (for you) if you find that weird...

cmake_install_rpath should just be an option so that it has the appending/prepending functionality that all other portfile options have. Then set

default configure.args {-DCMAKE_INSTALL_RPATH="[join ${cmake_install_rpath} ;]"}

to construct the path on the fly.

I must be doing something weird^H\^H\^H\^Hrong then:

options cmake.out_of_source cmake.build_dir cmake.set_osx_architectures cmake_install_rpath
#snip
default cmake_install_rpath ${prefix}/lib

default configure.args {-DCMAKE_INSTALL_RPATH="[join ${cmake_install_rpath} ;]"}

configure.args-append \
                    -DCMAKE_VERBOSE_MAKEFILE=ON \
                    -DCMAKE_COLOR_MAKEFILE=ON \
                    -DCMAKE_BUILD_TYPE=MacPorts \
                    -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON \
                    -DCMAKE_INSTALL_NAME_DIR=${prefix}/lib \
                    -DCMAKE_SYSTEM_PREFIX_PATH="${prefix}\;/usr" \
                    -DCMAKE_MODULE_PATH=${cmake_share_module_dir} \
                    -DCMAKE_FIND_FRAMEWORK=LAST \
                    -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
                    -Wno-dev

in a test Portfile:

PortGroup cmake 1.1
#snip
cmake_install_rpath-prepend ${qt_libs_dir}
ui_msg "cmake_install_rpath=${cmake_install_rpath}"
#snip
configure.args-append -Dfoo
#snip
ui_msg "configure.args=${configure.args}"

prints

cmake_install_rpath=/opt/local/libexec/qt5/lib /opt/local/lib
configure.args=-DCMAKE_INSTALL_RPATH="/opt/local/lib" -DCMAKE_VERBOSE_MAKEFILE=ON -DCMAKE_COLOR_MAKEFILE=ON -DCMAKE_BUILD_TYPE=MacPorts -DCMAKE_BUILD_WITH_INSTALL_RPATH=ON -DCMAKE_INSTALL_NAME_DIR=/opt/local/lib {-DCMAKE_SYSTEM_PREFIX_PATH="/opt/local;/usr"} -DCMAKE_MODULE_PATH=/opt/local/share/cmake/Modules -DCMAKE_FIND_FRAMEWORK=LAST -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -Wno-dev -Dfoo

in other words, the -prepend feature works, but the default configure.args statement doesn't. I also don't see how it could work and expand the final value of cmake_install_rpath as a default/initial value for configure.arg when there's no guarantee about the order in which cmake_install_rpath and configure.args get elements appended or prepended.

The only thing I could imagine is to append [join ${cmake_install_rpath}] in a pre-configure block,but even then you cannot be certain that the user will not add one of those too that touches cmake_install_rpath.

[mkae (Marko Käning)]

Above test port file contains cmake_install_rpath.

Did you mean cmake.install_rpath or am I misunderstanding something there?

[mkae (Marko Käning)]

Could you perhaps for testing attach a Portfile for some application which would make use of this port group?

[RJVB (René Bertin)]

Did you mean cmake.install_rpath or am I misunderstanding something there?

Since I got the option to work to satisfaction I gave it the same name as the old function, to be in line with the other options.

QtCurve-qt5 +qtonly uses the feature: ​https://github.com/RJVB/macstrop/tree/master/kde/QtCurve

I have verified that the setting is handed off to CMake correctly, I haven't rebuilt the port with that variant. I have however just tested the feature elsewhere, and it works as intended.

[mkae (Marko Käning)]

Okay, will then commit ASAP. :-)

[mkae (Marko Käning)]

Committed in r154368.

Please try using the new version 1.1 of PortGroup cmake!

I'll leave this ticket open until I get (positive) feedback from some of you.

[mkae (Marko Käning)]

Did anyone test this?

[mkae (Marko Käning)]

OK, I consider this ticket closed now.

[larryv (Lawrence Velázquez)]

Replying to RJVB:

• lines 204-243 : ensure there's a compile_commands.json file, and generate a .macports.${subport}.configure.cmd file that summarises the configure details, as a more permanent reference than the information available through the logfile. There's been some discussion of adding this to "base", if that happens this feature will evidently disappear or be refactored.

I’m not convinced that this should remain. Since it’s created in workpath, it still gets trashed on port clean, so I don’t see how it’s any more permanent than the logfile.