On 27/09/2021 11:38, Ævar Arnfjörð Bjarmason wrote:
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,
Yes you're right, looking more closely it is a context line in the range
diff that has changed, sorry.
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.
Yes especially when it splits the diff just to keep an empty line or
closing brace as context even though the line itself is not really
interesting. Python's diff-lib has an option to ignore "junk" when
determining which lines are unchanged, I keep meaning to see how it
affects the diff but have never got round to it (the library uses a
different slower algorithm for calculating the diff as well).
Best Wishes
Phillip
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.
