davea42 / libdwarf-code

Contains source for libdwarf, a library for reading DWARF2 and later DWARF. Contains source to create dwarfdump, a program which prints DWARF2 and later DWARF in readable format. Has a very limited DWARF writer set of functions in libdwarfp (producer library). Builds using GNU configure, meson, or cmake.
Other
170 stars 70 forks source link

Update zstd cmake usage to the provided by the upstream #225

Closed uilianries closed 8 months ago

uilianries commented 8 months ago

Hello! The project Zstd does not have an official cmake config file, neither find cmake available in the official cmake modules.

However, the project itself offers cmake support, including a config file and targets:

https://github.com/facebook/zstd/blob/dev/build/cmake/zstdConfig.cmake

https://github.com/facebook/zstd/blob/dev/build/cmake/CMakeLists.txt#L181

So, consuming zstd would be:

find_package(zstd CONFIG REQUIRED)

...

target_link_libraries(dwarf PRIVATE $<TARGET_EXISTS:zstd::libzstd_static>,zstd::libzstd_static,zstd::libzstd_shared>)

This would follow the correct way, following the current zstd project. In order to have effect, it would need to update libdwarf-code CMakeLists.txt, and FindZTD.cmake in this project, to follow the expect names.

In case you are inclined to accept the idea, I could open a PR with the fix.

davea42 commented 8 months ago

I hope @jeremy-rifkin and @flagarde will have comments on this.

I think it seems like a fine idea and a pull request would be good. Thanks!

uilianries commented 8 months ago

Having a second, part of this issue is implemented already: https://github.com/davea42/libdwarf-code/blob/main/CMakeLists.txt#L211

The only thing would be renaming the package to zstd instead of the current one ZSTD (all upper-case)

davea42 commented 8 months ago

Of course you do not mean to replace all 599 ZSTD with zstd, but I am not at all sure which instances need changing.

uilianries commented 8 months ago

@davea42 Sorry, I meant https://github.com/davea42/libdwarf-code/blob/main/CMakeLists.txt#L203 to use zstd, like:

find_package(zstd)