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

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

On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Sep 08 2021, Johannes Schindelin via GitGitGadget wrote:
>
>> Changes since v2:
>>
>>  * Adjusted the description of the list command in the manual page , as
>>    suggested by Bagas.
>>  * Addressed two style nits in cmd_run().
>>  * The documentation of git reconfigure -a was improved.
>>
>> Changes since v1:
>>
>>  * A couple typos were fixed
>>  * The code parsing the output of ls-remote was made more readable
>>  * The indentation used in scalar.txt now consistently uses tabs
>>  * We no longer hard-code core.bare = false when registering with Scalar
>
> In the summary I had on v1->v2 points 1-3 are for v2->v3, respectively,
> outstanding, addressed, outstanding:
>
>     https://lore.kernel.org/git/877dfupl7o.fsf@evledraar.gmail.com/
>
> In addition the discussion ending here:
> https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109082112270.55@tvgsbejvaqbjf.bet/
>
> For that point: I think it's fair enough not to properly handle the
> cleanup case in "scalar clone", but perhaps add a note in the commit
> message that unlike "git clone" this is known not to clean after itself
> properly on ctrl+c?

Seeing [1] about the planned re-roll I have the above a shot a few days
ago, see the original discussion at [2] (indirectly linked above).

The dependency graph isn't quite there yet, but I basically had it
working (some twiddling around adding a new top-level command name
needed). Result:
    
     .gitignore             |  1 +
     Documentation/Makefile |  4 ++++
     Makefile               | 27 +++++++++++++++++++++++++++
     3 files changed, 32 insertions(+)

And by comparison, your v3:
    
     Makefile                  |  8 +++++
     contrib/scalar/.gitignore |  5 +++
     contrib/scalar/Makefile   | 57 ++++++++++++++++++++++++++++++++++
     contrib/scalar/t/Makefile | 78 +++++++++++++++++++++++++++++++++++++++++++++++
     4 files changed, 148 insertions(+)

So that's a very pleasing reduction in complexity.

The WIP change for that is below, some oddities like the
s/scalarscalar/scalar/ (couldn't find the bit that generated that
built-in blurb at the time).

But for one the automatic lint integration in Documentation/ found one
nit, and "make test" etc. all work with this automatically.

All guarded behind a CONTRIB_SCALAR flag, so the end result isn't in any
way different.

Again, as pointed out in [2] the proposal isn't in any way to change
what an end user sees, just to make our build system less
complex. Stretching dependency graphs across Makefiles is a pain.

So just as an example I was improving the "sparse" and "hdr-check"
targets today, with this dropped into the main Makefile under a flag
stuff like that will Just Work, if it's under its own Makefile
infrastructure it'll need to be treated specially for every such change,
ditto "make TAGS" etc.

1. http://lore.kernel.org/git/nycvar.QRO.7.76.6.2109131531210.55@tvgsbejvaqbjf.bet;
2. https://lore.kernel.org/git/87mtoxwt63.fsf@evledraar.gmail.com/

diff --git a/.gitignore b/.gitignore
index 311841f9bed..491cb2177af 100644
--- a/.gitignore
+++ b/.gitignore
@@ -216,6 +216,7 @@
 /configure
 /.vscode/
 /tags
+/scalar
 /TAGS
 /cscope*
 /compile_commands.json
diff --git a/Documentation/Makefile b/Documentation/Makefile
index f5605b7767f..f0a03faf40f 100644
--- a/Documentation/Makefile
+++ b/Documentation/Makefile
@@ -19,6 +19,10 @@ MAN1_TXT += git.txt
 MAN1_TXT += gitk.txt
 MAN1_TXT += gitweb.txt
 
+ifdef CONTRIB_SCALAR
+MAN1_TXT += scalar.txt
+endif
+
 # man5 / man7 guides (note: new guides should also be added to command-list.txt)
 MAN5_TXT += gitattributes.txt
 MAN5_TXT += githooks.txt
diff --git a/Makefile b/Makefile
index 429c276058d..7407df45b2a 100644
--- a/Makefile
+++ b/Makefile
@@ -584,6 +584,7 @@ FUZZ_OBJS =
 FUZZ_PROGRAMS =
 GIT_OBJS =
 LIB_OBJS =
+NONGIT_PROGRAM_OBJS =
 OBJECTS =
 PROGRAM_OBJS =
 PROGRAMS =
@@ -691,10 +692,17 @@ PROGRAM_OBJS += shell.o
 .PHONY: program-objs
 program-objs: $(PROGRAM_OBJS)
 
