Bug 562164 – net-wireless/inspectrum does not respect CFLAGS (adds -O3)

• Product: Gentoo Linux
• Status: RESOLVED FIXED

[Description Julian Ospald 2015-10-03 22:26:37 UTC]

Created attachment 413632 [details]
1443911124-install-net-wireless_inspectrum-0.1:0::gentoo.out

[2/10] /usr/bin/clang++  -DQT_CORE_LIB -DQT_GUI_LIB -DQT_NO_DEBUG -DQT_WIDGETS_LIB  -DNDEBUG -march=core-avx2 -O2 -pipe -Wall -Qunused-arguments -fcolor-diagnostics  -std=gnu++11 -O3 -I. -I/var/tmp/paludis/net-wireless-inspectrum-0.1/work/inspectrum-0.1 -isystem /usr/include/qt5 -isystem /usr/include/qt5/QtWidgets -isystem /usr/include/qt5/QtGui -isystem /usr/include/qt5/QtCore -isystem /usr/lib64/qt5/mkspecs/linux-g++    -fPIC -MMD -MT CMakeFiles/inspectrum.dir/main.cpp.o -MF CMakeFiles/inspectrum.dir/main.cpp.o.d -o CMakeFiles/inspectrum.dir/main.cpp.o -c /var/tmp/paludis/net-wireless-inspectrum-0.1/work/inspectrum-0.1/main.cpp


https://github.com/miek/inspectrum/blob/master/CMakeLists.txt#L20

[Comment 1 Julian Ospald 2015-10-03 22:27:44 UTC]

Created attachment 413634 [details]
cave-info.txt

[Comment 2 Rick Farina (Zero_Chaos) 2015-10-04 15:06:01 UTC]

nice catch, thanks.  I'll send a patch upstream.

[Comment 3 Rick Farina (Zero_Chaos) 2015-10-05 15:05:50 UTC]

https://github.com/miek/inspectrum/commit/75e1049c6aea72916f97c89b90ac0319d700cb6e

is this good enough?  I don't really understand cmake well enough to easily do better, but this makes the -O3 overridable (yet default) which meets upstream's intention.

[Comment 4 Julian Ospald 2015-10-05 15:08:31 UTC]

IMO, it should be completely removed

custom CFLAGS/CXXFLAGS can easily be set and even cached in cmake (e.g. via ccmake). Only flags that are _required_ to build the program should be hardcoded.

There's really no good reason to hardcode optimziation flags at all, _especially_ if you do development.

[Comment 5 Rick Farina (Zero_Chaos) 2015-10-05 16:46:04 UTC]

(In reply to Julian Ospald (hasufell) from comment #4)
> IMO, it should be completely removed
> 
> custom CFLAGS/CXXFLAGS can easily be set and even cached in cmake (e.g. via
> ccmake). Only flags that are _required_ to build the program should be
> hardcoded.
> 
> There's really no good reason to hardcode optimziation flags at all,
> _especially_ if you do development.

While I agree with your opinion, do you see any actual harm in upstream setting an optimization default that gets overridden like this?

[Comment 6 Rick Farina (Zero_Chaos) 2015-10-05 17:57:09 UTC]

backported upstream's slightly more complete fix. thanks for the report.

[Comment 7 Julian Ospald 2015-10-05 19:10:42 UTC]

(In reply to Rick Farina (Zero_Chaos) from comment #5)
> (In reply to Julian Ospald (hasufell) from comment #4)
> > IMO, it should be completely removed
> > 
> > custom CFLAGS/CXXFLAGS can easily be set and even cached in cmake (e.g. via
> > ccmake). Only flags that are _required_ to build the program should be
> > hardcoded.
> > 
> > There's really no good reason to hardcode optimziation flags at all,
> > _especially_ if you do development.
> 
> While I agree with your opinion, do you see any actual harm in upstream
> setting an optimization default that gets overridden like this?

hmm, you could argue that it is unexpected behavior if the user has no -O* flag in his CFLAGS whatsoever, but then again that's rather a minor use case and we probably can expect people to do -O0 if they don't want any optimization

[Comment 8 Rick Farina (Zero_Chaos) 2015-10-05 21:16:33 UTC]

(In reply to Julian Ospald (hasufell) from comment #7)
> hmm, you could argue that it is unexpected behavior if the user has no -O*
> flag in his CFLAGS whatsoever, but then again that's rather a minor use case
> and we probably can expect people to do -O0 if they don't want any
> optimization

that was my assumption as well.  numerous autotools packages behave in this way as well.  I will leave it like this and if someone complains we can care then.