Re: [PATCH v5 00/15] Upstreaming the Scalar command

On Thu, Oct 07 2021, Johannes Schindelin via GitGitGadget wrote:

 * The OBJECTS list in the Makefile will now include Scalar.

So that looks like a partial fix for what I brought up in [1] [...]

Range-diff vs v4:

  1:  852ec003109 !  1:  7119a8efc21 scalar: create a rudimentary executable
     @@ Commit message
          Signed-off-by: Johannes Schindelin [off-list ref]
      
       ## Makefile ##
     -@@ Makefile: endif
     - .PHONY: objects
     - objects: $(OBJECTS)
     - 
     +@@ Makefile: OBJECTS += $(FUZZ_OBJS)
     + ifndef NO_CURL
     + 	OBJECTS += http.o http-walker.o remote-curl.o
     + endif
     ++
      +SCALAR_SOURCES := contrib/scalar/scalar.c
      +SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)
      +OBJECTS += $(SCALAR_OBJECTS)
      +
     - dep_files := $(foreach f,$(OBJECTS),$(dir $f).depend/$(notdir $f).d)
     - dep_dirs := $(addsuffix .depend,$(sort $(dir $(OBJECTS))))
     + .PHONY: objects
     + objects: $(OBJECTS)

Except that this & contrib/scalar/Makefile is still broken in multiple
ways. We now have two Makefiles that can build contrib/scalar/scalar:

    touch advice.h; make -j8 contrib/scalar/scalar

But try:

    $ touch advice.h; (cd contrib/scalar 2>/dev/null && make scalar)
    make: 'scalar' is up to date.

I.e. (I'm presuming in response to what I brought up in [1]) the
depenency graph in the top-level Makefile is correct in this specific
area. But it understands the ".depends" files (depending on
COMPUTE_HEADER_DEPENDENCIES), your sub-Makefile doesn't.

There's similar whack-a-mole issues in other areas, e.g.:

    make -C contrib/scalar/ test

Will break or not depending on whether you've built the top-level
git.

I noticed at least one other subtle breakage (first thing I checked
after those two).

I'm happy to send you a working patch to integrate that fixes all these
issues, it also integrates with "make install", this series leaves us
with a "scalar" binary, but no way to install it, if we just piggy-back
on the existing installation procedure.

The side-thread on the v3[3] that you most recently replied to is
conflating some suggestion of shipping this as a built-in, with the
purely build-system implementation details I'm suggesting here.

I did mention using it as a built-in in [4], but for the semi-related
issue of scalar.c copy/pasting less code from git.c. But that was in the
context of such a thing being purely a non-visible implementation
detail. I.e. it would still be "scalar", not "git scalar".

*That* suggestion is just a side-musing about whether it would be easier
to teach git.c to inspects its argv and have a special-case for
dispatching to cmd_scalar(), a user would never know the difference. I
think that might also be worthwhile, but I care *way* less about that
than making maintaining the Makefile a hassle, and it's an entirely
orthogonal suggestion.

1. https://lore.kernel.org/git/875yu9iolf.fsf@evledraar.gmail.com/ (local)
2. https://lore.kernel.org/git/87mtofnzv1.fsf@evledraar.gmail.com/ (local)
3. https://lore.kernel.org/git/xmqq1r5qzv35.fsf@gitster.g (local)
4. https://lore.kernel.org/git/87k0jhn0p9.fsf@evledraar.gmail.com/ (local)