search

free-audio / clap

Audio Plugin API
https://cleveraudio.org/
MIT License
1.76k stars 97 forks source link

Build compatibility with older C/C++/cmake tools #273

Closed sadko4u closed 1 year ago

sadko4u commented 1 year ago

System: 

cat /etc/os-release
NAME="openSUSE Leap"
VERSION="15.4"
ID="opensuse-leap"
ID_LIKE="suse opensuse"
VERSION_ID="15.4"
PRETTY_NAME="openSUSE Leap 15.4"
ANSI_COLOR="0;32"
CPE_NAME="cpe:/o:opensuse:leap:15.4"
BUG_REPORT_URL="https://bugs.opensuse.org"
HOME_URL="https://www.opensuse.org/"
DOCUMENTATION_URL="https://en.opensuse.org/Portal:Leap"
LOGO="distributor-logo-Leap"

Command: cmake .. -G Ninja -DCLAP_BUILD_TESTS=ON

Actual output:

-- CLAP version: 1.1.6
-- The C compiler identification is GNU 7.5.0
-- The CXX compiler identification is GNU 7.5.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Including CLAP tests, compile tests, and versions
-- Configuring done
CMake Error at CMakeLists.txt:55 (add_executable):
  C_STANDARD is set to invalid value '17'
Call Stack (most recent call first):
  CMakeLists.txt:75 (clap_compile_cpp)

GCC version:

gcc --version
gcc (SUSE Linux) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

CMAKE version:

cmake --version
cmake version 3.20.4

CMake suite maintained and supported by Kitware (kitware.com/cmake).
baconpaul commented 1 year ago

Well I suppose, waggishly, I could say that the test accurately shows that the 17 standard does indeed not compile on your system, so it has worked? Chuckle.

The more correct fix is to have https://github.com/free-audio/clap/blob/d6cbd4fd6540e80be816994f6f6bccc7933998ef/CMakeLists.txt#L75 that line either knock out those tests for older gcc or to do a pre-build test where it tries to compile with those flags using a can compile cmake check before it adds the target.

baconpaul commented 1 year ago

Oh I actually diagnosed it slightly incorrectly

https://cmake.org/cmake/help/latest/prop_tgt/C_STANDARD.html

the C_STANDARD 17 only is available as of CMake 3.21 so in this case we need to check CMAKE_VERSION there not compiler version.

baconpaul commented 1 year ago

So specifically

if(${CMAKE_VERSION} VERSION_LESS "3.21") 
    message(STATUS "Skipping C17 tests due to older CMAKE_VERSION ${CMAKE_VERSION}")
else()
   clap_compile_cpp(c17    c 17 17)
endif()

but i don't have any way to test that to see if it works.

abique commented 1 year ago

@sadko4u please can you test?

sadko4u commented 1 year ago

Verified. Yes, that worked. At least, CMAKE didn't give any error message.

baconpaul commented 1 year ago

Great. @abique need me to make a PR from it or can you get it with the other 1.1.7 changes?

baconpaul commented 1 year ago

Tossed in the change as PR #275 @abique