brgl / libgpiod

This is a mirror of the original repository over at kernel.org. This github page is for discussions and issue reporting only. PRs can be discussed here but the patches need to go through the linux-gpio mailing list.
https://git.kernel.org/pub/scm/libs/libgpiod/libgpiod.git/
Other
290 stars 102 forks source link

Wish/feature request: compile time evaluatable define(s) with API version #68

Closed mhei closed 5 months ago

mhei commented 5 months ago

Library users might want to know against which library ABI version they are actually compiling. This is even more important when ABI changes occur, e.g. between v1.6 and v2.x or similar. Users could then include some #ifdef stuff to allow supporting both versions within the same code base.

At the moment, this is AFAIK only possible using some pkg-config magic and creating such defines oneself.

brgl commented 5 months ago

I think you mean API? The header cannot guarantee the ABI version of the shared object.

The project (the C and C++ library parts of it anyway) uses pkg-config to expose version information and I'm not sure how else you want to link your programs against it but this is the official way of detecting it in the system.

mhei commented 5 months ago

Yes, of course - absolutely correct, I'm talking about the API not the ABI - sorry for the confusion (I changed it now in the issue title).

And yes, I know that pkg-config exposes this information. I'm not aware of any existing (example) implementation, so I created the following for openocd (see https://github.com/mhei/openocd/commit/472a8f92b1a71d595b334c7bc2ad371785b94379) You can see there that is requires some autotools magic so that I finally can build different objects attaching to different library versions (here I used this approach because I though this would keep the code more readable, but this is not that important here).

The question is whether libgpiod could not provide for example the following macros in the public gpiod header, e.g.:

#define GPIOD_VERSION_MAJOR 2
#define GPIOD_VERSION_MINOR 1
#define GPIOD_VERSION_REVISION 0

That would allow to just use

PKG_CHECK_MODULES([LIBGPIOD], [libgpiod], [use_libgpiod=yes], [use_libgpiod=no])

and later in the whatever.c file the "switching logic" based on the found version:

...
#include <gpiod.h>

...

#if GPIOD_VERSION_MAJOR == 2

// here I can use gpiod_api_version and struct gpiod_line_request etc.

#else

// here I can use gpiod_version_string and e.g. struct gpiod_line etc

#endif

The advantage I see is that in the linked example, I needed to create a is_libgpiodv2 variable on my own (and thus every library consumer has to re-invent/re-implement it again and again), but when libgpiod already ships with such defines, consumers can just use it.

brgl commented 5 months ago

The thing you did looks right to me. Do you think about having an ifdef hell in a single file for supporting both releases within the same compilation unit?

mhei commented 5 months ago

Exactly, this was my idea. I'm not yet sure, whether it is really a hell for smaller projects. I think, I'll have to try. That said, I have the feeling that you think that providing the version stuff via pkg-config is sufficient for libgpiod, right?

brgl commented 5 months ago

Indeed, I believe that a project should avoid supporting two major libgpiod releases in the first place (just switch to requiring v2) but if not possible, it should have the support for one and the other in separate .c files. I'm not 100% opposed to introducing these macros but this particular reason seems wrong to me.

mhei commented 5 months ago

I cannot image other reasons for such macros at the moment (and once introduced I think it will be used for this kind of stuff too :angel: ). But it is totally fine with me, I will just use the pkg-config way as described. So thanks for your feedback and the discussion here, really appreciated. I'll close this issue as "won't fix".