Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a minimum version for cmake #22

Merged
merged 1 commit into from Nov 11, 2017
Merged

Conversation

@david-moran
Copy link
Contributor

@david-moran david-moran commented Nov 11, 2017

cmake tool requires CMakeLists.txt to define a minimum version since its 2.6 version:

CMake Warning (dev) in CMakeLists.txt:
No cmake_minimum_required command is present. A line of code such as

cmake_minimum_required(VERSION 3.9)

should be added at the top of the file. The version specified may be lower
if you wish to support older CMake versions for this project. For more
information run "cmake --help-policy CMP0000".

if you set cmake_minimum_required to the first supported version you will get following error:

CMake Error at facil.io/CMakeLists.txt:47 (target_link_libraries):
The keyword signature for target_link_libraries has already been used with
the target "facil.io". All uses of target_link_libraries with a target
must be either all-keyword or all-plain.

The uses of the keyword signature are here:

  • facil.io/CMakeLists.txt:46 (target_link_libraries)

So you need to merge two target_link_libraries.

All steps above are needed because if you try to import facil.io from other cmake project using add_subdirectory, and that project have set the required version you will get the same errors:

[david@fedora facil.io-test]$ ls
CMakeLists.txt main.c
[david@fedora facil.io-test]$ cat CMakeLists.txt
project (faciliotest C)
cmake_minimum_required(VERSION 3.9)

add_subdirectory(facil.io)

add_executable(faciliotest main.c)

include_directories(facil.io/libdump/include)
target_link_libraries(faciliotest facil.io pthread)

[david@fedora facil.io-test]$ git clone https://github.com/boazsegev/facil.io.git
Cloning into 'facil.io'...
remote: Counting objects: 6201, done.
remote: Compressing objects: 100% (128/128), done.
remote: Total 6201 (delta 158), reused 206 (delta 114), pack-reused 5934
Receiving objects: 100% (6201/6201), 1.91 MiB | 3.65 MiB/s, done.
Resolving deltas: 100% (4378/4378), done.
[david@fedora facil.io-test]$ mkdir build
[david@fedora facil.io-test]$ cd build
[david@fedora build]$ cmake ..
-- The C compiler identification is GNU 7.2.1
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Detecting C compile features
-- Detecting C compile features - done
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE
CMake Error at facil.io/CMakeLists.txt:47 (target_link_libraries):
The keyword signature for target_link_libraries has already been used with
the target "facil.io". All uses of target_link_libraries with a target
must be either all-keyword or all-plain.

The uses of the keyword signature are here:

  • facil.io/CMakeLists.txt:46 (target_link_libraries)

-- Configuring incomplete, errors occurred!
See also "/home/david/Projects/facil.io-test/build/CMakeFiles/CMakeOutput.log".
See also "/home/david/Projects/facil.io-test/build/CMakeFiles/CMakeError.log".
[david@fedora build]$

@boazsegev
Copy link
Owner

@boazsegev boazsegev commented Nov 11, 2017

Hi @david-moran,

Thank you very much for this PR!!! 🎉👏🏻👏🏻🎉

I don't use CMake, so I really don't encounter these issues.

However, I should point out that the CMakeLists.txt file is automatically generated by the project's makefile.

I will add these changes to the generator script, but I would appreciate if you could look it over and make sure it works as expected.

Thanks!
B.

@boazsegev boazsegev merged commit a67a211 into boazsegev:master Nov 11, 2017
1 check passed
1 check passed
codacy/pr Good work! A positive pull request.
Details
@david-moran
Copy link
Contributor Author

@david-moran david-moran commented Nov 11, 2017

👍 Yes, everything it's working fine!!

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.