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

libbson detection fails in Linux. #375

Closed
darkshram opened this issue Dec 22, 2020 · 6 comments
Closed

libbson detection fails in Linux. #375

darkshram opened this issue Dec 22, 2020 · 6 comments
Labels
area:build Relates to compiling/buildsystem of fvwm help wanted Development help required (see `difficulty:*`) type:bug Something's broken!
Milestone

Comments

@darkshram
Copy link

Most Linux distributions use libbson-1.0/bson.h instead of bson/bson.h.

The code in fvwm3 uses this:

#if defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__sun__)
 #include <libbson-1.0/bson.h>
#else
#include <bson/bson.h>
#endif

Which means if not FreeBSD, OpenBSD or Solaris, header include will be bson/bson.h. So, the build will fail because there is no /usr/include/bson/bson.h in most Linux distributions.

Removing the conditionals allows to build fvwm3 in Linux.

diff -Naur fvwm3-1.0.2.orig/fvwm/builtins.h fvwm3-1.0.2/fvwm/builtins.h
--- fvwm3-1.0.2.orig/fvwm/builtins.h	2020-12-11 18:37:08.000000000 -0600
+++ fvwm3-1.0.2/fvwm/builtins.h	2020-12-21 21:18:36.846795167 -0600
@@ -3,11 +3,7 @@
 #ifndef FVWM_BUILTINS_H
 #define FVWM_BUILTINS_H
 
-#if defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__sun__)
 #include <libbson-1.0/bson.h>
-#else
-#include <bson/bson.h>
-#endif
 
 #include "fvwm.h"
 #include "screen.h"
diff -Naur fvwm3-1.0.2.orig/modules/FvwmMFL/FvwmMFL.c fvwm3-1.0.2/modules/FvwmMFL/FvwmMFL.c
--- fvwm3-1.0.2.orig/modules/FvwmMFL/FvwmMFL.c	2020-12-20 08:47:50.000000000 -0600
+++ fvwm3-1.0.2/modules/FvwmMFL/FvwmMFL.c	2020-12-21 21:18:45.505121652 -0600
@@ -30,11 +30,7 @@
 #include <stdbool.h>
 #include <unistd.h>
 
-#if defined(__FreeBSD__) || defined(__OpenBSD__) || defined(__sun__)
 #include <libbson-1.0/bson.h>
-#else
-#include <bson/bson.h>
-#endif
 
 #include <event2/event.h>
 /* FIXME: including event_struct.h won't be binary comaptible with future
@darkshram darkshram added the type:bug Something's broken! label Dec 22, 2020
@ThomasAdam
Copy link
Member

Hi @darkshram

This will need changes to configure.ac to detect this better; it's not enough to just remove those conditionals as this will break things elsewhere.

Is this something you could look at?

@ThomasAdam ThomasAdam added area:build Relates to compiling/buildsystem of fvwm help wanted Development help required (see `difficulty:*`) labels Dec 22, 2020
@ThomasAdam ThomasAdam added this to the 1.0.3 milestone Dec 22, 2020
@darkshram
Copy link
Author

I'll try something to detect the headers, but... maybe it' won't be necessary at all. Since version 1.0 (August 2014), libbson installs 'bson.h' as 'libbson-1.0/bson.h'. I don't believe it will break something somewhere, unless somebody is using a very old [and most likely insecure and therefore unsupported] version of libbson. bson/bson.h used to be installed as default with bson <= 0.8.4, June 2014. This is why I believe it will be easier to just ask for 'libbson-1.0 >= 1.0' from configure.ac. Libbson is currently being maintained as part of mongo-c-driver (latest version is 1.17.3 and it can be built separately if someone gets confused).

@slazav
Copy link
Contributor

slazav commented Dec 22, 2020

It should be just "#include <bson.h>". You already use pkg-config to detect the library and to set correct CFLAGS, -I/usr/include/libbson-1.0 in my case.

@ThomasAdam
Copy link
Member

ThomasAdam commented Dec 22, 2020

Hi @slazav

This isn't right because even though pkg-config detects the presence of libbson (in terms of the CFLAGS and LDFLAGS), it doesn't detect the difference in potential header locations. From what I've seen, just including:

#include <bson.h>

without any #ifdef guards will work on Linux and FreeBSD, but the reports I had suggest that others might not work due to them packaging libbson differently, or their version of libbson is older.

Either way, we have the capability of improving this further in configure.ac if we choose to.

I'll merge this change, and if we get a lot of resistance from elsewhere we can do something else.

@slazav
Copy link
Contributor

slazav commented Dec 22, 2020

I think if we use pkg-config we can expect correct paths from it. If in some system libbson package contains .pc file with a wrong path, it's an obvious error in the package. I can imaging that some old installations could contain no .pc file, but then configure will fail anyway.

@ThomasAdam
Copy link
Member

I think if we use pkg-config we can expect correct paths from it. If in some system libbson package contains .pc file with a wrong path, it's an obvious error in the package. I can imaging that some old installations could contain no .pc file, but then configure will fail anyway.

Yes -- and more to the point, although we can provide a fallback using AC_CHECK_HEADER if pkg-config were to fail, I suspect in that case we're dealing with systems with bigger problems, so let's see how we get on without needing to do that for now. It's easy enough to add it in the future.

@ThomasAdam ThomasAdam moved this to Done in FVWM3 Sep 18, 2022
@ThomasAdam ThomasAdam added this to FVWM3 Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:build Relates to compiling/buildsystem of fvwm help wanted Development help required (see `difficulty:*`) type:bug Something's broken!
Projects
Status: Done
Development

No branches or pull requests

3 participants