What's cooking in git.git (Nov 2021, #07; Mon, 29)

As promised, most of the topics that have been cooking in 'next' since the previous release cycle have been merged to 'master' to start a new cycle, with a small exception. The tip of 'next' has been rewound and there is 0 topics on it right now.

The exception is 'ns/tmp-objdir' and a few topics that depend on it, which are kicked out of 'next' to give the base topic a chance to lose the two "fixup!" band-aid. Careful re-reading of the topic by somebody with a fresh set of eyes (meaning, everybody, as this has been dormant for almost a month) is very much appreciated to move things forward. Also ns/remerge-diff that is Neeraj's rebase of the remerge-diff topic needs Elijah's Ack at least.

Many of the new patches in 'seen' since the last cycle ended are truly 'just seen' and not 'read' at all, as you can see from the lack of any topic description to speak of in the following list. Help is appreciated here, too. Before whipping them into some presentable shape, I wouldn't be able to take any more new topics.

Here are the topics that have been cooking in my tree. Commits prefixed with '+' are in 'next' (being in 'next' is a sign that a topic is stable enough to be used and are candidate to be in a future release). Commits prefixed with '-' are only in 'seen', which means nothing more than that I have found them of interest for some reason (like "it may have hard-to-resolve conflicts with another topic already in flight" or "this may turn out to be useful"). Do not read too much into a topic being in (or not in) 'seen'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them.

A few topics for regression fixes have been merged to 'next' and will be merged to 'master' shortly. There are many topics listed as "New" that are in 'seen', but I haven't read many of them (and I shouldn't have anyway, before dealing with regression fixes) and haven't gave any summary comments in this report, either, yet.

Copies of the source code to Git live in many repositories, and the following is a list of the ones I push into or their mirrors. Some repositories have only a subset of branches.

With maint, master, next, seen, todo:

	git://git.kernel.org/pub/scm/git/git.git/
	git://repo.or.cz/alt-git.git/
	https://kernel.googlesource.com/pub/scm/git/git/
	https://github.com/git/git/
	https://gitlab.com/git-vcs/git/

With all the integration branches and topics broken out:

	https://github.com/gitster/git/

Even though the preformatted documentation in HTML and man format are not sources, they are published in these repositories for convenience (replace "htmldocs" with "manpages" for the manual pages):

	git://git.kernel.org/pub/scm/git/git-htmldocs.git/
	https://github.com/gitster/git-htmldocs.git/

Release tarballs are available at:

	https://www.kernel.org/pub/software/scm/git/

-------------------------------------------------- [Graduated to 'master']

* ab/refs-errno-cleanup (2021-10-16) 21 commits
  (merged to 'next' on 2021-10-29 at 3f57147176)
 + refs API: post-migration API renaming [2/2]
 + refs API: post-migration API renaming [1/2]
 + refs API: don't expose "errno" in run_transaction_hook()
 + refs API: make expand_ref() & repo_dwim_log() not set errno
 + refs API: make resolve_ref_unsafe() not set errno
 + refs API: make refs_ref_exists() not set errno
 + refs API: make refs_resolve_refdup() not set errno
 + refs tests: ignore ignore errno in test-ref-store helper
 + refs API: ignore errno in worktree.c's find_shared_symref()
 + refs API: ignore errno in worktree.c's add_head_info()
 + refs API: make files_copy_or_rename_ref() et al not set errno
 + refs API: make loose_fill_ref_dir() not set errno
 + refs API: make resolve_gitlink_ref() not set errno
 + refs API: remove refs_read_ref_full() wrapper
 + refs/files: remove "name exist?" check in lock_ref_oid_basic()
 + reflog tests: add --updateref tests
 + refs API: make refs_rename_ref_available() static
 + refs API: make parse_loose_ref_contents() not set errno
 + refs API: make refs_read_raw_ref() not set errno
 + refs API: add a version of refs_resolve_ref_unsafe() with "errno"
 + branch tests: test for errno propagating on failing read

 The "remainder" of hn/refs-errno-cleanup topic.

* ab/sh-retire-helper-functions (2021-10-21) 6 commits
  (merged to 'next' on 2021-11-03 at 4b49d9f4be)
 + git-sh-setup: remove "sane_grep", it's not needed anymore
 + git-sh-setup: remove unused sane_egrep() function
 + git-instaweb: unconditionally assume that gitweb is mod_perl capable
 + Makefile: remove $(NO_CURL) from $(SCRIPT_DEFINES)
 + Makefile: remove $(GIT_VERSION) from $(SCRIPT_DEFINES)
 + Makefile: move git-SCRIPT-DEFINES adjacent to $(SCRIPT_DEFINES)
 (this branch is used by ab/make-dependency.)

 Make a few helper functions unused and then lose them.

* jc/fix-ref-sorting-parse (2021-10-20) 2 commits
  (merged to 'next' on 2021-10-29 at e3ec6e8e77)
 + for-each-ref: delay parsing of --sort=<atom> options
 + Merge branch 'ab/ref-filter-leakfix' into jc/fix-ref-sorting-parse

 Things like "git -c branch.sort=bogus branch new HEAD", i.e. the
 operation modes of the "git branch" command that do not need the
 sort key information, no longer errors out by seeing a bogus sort
 key.

* jc/tutorial-format-patch-base (2021-10-23) 1 commit
  (merged to 'next' on 2021-10-29 at 67ff1a5a77)
 + MyFirstContribution: teach to use "format-patch --base=auto"

 Teach and encourage first-time contributors to this project to
 state the base commit when they submit their topic.

* jc/unsetenv-returns-an-int (2021-10-29) 1 commit
  (merged to 'next' on 2021-11-01 at a11670a5a5)
 + unsetenv(3) returns int, not void

 The compatibility implementation for unsetenv(3) were written to
 mimic ancient, non-POSIX, variant seen in an old glibc; it has been
 changed to return an integer to match the more modern era.

* jk/loosen-urlmatch (2021-10-12) 1 commit
  (merged to 'next' on 2021-10-25 at f66ca39ebe)
 + urlmatch: add underscore to URL_HOST_CHARS

 Treat "_" as any other URL-valid characters in an URL when matching
 the per-URL configuration variable names.

* mc/clean-smudge-with-llp64 (2021-11-03) 8 commits
  (merged to 'next' on 2021-11-03 at fe5160a170)
 + clean/smudge: allow clean filters to process extremely large files
 + odb: guard against data loss checking out a huge file
 + git-compat-util: introduce more size_t helpers
 + odb: teach read_blob_entry to use size_t
 + t1051: introduce a smudge filter test for extremely large files
 + test-lib: add prerequisite for 64-bit platforms
 + test-tool genzeros: generate large amounts of data more efficiently
 + test-genzeros: allow more than 2G zeros in Windows

 The clean/smudge conversion code path has been prepared to better
 work on platforms where ulong is narrower than size_t.

* ow/stash-count-in-status-porcelain-output (2021-10-21) 2 commits
  (merged to 'next' on 2021-10-29 at 1a01b886e5)
 + status: print stash info with --porcelain=v2 --show-stash
 + status: count stash entries in separate function

 Allow "git status --porcelain=v2" to show the number of stash
 entries with --show-stash like the normal output does.

* so/stash-staged (2021-10-28) 2 commits
  (merged to 'next' on 2021-10-29 at 50335e8eeb)
 + stash: get rid of unused argument in stash_staged()
  (merged to 'next' on 2021-10-25 at 68b88e35c4)
 + stash: implement '--staged' option for 'push' and 'save'

 "git stash" learned the "--staged" option to stash away what has
 been added to the index (and nothing else).

* tb/plug-pack-bitmap-leaks (2021-10-28) 9 commits
  (merged to 'next' on 2021-11-01 at 47d218862b)
 + pack-bitmap.c: more aggressively free in free_bitmap_index()
 + pack-bitmap.c: don't leak type-level bitmaps
 + midx.c: write MIDX filenames to strbuf
 + builtin/multi-pack-index.c: don't leak concatenated options
 + builtin/repack.c: avoid leaking child arguments
 + builtin/pack-objects.c: don't leak memory via arguments
 + t/helper/test-read-midx.c: free MIDX within read_midx_file()
 + midx.c: don't leak MIDX from verify_midx_file
 + midx.c: clean up chunkfile after reading the MIDX

 Leakfix.

