Re: [PATCH v6 02/20] reftable: add LICENSE

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Han-Wen Nienhuys <hanwen@google.com>
Cc: Han-Wen Nienhuys via GitGitGadget <gitgitgadget@gmail.com>,
	git <git@vger.kernel.org>, Jeff King <peff@peff.net>,
	Ramsay Jones <ramsay@ramsayjones.plus.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Jonathan Tan <jonathantanmy@google.com>,
	Josh Steadmon <steadmon@google.com>,
	Emily Shaffer <emilyshaffer@google.com>,
	Patrick Steinhardt <ps@pks.im>,
	Felipe Contreras <felipe.contreras@gmail.com>,
	Derrick Stolee <stolee@gmail.com>,
	Han-Wen Nienhuys <hanwenn@gmail.com>
Subject: Re: [PATCH v6 02/20] reftable: add LICENSE
Date: Tue, 13 Apr 2021 15:41:15 +0200	[thread overview]
Message-ID: <87o8ei2tfo.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CAFQ2z_MQp99uPpdai9Hbh76h-PO3JUHmY-pasSXONjeRp8dTzA@mail.gmail.com>

On Tue, Apr 13 2021, Han-Wen Nienhuys wrote:

> On Tue, Apr 13, 2021 at 9:28 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>     > However, it is fundamentally blocked on someone willing to spend
>>     > some time reviewing the series; this seems to be out of my
>>     > control.
>>
>> I for one find it hard to follow re-rolls of this when past discussion
>> isn't clarified/updated in commit messages/CLs.
>>
>> So in particular in v3[2] there was a whole discussion about what what
>> this licence & external hosting of this codebase meant in practical
>> terms.
>>
>> I vaguely recall that that was clarified in some way by you, but didn't
>> find the relevant E-Mail. Something like Google's lawyers said something
>> to the effect that this could just be added to git.git, no? Maybe that's
>> incorrect, I don't remember.
>
> You're right, I didn't update the thread. The outcome is that the
> library for now lives in https://github.com/hanwen/reftable while it's
> not integrated in git.git. In this repo I can accept contributions
> (including fixup changes posted to the git list) without requiring
> CLAs.

And once this lands if there's patches to reftable/* on this ML?

Knowing would be good, a change to Documentation/SubmittingPatches et al
in the next iteration even better :)

>> I.e. if it's meant to be 100% externally maintained and "code-dumped"
>> into git.git like we do with sha1collisiondetection/ that raises one set
>> of concerns, but if it's meant to be "eaten" by git.git that poses
>> another set of questions for the code review here. I.e. much of what
>> you're doing in later patches in this series is introducing things that
>> are redundant/odd if viewed if we're supposing that this code is meant
>> to live in git.git.
>
> The code is meant to be eaten by git.git, but be amenable to being
> used by libgit2 with minimal modifications. To that end, it tries to
> use just functionality offered by git-compat-util.
>
> If you could point out specific oddities, that would be helpful.

Things like different coding style, e.g. $(git grep '[!=]= NULL' --
reftable/) & comment style.

Having Go-ish (or Google-ish?) *_test.c files instead of that living in
t/helper/*.

reftable/dump.c not using parse_options().

This having reftable_free() etc., which at least for in-tree use is
redundant, see 8d128513429 (grep/pcre2: actually make pcre2 use custom
allocator, 2021-02-18).

Wondering if internal libraries (e.g. in basic.c) are there to avoid
various kitchen-sink things existing in git.git itself, or because no
existing thing fit the purpose.

I'm not saying those things aren't OK or the "sorta-external" trade-off
needs to change, just that reviewing it not knowing what the expected
status is left me hanging.

> As said earlier, the BSD license here is the most liberal I could
> find, but I'm open to something else, if that makes integration in
> both git and libgit2 possible. This was not discussed further, and
> hence the "TODO: relicense?" comment.

I don't per-se have issues with the license, just questions about what
it means for the development flow.

Let's say Junio merges this to master, and someone discovers a bug in
it. Surely one of these happen:

 1. We just take the patch, if it's a tree-wide patch someone at
    reftable.git would need to either look at GPLv2 patch or clean-room
    duplicate it somehow.

 2. Junio refuses any suggested changes to the reftable/* directory, we
    tell people to submit an upstream PR, once that lands we update. So
    like sha1collisiondetection/ (well, sans the "Junio refusing", I
    think I un-forked that at least a couple of times).

One of those means that e.g. if the change is to a strbuf API reftable/*
needs we'd need to version that API or otherwise treat it as stable.

As an aside, to your [1] again: At least to me reading a series on-list
that has TODO comments in commit messages very much sounds like an RFC
but not calling itself that, maybe I'm alone in that, but if not
clearing this sort of thing up / documenting it might go far in getting
more reviews.

1. https://lore.kernel.org/git/CAFQ2z_P=MqT81gLjov6A471Z9sd69qQTep8KG8M8=LO9pJtkpQ@mail.gmail.com/