Re: [PATCH v2 00/12] fsmonitor: Implement fsmonitor for Linux

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 19, 2022, 20:14 UTC
Message-ID: <CAPig+cSCb6tA+Ox=xK+LuC+Hf=JRRBr3=xx9cJ0yQaguHm+HSQ@mail.gmail.com>
In-Reply-To: <221019.8635bjhbds.gmgdl@evledraar.gmail.com>

On Wed, Oct 19, 2022 at 3:16 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 16 quoted lines

> On Tue, Oct 18 2022, Eric Sunshine wrote:
> > On Tue, Oct 18, 2022 at 9:22 PM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >> In practice we've been catching SHA-implementation specific code early
> >> because the OSX implementation was different, but in this case it's
> >> OSX-only code, so it only supported the Apple Common Crypto backend.
> >
> > I don't know how germane it is to the current thread, but previous
> > discussions[1,2,3,4] favored dropping use of Apple's Common Crypto
> > altogether since it doesn't seem to buy us much (or anything) and is
> > incomplete; it doesn't support all of the OpenSSL API Git uses.
>
> Yeah, maybe. I'd be 100% OK with that happening, but I don't use OSX
> outside fo CI really, so I wanted to avoid scope creep to "non-SHA stuff
> we use OpenSSL/AppleCommonCrypto/whatever" for, in the cases where we
> can/do use OpenSSL/AppleCommonCrypto/whatever for SHA also.

Indeed, I was not suggesting that retirement of AppleCommonCrypto be tackled by you or by the series you posted to fix the build failure; doing so is well outside the scope of the immediate problem. I brought up those old discussions only as reference and as a pointer that whatever fix is eventually adopted for the immediate build problem need not necessarily bend over backward for AppleCommonCrypto support if a long-term goal is to drop AppleCommonCrypto (i.e. do as little as necessary to keep AppleCommonCrypto in a working state, but don't go overboard trying to give it first-class support since it will never be a complete replacement for OpenSSL).

Show 9 quoted lines

> But if you/someone can come up with a patch that confidently asserts
> that it's useless & drops it I'd be happy to either integrate it &
> submit it as part of this series if it makes things simpler, or
> alternatively re-roll on top of it.
>
> I'm just not confident in making that case myself, since I hardly use
> the platform, am not too familiar with the various concerns (aside from
> skimming the links above), and don't really have interest in pursuing
> that.

I was not suggesting holding up your series or integrating retirement of AppleCommonCrypto with it; they are separate concerns.

> OTOH if you do want to make the case for dropping Apple Common Crypto
> that would also presumably be much easier after this series, once we're
> past justifying the hurdle of "wait, we're switching the thing we use
> for SHA-1 by default, which we build practically everything in git on?"

My "don't know how germane it is to the current thread" observation was in response to the `fsmonitor` thread, before I ever saw the series you posted for fixing the build failure, so it wasn't given as any sort of feedback on your series, and wasn't asking you to incorporate retirement of AppleCommonCrypto into your series. That said, it does appear that dropping AppleCommonCrypto should become easier after your changes land if someone opts to tackle the task.