Re: [Discussion] The architecture of Scalar (and others) within Git

From: Derrick Stolee <stolee@gmail.com>
Date: Nov 4, 2021, 17:20 UTC
Message-ID: <9eb8fd45-c8a5-0320-6d38-56389bef193d@gmail.com>
In-Reply-To: <211029.86k0hww317.gmgdl@evledraar.gmail.com>

On 10/28/2021 2:58 PM, Ævar Arnfjörð Bjarmason wrote:

Show 6 quoted lines

> 
> On Wed, Oct 27 2021, Derrick Stolee wrote:
> 
>> I'm starting this discussion thread to create a new area to consider these
>> high-level concepts. Specifically: how should a new component like Scalar
>> be included in the Git codebase?

Thank you for your thoughts on this topic. I wanted to specifically reply to a few items while the discussion continues to simmer.

> E.g. the scalar series is now at v6, but apparently it hasn't been
> noticed that some of the tests were broken on OSX due to apparent errors
> with launchctl integration. That's because the patches had the code
> compiled, but didn't run the tests.

These failures are due to changes to Git maintenance during the 2.34 cycle, and we've noticed and fixed them in microsoft/git as we rebased on the release candidates.

This is one of the things that we expect with one extreme side of Git's "ownership" of Scalar: things could break Scalar that we won't notice until manually running the tests or taking upstream changes into our fork, but then we will fix them. Right now, that fix would be in a future iteration of the patch series.

Show 26 quoted lines

>> [...]
>> **Option 1:** One-directional coupling in `contrib/scalar/`
>>
>> This option was our initial choice because it minimizes the responsibility
>> of the Git community. While `contrib/scalar/scalar.c` depends on code in
>> `libgit.a`, the implementation does not require that code to change to
>> accommodate the needs of Scalar. The test suite and documentation is
>> separate.
>>
>> This does mean that changes to `libgit.a` could break Scalar without any
>> feedback to the developer that has not compiled Scalar. The Scalar
>> maintainers would then need to watch for this and send separate updates to
>> Scalar that fix these dependency breaks. This reduces developer friction,
>> but might cause some extra burden on the Git maintainer. If these "catch
>> up to dependency breaks" updates happen only after a release candidate is
>> out, then maybe this isn't too much of a burden. We don't typically have
>> release candidates for minor releases, so there is some risk there that
>> minor releases could include breaks.
>>
>> If we make our CI builds include Scalar by default, then the previous
>> paragraph about developer friction is negated.
> 
> Without going into exhaustive detail I'll just note as above that much
> of this describes a state that's got little or nothing to do with the
> contrib/scalar/* patches on-list, which are thoroughly
> bi-bidirectionally integrated, build (but don't test) by default etc.

You're right that I am describing a theoretical extreme that is not realized by the patch series in its current form. The extreme I'm describing is where we could revert Scalar out of the Git codebase with a simple 'rm -f contrib/scalar' and as minimal remaining work as possible. Johannes relaxed that condition in an attempt to reduce duplication in the Makefiles, but there is opportunity to redesign and get much closer to this extreme, if that's the consensus we reach.

Thanks, -Stolee