search

prittt / YACCLAB

YACCLAB: Yet Another Connected Components Labeling Benchmark
BSD 3-Clause "New" or "Revised" License
203 stars 37 forks source link

This breaks the build by default on most linux systems and non multilib OSX systems. #19

Closed Steve132 closed 4 years ago

Steve132 commented 4 years ago

https://github.com/prittt/YACCLAB/blob/56f8e739101feacf5384692cad33a361f24512b3/CMakeLists.txt#L83

There's essentially no reason to do this. It would be considered bad practice and broke my build for a good 45 minutes when it failed to compile against my (non-multi-lib headers) and then after I fixed that failed to link to my (non-multilib) toolchain.

If there's nothing specified, then it should default to nothing specified, which means "use the default for the current toolchain". The default for the current system should obviously be the default, and users can manually add it if they need in the CXX_FLAGS config on their own. In addition, there's no way the default should be -m32, because there's a vanishingly small number of 32-bit toolchains in the wild, especially by default.

The best thing to do here is to just delete lines 83-91 entirely. OR at the very least, default to nothing, which is the correct behavior most of the time.

CostantinoGrana commented 4 years ago

Since most of the OpenCV Linux installations we are working with were 32 bit, this was added in, in order to default to 32 bit compilation. We never observed any problem with that.

Anyway, we will remove the lines, so that the default behavior is used.

prittt commented 4 years ago

Done. Thank you for the tip @Steve132.