Re: What's cooking in git.git (Mar 2021, #07; Mon, 22)

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

On Tue, Mar 23 2021, Junio C Hamano wrote:

Quick notes on my outstanding submissions listed here:

> * ab/fsck-api-cleanup (2021-03-17) 19 commits
>  - fetch-pack: use new fsck API to printing dangling submodules
>  - fetch-pack: use file-scope static struct for fsck_options
>  - fetch-pack: don't needlessly copy fsck_options
>  - fsck.c: move gitmodules_{found,done} into fsck_options
>  - fsck.c: add an fsck_set_msg_type() API that takes enums
>  - fsck.c: pass along the fsck_msg_id in the fsck_error callback
>  - fsck.[ch]: move FOREACH_FSCK_MSG_ID & fsck_msg_id from *.c to *.h
>  - fsck.c: give "FOREACH_MSG_ID" a more specific name
>  - fsck.c: undefine temporary STR macro after use
>  - fsck.c: call parse_msg_type() early in fsck_set_msg_type()
>  - fsck.h: re-order and re-assign "enum fsck_msg_type"
>  - fsck.h: move FSCK_{FATAL,INFO,ERROR,WARN,IGNORE} into an enum
>  - fsck.c: refactor fsck_msg_type() to limit scope of "int msg_type"
>  - fsck.c: rename remaining fsck_msg_id "id" to "msg_id"
>  - fsck.c: move definition of msg_id into append_msg_id()
>  - fsck.c: rename variables in fsck_set_msg_type() for less confusion
>  - fsck.h: use "enum object_type" instead of "int"
>  - fsck.h: use designed initializers for FSCK_OPTIONS_{DEFAULT,STRICT}
>  - fsck.c: refactor and rename common config callback
>
>  Fsck API clean-up.
>  cf. <20210317182054.5986-1-avarab@gmail.com>

Re-rolled just now;
https://lore.kernel.org/git/cover-00.20-00000000000-20210328T130947Z-avarab@gmail.com/

> * ab/tests-cleanup-around-sha1 (2021-03-10) 4 commits
>  - tests: get rid of $_x05 from the test suite
>  - shortlog tests: rewrite to get rid of --abbrev=35 hardcoding
>  - test-lib: remove unused $_x40 and $_z40 variables
>  - git-bisect: remove unused SHA-1 $x40 shell variable
>
>  Remove variables that hold regexp and glob that match fixed number
>  of hexadecimal digits from the test suite.
>
>  Expecting a reroll.
>  At least the last one weakens a test and should be dropped; there
>  may be similar breakage due to not understanding what they are
>  trying to test.

Looking at those patches again I think it would be acceptable to merge
this having peeled off either 4/4 or that patch and 3/4?

But as noted in that discussion I submitted these thinking some other
abbrev patches of mine had landed, but I misrecalled that and they never
did.

I have a version of those locally, so I'd rather just focus on that. So
if you're not interested in peeling off the first 2 or 3 patches of this
for the trivial cleanuplet's just drop this one.

> * ab/make-cocci-dedup (2021-03-22) 4 commits
>  - Makefile/coccicheck: set SPATCH_BATCH_SIZE to 8
>  - Makefile/coccicheck: allow for setting xargs concurrency
>  - Makefile/coccicheck: speed up and fix bug with duplicate hunks
>  - Makefile/coccicheck: add comment heading for all SPATCH flags
>
>  Coccicheck update.
>
>  cf. <cover.1616414951.git.avarab@gmail.com>

Should be dropped per
https://lore.kernel.org/git/87k0psnzv5.fsf@evledraar.gmail.com/

> * ab/unexpected-object-type (2021-03-08) 7 commits
>  - tag: don't misreport type of tagged objects in errors
>  - object tests: add test for unexpected objects in tags
>  - object.c: add a utility function for "expected type X, got Y"
>  - tree.c: fix misindentation in parse_tree_gently()
>  - oid_object_info(): return "enum object_type"
>  - object.c: make type_from_string() return "enum object_type"
>  - object.c: refactor type_from_string_gently()
>
>  Error reporting upon object type mismatch has been improved
>
>  Expecting a reroll.
>  Looked good except for some rewrites.

I have a re-roll of this at
https://lore.kernel.org/git/cover-00.11-00000000000-20210328T021238Z-avarab@gmail.com/

It fixes the rewrites, but now there's some other outstanding feedback
on code in v1 (but not commented on at the time)...

> * ab/describe-tests-fix (2021-03-01) 10 commits
>  - test-lib: return 1 from test_expect_{success,failure}
>  - svn tests: refactor away a "set -e" in test body
>  - svn tests: remove legacy re-setup from init-clone test
>  - describe tests: support -C in "check_describe"
>  - describe tests: fix nested "test_expect_success" call
>  - describe tests: convert setup to use test_commit
>  - test-lib functions: add an --annotated-tag option to "test_commit"
>  - describe tests: always assert empty stderr from "describe"
>  - describe tests: refactor away from glob matching
>  - describe tests: improve test for --work-tree & --dirty
>
>  Various updates to tests around "git describe"
>
>  Expecting a reroll.
>  cf. <xmqq1rcj6hzr.fsf@gitster.g>

Hoping to re-roll this soon...