* tp/send-email-completion (2021-10-28) 2 commits
  (merged to 'next' on 2021-11-01 at 7104356cc0)
 + send-email docs: add format-patch options
 + send-email: programmatically generate bash completions

 The command line complation for "git send-email" options have been
 tweaked to make it easier to keep it in sync with the command itself.

-------------------------------------------------- [New Topics]

* ab/ambiguous-object-name (2021-11-26) 6 commits
 - object-name: re-use "struct strbuf" in show_ambiguous_object()
 - object-name: iterate ambiguous objects before showing header
 - object-name: show date for ambiguous tag objects
 - object-name: make ambiguous object output translatable
 - object-name: explicitly handle OBJ_BAD in show_ambiguous_object()
 - object-name tests: add tests for ambiguous object blind spots

* cb/add-p-single-key-fix (2021-11-28) 1 commit
 - add -p: avoid use of undefined $key when ReadKey -> EOF

* cb/mingw-gmtime-r (2021-11-27) 1 commit
 - mingw: avoid fallback for {local,gm}time_r()

* ds/trace2-regions-in-tests (2021-11-29) 2 commits
 - t/t*: remove custom GIT_TRACE2_EVENT_NESTING
 - test-lib.sh: set GIT_TRACE2_EVENT_NESTING

* hn/allow-bogus-oid-in-ref-tests (2021-11-29) 6 commits
 - t1430: create valid symrefs using test-helper
 - t1430: remove refs using test-tool
 - refs: add REF_SKIP_REFNAME_VERIFICATION flag
 - refs: allow skipping OID verification
 - refs: update comment.
 - test-ref-store: plug memory leak in cmd_delete_refs

* hn/t1404-df-limitation-is-ref-files-only (2021-11-29) 1 commit
 - t1404: mark directory/file conflict tests with REFFILES

* jc/reflog-iterator-callback-doc (2021-11-28) 1 commit
 - refs: document callback for reflog-ent iterators

* po/size-t-for-vs (2021-11-26) 3 commits
 - object-file.c: LLP64 compatibility, upcast unity for left shift
 - diffcore-delta.c: LLP64 compatibility, upcast unity for left shift
 - repack.c: LLP64 compatibility, upcast unity for left shift

* yn/complete-date-format-options (2021-11-25) 1 commit
 - completion: add human and auto: date format

* jc/grep-patterntype-default-doc (2021-11-29) 1 commit
 - grep: clarify what `grep.patternType=default` means

-------------------------------------------------- [Stalled]

* ar/submodule-update (2021-10-13) 9 commits
 . submodule--helper: rename helper functions
 . submodule--helper: remove unused helpers
 . submodule: move core cmd_update() logic to C
 . submodule--helper: run update using child process struct
 . submodule--helper: allow setting superprefix for init_submodule()
 . submodule--helper: refactor get_submodule_displaypath()
 . submodule--helper: rename helpers for update-clone
 . submodule--helper: get remote names from any repository
 . submodule--helper: split up ensure_core_worktree()

 Rewrite of "git submodule update" in C.

 Kicked out of 'seen' to make room for es/superproject-aware-submodules
 which is among the topics this topic stomps on.

* cf/fetch-set-upstream-while-detached (2021-07-06) 1 commit
 - fetch: fix segfault on --set-upstream while on a detached HEAD

 "git fetch --set-upstream" while on detached HEAD segfaulted
 instead of noticing that such an operation did not make sense.

 Getting tired of waiting for a reroll; will discard.

-------------------------------------------------- [Cooking]

* ab/ci-updates (2021-11-23) 5 commits
 - CI: don't run "make test" twice in one job
 - CI: use "$runs_on_pool", not "$jobname" to select packages & config
 - CI: rename the "Linux32" job to lower-case "linux32"
 - CI: use shorter names that fit in UX tooltips
 - CI: remove Travis CI support

 Drop support for TravisCI and update test workflows at GitHub.

* en/keep-cwd (2021-11-29) 11 commits
 - t2501: simplify the tests since we can now assume desired behavior
 - dir: new flag to remove_dir_recurse() to spare the original_cwd
 - dir: avoid incidentally removing the original_cwd in remove_path()
 - stash: do not attempt to remove startup_info->original_cwd
 - rebase: do not attempt to remove startup_info->original_cwd
 - clean: do not attempt to remove startup_info->original_cwd
 - symlinks: do not include startup_info->original_cwd in dir removal
 - unpack-trees: add special cwd handling
 - unpack-trees: refuse to remove startup_info->original_cwd
 - setup: introduce startup_info->original_cwd
 - t2501: add various tests for removing the current working directory

* ab/run-command (2021-11-25) 9 commits
 - run-command API: remove "env" member, always use "env_array"
 - difftool: use "env_array" to simplify memory management
 - run-command API: remove "argv" member, always use "args"
 - run-command API users: use strvec_push(), not argv construction
 - run-command API users: use strvec_pushl(), not argv construction
 - run-command tests: use strvec_pushv(), not argv assignment
 - run-command API users: use strvec_pushv(), not argv assignment
 - upload-archive: use regular "struct child_process" pattern
 - worktree: stop being overly intimate with run_command() internals

* hn/create-reflog-simplify (2021-11-22) 1 commit
 - refs: drop force_create argument of create_reflog API

 A small simplification of API.

 Will merge to 'next'.

* hn/reflog-tests (2021-11-25) 5 commits
 - refs/debug: trim trailing LF from reflog message
 - test-ref-store: tweaks to for-each-reflog-ent format
 - t1405: check for_each_reflog_ent_reverse() more thoroughly
 - test-ref-store: don't add newline to reflog message
 - show-branch: show reflog message

 Prepare tests on ref API to help testing reftable backends.

* jt/midx-doc-fix (2021-11-22) 1 commit
 - Doc: no midx and partial clone relation

 Docfix.

 Will merge to 'next'.

* tl/ls-tree-oid-only (2021-11-22) 1 commit
 - ls-tree.c: support `--oid-only` option for "git-ls-tree"

 "git ls-tree" learns "--oid-only" option, similar to "--name-only".

 Expecting a reroll.

* jk/t7006-sigpipe-tests-fix (2021-11-22) 3 commits
 - t7006: simplify exit-code checks for sigpipe tests
 - t7006: clean up SIGPIPE handling in trace2 tests
 - run-command: unify signal and regular logic for wait_or_whine()

 The function to cull a child process and determine the exit status
 had two separate code paths for normal callers and callers in a
 signal handler, and the latter did not yield correct value when the
 child has caught a signal.  The handling of the exit status has
 been unified for these two code paths.  An existing test with
 flakiness has also been corrected.

 Will merge to 'next'.

* em/missing-pager (2021-11-24) 1 commit
 - pager: fix crash when pager program doesn't exist

* ab/checkout-branch-info-leakfix (2021-11-18) 1 commit
 - checkout: fix "branch info" memory leaks

 Leakfix.

 Will merge to 'next'.

