Bug 235240

Summary: devel/libinotify: move sys/inotify.h into a subdirectory
Product: Ports & Packages Reporter: Jan Beich <jbeich>
Component: Individual Port(s)Assignee: Port Management Team <portmgr>
Status: Closed Not Accepted    
Severity: Affects Only Me CC: arrowd, decke, dominik.lisiak, freebsd-ports, haskell, java, joh.hendriks, jpaetzel, junichi, kde, kwm, lukas.slebodnik, madpilot, mickael.maillot, mono, mp39590, mr, olgeni, prj, rigoletto, sunpoet, swills, tcberner, timur, ultima, yonas, yuri
Priority: --- Keywords: patch
Version: LatestFlags: bugzilla: maintainer-feedback? (sunpoet)
decke: maintainer-feedback+
dominik.lisiak: maintainer-feedback+
jbeich: maintainer-feedback? (freebsd-ports)
arrowd: maintainer-feedback+
jbeich: maintainer-feedback? (java)
jbeich: maintainer-feedback? (joh.hendriks)
jbeich: maintainer-feedback? (jpaetzel)
jbeich: maintainer-feedback? (junichi)
tcberner: maintainer-feedback+
kwm: maintainer-feedback+
jbeich: maintainer-feedback? (lukas.slebodnik)
madpilot: maintainer-feedback+
jbeich: maintainer-feedback? (mickael.maillot)
jbeich: maintainer-feedback? (mono)
jbeich: maintainer-feedback? (mp39590)
jbeich: maintainer-feedback? (mr)
jbeich: maintainer-feedback? (olgeni)
rigoletto: maintainer-feedback+
jbeich: maintainer-feedback? (sunpoet)
swills: maintainer-feedback+
jbeich: maintainer-feedback? (takumiiinn)
jbeich: maintainer-feedback? (timur)
jbeich: maintainer-feedback? (ultima)
jbeich: maintainer-feedback? (yonas)
yuri: maintainer-feedback+
jbeich: exp-run?
Hardware: Any   
OS: Any   

Description Jan Beich freebsd_committer freebsd_triage 2019-01-27 11:45:48 UTC
libinotify installs sys/inotify.h in a common location where regular libraries are. Consumers may opportunistically pick it up over kqueue backend or if not required. Let's limit poisoning like OpenBSD port did.

See review D18990 for the patch and check your port didn't regress.
Comment 1 Jan Beich freebsd_committer freebsd_triage 2019-01-27 11:50:43 UTC
Can you check all ports for hidden consumers?

OPTIONS_SET += LIBINOTIFY INOTIFY FSMONITOR
Comment 2 Guido Falsi freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 2019-01-27 16:35:44 UTC
Looks good for kde@, as long as it is exp-ran.

mfg Tobias
Comment 4 Josh Paetzel freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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.
Comment 7 Dominik Lisiak 2019-03-31 22:28:58 UTC
I didn't apply the patch, but made a compatible fix in bug #236919 for security/ossec-hids-local.
Comment 8 Steve Wills freebsd_committer freebsd_triage 2019-05-03 21:39:10 UTC
Haven't tested it, but obs-qtwebkit change looks fine.
Comment 9 Phillip R. Jaenke 2020-10-23 12:43:20 UTC
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 freebsd_committer freebsd_triage 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.
Comment 11 mp39590 2021-09-15 11:34:43 UTC
What is the status of this bug? It has been up for more than 2 years, can it be closed?
Comment 12 Jan Beich freebsd_committer freebsd_triage 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 freebsd_committer freebsd_triage 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.