dolik-rce / pegof

PEG grammar optimizer and formatter
Other
9 stars 2 forks source link

Cmake: adding a cmake option to control whether to build and run tests #20

Closed leleliu008 closed 1 month ago

dolik-rce commented 1 month ago

I'm struggling to understand why would you need this? If you don't want to run tests, then just tell cmake to only compile the release binary:

cmake --build <dir> --target pegof

This should do exactly the same thing as if setting ENABLE_TESTS to false.

leleliu008 commented 1 month ago

@dolik-rce

cmake -S . -B build.d
cmake --build build.d
cmake --install build.d

this is the most standardized process to cmake.

dolik-rce commented 1 month ago

Ok, now I see why you want this. But there is much simpler way, just remove pegof_test from the default target:

set_target_properties(pegof_test PROPERTIES EXCLUDE_FROM_ALL 1)

It will not be compiled by default, unless you invoke cmake with --target test or --target test_all.

leleliu008 commented 1 month ago

It's better to build and run test by default.

On musl-based Linux system, tests can not be run due to lack of sanitizer. In this case, we need to allow users to disable running tests.

dolik-rce commented 1 month ago

On musl-based Linux system, tests can not be run due to lack of sanitizer. In this case, we need to allow users to disable running tests.

I've noticed that too, perhaps it would be best to skip the sanitizer options on systems that do not support them. This commit should solve it: 1f501af24417adf985d17bf99eebeba4d4097295.

leleliu008 commented 1 month ago

That's not the proper way to use sanitizers. only compiler driver itself known where to find these libraies usually. for example, clang's location: lib/clang/17.0.2/lib/linux/libclang_rt.asan-aarch64-android.so

dolik-rce commented 1 month ago

My mistake, I have wrongly assumed that find_library checks work differently. I have changed the commit (06e4cd86f489b5b52a81246504f77db62c8be5ea) to use try_compile instead, which should correctly detect if the linker flags are supported for current compiler.

leleliu008 commented 1 month ago

https://cmake.org/cmake/help/latest/command/try_compile.html

try_compile command SOURCE_FROM_CONTENT option was introduced in cmake-3.25, but our request is cmake_minimum_required(VERSION 3.11)

dolik-rce commented 1 month ago

try_compile command SOURCE_FROM_CONTENT option was introduced in cmake-3.25, but our request is cmake_minimum_required(VERSION 3.11)

That's interesting, it seems to work in ubuntu 20.04, even though there is cmake 3.16.3. If it turns out to be a problem, we can always use write_file to create a file and run try_compile on that.

leleliu008 commented 1 month ago
CMake Error: The source directory "check.c" does not exist.
Specify --help for usage, or press the help button on the CMake GUI.
CMake Error at CMakeLists.txt:12 (try_compile):
  Failed to configure test project build system.
Call Stack (most recent call first):
  CMakeLists.txt:26 (check_linker_options)
dolik-rce commented 1 month ago

Where did you get that error? I can fix it, but it would be much easier to know how to test.

leleliu008 commented 1 month ago

https://github.com/dolik-rce/pegof/pull/19 alpine-3.17

dolik-rce commented 1 month ago

CMake Error: The source directory "check.c" does not exist.

Should be fixed now in devel.

leleliu008 commented 1 month ago

Yes, fixed.