Comment 1 Jan Beich 2019-01-27 11:50:43 UTC

Can you check all ports for hidden consumers?

OPTIONS_SET += LIBINOTIFY INOTIFY FSMONITOR

Comment 2 Guido Falsi 2019-01-27 14:08:07 UTC

I filed some feedback in the code review.

Short version is, I think this could be better addressed creating an inotify USES.

I'm restraining my approval until such concern is addressed.

Posting here to avoid timing out.

Comment 3 Tobias C. Berner 2019-01-27 16:35:44 UTC

Looks good for kde@, as long as it is exp-ran.

mfg Tobias

Comment 4 Josh Paetzel 2019-01-28 19:50:49 UTC

I patched devel/libinotify, then installed devel/py-pyinotify and confirmed it works properly.  All clear from me.

Comment 5 Po-Chuan Hsieh 2019-01-30 19:33:02 UTC

(In reply to Jan Beich from comment #0)

I'm OK to change where inotify.h installs. But I do not understand why we have to move it. Could you please give a case that "consumers may opportunistically pick it up over kqueue backend or if not required"? After this change, do we have to teach every new ports how to find the new home of inotify.h?

Thanks.

Comment 6 Jan Beich 2019-02-04 16:09:33 UTC

(In reply to Sunpoet Po-Chuan Hsieh from comment #5)
> Could you please give a case that "consumers may opportunistically
> pick it up over kqueue backend or if not required"?

Look for ports with CONFIGURE_ENV=ac_cv_header_sys_inotify_h=no.

> After this change, do we have to teach every new ports how to find the new home of inotify.h?

No. Only those not using pkg-config. -linotify doesn't exist on Linux, so it always comes with a .pc file (unless older than 20170711). autotools and meson support pkg-config just fine, cmake uses modules to reinvent pkg-config but the support is good if required module exists, gmake and bmake[1] can cache pkg-config output via $(shell ...) and ${SH:!...!}.

[1] bmake caching have to be in submake *after* pkg-config is installed as ${SH:!...!} is evaluated at parsing time. gmake isn't affected because it's only used to parse vendor's Makefile while port's Makefile is parsed by bmake.

I didn't apply the patch, but made a compatible fix in bug #236919 for security/ossec-hids-local.

Comment 8 Steve Wills 2019-05-03 21:39:10 UTC

Haven't tested it, but obs-qtwebkit change looks fine.

This will break mono and all mono consumers using the System.IO namespace.
We already handled kqueue vs inotify poisoning upstream by specifically looking to /usr/local/include for inotify.h.

Comment 10 Po-Chuan Hsieh 2020-11-24 20:16:01 UTC

(In reply to Jan Beich from comment #6)

There's only 8 CONFIGURE_ENV=ac_cv_header_sys_inotify_h=no in the ports tree. Do we really need such a huge local patch which affects more than 40 ports.

What is the status of this bug? It has been up for more than 2 years, can it be closed?

Comment 12 Jan Beich 2021-09-15 12:49:14 UTC

(In reply to Po-Chuan Hsieh from comment #10)
Those 8 are marked landmines, more are likely awaiting for someone to step on and waste lots of time debugging. The patch increases reproducibility by making the intent to use libinotify explicit. Avoiding surprises is why FreeBSD long ago removed /usr/local/include default in base compiler.

(In reply to mp39590 from comment #11)
Originally stalled due to style issues (pkg-config vs. directly adjusting flags)  but then Mono upstream started hardcoding /usr/local/include, sabotaging not only this effort but also LOCALBASE != /usr/local.

Comment 13 Jan Beich 2021-09-15 13:12:00 UTC

The patch here is not salvageable anymore. Some unintentional consumers are easy to discover due to broken build but in libraries and plugins unless (--no-undefined or -z defs) it may propagate to runtime.