`ll.h` includes SquashFUSE's `config.h`

vasi / squashfuse

FUSE filesystem to mount squashfs archives

Other

288 stars 66 forks source link

`ll.h` includes SquashFUSE's `config.h` #65

Open reidpr opened 2 years ago

reidpr commented 2 years ago

Thanks for the library support (PR #59); this helps us a lot!!

One challenge we ran into is that ll.h includes (via some intermediate headers IIRC) the Autoconf-produced config.h. This clashes with two things:

our own Autoconf-produced config.h, and
something in the system headers, because it defines _POSIX_C_SOURCE to a different value than our project.

We did find a workaround:

#define SQFS_CONFIG_H
#define FUSE_USE_VERSION 32
#include <squashfuse/ll.h>

(Note that we are libfuse3 only so that's why it seemed OK to define FUSE_USE_VERSION unconditionally.)

But it would be nice if there were an externally-facing header to include.

DrDaveD commented 1 year ago

Can you submit a PR with a suggested fix?

reidpr commented 1 year ago

We don’t have the expertise on this project to submit a PR; sorry.

DrDaveD commented 1 year ago

I don't really understand how your workaround works or what it does. I don't see how it can get away with ignoring all the config.h definitions determined by configure.

reidpr commented 1 year ago

config.h is an internal header and shouldn't be exposed to other programs using the library. Thus, it was fine to have it included by ll.h when there was no shared library, but now that there is, including that file may break any library user that also uses Autoconf.

The workaround above works by preventing ll.h from defining the specific symbols that clash with Charliecloud's build. It is not a general fix. The general fix would be to separate the SquashFUSE headers into internal and external (which requires knowledge of SquashFUSE we don't have).

DrDaveD commented 1 year ago

I see, that makes sense. Perhaps the developer of the library interface @haampie could come up with a PR for that?

vasi commented 11 months ago

Here are the config.h-related things in squashfuse headers:

squashfs_fs.h: SQFS_MULTITHREADED. We only use this to decide the number of cached blocks, which is only for internal use. We should move that to the private header.
squashfs_fs.h: HAVE_LINUX_TYPES_LE16. The replacement for this should always work, maybe we should just do that unconditionally?
ll.h: FUSE_XATTR_POSITION and FUSE_USE_VERSION (for the extra fuse_chan field in sqfs_ll_chan. These are really bad, because they're API changes based on configure! If you compile against squashfuse, and then later squashfuse is rebuild with different configure args, these could theoretically break. But honestly, they're both internals that never should have been exposed in a public header--we probably need to figure out what is really internal vs. external, and keep the internals private.