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

On Fri, Oct 22 2021, Johannes Schindelin wrote:

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.

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".
 

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.

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.