On Mon, Sep 27 2021, Phillip Wood wrote: > On 26/09/2021 20:03, Ævar Arnfjörð Bjarmason wrote: >> This series is an incremental restart of the now-ejected >> es/config-based-hooks and ab/config-based-hooks-base topics. See [1] >> for a summary of the plan and progression. >> In v2 the "sed" invocation that generates the new hook-list.h has >> been >> changed to be portable under POSIX. See the thread starting at >> https://lore.kernel.org/git/92471ff9-7573-c3e4-e9fd-63a5cbf5738f@xxxxxxxxx/; >> The portability issue is AFAICT theoretical in that any "sed" >> command >> I've tried accepts the old version (I tried the large list of OS's >> listed in [2]), but better safe than sorry. >> Other changes: >> * I noticed that the run-command.h inclusion in transport.c become >> redundant, I removed that and validated the other ones that have >> the new hook.h, they all still need run-command.h. >> * A whitespace change in v1 in a change to the Makefile makes the >> diff for 8/8 easier to read. >> 1. http://lore.kernel.org/git/cover-0.8-00000000000-20210923T095326Z-avarab@xxxxxxxxx >> 2. https://lore.kernel.org/git/87fstt3gzd.fsf@xxxxxxxxxxxxxxxxxxx/ >> [...] >> 8: 80aae4d5c13 ! 8: 7420267ce09 hook-list.h: add a generated list of hooks, like config-list.h >> @@ Makefile: XDIFF_LIB = xdiff/lib.a >> generated-hdrs: $(GENERATED_H) >> @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) >> $(GITLIBS) >> + $(filter %.o,$^) $(LIBS) >> help.sp help.s help.o: command-list.h >> ++hook.sp hook.s hook.o: hook-list.h >> -builtin/help.sp builtin/help.s builtin/help.o: >> config-list.h GIT-PREFIX >> -+hook.sp hook.s hook.o: hook-list.h >> -+ >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX > > This is billed as a whitespace change above but this line has actually > changed since the last version - was that intentional? I think you're mistaken here, it is a whitespace-only change to the end-state, but the diff and range-diff are confusing. If I diff the two Makefiles I end up with when applying v1 and v2 I get: @@ -2217,7 +2210,6 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(filter %.o,$^) $(LIBS) help.sp help.s help.o: command-list.h - hook.sp hook.s hook.o: hook-list.h builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX I.e. only the line between the old command-list.h and new hook-list.h line is gone. But the diff for v1 is D/A/A/A and for v2 A/D/A (D = Deletion, A = Addition). I.e. it's one of those times when "git diff" produces a valid diff, and one that's actually smaller than couldu have been produced with a v2-like diff given the change in v1. As an aside I've sometimes wished we had a --diff-algorithm=maximal or something, i.e. there's a lot of cases where by just adding one line to the diff you can produce a bigger but IMO less confusing one. In any case, the end state looks better & the diff for v2 is more intuitive to look at. >> builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ >> '-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \ >> @@ generate-hooklist.sh (new) >> +static const char *hook_name_list[] = { >> +EOF >> + >> -+sed -n -e '/^~~~~*$/ {x; s/^.*$/ "&",/; p;}; x' \ >> ++sed -n \ >> ++ -e '/^~~~~*$/ {x; s/^.*$/ "&",/; p;}' \ >> ++ -e 'x' \ >> + <Documentation/githooks.txt | >> + LC_ALL=C sort >> + > > The sed change looks good Thanks for confirming.
