SimonKagstrom / kcov

Code coverage tool for compiled programs, Python and Bash which uses debugging information to collect and report data without special compilation options
http://simonkagstrom.github.io/kcov/
GNU General Public License v2.0
712 stars 109 forks source link

Make cmake script fail when it cannot find certain dependencies #354

Closed haampie closed 3 years ago

haampie commented 3 years ago

I tried to build this package, but it did not install the kcov binary because cmake couldn't find a couple dependencies.

Could we maybe replace things like

+ find_package (Bfd)
- find_package (Bfd REQUIRED)

for linux so that the kcov executable actually gets built instead of silently ignored?

A popular approach is to introduce variables like USE_BFD and USE_ELFUTILS with sensible defaults, let the user optionally set those, and then do:

if (USE_BFD)
  find_package(Bfd REQUIRED)
endif()

if (USE_ELFUTILS)
  find_package(ElfUtils REQUIRED)
endif ()
SimonKagstrom commented 3 years ago

Well, I did some digging to remember why I did it this way. It's actually issue #83 , because libbfd isn't technically needed even on Linux. It's used for the --verify option. That it doesn't build kcov itself without it seems like a regression, the original intention was to skip just the address verifier library if libbfd isn't available.

That said, I'd be OK to make it mandatory on Linux (and maybe FreeBSD).

haampie commented 3 years ago

Thanks for digging that up :)

I think you can leave it an optional dependency, but allow the user to add a flag like WITH_ADDRESS_VERIFIER=On/Off, and if on, do find_package(... REQUIRED).

That way package managers can explicitly turn it on or off and specify dependencies accordingly

SimonKagstrom commented 3 years ago

But is the problem Linux or OSX here? From the spack issue, it seems like the build issue is on OSX if it finds libbfd?

I'm inclined in simply making libbfd mandatory on Linux/FreeBSD and not looking for it at all on other platforms (i.e., OSX).

haampie commented 3 years ago

Ah, in spack elfutils and libiberty etc were not listed as dependencies which caused kcov to be skipped for me, so I added those. Somehow that is not the case on macos, so someone else removed them for macos.

SimonKagstrom commented 3 years ago

Yes, the kcov-system-daemon can be built separately from the rest without those dependencies. I guess it hasn't been used by anyone other than myself, so perhaps it's time to enforce some of it.

Anyway, I pushed libbfd being required for Linux+FreeBSD, so hopefully it will work betetr after that.

jonhoo commented 2 years ago

@SimonKagstrom Note that libbfd (unlike elfutils) is GPLv3, which means that kcov now requires GPLv3-licensed libraries to link and run, which are often not available in commercial settings, making kcov v39 hard to use in such contexts.

SimonKagstrom commented 2 years ago

Yes, that is true. I guess it would be better to make it non-required again.

It will be missing the --verify functionality then though, which does help a lot (especially it seems for Rust binaries).

jonhoo commented 2 years ago

Yeah, it is unfortunate, since --verify is super handy, but better to have some (hard to debug) access to coverage than none :sweat_smile: