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

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

[Junio C Hamano]

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'.

jk/loosen-urlmatch, was What's cooking in git.git (Nov 2021, #07; Mon, 29)

[Jeff King]

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

> --------------------------------------------------
> [Graduated to 'master']
>
> [...]
> * 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.

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

Re: jk/loosen-urlmatch, was What's cooking in git.git (Nov 2021, #07; Mon, 29)

[brian m. carlson]

[-- Attachment #1: Type: text/plain, Size: 2030 bytes --]

On 2021-11-30 at 07:33:25, Jeff King wrote:
> On Mon, Nov 29, 2021 at 06:16:54PM -0800, Junio C Hamano wrote:
> 
> > --------------------------------------------------
> > [Graduated to 'master']
> >
> > [...]
> > * 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.
> 
> 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.

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.

> 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 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.
-- 
brian m. carlson (he/him or they/them)
Toronto, Ontario, CA

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

ab/run-command + em/missing-pager (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Ævar Arnfjörð Bjarmason]

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

> * 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

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).

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

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

ab/ci-updates (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Ævar Arnfjörð Bjarmason]

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

> * 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

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/

ab/config-based-hooks-2 (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Ævar Arnfjörð Bjarmason]

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

> * 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".

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/

fs/test-prereq (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Ævar Arnfjörð Bjarmason]

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

> * 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'?

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.

Re: ab/run-command + em/missing-pager (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Jeff King]

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

> > * em/missing-pager (2021-11-24) 1 commit
> > - pager: fix crash when pager program doesn't exist
> 
> 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.

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

jc/c99-var-decl-in-for-loop (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Ævar Arnfjörð Bjarmason]

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

> * 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'?

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.

Re: ab/run-command + em/missing-pager (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Eric Sunshine]

On Tue, Nov 30, 2021 at 4:07 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Mon, Nov 29 2021, Junio C Hamano wrote:
> > * ab/run-command (2021-11-25) 9 commits
> >  - worktree: stop being overly intimate with run_command() internals
>
> 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. https://lore.kernel.org/git/CAPig+cRi6SeuV7k_+9JCcnf79daLZp5B=EyHK-KxC1VGN0B4ig@mail.gmail.com/

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.

ns/tmp-objdir and ns/remerge-diff

[Elijah Newren]

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> 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.

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/

>  Also ns/remerge-diff that is Neeraj's rebase of the
> remerge-diff topic needs Elijah's Ack at least.

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

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

[Elijah Newren]

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:
>
> * pw/xdiff-classify-record-in-histogram (2021-11-18) 3 commits
> ...
>  Will merge to 'next'.
>
>
> * ab/mark-leak-free-tests-even-more (2021-11-01) 15 commits
> ...
>  Will merge to 'next'.
>
>
> * pw/diff-color-moved-fix (2021-10-27) 15 commits
> ...
>  Will merge to 'next'.
>
>
> * vd/sparse-sparsity-fix-on-read (2021-11-24) 4 commits
> ...
>  Will merge to 'next'.

Thumb up on all these from me.

> * vd/sparse-reset (2021-11-29) 8 commits
> ...
>  Will merge to 'next'.

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.

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

[Elijah Newren]

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> * 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'.

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).

en/zdiff3 (was: Re: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Elijah Newren]

On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> * 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?

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.

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

[Aleen 徐沛文]

> 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

Re: fs/test-prereq (was: What's cooking in git.git (Nov 2021, #07; Mon, 29))

[Fabian Stelzer]

On 30.11.2021 22:17, Ævar Arnfjörð Bjarmason wrote:
>
>On Mon, Nov 29 2021, Junio C Hamano wrote:
>
>> * 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'?
>
>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.

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

Thanks

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

[Fabian Stelzer]

On 29.11.2021 18:16, Junio C Hamano wrote:
>
> * fs/ssh-signing-other-keytypes (2021-11-19) 2 commits
> - ssh signing: make sign/amend test more resilient
> - ssh signing: support non ssh-* keytypes
>

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.

>
> * 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'?
>

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

Thanks

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

[Victoria Dye]

Elijah Newren wrote:
>> * vd/sparse-reset (2021-11-29) 8 commits
>> ...
>>  Will merge to 'next'.
> 
> 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.
> 

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.

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

[Elijah Newren]

On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> 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.

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.

>     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'.

That would be good.

Re: en/zdiff3

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> On Tue, Nov 30, 2021 at 6:49 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> * 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?
>
> 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.

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

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

[Junio C Hamano]

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).

[PATCH 0/3] unused-parameter cleanups on top of pw/xdiff-classify-record-in-histogram

[Jeff King]

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

> * 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'.

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

[PATCH 1/3] xdiff: drop CMP_ENV macro from xhistogram

[Jeff King]

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(-)

diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
index 6c1c88a69a..399cc236d7 100644
--- a/xdiff/xhistogram.c
+++ b/xdiff/xhistogram.c
@@ -95,9 +95,6 @@ static int cmp_recs(xpparam_t const *xpp,
 
 }
 
