mathstuf / eventd-plugin-journald

Journald event source for eventd
GNU Lesser General Public License v3.0
0 stars 0 forks source link

CMake fixes and tweaks #4

Closed sardemff7 closed 9 years ago

sardemff7 commented 9 years ago

The first commit is a necessary fix for the renames I did in eventd.

Second one is for proper linking. With -Wl,--no-undefined your plugin fails to link, because evhelpers_* functions are undefined.

Third one is to install your plugin exactly where eventd is expecting it. The pluginsdir variable in libeventd-plugin.pc is provided exactly for that purpose, so let’s just use it!

mathstuf commented 9 years ago

And other than that, the CMake style is…not matching, but I can fix that :) .

sardemff7 commented 9 years ago

No problem, what are the style issues? :-)

mathstuf commented 9 years ago

I still don't understand why those nifty "read the config as type" functions are not meant to be used, but whatever.

sardemff7 commented 9 years ago

It is not that you must not use them, just that you have to do that because it makes it clear that it is not public API. :-) This way, I can WONTFIX whichever bug report about it. ;-)

mathstuf commented 9 years ago

I guess for here, my question is why they aren't public API. Not making them so will have some plugins understanding True rather than true due to them being Python, using a different list syntax, or other oddities that will get annoying once more than one alternative exists.

sardemff7 commented 9 years ago

As I said, I do not want to maintain this API in any stable way. I want to break it whenever I need to without bothering with -version-info and friends. It is perfectly ok for people to use it, but it is their burden to call AC_SEARCH_LIBS or find_library to link to it.

Also, it only uses g_key_file_get_* functions for stuff like True or true. More complex data, like enums, are already plugin-specific. So as long as you use the provided GKeyFile, it should just work. :-) [Edit] And the enum stuff is libnkutils so already public, except as a submodule, so I do not have any stable issue there, since it it statically linked.

sardemff7 commented 9 years ago

I added the checks you wanted and renamed the macro to pkg_get_variable (as in your patch to CMake).

mathstuf commented 9 years ago

Merged and I did some cleanup on top of it. Thanks!