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 <johannes.schindelin@xxxxxx>
>
> ## 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@xxxxxxxxxxxxxxxxxxx/
2. https://lore.kernel.org/git/87mtofnzv1.fsf@xxxxxxxxxxxxxxxxxxx/
3. https://lore.kernel.org/git/xmqq1r5qzv35.fsf@gitster.g
4. https://lore.kernel.org/git/87k0jhn0p9.fsf@xxxxxxxxxxxxxxxxxxx/