-#define CMP_ENV(xpp, env, s1, l1, s2, l2) \
-	(cmp_recs(xpp, REC(env, s1, l1), REC(env, s2, l2)))
-
 #define CMP(i, s1, l1, s2, l2) \
 	(cmp_recs(i->xpp, REC(i->env, s1, l1), REC(i->env, s2, l2)))
 
-- 
2.34.1.436.g8c445b282e

[PATCH 2/3] xdiff: drop xpparam_t parameter from histogram cmp_recs()

[Jeff King]

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(-)

diff --git a/xdiff/xhistogram.c b/xdiff/xhistogram.c
index 399cc236d7..80794748b0 100644
--- a/xdiff/xhistogram.c
+++ b/xdiff/xhistogram.c
@@ -88,15 +88,14 @@ struct region {
 #define REC(env, s, l) \
 	(env->xdf##s.recs[l - 1])
 
-static int cmp_recs(xpparam_t const *xpp,
-	xrecord_t *r1, xrecord_t *r2)
+static int cmp_recs(xrecord_t *r1, xrecord_t *r2)
 {
 	return r1->ha == r2->ha;
 
 }
 
 #define CMP(i, s1, l1, s2, l2) \
-	(cmp_recs(i->xpp, REC(i->env, s1, l1), REC(i->env, s2, l2)))
+	(cmp_recs(REC(i->env, s1, l1), REC(i->env, s2, l2)))
 
 #define TABLE_HASH(index, side, line) \
 	XDL_HASHLONG((REC(index->env, side, line))->ha, index->table_bits)
-- 
2.34.1.436.g8c445b282e

[PATCH 3/3] xdiff: drop unused flags parameter from recs_match

[Jeff King]

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(-)

diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 6a3b9280be..69689fab24 100644
--- a/xdiff/xdiffi.c
+++ b/xdiff/xdiffi.c
@@ -390,7 +390,7 @@ static xdchange_t *xdl_add_change(xdchange_t *xscr, long i1, long i2, long chg1,
 }
 
 
-static int recs_match(xrecord_t *rec1, xrecord_t *rec2, long flags)
+static int recs_match(xrecord_t *rec1, xrecord_t *rec2)
 {
 	return (rec1->ha == rec2->ha);
 }
@@ -756,10 +756,10 @@ static inline int group_previous(xdfile_t *xdf, struct xdlgroup *g)
  * following group, expand this group to include it. Return 0 on success or -1
  * if g cannot be slid down.
  */
-static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g, long flags)
+static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g)
 {
 	if (g->end < xdf->nrec &&
-	    recs_match(xdf->recs[g->start], xdf->recs[g->end], flags)) {
+	    recs_match(xdf->recs[g->start], xdf->recs[g->end])) {
 		xdf->rchg[g->start++] = 0;
 		xdf->rchg[g->end++] = 1;
 
@@ -777,10 +777,10 @@ static int group_slide_down(xdfile_t *xdf, struct xdlgroup *g, long flags)
  * into a previous group, expand this group to include it. Return 0 on success
  * or -1 if g cannot be slid up.
  */
-static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g, long flags)
+static int group_slide_up(xdfile_t *xdf, struct xdlgroup *g)
 {
 	if (g->start > 0 &&
-	    recs_match(xdf->recs[g->start - 1], xdf->recs[g->end - 1], flags)) {
+	    recs_match(xdf->recs[g->start - 1], xdf->recs[g->end - 1])) {
 		xdf->rchg[--g->start] = 1;
 		xdf->rchg[--g->end] = 0;
 
@@ -830,7 +830,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			end_matching_other = -1;
 
 			/* Shift the group backward as much as possible: */
-			while (!group_slide_up(xdf, &g, flags))
+			while (!group_slide_up(xdf, &g))
 				if (group_previous(xdfo, &go))
 					BUG("group sync broken sliding up");
 
@@ -845,7 +845,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 
 			/* Now shift the group forward as far as possible: */
 			while (1) {
-				if (group_slide_down(xdf, &g, flags))
+				if (group_slide_down(xdf, &g))
 					break;
 				if (group_next(xdfo, &go))
 					BUG("group sync broken sliding down");
@@ -872,7 +872,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			 * other file that it can align with.
 			 */
 			while (go.end == go.start) {
-				if (group_slide_up(xdf, &g, flags))
+				if (group_slide_up(xdf, &g))
 					BUG("match disappeared");
 				if (group_previous(xdfo, &go))
 					BUG("group sync broken sliding to match");
@@ -915,7 +915,7 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo, long flags) {
 			}
 
 			while (g.end > best_shift) {
-				if (group_slide_up(xdf, &g, flags))
+				if (group_slide_up(xdf, &g))
 					BUG("best shift unreached");
 				if (group_previous(xdfo, &go))
 					BUG("group sync broken sliding to blank line");
-- 
2.34.1.436.g8c445b282e

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

[Ævar Arnfjörð Bjarmason]

On Wed, Dec 01 2021, Elijah Newren wrote:

> On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> 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.
>
> 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.

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...

> 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.

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.

Re: ns/tmp-objdir and ns/remerge-diff

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> 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/
>
>>  Also ns/remerge-diff that is Neeraj's rebase of the
>> remerge-diff topic needs Elijah's Ack at least.
>
> Mark it as expecting a re-roll; I've been waiting for ns/tmp-objdir to
> settle so I can rebase on it.

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.

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

[Elijah Newren]

On Fri, Dec 3, 2021 at 10:37 AM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Wed, Dec 01 2021, Elijah Newren wrote:
>
> > On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> 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.
> >
> > 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.
>
> 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...

:-)

> > 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.
>
> 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()).

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:

> 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.

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'.

> 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.

`--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.

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

[Ævar Arnfjörð Bjarmason]

On Fri, Dec 03 2021, Elijah Newren wrote:

> On Fri, Dec 3, 2021 at 10:37 AM Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Wed, Dec 01 2021, Elijah Newren wrote:
>>
>> > On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> 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.
>> >
>> > 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.
>>
>> 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...
>
> :-)
>
>> > 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.
>>
>> 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()).
>
> 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:
>
>> 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.
>
> 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'.
>
>> 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.
>
> `--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.

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".

Re: ns/tmp-objdir and ns/remerge-diff

[Neeraj Singh]

On Fri, Dec 03, 2021 at 11:21:13AM -0800, Junio C Hamano wrote:
> Elijah Newren <newren@gmail.com> writes:
> 
> > 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/
> >
> >>  Also ns/remerge-diff that is Neeraj's rebase of the
> >> remerge-diff topic needs Elijah's Ack at least.
> >
> > Mark it as expecting a re-roll; I've been waiting for ns/tmp-objdir to
> > settle so I can rebase on it.
> 
> 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.

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

Re: ns/tmp-objdir and ns/remerge-diff

[Elijah Newren]

On Fri, Dec 3, 2021 at 6:58 PM Neeraj Singh <nksingh85@gmail.com> wrote:
>
> On Fri, Dec 03, 2021 at 11:21:13AM -0800, Junio C Hamano wrote:
> > Elijah Newren <newren@gmail.com> writes:
> >
> > > 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/
> > >
> > >>  Also ns/remerge-diff that is Neeraj's rebase of the
> > >> remerge-diff topic needs Elijah's Ack at least.
> > >
> > > Mark it as expecting a re-roll; I've been waiting for ns/tmp-objdir to
> > > settle so I can rebase on it.
> >
> > 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.
>
> I submitted a new PR (with a new mail thread) for ns/tmp-objdir.  Hopefully
> that one can sail in smoothly now.

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.  :-)

> 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.

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

[Aleen 徐沛文]

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

> On Fri, Dec 03 2021, Elijah Newren wrote:
> 
> > On Fri, Dec 3, 2021 at 10:37 AM Ævar Arnfjörð Bjarmason
> > <avarab@gmail.com> wrote:
> >>
> >> On Wed, Dec 01 2021, Elijah Newren wrote:
> >>
> >> > On Tue, Nov 30, 2021 at 5:42 PM Aleen 徐沛文 <pwxu@coremail.cn> 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.
> >> >
> >> > 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.
> >>
> >> 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...
> >
> > :-)
> >
> >> > 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.
> >>
> >> 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()).
> >
> > 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:
> >
> >> 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.
> >
> > 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'.
> >
> >> 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.
> >
> > `--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.
> 
> 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".

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

[Junio C Hamano]

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

> 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.

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).

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

[Aleen 徐沛文]

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;


> > 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.
> 
> 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).

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

[Aleen 徐沛文]

>     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);

Sorry for the missing "break" here.

>     case DIE_EMPTY_COMMIT:
>         am_destroy(state);
>         die(_("Patch is empty."));
>         break;

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

[Junio C Hamano]

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

> 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, 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.

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

[Elijah Newren]

On Sun, Dec 5, 2021 at 10:28 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Aleen 徐沛文 <pwxu@coremail.cn> writes:
>
> > 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.
>
> 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

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.

> (on the other hand, I find 'ask' highly
> unnatural since we do not ask anything---we just throw the control
> back the user).

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

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

[Junio C Hamano]

Elijah Newren <newren@gmail.com> writes:

> 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.

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.

>> (on the other hand, I find 'ask' highly
>> unnatural since we do not ask anything---we just throw the control
>> back the user).
>
> Okay, but what about my previous suggestions of 'stop' or 'interrupt'?

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.

Re: [PATCH 0/3] unused-parameter cleanups on top of pw/xdiff-classify-record-in-histogram

[Phillip Wood]

Hi Peff

On 03/12/2021 05:10, Jeff King wrote:
> On Mon, Nov 29, 2021 at 06:16:54PM -0800, Junio C Hamano wrote:
> 
>> * 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'.
> 
> 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

Thanks, all three patches look good to me

Best Wishes

Phillip

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

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

[Aleen 徐沛文]

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

> 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.

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

[Junio C Hamano]

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

>> 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.

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

>     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)'?

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).

>> 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.

This still stands, too.

Thanks.

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

[Aleen 徐沛文]

> 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.

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.