On Thu, Jun 23 2022, Derrick Stolee wrote:
> On 6/23/2022 6:26 AM, Ævar Arnfjörð Bjarmason wrote:
>> This one-patch series integrates the "scalar" command to the
>> top-level, meaning we build the "scalar" binary by default, and run
>> its tests on "make test" and in CI. We'll also build and test its
>> documentation. We now also have "install" support, both for the
>> program and its docs, but you'll need to:
>>
>> make <install-target> INSTALL_SCALAR=Y
>>
>> I'm sending this out now to avoid needless duplicate work.
>
> As mentioned on the list earlier, Victoria is taking over the
> remaining work to complete the Scalar project. Nothing has been
> sent to the list because we didn't want to cause a distraction
> from the release window.
I was on the fence about sending this out, but given that the "CI"
thread was going on until the start of that window, and wanting to save
her the work of re-discovering the subtle issues with the integration
I'd already fixed I thought it was better ot send it out.
> Victoria is taking time to incorporate your previous thoughts on
> how Scalar is built and its location in the codebase and create
> a complete narrative of how to get from our current state to that
> point.
I wasn't sure she was even aware of it, and given that the WIP patch I
saw in my "git fetch" was pretty much a subset of the upthread v1 it
seemed that there was needless duplicate work going on.
It seemed clear that that WIP patch was attempting to head in the same
direction, but hadn't yet discovered some of the hurdles with
e.g. documentation building & installation that I'd fixed
already. There's also the CMake integration, which I finished up for
this v2.
> To that point, this thread is the duplicate work you're trying
> to avoid. Please instead wait for Victoria to present her plan in
> July instead of moving forward with this topic.
By "duplicate work" I mean that back in October of last year I sent a
patch that was a more complete version of some WIP code I spotted
written in the past few days.
But you're probably talking about duplication in the sense of some
larger integration of scalar changes into git.git? Or at least that's
the only way I can make sense of the seemingly reversed chronology.
I'm all too happy to leave that to someone else that's more interested,
perhaps this will save them some time.
Although (sans release window etc.) I think this change is in a state
that's ready for pickup, and it seems that here's a consensus on this
direction from everyone involved.
Although perhaps there's some minor disagreement about details, such as
whether we should have an "optionalcontrib" documentation section etc.
I do think the current state on "master" is slightly confusing, one
oddity I spotted is that we've already asked translators to translate
contrib/scalar/scalar.c, but we never install it, and it's "in
contrib".
It seems that part was unintentional in 0a43fb22026 (scalar: create a
rudimentary executable, 2021-12-03), and later formalized in 9f555783c0b
(Makefile: generate "po/git.pot" from stable LOCALIZED_C,
2022-05-26). Maybe it's all water under the bridge at this point,
i.e. we'd rather keep it than throw away now-existing translations...
FWIW I have this local change queued on top of this v2, it's all
cosmetic, but probably a good idea.
The $(SCALAR_SOURCES) bit is something I missed, but which Victoria
didn't in her WIP patch (I stole it from there). That part had been
added since October, so I managed to miss it when rebasing, it's the
part that's been adding contrib/scalar/scalar.c to po/git.pot (see
9f555783c0b).
1: 9743e2a1e6a ! 1: 11404988785 scalar: reorganize from contrib/, still keep it "a contrib command"
@@ Commit message
## .gitignore ##
@@
+ /config.mak.append
/configure
/.vscode/
- /tags
+/scalar
+ /tags
/TAGS
/cscope*
- /compile_commands.json
## Documentation/Makefile ##
@@ Documentation/Makefile: MAN1_TXT += $(filter-out \
@@ Makefile: ifndef NO_CURL
endif
-SCALAR_SOURCES := contrib/scalar/scalar.c
-+SCALAR_SOURCES := scalar.c
- SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)
+-SCALAR_OBJECTS := $(SCALAR_SOURCES:c=o)
++SCALAR_OBJECTS := scalar.o
OBJECTS += $(SCALAR_OBJECTS)
+ .PHONY: objects
@@ Makefile: $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS $(GITLIBS
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $(filter %.o,$^) \
$(CURL_LIBCURL) $(EXPAT_LIBEXPAT) $(LIBS)
@@ Makefile: $(REMOTE_CURL_PRIMARY): remote-curl.o http.o http-walker.o GIT-LDFLAGS
$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
$(filter %.o,$^) $(LIBS)
+@@ Makefile: XGETTEXT_FLAGS_SH = $(XGETTEXT_FLAGS) --language=Shell \
+ XGETTEXT_FLAGS_PERL = $(XGETTEXT_FLAGS) --language=Perl \
+ --keyword=__ --keyword=N__ --keyword="__n:1,2"
+ MSGMERGE_FLAGS = --add-location --backup=off --update
+-LOCALIZED_C = $(sort $(FOUND_C_SOURCES) $(FOUND_H_SOURCES) $(SCALAR_SOURCES) \
+- $(GENERATED_H))
++LOCALIZED_C = $(sort $(FOUND_C_SOURCES) $(FOUND_H_SOURCES) $(GENERATED_H))
+ LOCALIZED_SH = $(sort $(SCRIPT_SH) git-sh-setup.sh)
+ LOCALIZED_PERL = $(sort $(SCRIPT_PERL))
+
@@ Makefile: GIT-PYTHON-VARS: FORCE
fi
endif