* ab/make-dependency (2021-11-18) 24 commits
 - Makefile: move ".SUFFIXES" rule to shared.mak
 - Makefile: define $(LIB_H) in terms of $(FIND_SOURCE_FILES)
 - Makefile: disable GNU make built-in wildcard rules
 - Makefile: use $(file) I/O instead of "FORCE" when possible
 - Makefile: correct the dependency graph of hook-list.h
 - Makefiles: add and use wildcard "mkdir -p" template
 - Makefile: use $(wspfx) for $(QUIET...) in shared.mak
 - Makefile: add "$(QUIET)" boilerplate to shared.mak
 - Makefile: add a "TRACK_template" for GIT-*{FLAGS,DEFINES,...}
 - Makefile: re-add and use the "shellquote" macros
 - Makefile: move $(comma), $(empty) and $(space) to shared.mak
 - Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
 - Makefile: stop needing @@GIT_VERSION@@ in *.perl scripts
 - Makefile: create a GIT-PYTHON-DEFINES, like "PERL"
 - Makefile: correct "GIT-PERL-{DEFINES,HEADER}" dependency graph
 - Makefile: adjust Perl-related comments & whitespace
 - Makefile: change "ifndef NO_PERL" to "ifdef NO_PERL"
 - Makefile: guard Perl-only variable assignments
 - Makefile: remove "mv $@ $@+" dance redundant to .DELETE_ON_ERROR
 - Makefile: clean perl/build/ even with NO_PERL=Y
 - Makefile: use "=" not ":=" for po/* and perl/*
 - Makefile: don't set up "perl/build" rules under NO_PERL=Y
 - Makefile: don't invoke msgfmt with --statistics
 - Merge branch 'ab/sh-retire-helper-functions' into ab/make-dependency

 Dependency clean-up.

* ah/advice-pull-has-no-preference-between-rebase-and-merge (2021-11-19) 1 commit
 - pull: don't say that merge is "the default strategy"

 The advice message given by "git pull" when the user hasn't made a
 choice between merge and rebase still said that the merge is the
 default, which no longer is the case.  This has been corrected.

 Will merge to 'next'.

* ds/fetch-pull-with-sparse-index (2021-11-29) 3 commits
 - ls-files: add --sparse option
 - fetch/pull: use the sparse index
 - Merge branch 'ld/sparse-diff-blame' into ds/fetch-pull-with-sparse-index
 (this branch uses ld/sparse-diff-blame and vd/sparse-reset.)

* fs/ssh-signing-other-keytypes (2021-11-19) 2 commits
 - ssh signing: make sign/amend test more resilient
 - ssh signing: support non ssh-* keytypes

* fs/test-prereq (2021-11-20) 3 commits
 - test-lib: make BAIL_OUT() work in tests and prereq
 - test-lib: introduce required prereq for test runs
 - test-lib: show missing prereq summary

 The test framework learns to list unsatisfied test prerequisites,
 and optionally error out when prerequisites that are expected to be
 satisfied are not.

 Will merge to 'next'?

* jk/t5319-midx-corruption-test-deflake (2021-11-18) 1 commit
 - t5319: corrupt more bytes of the midx checksum

 Test fix.

 Will merge to 'next'.

* js/trace2-avoid-recursive-errors (2021-11-18) 1 commit
 - trace2: disable tr2_dst before warning on write errors

 trace2 error code path fix.

 Will merge to 'next'.

* pw/xdiff-classify-record-in-histogram (2021-11-18) 3 commits
 - xdiff: simplify comparison
 - xdiff: avoid unnecessary memory allocations
 - diff histogram: intern strings

 "diff --histogram" optimization.

 Will merge to 'next'.

* rs/mergesort (2021-11-18) 1 commit
 - mergesort: avoid left shift overflow

 Bitop fix for platforms whose "long" is 32-bit.

 Will merge to 'next'.

* xw/am-empty (2021-11-22) 2 commits
 - am: support --empty=<option> to handle empty patches
 - doc: git-format-patch: describe the option --always

 "git am" learns "--empty=(die|drop|keep)" option to tweak what is
 done to a piece of e-mail without a patch in it.

 Will merge to 'next'.

* hk/ci-checkwhitespace-commentfix (2021-11-19) 1 commit
 - ci(check-whitespace): update stale file top comments

 Comment fix.

 Will merge to 'next'.

* jk/fetch-pack-avoid-sigpipe-to-index-pack (2021-11-19) 1 commit
 - fetch-pack: ignore SIGPIPE when writing to index-pack

 "git fetch", when received a bad packfile, can fail with SIGPIPE.
 This wasn't wrong per-se, but we now detect the situation and fail
 in a more predictable way.

 Will merge to 'next'.

* jk/refs-g11-workaround (2021-11-19) 1 commit
 - refs: work around gcc-11 warning with REF_HAVE_NEW

 Workaround for a false-alarm by gcc-11

 Will merge to 'next'.

* mp/absorb-submodule-git-dir-upon-deinit (2021-11-19) 1 commit
 - submodule: absorb git dir instead of dying on deinit

 "git submodule deinit" for a submodule whose .git metadata
 directory is embedded in its working tree refused to work, until
 the submodule gets converted to use the "absorbed" form where the
 metadata directory is stored in superproject, and a gitfile at the
 top-level of the working tree of the submodule points at it.  The
 command is taught to convert such submodules to the absorbed form
 as needed.

 Will merge to 'next'?

* rj/receive-pack-avoid-sigpipe-during-status-reporting (2021-11-10) 1 commit
 - receive-pack: ignore SIGPIPE while reporting status to client

 When the "git push" command is killed while the receiving end is
 trying to report what happened to the ref update proposals, the
 latter used to die, due to SIGPIPE.  The code now ignores SIGPIPE
 to increase our chances to run the post-receive hook after it
 happens.

 Will merge to 'next'?

* ab/parse-options-cleanup (2021-11-10) 1 commit
 - parse-options.c: use "enum parse_opt_result" for parse_nodash_opt()

 Change the type of an internal function to return an enum (instead
 of int) and replace -2 that was used to signal an error with -1.

* cw/protocol-v2-doc-fix (2021-11-11) 1 commit
 - protocol-v2.txt: align delim-pkt spec with usage

 Doc update.

 Will merge to 'next'.

* jt/pack-header-lshift-overflow (2021-11-11) 1 commit
 - packfile: avoid overflowing shift during decode

 The code to decode the length of packed object size has been
 corrected.

 Will merge to 'next'.

* bc/require-c99 (2021-11-18) 1 commit
 . git-compat-util: add a test balloon for C99 support

 Weather balloon to break people with compilers that do not support
 C99.

 Expecting a reroll.

* jc/c99-var-decl-in-for-loop (2021-11-22) 1 commit
 - revision: use C99 declaration of variable in for() loop

 Weather balloon to break compilers that do not grok variable
 declaration in the for() loop.

 Will merge to 'next'?

* tl/midx-docfix (2021-11-18) 1 commit
 - midx: fix a formatting issue in "multi-pack-index.txt"

 Doc mark-up fix.

 Will merge to 'next'.

* ak/protect-any-current-branch (2021-11-15) 8 commits
 - branch: protect branches checked out in all worktrees
 - receive-pack: protect current branch for bare repository worktree
 - receive-pack: clean dead code from update_worktree()
 - fetch: protect branches checked out in all worktrees
 - worktree: simplify find_shared_symref() memory ownership model
 - branch: lowercase error messages
 - receive-pack: lowercase error messages
 - fetch: lowercase error messages

 "git fetch" without the "--update-head-ok" option ought to protect
 a checked out branch from getting updated, to prevent the working
 tree that checks it out to go out of sync.  The code was written
 before the use of "git worktree" got widespread, and only checked
 the branch that was checked out in the current worktree, which has
 been updated.
 (originally called ak/fetch-not-overwrite-any-current-branch)

 Will merge to 'next'?

* jk/test-bitmap-fix (2021-11-05) 1 commit
 - test_bitmap_hashes(): handle repository without bitmaps

 Tighten code for testing pack-bitmap.

 Will merge to 'next'.

* jk/jump-merge-with-pathspec (2021-11-09) 1 commit
 - git-jump: pass "merge" arguments to ls-files

 The "merge" subcommand of "git jump" (in contrib/) silently ignored
 pathspec and other parameters.

 Will merge to 'next'.

* jk/strbuf-addftime-seconds-since-epoch (2021-11-04) 1 commit
 - strbuf_addftime(): handle "%s" manually

 The "--date=format:<strftime>" gained a workaround for the lack of
 system support for a non-local timezone to handle "%s" placeholder.

 Will merge to 'next'.

* js/ci-no-directional-formatting (2021-11-04) 1 commit
 - ci: disallow directional formatting

 CI has been taught to catch some Unicode directional formatting
 sequence that can be used in certain mischief.

 Will merge to 'next'.

* tw/var-default-branch (2021-11-03) 1 commit
 - var: add GIT_DEFAULT_BRANCH variable

 "git var GIT_DEFAULT_BRANCH" is a way to see what name is used for
 the newly created branch if "git init" is run.

 Will merge to 'next'.

* ab/mark-leak-free-tests-even-more (2021-11-01) 15 commits
 - leak tests: mark some fast-import tests as passing with SANITIZE=leak
 - leak tests: mark some config tests as passing with SANITIZE=leak
 - leak tests: mark some status tests as passing with SANITIZE=leak
 - leak tests: mark some clone tests as passing with SANITIZE=leak
 - leak tests: mark some add tests as passing with SANITIZE=leak
 - leak tests: mark some diff tests as passing with SANITIZE=leak
 - leak tests: mark some apply tests as passing with SANITIZE=leak
 - leak tests: mark some notes tests as passing with SANITIZE=leak
 - leak tests: mark some update-index tests as passing with SANITIZE=leak
 - leak tests: mark some rev-parse tests as passing with SANITIZE=leak
 - leak tests: mark some rev-list tests as passing with SANITIZE=leak
 - leak tests: mark some misc tests as passing with SANITIZE=leak
 - leak tests: mark most gettext tests as passing with SANITIZE=leak
 - leak tests: mark "sort" test as passing SANITIZE=leak
 - leak tests: mark a read-tree test as passing SANITIZE=leak

 More tests are marked as leak-free.

 Will merge to 'next'.

* if/redact-packfile-uri (2021-11-11) 2 commits
 - http-fetch: redact url on die() message
 - fetch-pack: redact packfile urls in traces

 Redact the path part of packfile URI that appears in the trace output.

 Will merge to 'next'.

* ew/test-wo-fsync (2021-10-29) 1 commit
 - tests: disable fsync everywhere

 Allow running our tests while disabling internal fsync.

* ja/doc-cleanup (2021-11-09) 10 commits
 - init doc: --shared=0xxx does not give umask but perm bits
 - doc: git-init: clarify file modes in octal.
 - doc: git-http-push: describe the refs as pattern pairs
 - doc: uniformize <URL> placeholders' case
 - doc: use three dots for indicating repetition instead of star
 - doc: git-ls-files: express options as optional alternatives
 - doc: use only hyphens as word separators in placeholders
 - doc: express grammar placeholders between angle brackets
 - doc: split placeholders as individual tokens
 - doc: fix git credential synopsis

 Doc update.

 Will merge to 'next'.

* re/color-default-reset (2021-10-28) 3 commits
 - color: allow colors to be prefixed with "reset"
 - color: support "default" to restore fg/bg color
 - color: add missing GIT_COLOR_* white/black constants

 "default" and "reset" colors have been added to our palette.

 Will merge to 'next'?

* jc/fix-first-object-walk (2021-10-29) 2 commits
 - docs: add headers in MyFirstObjectWalk
 - docs: fix places that break compilation in MyFirstObjectWalk

 Doc update.

 Will merge to 'next'.

* pw/diff-color-moved-fix (2021-10-27) 15 commits
 - diff --color-moved: intern strings
 - diff: use designated initializers for emitted_diff_symbol
 - diff --color-moved-ws=allow-indentation-change: improve hash lookups
 - diff --color-moved: stop clearing potential moved blocks
 - diff --color-moved: shrink potential moved blocks as we go
 - diff --color-moved: unify moved block growth functions
 - diff --color-moved: call comparison function directly
 - diff --color-moved-ws=allow-indentation-change: simplify and optimize
 - diff: simplify allow-indentation-change delta calculation
 - diff --color-moved: avoid false short line matches and bad zerba coloring
 - diff --color-moved=zebra: fix alternate coloring
 - diff --color-moved: rewind when discarding pmb
 - diff --color-moved: factor out function
 - diff --color-moved: clear all flags on blocks that are too short
 - diff --color-moved: add perf tests

 Long-overdue correctness and performance update to "diff
 --color-moved" feature.

 Will merge to 'next'.

* ab/generate-command-list (2021-11-05) 10 commits
 - generate-cmdlist.sh: don't parse command-list.txt thrice
 - generate-cmdlist.sh: replace "grep' invocation with a shell version
 - generate-cmdlist.sh: do not shell out to "sed"
 - generate-cmdlist.sh: stop sorting category lines
 - generate-cmdlist.sh: replace for loop by printf's auto-repeat feature
 - generate-cmdlist.sh: run "grep | sort", not "sort | grep"
 - generate-cmdlist.sh: don't call get_categories() from category_list()
 - generate-cmdlist.sh: spawn fewer processes
 - generate-cmdlist.sh: trivial whitespace change
 - command-list.txt: sort with "LC_ALL=C sort"

 Build optimization.

 Will merge to 'next'.

* es/pretty-describe-more (2021-11-01) 3 commits
 - pretty: add abbrev option to %(describe)
 - pretty: add tag option to %(describe)
 - pretty.c: rework describe options parsing for better extensibility

 Extend "git log --format=%(describe)" placeholder to allow passing
 selected command-line options to the underlying "git describe"
 command.

 Will merge to 'next'?

* fs/ssh-signing-key-lifetime (2021-11-18) 10 commits
 - ssh signing: verify ssh-keygen in test prereq
 - ssh signing: make fmt-merge-msg consider key lifetime
 - ssh signing: make verify-tag consider key lifetime
 - ssh signing: make git log verify key lifetime
 - ssh signing: make verify-commit consider key lifetime
 - ssh signing: add key lifetime test prereqs
 - ssh signing: use sigc struct to pass payload
 - Merge branch 'ad/ssh-signing-testfix' into fs/ssh-signing-key-lifetime
 - Merge branch 'fs/ssh-signing-fix' into fs/ssh-signing-key-lifetime
 - Merge branch 'fs/ssh-signing' into fs/ssh-signing-key-lifetime

 Extend the signing of objects with SSH keys and learn to pay
 attention to the key validity time range when verifying.

 Will merge to 'next'?

* jc/doc-submitting-patches-choice-of-base (2021-10-25) 2 commits
 - (wip) reword the final review part
 - SubmittingPatchs: clarify choice of base and testing

 Extend the guidance to choose the base commit to build your work
 on, and hint/nudge contributors to read others' changes.

* gc/remote-with-fewer-static-global-variables (2021-11-18) 5 commits
 - remote: die if branch is not found in repository
 - remote: remove the_repository->remote_state from static methods
 - remote: use remote_state parameter internally
 - remote: move static variables into per-repository struct
 - t5516: add test case for pushing remote refspecs

 Code clean-up to eventually allow information on remotes defined
 for an arbitrary repository to be read.

 Will merge to 'next'.

* ab/config-based-hooks-2 (2021-11-24) 18 commits
 - run-command: remove old run_hook_{le,ve}() hook API
 - receive-pack: convert push-to-checkout hook to hook.h
 - read-cache: convert post-index-change to use hook.h
 - commit: convert {pre-commit,prepare-commit-msg} hook to hook.h
 - git-p4: use 'git hook' to run hooks
 - send-email: use 'git hook run' for 'sendemail-validate'
 - git hook run: add an --ignore-missing flag
 - hooks: convert worktree 'post-checkout' hook to hook library
 - hooks: convert non-worktree 'post-checkout' hook to hook library
 - merge: convert post-merge to use hook.h
 - am: convert applypatch-msg to use hook.h
 - rebase: convert pre-rebase to use hook.h
 - hook API: add a run_hooks_l() wrapper
 - am: convert {pre,post}-applypatch to use hook.h
 - gc: use hook library for pre-auto-gc hook
 - hook API: add a run_hooks() wrapper
 - hook: add 'run' subcommand
 - Merge branch 'ab/config-based-hooks-1' into ab/config-based-hooks-2

 More "config-based hooks".

* vd/sparse-sparsity-fix-on-read (2021-11-24) 4 commits
 - sparse-index: update do_read_index to ensure correct sparsity
 - sparse-index: add ensure_correct_sparsity function
 - sparse-index: avoid unnecessary cache tree clearing
 - test-read-cache.c: prepare_repo_settings after config init

 Ensure that the sparseness of the in-core index matches the
 index.sparse configuration specified by the repository immediately
 after the on-disk index file is read.

 Will merge to 'next'.

* js/branch-track-inherit (2021-11-18) 1 commit
 - branch: add flags and config to inherit tracking

 "git -c branch.autosetupmerge=inherit branch new old" makes "new"
 to have the same upstream as the "old" branch, instead of marking
 "old" itself as its upstream.

* jh/builtin-fsmonitor-part2 (2021-10-21) 29 commits
 - t7527: test status with untracked-cache and fsmonitor--daemon
 - fsmonitor: force update index after large responses
 - fsmonitor--daemon: use a cookie file to sync with file system
 - fsmonitor--daemon: periodically truncate list of modified files
 - t/perf/p7519: add fsmonitor--daemon test cases
 - t/perf/p7519: speed up test on Windows
 - t/helper/test-chmtime: skip directories on Windows
 - t/perf: avoid copying builtin fsmonitor files into test repo
 - t7527: create test for fsmonitor--daemon
 - t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
 - help: include fsmonitor--daemon feature flag in version info
 - fsmonitor--daemon: implement handle_client callback
 - compat/fsmonitor/fsm-listen-darwin: implement FSEvent listener on MacOS
 - compat/fsmonitor/fsm-listen-darwin: add macos header files for FSEvent
 - compat/fsmonitor/fsm-listen-win32: implement FSMonitor backend on Windows
 - fsmonitor--daemon: create token-based changed path cache
 - fsmonitor--daemon: define token-ids
 - fsmonitor--daemon: add pathname classification
 - fsmonitor--daemon: implement 'start' command
 - fsmonitor--daemon: implement 'run' command
 - compat/fsmonitor/fsm-listen-darwin: stub in backend for Darwin
 - compat/fsmonitor/fsm-listen-win32: stub in backend for Windows
 - fsmonitor--daemon: implement 'stop' and 'status' commands
 - fsmonitor--daemon: add a built-in fsmonitor daemon
 - fsmonitor: document builtin fsmonitor
 - fsmonitor: use IPC to query the builtin FSMonitor daemon
 - fsmonitor: config settings are repository-specific
 - fsmonitor-ipc: create client routines for git-fsmonitor--daemon
 - fsmonitor: enhance existing comments

 Built-in fsmonitor (part 2).

* ld/sparse-diff-blame (2021-11-29) 3 commits
 - blame: enable and test the sparse index
 - diff: enable and test the sparse index
 - Merge branch 'vd/sparse-reset' into ld/sparse-diff-blame
 (this branch is used by ds/fetch-pull-with-sparse-index; uses vd/sparse-reset.)

 Teach diff and blame to work well with sparse index.

 Expecting a reroll.
 cf. <YXgpqJIS2OgOgS+k@nand.local>

* ns/remerge-diff (2021-10-27) 9 commits
 - doc/diff-options: explain the new --remerge-diff option
 - show, log: adapt Elijah Newren's changes to common tmp-objdir API
 - show, log: provide a --remerge-diff capability
 - merge-ort: capture and print ll-merge warnings in our preferred fashion
 - ll-merge: add API for capturing warnings in a strbuf instead of stderr
 - merge-ort: add ability to record conflict messages in a file
 - merge-ort: mark a few more conflict messages as omittable
 - Merge branch 'ns/tmp-objdir' into ns/remerge-diff
 - Merge branch 'ns/tmp-objdir' into ns/remerge-diff
 (this branch uses ns/tmp-objdir.)

 A new presentation for two-parent merge "--remerge-diff" can be
 used to show the difference between mechanical (and possibly
 conflicted) merge results and the recorded resolution.

 On hold.
 This is Elijah's remerge-diff rebased on ns/tmp-objdir to share the
 "create objects temporarily, only to discard without committing
 them to longer-term storage" infrastructure with another topic.

* ns/tmp-objdir (2021-10-27) 4 commits
 - fixup! tmp-objdir: new API for creating temporary writable databases
 - fixup! tmp-objdir: new API for creating temporary writable databases
 - tmp-objdir: disable ref updates when replacing the primary odb
 - tmp-objdir: new API for creating temporary writable databases
 (this branch is used by ns/batched-fsync and ns/remerge-diff.)

 New interface into the tmp-objdir API to help in-core use of the
 quarantine feature.

 On hold.
 cf. <xmqqo87auqda.fsf@gitster.g>

* vd/sparse-reset (2021-11-29) 8 commits
 - unpack-trees: improve performance of next_cache_entry
 - reset: make --mixed sparse-aware
 - reset: make sparse-aware (except --mixed)
 - reset: integrate with sparse index
 - reset: expand test coverage for sparse checkouts
 - sparse-index: update command for expand/collapse test
 - reset: preserve skip-worktree bit in mixed reset
 - reset: rename is_missing to !is_in_reset_tree
 (this branch is used by ds/fetch-pull-with-sparse-index and ld/sparse-diff-blame.)

 Various operating modes of "git reset" have been made to work
 better with the sparse index.

 Will merge to 'next'.

* pw/fix-some-issues-in-reset-head (2021-10-01) 12 commits
 - rebase -m: don't fork git checkout
 - rebase --apply: set ORIG_HEAD correctly
 - rebase --apply: fix reflog
 - reset_head(): take struct rebase_head_opts
 - rebase: cleanup reset_head() calls
 - reset_head(): make default_reflog_action optional
 - reset_head(): factor out ref updates
 - reset_head(): remove action parameter
 - reset_head(): don't run checkout hook if there is an error
 - reset_head(): fix checkout
 - rebase: factor out checkout for up to date branch
 - Merge branch 'pw/rebase-of-a-tag-fix' into pw/fix-some-issues-in-reset-head

 Fix "some issues" in a helper function reset_head().

 Expecting a reroll.
 Needs a lot better explanation, including what the issues are,
 which codepaths the helper is used and to do what, and tests to
 protect the fixes.

* es/superproject-aware-submodules (2021-11-18) 5 commits
 - submodule: use config to find superproject worktree
 - submodule: record superproject gitdir during 'update'
 - submodule: record superproject gitdir during absorbgitdirs
 - introduce submodule.superprojectGitDir record
 - t7400-submodule-basic: modernize inspect() helper

 A configuration variable in a submodule points at the location of
 the superproject it is bound to (RFC).

* ns/batched-fsync (2021-10-27) 9 commits
 - Merge branch 'ns/tmp-objdir' into ns/batched-fsync
 - core.fsyncobjectfiles: performance tests for add and stash
 - core.fsyncobjectfiles: tests for batch mode
 - unpack-objects: use the bulk-checkin infrastructure
 - update-index: use the bulk-checkin infrastructure
 - core.fsyncobjectfiles: add windows support for batch mode
 - core.fsyncobjectfiles: batched disk flushes
 - bulk-checkin: rename 'state' variable and separate 'plugged' boolean
 - Merge branch 'ns/tmp-objdir' into ns/batched-fsync
 (this branch uses ns/tmp-objdir.)

 The "core.fsyncobjectfiles" configuration variable can now be set
 to "batch" for improved performance.

 On hold.
 cf. <xmqqo87auqda.fsf@gitster.g>

* en/zdiff3 (2021-09-20) 2 commits
 - update documentation for new zdiff3 conflictStyle
 - xdiff: implement a zealous diff3, or "zdiff3"

 "Zealous diff3" style of merge conflict presentation has been added.

 What's the status of this thing?

* js/scalar (2021-11-19) 17 commits
 - scalar: implement the `version` command
 - scalar: implement the `delete` command
 - scalar: teach 'reconfigure' to optionally handle all registered enlistments
 - scalar: allow reconfiguring an existing enlistment
 - scalar: implement the `run` command
 - scalar: teach 'clone' to support the --single-branch option
 - scalar: implement the `clone` subcommand
 - scalar: implement 'scalar list'
 - scalar: let 'unregister' handle a deleted enlistment directory gracefully
 - scalar: 'unregister' stops background maintenance
 - scalar: 'register' sets recommended config and starts maintenance
 - ci: also run the `scalar` tests
 - cmake: optionally build `scalar`, too
 - scalar: create test infrastructure
 - scalar: start documenting the command
 - scalar: create a rudimentary executable
 - scalar: add a README with a roadmap

 Add pieces from "scalar" to contrib/.

* ms/customizable-ident-expansion (2021-09-01) 1 commit
 - keyword expansion: make "$Id$" string configurable

 Instead of "$Id$", user-specified string (like $FreeBSD$) can be
 used as an in-blob placeholder for keyword expansion.

* ab/only-single-progress-at-once (2021-11-03) 8 commits
 - progress.c: add & assert a "global_progress" variable
 - various *.c: use isatty(0|2), not isatty(STDIN_FILENO|STDERR_FILENO)
 - pack-bitmap-write.c: don't return without stop_progress()
 - progress.c: add temporary variable from progress struct
 - progress.c tests: test some invalid usage
 - progress.c tests: make start/stop commands on stdin
 - progress.c test helper: add missing braces
 - leak tests: fix a memory leaks in "test-progress" helper

 Further tweaks on progress API.

* hn/reftable (2021-10-08) 19 commits
 - Add "test-tool dump-reftable" command.
 - reftable: add dump utility
 - reftable: implement stack, a mutable database of reftable files.
 - reftable: implement refname validation
 - reftable: add merged table view
 - reftable: add a heap-based priority queue for reftable records
 - reftable: reftable file level tests
 - reftable: read reftable files
 - reftable: generic interface to tables
 - reftable: write reftable files
 - reftable: a generic binary tree implementation
 - reftable: reading/writing blocks
 - Provide zlib's uncompress2 from compat/zlib-compat.c
 - reftable: (de)serialization for the polymorphic record type.
 - reftable: add blocksource, an abstraction for random access reads
 - reftable: utility functions
 - reftable: add error related functionality
 - reftable: add LICENSE
 - hash.h: provide constants for the hash IDs

 The "reftable" backend for the refs API, without integrating into
 the refs subsystem.

 Will merge to 'next'.

On Mon, Nov 29, 2021 at 06:16:54PM -0800, Junio C Hamano wrote:

Sorry, I hadn't noticed that this one had even made it to 'next', and was surprised to see it graduate.

I think brian corrected my assertion in the commit message that RFC 1738 says that underscores are OK. They are for URIs in general, but not the specific case of hostnames in HTTP schemes.

Now that isn't strictly a reason to drop the patch. Even though underscores aren't allowed, they do work in some limited circumstances, and curl is happy to take them. So in some sense this is harmonizing our urlmatch behavior with curl for an iffy-but-workable practice, and there may be value in that. But it does take us further away from the standards, which could possibly have surprising consequences down the road.

I don't have a strong feeling either way on reverting it at this point. But I wanted to make sure that if we kept it in, we were doing so consciously, and not just because folks involved in the discussion didn't realize it was still working its way through the process.

I.e., if brian is likewise surprised and wants to object, this is now the time. :)

-Peff

On 2021-11-30 at 07:33:25, Jeff King wrote:

From my memory of the research I did, they're okay for URIs according to the grammar, but they're acceptable only if you're using something that isn't DNS, which as a practical matter is never the case.

I don't think it's necessarily worth reverting, but we should avoid continuing to make further improvements in this area. In other words, we shouldn't take this as an opportunity to support more of this.

On Mon, Nov 29 2021, Junio C Hamano wrote:

I think the only outstanding thing for this topic is Eric's [1] comment (on his own code). I think that variable shadowing is OK.

1/2 of that patch will also be rewritten in the in-flight hook topic (but the "reset --hard" shadowing is left in place).

As noted in [2] I'm happy to get this more isolated fix first. I'd missed that there was a re-submission[3] until now (since In-Reply-To wasn't maintained).

The code change in [3] isn't needed anymore when combined with my ab/run-command.

Depending on how you're planning to advance these perhaps you'd like to revert that as it's merged with ab/run-command, or I can re-roll ab/run-command on top of it if you'd like.

We could also just leave that now-redundant child_process_init() in pager.c, but having something that'll amount to cargo-culting to get around a bug in dead code doesn't seem ideal. It would be nice to have the API use reflect "argv" and "env" being gone.

1. https://lore.kernel.org/git/CAPig+cRi6SeuV7k_+9JCcnf79daLZp5B=EyHK-KxC1VGN0B4ig@mail.gmail.com/
2. https://lore.kernel.org/git/211124.865ysie2br.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/20211125000239.2336-1-ematsumiya@suse.de

On Tue, Nov 30, 2021 at 09:54:33PM +0100, Ævar Arnfjörð Bjarmason wrote:

IMHO that extra init is still doing the right thing, because it means the struct is in the same known state in every run of setup_pager(). Fixing the argv/env thing stops the immediate bug, but there are other potential ones. E.g., starting the command will overwrite the in/out/err parameters; we're OK in this instance because we unconditionally overwrite them.

So I think reusing a child_process struct without reinitializing it is always a bit iffy, though it _usually_ works in practice. We should model good use of the API by initializing on each use.

-Peff

On Tue, Nov 30, 2021 at 4:07 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

I'm also fine with the variable shadowing and don't consider it an outstanding issue with the topic (it was just a minor observation in case anyone complained). I didn't see anything else worth commenting on in the remainder of the series.

On Mon, Nov 29 2021, Junio C Hamano wrote:

It looked like there was general happiness with the approach of having the dead code gone and the GitHub tooltips being more useful.

Given that this is relatively trivial, and if it was causing trouble we'd likely know by now it would be nice to get this merged down sooner than later.

As noted in the latest thread on js/scalar there's outstanding interactions with that larger topic (including some semantic merge "conflicts"). So having it in "master" to base on would also remove its interaction with that topic & any other CI modification as a variable.

1. https://lore.kernel.org/git/211130.86mtlleqhm.gmgdl@evledraar.gmail.com/

On Mon, Nov 29 2021, Junio C Hamano wrote:

I know Emily's keen to get to the conclusion of the "config-based-hooks" story. I think it's gotten a lot of careful review already, and it would be nice to have it land soon in this cycle so the follow-up topics can be queued after it.

I re-rolled it a ~week ago[1], but that was a trivial change to save you from dealing with a compilation error as it got merged with my own "ab/run-command" (it was using "env", not "env_array")[1]. Other than that there's been no change to it for the last month since the v4[2].

1. https://lore.kernel.org/git/cover-v5-00.17-00000000000-20211123T114206Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-v4-00.17-00000000000-20211101T184938Z-avarab@gmail.com/

On Mon, Nov 29 2021, Junio C Hamano wrote:

My reading of https://lore.kernel.org/git/20211130143821.7dz5jj2z2x2q2ytn@fs/ is that Fabian's planning to re-roll it.

I replied today to that to clarify some edge cases around TAP.

On 30.11.2021 22:17, Ævar Arnfjörð Bjarmason wrote:

Yes, thanks for that. I resubmitted a new version with the better comment.

Thanks

On Mon, Nov 29 2021, Junio C Hamano wrote:

I think that sounds good!

The change is trivial, and due to the nature of it having it sooner than later this cycle can only be better, in case there's some obscure system we didn't consider, catching that before the next release would be preferable.

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:

ns/tmp-objdir had a re-roll that has not been picked up, at [1] -- perhaps because it's an combination of ns/tmp-objdir and ns/batched-fsync (it'd be nicer to have those two split). I gave the ns/tmp-objdir part another read over and was only able to spot two small things. I think you should mark it as expecting a reroll based on [2] ("Good catch. I'll fix this.") and [3] ("I'll take this suggestion."), but I think it could be merged to next quickly after that.

[1] https://lore.kernel.org/git/pull.1076.v9.git.git.1637020263.gitgitgadget@gmail.com/ [2] https://lore.kernel.org/git/CANQDOddCC7+gGUy1VBxxwvN7ieP+N8mQhbxK2xx6ySqZc6U7-g@mail.gmail.com/ [3] https://lore.kernel.org/git/CANQDOdd7EHUqD_JBdO9ArpvOQYUnU9GSL6EVR7W7XXgNASZyhQ@mail.gmail.com/

Mark it as expecting a re-roll; I've been waiting for ns/tmp-objdir to settle so I can rebase on it.

Elijah Newren <newren@gmail.com> writes:

I took a quick look at the rerolled one on list, and I agree that keeping tmp-objdir and batched-fsync as two separate topics makes sense, since the former can graduate much more smoothly and quickly, and it can have other dependant topics.

So I'll mark all three (ns/tmp-objdir, ns/batched-fsync and remerge-diff) as "Expecting a reroll".

As I announced, I won't be taking any new topics or new rerolls today (or possibly tomorrow) until I can sift the topics I've already seen to come up with a tested set of candidate topics to merge to 'next', so there is no need to rush.

Thanks.

On Fri, Dec 03, 2021 at 11:21:13AM -0800, Junio C Hamano wrote:

I submitted a new PR (with a new mail thread) for ns/tmp-objdir. Hopefully that one can sail in smoothly now.

ns/batched-fsync will take a bit more time to settle. I'm going to post a new series called ns/core-fsync, which is focused on the extensible interface for syncing parts of the tree.

Thanks, Neeraj

On Fri, Dec 3, 2021 at 6:58 PM Neeraj Singh <nksingh85@gmail.com> wrote:

Thanks for sending that out. I wasn't cc'ed on them, and I'm dealing with multi-day delay of git emails that I'm not cc'ed on, but I read them over on lore.kernel.org/git and those two patches look good and appear to me to be ready for next. :-)

Not sure if it help/hurts/matters, but thought I'd send a thumbs up on several of the topics. For topics I have comments other than a thumbs up for, I'll post separate messages...

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:

Thumb up on all these from me.

There were two tiny issues pointed out at https://lore.kernel.org/git/9c5b7067-b0e3-0153-5cd3-042bae92f91e@github.com/, but it'd be fine to merge the series as-is and consider those as later potential improvements.

Elijah Newren wrote:

In the interest of avoiding further delays to (and maintenance burden for) the other in-flight series dependent on this one, I'd prefer including those fixups in my next set of sparse index integrations.

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:

Please don't, at least not this version. There have been newer submissions with three commits.

I also still find the word 'die' confusing, since to me it suggests aborting the whole am operation, and the documentation does not dispel that concern. Even if you don't like 'ask' (for consistency with git-rebase), I think 'stop' or 'interrupt' would be much better options than 'die'. If you really want it to be 'die', I think the behavior needs to be explained in the documentation, rather than just assumed that users will understand it (because I didn't understand it until I read the code).

Dears Newren,

    I don't think 'stop' and 'interrupt' words are better to explain more than 'die'
    because they still indicate that it will stop or interrupt the whole am session,
    rather than stop in the middle of it. It may be a good choice to just add an
    additional description about the behaviour when not passing the '--empty' option
    or passing '--empty=die'.

Aleen

On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> wrote:

Since you've been through several rounds of revisions already, if this is the only remaining issue with your series, I wouldn't try to hold it up for this issue; I just thought it could be fixed while you were working on the --allow-empty stuff.

However, while I don't think it's worth holding up your series for just this issue, I would definitely submit a follow-up RFC patch to fix the wording, because I do disagree with your assertion here pretty strongly. Let's look at the meanings of the terms:

die: connotes something pretty final and irreversible -- people tend not to revive after death as far as recorded history goes; most such recorded instances tend to be causes for people to debate the definition of 'dead'.

stop: could be final, but is often used in a transitory setting: "stop and go traffic", "stopped to catch my breath", "the train will stop at this station", "stop! I want to get out", etc.

interrupt: seems to nearly always be used as a transitory thing

Now, in the computer science context, all three terms come up relative to processes. You can interrupt a process (the kernel does all the time), but it'll usually continue afterwards. Or you can give it a SIGINT (interrupt from keyboard signal), which the process can catch and ignore. You can stop a process (and SIGSTOP cannot be caught), but you can also continue it later. die essentially means the process is going to be gone for good (at least short of some kind of black magic like a reversible debugger such as rr).

So, I think it's much more likely that 'die' will be misunderstood to mean abortion of the entire am-process, than that 'stop' or 'interrupt' would.

That would be good.

On Wed, Dec 01 2021, Elijah Newren wrote:

FWIW I think it's worth getting the UX issue right, tweaking it is relatively easy, and if we can decide on what the thing is called then...

Why are we exposing an --empty=die at all? It's what we do by default, so why have it? The user can just not provide the "--empty" option, then they'll get the current behavior of die_user_resolve(), which seems to have inpired the name for this "die" (it exits with code 128, just like die()).

Once we get rid of "die" the rest of the UI can follow the example of the existing "git rebase" options:

    --empty={drop,keep,ask}

I.e. the "drop" and "keep" will be the same, no "ask" currently, but it can be implemented in the future.

Maybe I'm missing something, I haven't used "am" in this way (or rebase with --empty=*), but this just seems to me to be needlessly exposing a "die" (or "stop" or whatever) because it's how we implement this.

Whereas for the UX we don't need to call it anything except the absence of an --empty option, or perhaps --no-empty.

On Fri, Dec 3, 2021 at 10:37 AM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

:-)

That's an interesting angle to take; I hadn't thought of that. It's worth considering.

We do often introduce options equivalent to the default as a way to either countermand an earlier option (e.g. --do-walk overrides --no-walk in git log), or because we want to allow new config options that change the default while allowing the user to explicitly request something different (e.g. --no-renames was introduced at the same time as diff.renames), or because we may want to change the default behavior and want users to be able to explicitly request a certain type of behavior (e.g. rename detection is the default and --no-renames overrides).

It's not clear to me whether that type of flexibility is needed or whether we can just leave it unnamed. Three points that may affect that decision: (a) the default (and actually, hardcoded) behavior before this series for git-am was 'drop', (b) the default behavior for git-rebase is 'drop' (though it only affects commits which become empty, something we can't determine in the context of am) and (c) there was one point during the series that the author asked about just removing the 'die' implementation and picking a different default.

The above three points suggest to me that there might reasonably be config added to control this or that the default could change, and thus that it might be useful to name the interrupt-the-operation behavior so that users can explicitly request it. But that's somewhere around three levels of chained "might" conditions, so... :shrug:

Um, there are minor contextual differences, but what rebase calls "ask" (interrupt the operation and tell the users what commands they can use to keep or drop the commit and then resume the operation) _is_ implemented by this series -- it's just being called "die" here.

That's the kind of maddening inconsistency in Git that users complain about that I really think we should avoid adding to. If for some reason 'ask' from rebase seems like a bad choice, then I think we should pick a new name for this interrupt-the-operation behavior that makes sense (unlike 'die') for git-am and add it to git-rebase as a preferred synonym to 'ask'.

`--no-empty` would semantically be read by users to mean get rid of empty commits, which would be a synonym of 'drop'. I think it'd be as confusing as 'die' (and maybe even more so) for naming the interrupt-the-operation behavior.

On Fri, Dec 03 2021, Elijah Newren wrote:

Ah, I didn't look into the finer details. Yes if it does maps to "ask" in rebase we could just use that, so would this be consistent?:

    --empty=die -> --empty=ask
    --empty=drop -> (ditto, no change)
    --empty=keep -> (ditto, no change)

I think "ask" is a bit of a weird term for this, but I think consistency trumps a while-we're-at-it fix here.

Whatever new synonym we'd come up with (if that would be justified, that itself would add to confusion) could be done as a follow-up and implemented for both "rebase" and "am".

Dears Hamano,

    In my opinion, the '--empty' should act as a strategy option like the
    'X' option to 'git-rebase'. Since that the default behaviour of not passing
    the option is stopped in the middle of an am session, should the 'die' value
    dies the whole process but not the middle state? It may also make it not
    confusing.

Aleen

Aleen 徐沛文 <pwxu@coremail.cn> writes:

Hmph, unlike "git rebase" or "git merge", "git am" does not employ different strategy backends, so "-X<option>" would be out of place, I would think.

Among what we already have, what kills the entire session is called "git am --abort". Since I do not find it unnatural to say "'git am' dies" when it stops and gives control back to the user, so that the user can decide what to do next, I am not sure where the aversion to the word comes from (on the other hand, I find 'ask' highly unnatural since we do not ask anything---we just throw the control back the user).

The confusing point seems why "git am" stops into the middle and gives control back to the user when passing "die". In common cases, "die" should stop the whole process, and is it better to distinguish the case when passing "die" from the default behaviour? Like what the following snippet is implemented:

    case ERR_EMPTY_COMMIT:
        printf_ln(_("Patch is empty.\n"
                    "If you want to record it as an empty commit, run \"git am --allow-empty\"."));
	die_user_resolve(state);
    case DIE_EMPTY_COMMIT:
        am_destroy(state);
        die(_("Patch is empty."));
        break;

Sorry for the missing "break" here.

Aleen 徐沛文 <pwxu@coremail.cn> writes:

Sorry, but I am afraid that I still don't get it.  

As we can see, the ERR_EMPTY_COMMIT case already exists and that is the behaviour we want when we say "create commits from the messages with patches, stop and give me control back when you see an empty commit, so that I can decide what to do". And one of the things you can do at that point is "am --abort" that causes the am_destroy() to be called.

That is very much in line with the behaviour the users are used to see from "git am" when it detects conditions other than "there is no patch" that needs to stop and give control back to the user, e.g. when the patch does not apply cleanly or the log message did not pass msg hook. am_destroy() is destructive, and limiting such a destructive operation to "am --abort" would avoid mistakes and surprises. I do not know where you got "In common cases, die should stop the whole" from, but it is not a friendly thing to do to our users.

Why should the "in addition, stop when there is no patch", which is what we already handle just fine, needs to become different? More importantly, is it worth forcing the users to be aware of the difference and be extra careful to avoid it?

It is my understanding that the ONLY reason the patch proposes to add an option other than "skip the step" and "create an empty commit" is to allow an earlier "--empty=skip" on the command line to be overridden by a later "--empty=die". If that option does not make the command behave identically to "--empty=<anything>" option on the command line, i.e. ERR_EMPTY_COMMIT case, it does not serve the intended purpose of overriding the earlier option to revert the behaviour back to the default.

By the way, I agree with an earlier comment (I think it was from Dscho) that these names should say "${DO_THIS}_ON_EMPTY_COMMIT"; the above without "ON" feels somewhat strange.

Thanks.

Dears Hamano,

    I have doubted that since that the default behaviour is that stopping
    when meeting commit messages lacking a patch and giving control back
    to the user, is that necessary to provide duplicated '--empty=die'?
    Should we just provide '--empty=(drop|keep)'?

Aleen

Aleen 徐沛文 <pwxu@coremail.cn> writes:

[jc: do not top-post, as people read from top to bottom]

Adding an option that aborts and trashes the state directory is much worse than not having a choice other than drop and keep, which is in turn a bit worse than having a way to countermand an option that was given earlier on the command line.

If we were to have an option other than drop/keep, as Elijah suggested, it may make sense to call it "stop" rather than "die" (I think "ask" is a mistake, as we do not ask anything to the user).

This still stands, too.

Thanks.

Another option to countermand an option is only meaningful when it can be applied as a resume value but not in such a case, I think. "--empty=<option>" is only kept in each am session, and it seems we can't countermand the option after the session.

On Sun, Dec 5, 2021 at 10:28 PM Junio C Hamano <gitster@pobox.com> wrote:

When I first read the documentation, it sounded to me like it was implying an abort. I find 'die' very unnatural as a way to explain this behavior; it's too strong of a word.

Okay, but what about my previous suggestions of 'stop' or 'interrupt'?

Elijah Newren <newren@gmail.com> writes:

The way we currently document "git am", we have no word "die" used anywhere in the page. The existing mention of the behaviour are found in these places:

    --continue::
    -r::
    --resolved::
            After a patch failure (e.g. attempting to apply
            conflicting patch), the user has applied it by hand and
            the index file stores the result of the application.
            Make a commit using the authorship and commit log
            extracted from the e-mail message and the current index
            file, and continue.

This uses "failure", and "the user has applied" implies that the user somehow got control back. We give "error:" messages to state that the patch does not apply from apply.c::apply_all_patches() and the caller silently exits, without calling die.

    --show-current-patch[=(diff|raw)]::
            Show the message at which `git am` has stopped due to
            conflicts.  If `raw` is specified, show the raw contents of
            the e-mail message; if `diff`, show the diff portion only.
            Defaults to `raw`.

This uses "has stopped" to describe the same situation.

I agree that "stop" would be a good word that is already used to describe "the command cannot make further progress without assistance by the user, so it stops and gives control back".

After that, the user can say "am --skip", "am --abort", or edit plus "am --continue".

Thanks.

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:

I resubmitted it before you added this question, and have pointed out the resubmission in response to the past "What's cooking" emails, but perhaps it'll be easier for me to just resubmit with no changes. I'll do that.

Elijah Newren <newren@gmail.com> writes:

Thanks. Yes, for anything that is older than a month, it always is easier that way.

On 29.11.2021 18:16, Junio C Hamano wrote:

I will send a rebased version of this one on top of fs/ssh-signing-key-lifetime. Otherwise those two will result in a (simple) merge conflict in the test prereqs.

I just resubmitted a new version which cleans up the prereq setup.

Thanks

As of this evening, my tree is not yet in a good enough shape to issue the next iteration of "What's cooking" report, but I've managed to merge some topics to 'next'. No, nothing exciting is in there. This first batch of topics consists of stuff that would have been in 'next' or even in the release if the previous cycle were longer by a week or two.

I plan to reserve time for picking and trial-merging other topics that we want to have in 'next' tomorrow, so that I can finish the "What's cooking" report with more topics marked for 'next'.

To make sure I can make progress, I'll promise that I will not look at or comment on patches that are sent to the list tomorrow (or in the coming few days), except that a reroll of an existing topic will be taken as a sign that the stale version I have in my tree needs to be replaced before it can be merged to 'next' (but I won't pick the new iteration up---I know from experience that it would stall and delay me).

On Mon, Nov 29, 2021 at 06:16:54PM -0800, Junio C Hamano wrote:

This being merged to 'next' tickled my -Wunused-parameter topic, so there were a few more opportunities for cleanup.

Definitely not urgent, but I think worth doing. Prepared on top of the topic branch.

  [1/3]: xdiff: drop CMP_ENV macro from xhistogram
  [2/3]: xdiff: drop xpparam_t parameter from histogram cmp_recs()
  [3/3]: xdiff: drop unused flags parameter from recs_match

 xdiff/xdiffi.c     | 18 +++++++++---------
 xdiff/xhistogram.c |  8 ++------
 2 files changed, 11 insertions(+), 15 deletions(-)

-Peff

This macro has been unused since 43ca7530df (xdiff/xhistogram: rely on xdl_trim_ends(), 2011-08-01). The function that called it went away there, and its use in the CMP() macro was inlined. It probably should have been deleted then, but nobody noticed.

Signed-off-by: Jeff King <peff@peff.net>
---
This one isn't strictly enabled by Phillip's new patches, but it cleans
up an unused caller of cmp_recs() so we don't have to worry about it in
the next patch.

 xdiff/xhistogram.c | 3 ---
 1 file changed, 3 deletions(-)

Since 663c5ad035 (diff histogram: intern strings, 2021-11-17), our cmp_recs() does not call xdl_recmatch(), and thus no longer needs an xpparam_t struct from which to get the flags. We can drop the unused parameter from the function, as well as the macro which wraps it.

There's no functional change here; it's just simplifying things (and making -Wunused-parameter happier).

Signed-off-by: Jeff King <peff@peff.net>
---
 xdiff/xhistogram.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

Since 6b13bc3232 (xdiff: simplify comparison, 2021-11-17), we do not call xdl_recmatch() from xdiffi.c's recs_match(), so we no longer need the "flags" parameter. That in turn lets us drop the flags parameters from the group-slide functions which call it.

There's no functional change here; it's just making these functions a little simpler.

Signed-off-by: Jeff King <peff@peff.net>
---
 xdiff/xdiffi.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Hi Peff

On 03/12/2021 05:10, Jeff King wrote:

Thanks, all three patches look good to me

Best Wishes

Phillip