+ifdef CONTRIB_SCALAR
+NONGIT_PROGRAM_OBJS += scalar.o
+endif
+.PHONY: nongit-program-objs
+nongit-program-objs: $(NONGIT_PROGRAM_OBJS)
+
 # Binary suffix, set to .exe for Windows builds
 X =
 
 PROGRAMS += $(patsubst %.o,git-%$X,$(PROGRAM_OBJS))
+PROGRAMS += $(patsubst %.o,%$X,$(NONGIT_PROGRAM_OBJS))
 
 TEST_BUILTINS_OBJS += test-advise.o
 TEST_BUILTINS_OBJS += test-bitmap.o
@@ -800,6 +808,9 @@ BINDIR_PROGRAMS_NEED_X += git-receive-pack
 BINDIR_PROGRAMS_NEED_X += git-shell
 BINDIR_PROGRAMS_NEED_X += git-upload-archive
 BINDIR_PROGRAMS_NEED_X += git-upload-pack
+ifdef CONTRIB_SCALAR
+BINDIR_PROGRAMS_NEED_X += scalar
+endif
 
 BINDIR_PROGRAMS_NO_X += git-cvsserver
 
@@ -1247,6 +1258,10 @@ else
 ALL_COMMANDS_TO_INSTALL += git-receive-pack$(X)
 ALL_COMMANDS_TO_INSTALL += git-upload-archive$(X)
 ALL_COMMANDS_TO_INSTALL += git-upload-pack$(X)
+
+ifdef CONTRIB_SCALAR
+ALL_COMMANDS_TO_INSTALL += scalar$(X)
+endif
 endif
 
 ALL_CFLAGS = $(DEVELOPER_CFLAGS) $(CPPFLAGS) $(CFLAGS)
@@ -2459,6 +2474,7 @@ git-objs: $(GIT_OBJS)
 
 OBJECTS += $(GIT_OBJS)
 OBJECTS += $(PROGRAM_OBJS)
+OBJECTS += $(NONGIT_PROGRAM_OBJS)
 OBJECTS += $(TEST_OBJS)
 OBJECTS += $(XDIFF_OBJS)
 OBJECTS += $(FUZZ_OBJS)
@@ -2582,6 +2598,9 @@ compat/nedmalloc/nedmalloc.sp compat/nedmalloc/nedmalloc.o: EXTRA_CPPFLAGS = \
 compat/nedmalloc/nedmalloc.sp: SP_EXTRA_FLAGS += -Wno-non-pointer-null
 endif
 
+ifdef CONTRIB_SCALAR
+# TODO: Implicit rule for git-scalar here
+endif
 git-%$X: %.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
 
@@ -2596,6 +2615,11 @@ git-http-push$X: http.o http-push.o GIT-LDFLAGS $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
 		$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
 
+ifdef CONTRIB_SCALAR
+scalar: scalar.o $(GITLIBS)
+	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) $(LIBS)
+endif
+
 $(REMOTE_CURL_ALIASES): $(REMOTE_CURL_PRIMARY)
 	$(QUIET_LNCP)$(RM) $@ && \
 	ln $< $@ 2>/dev/null || \
@@ -2800,6 +2824,7 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo SHELL_PATH=\''$(subst ','\'',$(SHELL_PATH_SQ))'\' >$@+
 	@echo TEST_SHELL_PATH=\''$(subst ','\'',$(TEST_SHELL_PATH_SQ))'\' >>$@+
 	@echo PERL_PATH=\''$(subst ','\'',$(PERL_PATH_SQ))'\' >>$@+
+	@echo CONTRIB_SCALAR=\''$(subst ','\'',$(subst ','\'',$(CONTRIB_SCALAR)))'\' >>$@+
 	@echo DIFF=\''$(subst ','\'',$(subst ','\'',$(DIFF)))'\' >>$@+
 	@echo PYTHON_PATH=\''$(subst ','\'',$(PYTHON_PATH_SQ))'\' >>$@+
 	@echo TAR=\''$(subst ','\'',$(subst ','\'',$(TAR)))'\' >>$@+
@@ -2881,6 +2906,8 @@ bin-wrappers/%: wrap-for-bin.sh
 	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
 	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%$(X),$(@F))$(patsubst git%,$(X),$(filter $(@F),$(BINDIR_PROGRAMS_NEED_X)))|' < $< > $@ && \
+	sed -e 's|scalarscalar|scalar|' <$@ >$@+ && \
+	mv $@+ $@ && \
 	chmod +x $@