Re: [PATCH v3 00/15] [RFC] Upstreaming the Scalar command
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2021-09-13 15:28:24
Subsystem:
documentation, kernel build + files below scripts/ (unless maintained elsewhere), the rest · Maintainers:
Jonathan Corbet, Nathan Chancellor, Nicolas Schier, Linus Torvalds
On Thu, Sep 09 2021, Ævar Arnfjörð Bjarmason wrote:
On Wed, Sep 08 2021, Johannes Schindelin via GitGitGadget wrote:quoted
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 ScalarIn 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/ (local) In addition the discussion ending here: https://lore.kernel.org/git/nycvar.QRO.7.76.6.2109082112270.55@tvgsbejvaqbjf.bet/ (local) 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 (local);
2. https://lore.kernel.org/git/87mtoxwt63.fsf@evledraar.gmail.com/ (local)
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 $@