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

FindLog4cplus.cmake broken #116

Closed
Anticom opened this issue Sep 14, 2016 · 5 comments
Closed

FindLog4cplus.cmake broken #116

Anticom opened this issue Sep 14, 2016 · 5 comments

Comments

@Anticom
Copy link

Anticom commented Sep 14, 2016

When changing the find-package of log4cplus to required (find_package(Log4cplus REQUIRED)), cmake on my machine fails to find it. I'm not quite sure why that is.

Steps to reproduce (Ubuntu 16.04.1 Xenial LTS, 64 Bit):

  1. Set the find-package in CMakeLists.txt to REQUIRED.
  2. sudo apt-get install liblog4cplus-dev
  3. Try to build cmake with USE_LOG4CPLUS enabled

CMake should fail to find the log4cplus library.

I've written a FindLog4cplus.cmake module that works for me but lacks some configurability that your existing module provides.
You can find it here.

@snikulov
Copy link
Contributor

@Anticom can be Xenial has bumped version from 1.0.4 to 1.1.2, so it's possible.
Will check it later.
Also you can try change set(Log4Cplus_USE_STATIC_LIBS 1) to set(Log4Cplus_USE_STATIC_LIBS 0) in pion's CMakeLists.txt here https://github.com/splunk/pion/blob/develop/CMakeLists.txt#L119
It also can help you.

@Anticom
Copy link
Author

Anticom commented Sep 14, 2016

@snikulov Indeed the version on my machine is 1.1.2. However I've looked at your FindLog4cplus.cmake and i don't see how this module is breaking anything on my machine. However Log4Cplus_USE_STATIC_LIBS being set to 1 by default looks like causing the issue for me.

Shouldn't find_path handle this on its own? Either way IMO it should be an option so the user has control over it.

@snikulov
Copy link
Contributor

snikulov commented Sep 14, 2016

Shouldn't find_path handle this on its own? - Nop.
It specific for library authors. One adds S prefix, other add -static. No unified way.

Just remove Log4Cplus_USE_STATIC_LIBS at all from pion's CMakeLists.txt .
Then module will search dynamic by default, unless you explicitly not set it using -D

@Anticom
Copy link
Author

Anticom commented Sep 14, 2016

Since we're building pion using Yocto i'd have to create a patch for this which would be unfortunate for such a trivial change. Is there any reason to not make it an option - besides eventually breaking working builds that use a static version of log4cplus - ?

If this is your only concern I'd vote for this "breaking" change.

@snikulov
Copy link
Contributor

I think it is smaller then add option and use this option later.
Just remove one line and you'll give this option for user.

firedaemon-cto pushed a commit to FireDaemon/pion that referenced this issue Aug 27, 2020
* change gettimeofday to be clearer - issue splunk#127

* clarify date_number docs - github issue splunk#125

* add docs for from_iso_extended_string function - issue splunk#116

* update documentation for 1.73 release - changes.xml
@Anticom Anticom closed this as completed Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants