Re: gvfs patch to remove lsof dependency [review D56224]
Guido Falsi <[email protected]>
| Newsgroups | gmane.os.freebsd.devel.gnome |
|---|---|
| Message-ID | <[email protected]> |
On 4/15/26 20:50, Charlie Li wrote: > Guido Falsi wrote: >> Hi, >> >> As subject states I have prepared a patch to gvfs that removes the >> need to have lsof installed, replacing its usage with fuser(1), which >> is included in the base system. [1] >> >> I was moved into doing this after lsof broke (again) on head. >> >> There is a single use of lsof in gvfs, for which the port forces any >> desktop machine to depend on lsof. So I think this could be a very >> good improvement for our ports tree, removing this extra dependency, >> for a very single and minor use. >> > I also thought of looking for where lsof is used; good job for finding it. Thanks for your feedback! >> I also sent the patch upstream [2], so if anyone has contacts there >> and can push for it or at least get some feedback that would be very >> helpful! >> >> Anyway, I am usually against diverging from upstream, but in this case >> I think the advantages are strong enough to warrant an exception. >> >> So I'm asking if anyone from gnome@ could approve this change. >> > On first read it looks fine, but not approving yet. More general > comments here, any specifics about port changes in phab. I'm obviously open to suggestions to improve it. >> [1] https://reviews.freebsd.org/D56224 >> >> [2] https://gitlab.gnome.org/GNOME/gvfs/-/merge_requests/321 >> > Upstream will get to it when they get to it. Looks $life is why > everything since the last commit hasn't been actioned yet, let alone > this merge request. Sure, I'm not in any kind of hurry. OTOH I want to avoid stagnation and the patch being forgotten. > > I would like upstream's feedback on how you structured the meson option. > It could turn out that they may want to make the fuser(1) method > default. More importantly, the testing pipeline ran on the merge request > is still against lsof; I would like to see the results of it ran against > fuser(1), both our variant and Linux's. I left the default as is intentionally. Usually patches are easier to get accepted if they do not change the status quo. If upstream is interested I'll be really happy to enable my code by default. I'm running with my patch enabled on FreeBSD with XFCE, and works as expected, lsof is not installed anymore on my desktop machines. I have not really tested the patch on Linux. I did perform some simple test with a minimal C code on arch Linux to validate the idea. That's why I use different options for Linux, their fuser(1) has different command line options. I think I can get a test run on their CI with a separate branch with the default switched. I'll try that in the next few days. Thanks again for your feedback! -- Guido Falsi <[email protected]>