Re: js/scalar, was Re: What's cooking in git.git (Oct 2021, #05; Mon, 18)

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Fri, Oct 22 2021, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Fri, 22 Oct 2021, Ævar Arnfjörð Bjarmason wrote:
>
>> On Thu, Oct 21 2021, Johannes Schindelin wrote:
>>
>> > tl;dr it isn't worth your nor my time for you to focus on the build
>> > process in contrib/scalar/ at this moment.
>
> I still stand by these words, and I think you completely glanced over this
> problem. Your focus seems to lie exclusively on those "dependency
> problems". But apart from you, who cares if `libgit.a` is not rebuilt in
> obscure and rare circumstances _when building something in `contrib/`_?[...]

You glanced over and didn't reply to the "This invariant that something
must be[...]" part of the E-Mail you're replying to, which I think gets
to the crux of the issue.

I.e. you haven't answered why you actively prefer a broken build system
integration when a non-broken and less complex one is available, other
than some vague and unclear references to the effect that "it's a
contrib thing", without really explaining what that means, or why it
necessitates a broken and more complex build system.

It's not that libgit.a isn't rebuilt, that part isn't broken, it's built
by the top-level Makefile. It's that if you:

    make -C contrib/scalar

And then change anything it depends on, short of the top-level
rebuilding libgit.a it won't be re-built, so for anyone hacking on it
that Makefile is rather useless. It *does* work to do:

    make contrib/scalar/scalar

Because that uses the top-level Makefile, will inspect
contrib/scalar/.depends etc. So your patches already mostly make
contrib/scalar/Makefile redundant anyway, hence my question about (and
patches) why we can't just drop it entirely.

As an aside your commit message instructs users to run:

     make -C contrib/scalar/Makefile

But that command can't work (you either meant -f <path>, or -C <dir>).

You keep narrowly focusing on the bit where your *.o dependencies are
broken, but I've pointed out a bunch of other things that are broken,
including but not limited to: "make TAGS coccicheck check-docs install
install-doc" etc.

> [...]The code in `contrib/scalar/` is transitional.[...]

OK. But it's being proposed for merge to "master", I don't see how "we
don't expect this to stay for long" translates into "it's OK that it's
broken while it's here".
 
> Just forget about Scalar's build process. Forget about getting its CI to
> work. I have all that figured out already. It is all working as well as
> needed.

You're proposing to integrate this into git.git, so it really does
matter that it's useful to others and plays well with existing
components.

When a release of this is available we'd expect third-party packagers to
consider installing it alongside git. If that's not at all the intent
then at the very least the commit messages & documentation are sorely
lacking.

And it's not OK that some in-tree *.c code that uses various bits of
libgit.a and will break if changes to that code don't take it into
account doesn't have meaningful testing CI/integration.

E.g. I was hacking on something the other day, and since I build (my
version of) your scalar patches had a breakage due (my local changes to)
an API where scalar was the one remaining in-tree user of an API
function. Luckily I ran its tests by default, and didn't need to go out
of my way to do so.

All of the above is in the context that I've offered you patches to fix
all these issues.

I haven't sent anything but an inline WIP demo of that to the list
because we're still having this discussion where you're categorically
refusing to even consider fixes to things in your series that are
demonstrably broken.

>> > Having said that, I do appreciate your interest in this patch series, and
>> > I have suggestions at the end of this mail how we could collaborate on it
>> > in a more fruitful manner.
>
> I would still like to invite you to think along more productive lines.
> It's not about where Scalar's build mechanics are right now. It's where we
> can take _Git_ to do what Scalar already does.

That's also interesting, but right now we're not discussing that
subsequent set of patches, but your integration of scalar in-tree. Let's
fix those issues first.