bcpierce00 / unison

Unison file synchronizer
GNU General Public License v3.0
4.08k stars 229 forks source link

Enable building inotify fsmonitor on BSDs #921

Closed tleedjarv closed 7 months ago

tleedjarv commented 1 year ago

Even though inotify is a Linux interface, there are other OS which provide a compatible implementation (BSDs, even Darwin, and some illumos-based OS) and may not have a native fsmonitor implementation of their own.

Enable building inotify fsmonitor on BSDs. Tested on FreeBSD, OpenBSD, NetBSD, DragonFly.

A configure-style test is made, searching for libinotify on a couple of hardcoded paths in addition to locations provided in CFLAGS/LDFLAGS and default locations by system configuration.

gdt commented 1 year ago

Just starting to look, I see a list of dirs getting -L, but not -R. I am not comfortable with including as -L a list of dirs that might be right in various environments. Really, this is a a sign that unison doesn't have a build system like autoconf.

On my system, the libinotify package comes with pkg-config, so it seems straightforward, even simpler, to use pkg-config to find ldflags, cflags and cppflags (waves hands, but this is normal). That would avoid spurious -L, and get the -R correct. I will have a look at how hard this is.

On my system, just building did not apparently get me libinotify linked, but I haven't really dug in yet.

tleedjarv commented 1 year ago

pkg-config sounds like a good idea. Is it expected to be available on BSD systems?

gdt commented 1 year ago

The packaging world basically requires it, because it is the standard approach (even if only 50%) for programs to find dependencies. So pkgsrc has a pkg-config package, and any program that uses pkg-config to find things depends on it. Similarly for everything else, which is actually the same situation as GNU/Linux. I am totally ok with listing it as a dependency for unison to build.

On windows, native and cross, I have no idea.

tleedjarv commented 1 year ago

I've used pkg-config now. I also left in the linking check so that it can still work even without pkg-config.

Now, just swapping in pkg-config doesn't do anything about -R, but isn't that a libinotify packaging bug then? What is the standard approach here?

I don't think pkg-config needs to be defined as dependency for unison, though. For one thing, fsmonitor is itself optional. For another, pkg-config is not required; fsmonitor can be built fine without pkg-config (completely automatic if -L and -I are not needed; but will also work fine by setting proper CFLAGS and LDFLAGS). Anyway, this is a decision for packagers.

gdt commented 1 year ago

Whether -R is needed depends on the system and the location of the library. it is the responsibility of the libinotify package to install pkg-config data that gets this right, and it is normal to just rely on that data. It seems to be wrong in the pkgsrc package.

It is also normal to respect LDFLAGS from the environment (or treat them as additional) and thus people passing -L/usr/pkg/lib -R/usr/pkg/lib can work around broken pkg-config files.

Our make setup is already too complicated and it's a bug that it isn't using something like autoconf, so I do not want to go down the path of reinventing adaptation at all, not even a little bit. Thus I lean pretty hard to just use pkg-config, and fail if pkg-config isn't there. While dependencies have a cost, things that are needed by such a large fraction of the world are not painful; it's hard to imagine a system without pkg-config that is used to build even a handful of randomly chosen programs, and I know of no posixy environment where it is difficult to deal with.

I also would like the build path to just have one approach, because people are going to have trouble and file tickets, and it will be easier to work through a single approach.

This all raises the question of the dune build :-)

gdt commented 1 year ago

pkg-config floated on users.

tleedjarv commented 1 year ago

Whether -R is needed depends on the system and the location of the library. it is the responsibility of the libinotify package to install pkg-config data that gets this right, and it is normal to just rely on that data. It seems to be wrong in the pkgsrc package.

As I thought. This question is closed then.

tleedjarv commented 1 year ago

Our make setup is already too complicated and it's a bug that it isn't using something like autoconf, so I do not want to go down the path of reinventing adaptation at all, not even a little bit. Thus I lean pretty hard to just use pkg-config, and fail if pkg-config isn't there. While dependencies have a cost, things that are needed by such a large fraction of the world are not painful; it's hard to imagine a system without pkg-config that is used to build even a handful of randomly chosen programs, and I know of no posixy environment where it is difficult to deal with.

The discussion got a bit broader so I'm going to comment strictly within scope of this PR. I also don't want to have the broader discussion in the context of this PR so I'm fine ripping the offending parts out of the PR and only leaving the option of manual invocation.

I'm not keen on making pkg-config a hard requirement because of a few reasons. pkg-config is not required to actually build fsmonitor. pkg-config is not even required to auto-detect existence of libinotify. FreeBSD Ports, for example, places (I think) libinotify library and headers in standard locations. Even with non-standard locations, CFLAGS and LDFLAGS can be set accordingly by the user/packager (which could be done from output of pkg-config). Finally, making pkg-config a hard requirement would not simplify the makefile. It would just shift the complexity elsewhere. To actually reduce complexity, I would rip out pkg-config completely and just expect the user/packager to set CFLAGS and LDFLAGS correctly, where needed. In the end, if we don't want any auto-detection for this at all then we can instruct users/packagers to build the fsmonitor explicitly. Then the build can simply fail, as opposed to with 'make world' where it would need to be skipped.

olafhering commented 1 year ago

The relevant dune files have to be adjusted, so that the output of pkg-config is used for building. dune-configurator provides an easy interface to interact with pkg-config.

gdt commented 1 year ago

To actually reduce complexity, I would rip out pkg-config completely and just expect the user/packager to set CFLAGS and LDFLAGS correctly, where needed. I

This does not reduce complexity; it moves it to the user. There is lines of code that are included by reference, but that's not the right metric IMHO. I think build systems should reduce the needed mount of human thought and work, while remaining sound. But as you say, we should settle the build issue, and then after that, this can adapt to the new plan easily.