Add cmake support #31
Conversation
* jmoore_errnofix: Added #include <errno.h> to files that use errno.
|
Would love to see CMake support for this repository. @aclements could this request be accepted? |
|
Just FYI, it's been a long time since I touched anything to do with CMake, so I'm unlikely to be able to do contribute anything additional here. I had totally forgotten about this until I saw the update from a week ago. |
|
Looks like there's another PR that includes cmake - #37 |
|
I wish this PR (or #37) would be accepted. CMake is the most popular tool for managing C++ build process. @aclements, it would be great, if you take a look at this PR. |
| @@ -2,6 +2,8 @@ cmake_minimum_required(VERSION 3.6) | |||
|
|
|||
| project (elfin) | |||
|
|
|||
| set(CMAKE_CXX_STANDARD 17) | |||
|
|
||
| project (elfin) | ||
|
|
||
| set(CMAKE_CXX_STANDARD 17) |
There was a problem hiding this comment.
| set(CMAKE_CXX_STANDARD 17) | |
| set(CMAKE_CXX_STANDARD 11) |
| add_subdirectory(elf elfdir) | ||
| add_subdirectory(dwarf dwarfdir) | ||
| add_subdirectory(examples examplesdir) |
There was a problem hiding this comment.
Adding an immediate subdirectory in the source tree as a different directory in the build directory is not necessary.
Also, optionally building the examples is nice as they are not needed when e.g. only wanting to build the library or adding libelfin as a subdirectory/subproject.
| add_subdirectory(elf elfdir) | |
| add_subdirectory(dwarf dwarfdir) | |
| add_subdirectory(examples examplesdir) | |
| option(BUILD_LIBELFIN_EXAMPLES "Build libelfin examples" ON) | |
| add_subdirectory(elf) | |
| add_subdirectory(dwarf) | |
| if(BUILD_LIBELFIN_EXAMPLES) | |
| add_subdirectory(examples) | |
| endif() |
| set(dwarfH ${CMAKE_CURRENT_LIST_DIR}/dwarf++.hh) | ||
| set(dataH ${CMAKE_CURRENT_LIST_DIR}/data.hh) |
There was a problem hiding this comment.
| set(dwarfH ${CMAKE_CURRENT_LIST_DIR}/dwarf++.hh) | |
| set(dataH ${CMAKE_CURRENT_LIST_DIR}/data.hh) | |
| set(dwarfH ${CMAKE_CURRENT_SOURCE_DIR}/dwarf++.hh) | |
| set(dataH ${CMAKE_CURRENT_SOURCE_DIR}/data.hh) |
| add_library(dwarf dwarf.cc cursor.cc die.cc value.cc abbrev.cc | ||
| expr.cc rangelist.cc line.cc attrs.cc | ||
| die_str_map.cc elf.cc ${to_string_src}) | ||
| target_include_directories(dwarf PUBLIC ${CMAKE_CURRENT_LIST_DIR}) |
There was a problem hiding this comment.
| target_include_directories(dwarf PUBLIC ${CMAKE_CURRENT_LIST_DIR}) | |
| target_include_directories(dwarf PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}) |
|
|
||
| add_custom_command( | ||
| OUTPUT ${to_string_src} | ||
| COMMAND bash ${configured_to_string_script} ${dwarfH} ${dataH} ${to_string_src}) |
There was a problem hiding this comment.
bash is not really cross platform (it is less often available on Windows then python).
I would just use a add_custom_command with all commands from the bash script.
See conan-io/conan-center-index#3283 for an example.
|
|
||
| project(elf) | ||
|
|
||
| set(toStringInputHeader ${CMAKE_CURRENT_LIST_DIR}/data.hh) |
There was a problem hiding this comment.
| set(toStringInputHeader ${CMAKE_CURRENT_LIST_DIR}/data.hh) | |
| set(toStringInputHeader ${CMAKE_CURRENT_SOURCE_DIR}/data.hh) |
| configure_file(create_to_string_code.sh ${configured_to_string_script}) | ||
|
|
||
| add_custom_command( | ||
| OUTPUT ${to_string_src} |
There was a problem hiding this comment.
Same remark as the dwarf library.
I would not use bash but add these commands directly here.
See conan-io/conan-center-index#3283 again
| COMMAND bash ${configured_to_string_script} ${toStringInputHeader} ${to_string_src}) | ||
| add_custom_target(elf_to_string_target DEPENDS ${to_string_src}) | ||
| add_library(elf elf.cc mmap_loader.cc ${to_string_src}) | ||
| add_dependencies(elf elf_to_string_target) |
There was a problem hiding this comment.
This dependency (or add_custom_target) should not be necessary.
As the generated source is added to the sources of the add_library, cmake knows already that the generated source is a dependency of the library and that its source should be generated first.
|
In addition to my remarks, cmake should also be tested on @banshee |
|
No, it's been two years since I touched this and I haven't written anything in CMake for, hmm, can't actually remember. I'm totally happy to do anything to help (that doesn't involve relearning CMake...), but my suggestion would be to take the file you ended up with and make it a new PR. Abandoning this PR would be totally fine with me. |
|
My next suggestion - if you want to put together a better PR that other people could also pull in, I'll happily close this one in favor of yours. Things like "err, bash isn't exactly portable" are 100% true, and I think my original CMake files were pretty much the fastest thing I could get working. Given that it's been here for two years, I suspect it's never going to make it into the main line, but it'd probably be good to only have one outstanding PR that people can grab. |
|
@banshee |
|
Closing this so it won't show up in my PR list any more. |
I added cmake support for myself (I wanted to use CLion), and I'm happy to add it in to the project if it's interesting for other people. There's some cleanup work to be done here before merge (although it's working), I just wanted to see if it's worth doing.