Hi Ævar,

On Thu, 1 Jul 2021, Ævar Arnfjörð Bjarmason wrote:

> On Thu, Jul 01 2021, Jeff Hostetler wrote:
>
> > On 7/1/21 1:40 PM, Ævar Arnfjörð Bjarmason wrote:
> >
> >> Any other testing of it is stalled by there being no linux backend
> >> for it as part of this series. I see from spelunking repos that
> >> Johannes had a WIP compat/fsmonitor/linux.c which looks like it
> >> could/should mostly work, but the API names all changed since then,
> >> and after a short try I gave up on trying to rebase it.

I am a bit surprised that you gave up so easily, it cannot be that hard if
you use `git rebase -i` smartly.

But I think there is an even bigger obstacle lurking than just the
challenge of rebasing that experimental backend.

> > The early Linux version was dropped because inotify does not give
> > recursive coverage -- only the requested directory.  Using inotify
> > requires adding a watch to each subdirectory (recursively) in the
> > worktree.  There's a system limit on the maximum number of watched
> > directories (defaults to 8K IIRC) and that limit is system-wide.
> >
> > Since the whole point was to support large very large repos, using
> > inotify was a non-starter, so I removed the Linux version from our
> > patch series.  For example, the first repo I tried it on (outside of
> > the test suite) had 25K subdirectories.
> >
> > I'm told there is a new fanotify API in recent Linux kernels that is a
> > better fit for what we need, but we haven't had time to investigate it
> > yet.
>
> That default limit is a bit annoying, but I don't see how it's a blocker
> in any way.

Let me help you to see it.

So let's assume that you start FSMonitor-enabled Git, with the default
values. What is going to happen if you have any decently-sized worktree?
You run out of handles. What then? Throw your hands in the air? Stop
working? Report incorrect results?

Those are real design challenges, and together with the race problems Jeff
mentioned, they pose a much bigger obstacle than the rebasing you
mentioned above.

> You simply adjust the limit. E.g. I deployed and tested the hook version
> of inotify (using watchman) in a sizable development environment, and
> written my own code using the API. This was all before fanotify(7)
> existed. IIRC I set most of the limits to 2^24 or 2^20. I've used it
> with really large Git repos, including with David Turner's
> 2015-04-03-1M-git for testing (`git ls-files | wc -l` on that is around
> a quarter-million).
>
> If you have a humongous repository and don't have root on your own
> machine you're out of luck. But I think most people who'd use this are
> either using their own laptop, or e.g. in a corporate setting where
> administrators would tweak the sysctl limits given the performance
> advantages (as I did).

This conjecture that most people who'd use this are using their own laptop
or have a corporate setting where administrators would tweak the sysctl
limits according to engineers' wishes strikes me as totally made up from
thin air, nothing else.

In other words, I find it an incredibly unconvincing argument.

I prefer not to address the rest of your mail, as I found it not only a
lengthy tangent (basically trying to talk Jeff into adding Linux support
in what could have been a much shorter mail), but actively distracting
from the already long patch series Jeff presented. It is so long, in fact,
that we had to put in an exemption in GitGitGadget because it is already
longer than a usually-unreasonable 30 patches. Also, at this point,
insisting on Linux support (in so many words) is unhelpful.

Let me summarize why I think this is unhelpful: In Git, it is our
tradition to develop incrementally, for better or worse. Jeff's effort
brought us to a point where we already have Windows and macOS support,
i.e. support for the most prevalent development platforms (see e.g.
https://insights.stackoverflow.com/survey/2020#technology-developers-primary-operating-systems).
We already established multiple obstacles for Linux support, therefore
demanding Linux support to be included Right Now would increase the patch
series even further, making it even less reviewable, being even less
incremental, hold up the current known-to-work-well state, force Jeff to
work on something he probably cannot work on right now, and therefore
delaying the entire effort even further.

Ciao,
Johannes