support user's CFLAGS/CXXFLAGS

[marcindulak]

The issue arose when packaging hyperscan for Fedora/EPEL:

"In lines 179-244, this CMakeList.txt implements a scheme which manipulates and overrides CFLAGS/CXXFLAGS to values which are incompatible to Fedora (== this package is being miscompiled). This alone qualifies this package as non-eligible for Fedora, unless this issue can be worked around"

https://bugzilla.redhat.com/show_bug.cgi?id=1372866

[jmtaylor90]

During the testing for the 4.4.0 a scratch build[0] reports:

CMake Warning:
Manually-specified variables were not used by the project:
CMAKE_CXX_FLAGS_RELEASE
CMAKE_C_FLAGS_RELEASE
CMAKE_Fortran_FLAGS_RELEASE
CMAKE_INSTALL_LIBDIR
INCLUDE_INSTALL_DIR
LIB_INSTALL_DIR
LIB_SUFFIX
SHARE_INSTALL_PREFIX
SYSCONF_INSTALL_DIR

As Marcin noted, it is necessary for the fedora build system to be able to specify/override the defaults.

[0] https://kojipkgs.fedoraproject.org//work/tasks/3020/18093020/build.log

[mdb256]

The short answer is: I can see we have some work to do in handling install locations and lib names, so I'll look at that.

The longer answer adds:

Some of the comments in that bugzilla entry relate to v4.3 - we did have some problems with our handling of CFLAGS/CXXFLAGS that are corrected in v4.4. The commit where I undid the flag ignoring/clobbering is 2e5a2ab

However I think the misunderstanding is partly because Hyperscan is x86-specific. There is no support for other architectures, so the comment about generality and portability is not relevant.

We are also opinionated about our use of optimisation flags, because Hyperscan is a high-performance library, so the extra performance we get from -O3 over -O2 really does matter (we do a lot of performance testing). This is only applied to the runtime side of Hyperscan - the C++ compile-side is built with -O2.

[marcindulak]

The need of custom CFLAGS/CXXFLAGS comes from the campaign for build hardening in the distributions.
https://bugzilla.redhat.com/show_bug.cgi?id=1387475
https://wiki.debian.org/Hardening

The choice of optimization level is left to the packager, or more precisely:

"Overriding these flags for performance optimizations (for instance, -O3 instead of -O2) is generally discouraged. If you can present benchmarks that show a significant speedup for this particular code, this could be revisited on a case-by-case basis. Adding to and overriding or filtering parts of these flags is permitted if there's a good reason to do so; the rationale for doing so must be documented in the specfile."

https://fedoraproject.org/wiki/Packaging:Guidelines#Compiler_flags

[jmtaylor90]

Thanks for the quick response and taking a look @mdb256

[mdb256]

It seems fixing the install locations is a pretty simple change - I've created this commit which will apply against v4.4.1: mdb256@a81865f

I think it also helps Debian multilib packaging as well. Comments welcome of course.

We will be able to add these changes into the next release, but for v4.4.1 it should be usable as a patch in the rpmbuild process.

[jmtaylor90]

Thanks @mdb256, I created/applied the patch and things look good from my end. I have updated the BZ referenced above and requested a revisit of the issues noted.

JT

[mdb256]

That fix was in ce50ff6 which is in v4.5.0