> * ab/make-cleanup (2021-02-23) 5 commits
>   (merged to 'next' on 2021-03-22 at e9e16c9fc4)
>  + Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets
>  + Makefile: split OBJECTS into OBJECTS and GIT_OBJS
>  + Makefile: sort OBJECTS assignment for subsequent change
>  + Makefile: split up long OBJECTS line
>  + Makefile: guard against TEST_OBJS in the environment
>
>  Originally merged to 'next' on 2021-03-19
>
>  Reorganize Makefile to allow building git.o and other essential
>  objects without extra stuff needed only for testing.
>
>  Will merge to 'master'.

Thanks, I'd seen before that that merge peeled off (Makefile: build
"$(FUZZ_OBJS)" in CI, not under "all", 2021-01-28) per list discussion
about wanting to have a canary for the fuzz objects.

Is there any interest in an "all" target that's narrowly tailored to
just build the git you asked for via other options? Maybe "make all
NO_CANARY=Y" ?

> * ab/pickaxe-pcre2 (2021-02-18) 24 commits
>  - pickaxe -G: don't special-case create/delete
>  - pickaxe -G: terminate early on matching lines
>  - xdiff-interface: support early exit in xdiff_outf()
>  - xdiff-interface: allow early return from xdiff_emit_{line,hunk}_fn
>  - pickaxe -S: slightly optimize contains()
>  - pickaxe: rename variables in has_changes() for brevity
>  - pickaxe -S: support content with NULs under --pickaxe-regex
>  - pickaxe: assert that we must have a needle under -G or -S
>  - pickaxe: refactor function selection in diffcore-pickaxe()
>  - perf: add performance test for pickaxe
>  - pickaxe/style: consolidate declarations and assignments
>  - diff.h: move pickaxe fields together again
>  - pickaxe: die when --find-object and --pickaxe-all are combined
>  - pickaxe: die when -G and --pickaxe-regex are combined
>  - pickaxe tests: test for -G, -S and --find-object incompatibility
>  - pickaxe tests: add test for "log -S" not being a regex
>  - pickaxe tests: add test for diffgrep_consume() internals
>  - pickaxe tests: refactor to use test_commit --append --printf
>  - test-lib functions: add --printf option to test_commit
>  - test-lib-functions: reword "test_commit --append" docs
>  - test-lib-functions: document and test test_commit --no-tag
>  - grep/pcre2 tests: reword comments referring to kwset
>  - Merge branch 'jk/rev-list-disk-usage' into ab/pickaxe-pcre2
>  - Merge branch 'ab/test-lib' into ab/pickaxe-pcre2
>
>  Rewrite the backend for "diff -G/-S" to use pcre2 engine when
>  available.
>
>  Ready???

I think it's been ready for while, but unfortunately it hasn't gotten
much/any review.

The changes to the C code are all rather trivial (just the "change
return; to return 0" is rather verbose). 

> * ab/tree-walk-with-object-type (2021-03-17) 32 commits
>  . tree-walk.h API: add a tree_entry_extract_type() function
>  . blame: emit a better error on 'git blame directory'
>  . tree-walk.h API: add a get_tree_entry_path() function
>  . tree-walk.h API: add get_tree_entry_all()
>  . tree-walk.h API: add a tree_entry_extract_all() function
>  . tree-entry.h API: rename tree_entry_extract() to tree_entry_extract_mode()
>  . tree-walk.h API: document and format tree_entry_extract()
>  . tree-walk.h API: add get_tree_entry_type()
>  . match-trees: use "tmp" for mode in shift_tree_by()
>  . tree-walk.h API: rename get_tree_entry() to get_tree_entry_mode()
>  . tree-walk.h API: formatting changes for subsequent commit
>  . tree-walk.h users: use temporary variable(s) for "mode"
>  . fsck.c: switch on "object_type" in fsck_walk_tree()
>  . merge-ort: correct reference to test in 62fdec17a11
>  . merge-tree tests: test for the mode comparison in same_entry()
>  . tree-walk.h users: migrate miscellaneous "mode" to "object_type"
>  . tree-walk.h users: refactor chained "mode" if/else into switch
>  . tree-walk.h users: migrate "p->mode &&" pattern
>  . tree.h API: make read_tree_fn_t take an "enum object_type"
>  . archive: get rid of 'stage' parameter
>  . tree.h users: format argument lists in archive.c
>  . tree.h: format argument lists of read_tree_recursive() users
>  . tree-walk.h users: switch object_type(...) to new .object_type
>  . cache.h: have base_name_compare() take "is tree?", not "mode"
>  . diff tests: test that "mode" is passed when sorting
>  . mktree tests: test that "mode" is passed when sorting
>  . fast-import tests: test for sorting dir/file foo v.s. foo.txt
>  . tree-walk.c: migrate to using new "object_type" field when possible
>  . tree-walk.h: add object_type member to name_entry
>  . cache.h: add a comment to object_type()
>  . notes & match-trees: use name_entry's "pathlen" member
>  . diff.c: remove redundant canon_mode() call
>  (this branch uses ab/read-tree.)
>
>  Code clean-up.
>
>  I am not exactly sure where this series wants to go, other than
>  unnecessarily churning the code.  Seems to break "diff --no-index"
>  rather badly, too (e.g. t4050, t4002, among others).

I see this got ejected from "seen" around the time of the release.

As noted in
https://lore.kernel.org/git/87o8fcqrg8.fsf@evledraar.gmail.com/ I do
think it's a useful eventual step to simplify the various API use around
tree-walk to the desires of its callers (who really care about object
types, not what mode bits *say* about object types). As noted this
doesn't do that yet, but is necessary for that.

More importantly, I do need the improved tree-walk API this is leading
up to for fixing a long standing bug in us not validating tree mode bits
in fsck.