What's cooking in git.git (Aug 2020, #07; Thu, 27)

What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Junio C Hamano]

Here are the topics that have been cooking.  Commits prefixed with '-' are
only in 'seen' (formerly 'pu'---proposed updates) while commits prefixed
with '+' are in 'next'.  The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.

You can find the changes described here in the integration branches of the
repositories listed at

    http://git-blame.blogspot.com/p/git-public-repositories.html

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

* en/mem-pool (2020-08-18) 3 commits
  (merged to 'next' on 2020-08-19 at eff9ad46f0)
 + mem-pool: use consistent pool variable name
 + mem-pool: use more standard initialization and finalization
 + mem-pool: add convenience functions for strdup and strndup

 API update.


* hn/refs-fetch-head-is-special (2020-08-19) 4 commits
  (merged to 'next' on 2020-08-21 at def233ab43)
 + refs: read FETCH_HEAD and MERGE_HEAD generically
 + refs: move gitdir into base ref_store
 + refs: fix comment about submodule ref_stores
 + refs: split off reading loose ref data in separate function
 (this branch is used by hn/refs-pseudorefs.)

 The FETCH_HEAD is now always read from the filesystem regardless of
 the ref backend in use, as its format is much richer than the
 normal refs, and written directly by "git fetch" as a plain file..


* jk/leakfix (2020-08-17) 7 commits
  (merged to 'next' on 2020-08-21 at a8b25a2657)
 + submodule--helper: fix leak of core.worktree value
 + config: fix leak in git_config_get_expiry_in_days()
 + config: drop git_config_get_string_const()
 + config: fix leaks from git_config_get_string_const()
 + checkout: fix leak of non-existent branch names
 + submodule--helper: use strbuf_release() to free strbufs
 + clear_pattern_list(): clear embedded hashmaps

 Code clean-up.


* rz/complete-more-options (2020-08-19) 2 commits
  (merged to 'next' on 2020-08-21 at ba8f4c8cb1)
 + completion: add GIT_COMPLETION_SHOW_ALL env var
 + parse-options: add --git-completion-helper-all

 Command line completion (in contrib/) usually omits redundant,
 deprecated and/or dangerous options from its output; it learned to
 optionally include all of them.

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

* jc/remove-pack-redundant (2020-08-25) 1 commit
 - pack-redundant: gauge the usage before proposing its removal

 The first step to remove "git pack-redundant" by soliciting
 objections.


* ps/ref-transaction-hook (2020-08-25) 1 commit
  (merged to 'next' on 2020-08-27 at 49b3fb8349)
 + refs: remove lookup cache for reference-transaction hook

 Code simplification by removing ineffective optimization.

 Will merge to 'master'.


* jc/undash-in-tree-git-callers (2020-08-27) 3 commits
  (merged to 'next' on 2020-08-27 at 671fa2f87e)
 + credential-cache: use child_process.args
 + cvsexportcommit: do not run git programs in dashed form
 + transport-helper: do not run git-remote-ext etc. in dashed form
 (this branch is used by jc/war-on-dashed-git.)

 A handful of places in in-tree code still relied on being able to
 execute the git subcommands, especially built-ins, in "git-foo"
 form, which have been corrected.

 Will merge to 'master'.


* jc/war-on-dashed-git (2020-08-27) 1 commit
 - git: catch an attempt to run "git-foo"
 (this branch uses jc/undash-in-tree-git-callers.)

 The first step to remove on-disk binaries for built-in subcommands
 by soliciting objections.

 On hold for now.


* jk/rev-input-given-fix (2020-08-26) 1 commit
  (merged to 'next' on 2020-08-27 at da291a327c)
 + revision: set rev_input_given in handle_revision_arg()

 Feeding "$ZERO_OID" to "git log --ignore-missing --stdin", and
 running "git log --ignore-missing $ZERO_OID" fell back to start
 digging from HEAD; it has been corrected to become a no-op, like
 "git log --tags=no-tag-matches-this-pattern" does.

 Will merge to 'master'.


* tb/repack-clearing-midx (2020-08-26) 1 commit
  (merged to 'next' on 2020-08-27 at a465875cbb)
 + builtin/repack.c: invalidate MIDX only when necessary

 When a packfile is removed by "git repack", multi-pack-index gets
 cleared; the code was taught to do so less aggressively by first
 checking if the midx actually refers to a pack that no longer
 exists.

 Will merge to 'master'.


* jc/run-command-use-embedded-args (2020-08-26) 1 commit
  (merged to 'next' on 2020-08-27 at c2b688e8e9)
 + run_command: teach API users to use embedded 'args' more

 Various callers of run_command API has been modernized.

 Will merge to 'master'.


* es/worktree-repair (2020-08-27) 5 commits
 - init: make --separate-git-dir work from within linked worktree
 - init: teach --separate-git-dir to repair linked worktrees
 - worktree: teach "repair" to fix outgoing links to worktrees
 - worktree: teach "repair" to fix worktree back-links to main worktree
 - worktree: add skeleton "repair" command

 "git worktree repair" command to correct on-disk pointers between
 the repository and its secondary working trees.

 Expecting a reroll.


* jk/worktree-check-clean-leakfix (2020-08-27) 1 commit
 - worktree: fix leak in check_clean_worktree()

 Leakfix.

 Will merge to 'next'.


* so/pretty-abbrev-doc (2020-08-27) 1 commit
 - pretty-options.txt: fix --no-abbrev-commit description

 Documentation update for "--no-abbrev-commit".

 Will merge to 'next'.


* ss/submodule-summary-in-c-fixes (2020-08-27) 3 commits
 - t7421: eliminate 'grep' check in t7421.4 for mingw compatibility
 - submodule: fix style in function definition
 - submodule: eliminate unused parameters from print_submodule_summary()
 (this branch uses ss/submodule-summary-in-c.)

 Fixups to a topic in 'next'.

 Will merge to 'next'.

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

* tb/bloom-improvements (2020-08-11) 14 commits
 - builtin/commit-graph.c: introduce '--max-new-filters=<n>'
 - commit-graph: rename 'split_commit_graph_opts'
 - commit-graph: add large-filters bitmap chunk
 - commit-graph.c: sort index into commits list
 - bloom/diff: properly short-circuit on max_changes
 - bloom: use provided 'struct bloom_filter_settings'
 - csum-file.h: introduce 'hashwrite_be64()'
 - bloom: split 'get_bloom_filter()' in two
 - commit-graph.c: store maximum changed paths
 - commit-graph: respect 'commitGraph.readChangedPaths'
 - t/helper/test-read-graph.c: prepare repo settings
 - commit-graph: pass a 'struct repository *' in more places
 - t4216: use an '&&'-chain
 - commit-graph: introduce 'get_bloom_filter_settings()'

 Misc Bloom filter improvements.

 Expecting a reroll.
 It seems that the review is getting closer to result in another update.
 cf. <20200811220503.GC66656@syl.lan>


* es/config-hooks (2020-07-30) 6 commits
 - hook: add 'run' subcommand
 - parse-options: parse into argv_array
 - hook: add --porcelain to list command
 - hook: add list command
 - hook: scaffolding for git-hook subcommand
 - doc: propose hooks managed by the config

 The "hooks defined in config" topic.

 Expecting a reroll.
 Now jk/strvec is in 'master', we may want to see the topic reworked
 on top of it.  Are there unresolved issues, or does the topic need
 a round of detailed review?
 cf. <xmqqmu3i9kvg.fsf@gitster.c.googlers.com>


* mt/grep-sparse-checkout (2020-06-12) 6 commits
 - config: add setting to ignore sparsity patterns in some cmds
 - grep: honor sparse checkout patterns
 - config: correctly read worktree configs in submodules
 - t/helper/test-config: facilitate addition of new cli options
 - t/helper/test-config: return exit codes consistently
 - doc: grep: unify info on configuration variables

 "git grep" has been tweaked to be limited to the sparse checkout
 paths.

 Review needed on 4/6; otherwise looking sane.
 cf. <CABPp-BGdEyEeajYZj_rdxp=MyEQdszuyjVTax=hhYj3fOtRQUQ@mail.gmail.com>


* ls/mergetool-meld-auto-merge (2020-07-12) 2 commits
 - SQUASH???
 - Support auto-merge for meld to follow the vim-diff behavior

 The 'meld' backend of the "git mergetool" learned to give the
 underlying 'meld' the '--auto-merge' option, which would help
 reduce the amount of text that requires manual merging.

 Expecting a reroll.


* mf/submodule-summary-with-correct-repository (2020-06-24) 2 commits
 - submodule: use submodule repository when preparing summary
 - revision: use repository from rev_info when parsing commits

 "git diff/show" on a change that involves a submodule used to read
 the information on commits in the submodule from a wrong repository
 and gave a wrong information when the commit-graph is involved.

 Needs tests.


* dr/push-remoteref-fix (2020-04-23) 1 commit
 - remote.c: fix handling of %(push:remoteref)

 The "%(push:remoteref)" placeholder in the "--format=" argument of
 "git format-patch" (and friends) only showed what got explicitly
 configured, not what ref at the receiving end would be updated when
 "git push" was used, as it ignored the default behaviour (e.g. update
 the same ref as the source).

 Expecting a reroll.
 cf. <20200416152145.wp2zeibxmuyas6y6@feanor>


* mr/bisect-in-c-2 (2020-07-17) 14 commits
 . SQUASH??? do not add new users of git_path_bisect_head()
 . bisect--helper: retire `--bisect-autostart` subcommand
 . bisect--helper: retire `--write-terms` subcommand
 . bisect--helper: retire `--check-expected-revs` subcommand
 . bisect--helper: reimplement `bisect_state` & `bisect_head` shell functions in C
 . bisect--helper: retire `--next-all` subcommand
 . bisect--helper: retire `--bisect-clean-state` subcommand
 . bisect--helper: finish porting `bisect_start()` to C
 . bisect--helper: reimplement `bisect_next` and `bisect_auto_next` shell functions in C
 . bisect: call 'clear_commit_marks_all()' in 'bisect_next_all()'
 . bisect--helper: reimplement `bisect_autostart` shell function in C
 . bisect--helper: introduce new `write_in_file()` function
 . bisect--helper: use '-res' in 'cmd_bisect__helper' return
 . bisect--helper: BUG() in cmd_*() on invalid subcommand

 Rewrite of the remainder of "git bisect" script in C continues.

 Needs more work.
 Ejected out of 'seen'; al/bisect-first-parent topic has a bit of
 textual conflict with this topic.


* mk/use-size-t-in-zlib (2018-10-15) 1 commit
 - zlib.c: use size_t for size

 The wrapper to call into zlib followed our long tradition to use
 "unsigned long" for sizes of regions in memory, which have been
 updated to use "size_t".

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

* jk/refspecs-cleanup (2020-08-17) 2 commits
  (merged to 'next' on 2020-08-24 at 807a080ebf)
 + refspec: make sure stack refspec_item variables are zeroed
 + refspec: fix documentation referring to refspec_item
 (this branch is used by jk/refspecs-negative.)

 Preliminary code clean-up before introducing "negative refspec".

 Will merge to 'master'.


* rs/checkout-no-overlay-pathspec-fix (2020-08-22) 1 commit
  (merged to 'next' on 2020-08-27 at 277e39346d)
 + checkout, restore: make pathspec recursive

 "git restore/checkout --no-overlay" with wildcarded pathspec
 mistakenly removed matching paths in subdirectories, which has been
 corrected.

 Will merge to 'master'.


* al/bisect-first-parent (2020-08-22) 1 commit
  (merged to 'next' on 2020-08-24 at f95fbf45a6)
 + bisect: add first-parent option to documentation

 Finishing touches.

 Will merge to 'master'.


* js/no-builtins-on-disk-option (2020-08-24) 3 commits
 - ci: stop linking built-ins to the dashed versions
 - install: optionally skip linking/copying the built-ins
 - msvc: copy the correct `.pdb` files in the Makefile target `install`

 The installation procedure learned to optionally omit "git-foo"
 executable files for each 'foo' built-in subcommand, which are only
 required by old timers that still rely on the age old promise that
 prepending "git --exec-path" output to PATH early in their script
 will keep the "git-foo" calls they wrote working.

 The old attempt to remove these executables from the disk failed in
 the 1.6 era; it may be worth attempting again, but I think it is
 worth to keep this topic separate from such a policy change to help
 it graduate early.

 cf. https://public-inbox.org/git/7vprnzt7d5.fsf@gitster.siamese.dyndns.org/


* jt/threaded-index-pack (2020-08-27) 9 commits
 - builtin/index-pack.c: fix some sparse warnings
 - fixup! index-pack: make quantum of work smaller
 - index-pack: make quantum of work smaller
 - index-pack: make resolve_delta() assume base data
 - index-pack: calculate {ref,ofs}_{first,last} early
 - index-pack: remove redundant child field
 - index-pack: unify threaded and unthreaded code
 - index-pack: remove redundant parameter
 - Documentation: deltaBaseCacheLimit is per-thread

 "git index-pack" learned to resolve deltified objects with greater
 parallelism.


* hn/refs-pseudorefs (2020-08-21) 4 commits
  (merged to 'next' on 2020-08-24 at 3579abe8ff)
 + sequencer: treat REVERT_HEAD as a pseudo ref
 + builtin/commit: suggest update-ref for pseudoref removal
 + sequencer: treat CHERRY_PICK_HEAD as a pseudo ref
 + refs: make refs_ref_exists public

 Accesses to two pseudorefs have been updated to properly use ref
 API.

 Will merge to 'master'.


* jt/promisor-pack-fix (2020-08-20) 1 commit
  (merged to 'next' on 2020-08-24 at cd26d30d8d)
 + fetch-pack: in partial clone, pass --promisor

 Updates into a lazy/partial clone with a submodule did not work
 well with transfer.fsckobjects set.

 Will merge to 'master'.


* hv/ref-filter-trailers-atom-parsing-fix (2020-08-21) 2 commits
  (merged to 'next' on 2020-08-24 at 79b27f3263)
 + ref-filter: 'contents:trailers' show error if `:` is missing
 + t6300: unify %(trailers) and %(contents:trailers) tests

 The parser for "git for-each-ref --format=..." was too loose when
 parsing the "%(trailers...)" atom, and forgot that "trailers" and
 "trailers:<modifers>" are the only two allowed forms, which has
 been corrected.

 Will merge to 'master'.


* jc/ident-whose-ident (2020-08-21) 1 commit
  (merged to 'next' on 2020-08-27 at caf5149c28)
 + ident: say whose identity is missing when giving user.name hint

 Error message update.

 Will merge to 'master'.


* jk/index-pack-w-more-threads (2020-08-21) 3 commits
  (merged to 'next' on 2020-08-24 at 18f18a5b66)
 + index-pack: adjust default threading cap
 + p5302: count up to online-cpus for thread tests
 + p5302: disable thread-count parameter tests by default

 Long ago, we decided to use 3 threads by default when running the
 index-pack task in parallel, which has been adjusted a bit upwards.

 Will merge to 'master'.


* rp/apply-cached-doc (2020-08-20) 1 commit
  (merged to 'next' on 2020-08-27 at 1d610f08ea)
 + git-apply.txt: update descriptions of --cached, --index

 The description of --cached/--index options in "git apply --help"
 has been updated.

 Will merge to 'master'.


* dd/diff-customize-index-line-abbrev (2020-08-21) 2 commits
  (merged to 'next' on 2020-08-24 at 74e842a2c8)
 + diff: index-line: respect --abbrev in object's name
 + t4013: improve diff-post-processor logic

 The output from the "diff" family of the commands had abbreviated
 object names of blobs involved in the patch, but its length was not
 affected by the --abbrev option.  Now it is.

 Will merge to 'master'.


* hv/ref-filter-misc (2020-08-17) 9 commits
  (merged to 'next' on 2020-08-27 at c015fa6b0f)
 + ref-filter: add `sanitize` option for 'subject' atom
 + format-support: move `format_sanitized_subject()` from pretty
 + pretty: refactor `format_sanitized_subject()`
 + ref-filter: add `short` modifier to 'parent' atom
 + ref-filter: add `short` modifier to 'tree' atom
 + ref-filter: rename `objectname` related functions and fields
 + ref-filter: modify error messages in `grab_objectname()`
 + ref-filter: refactor `grab_objectname()`
 + ref-filter: support different email formats

 The "--format=" option to the "for-each-ref" command and friends
 learned a few more tricks, e.g. the ":short" suffix that applies to
 "objectname" now also can be used for "parent", "tree", etc.

 Will merge to 'master'.


* jk/refspecs-negative (2020-08-21) 1 commit
 - refspec: add support for negative refspecs
 (this branch uses jk/refspecs-cleanup.)

 "negative refspecs"


* jt/fetch-pack-loosen-validation-with-packfile-uri (2020-08-24) 3 commits
  (merged to 'next' on 2020-08-27 at efd171f172)
 + fetch-pack: make packfile URIs work with transfer.fsckobjects
 + fetch-pack: document only_packfile in get_pack()
 + (various): document from_promisor parameter

 Bugfix for "git fetch" when the packfile URI capability is in use.

 Will merge to 'master'.


* mr/diff-hide-stat-wo-textual-change (2020-08-19) 1 commit
  (merged to 'next' on 2020-08-27 at b9e97254ae)
 + diff: teach --stat to ignore uninteresting modifications

 "git diff --stat -w" showed 0-line changes for paths whose changes
 were only whitespaces, which was not intuitive.  We now omit such
 paths from the stat output.

 Will merge to 'master'.


* pw/add-p-allowed-options-fix (2020-08-17) 2 commits
  (merged to 'next' on 2020-08-27 at 6cd62753f7)
 + add -p: fix checking of user input
 + add -p: use ALLOC_GROW_BY instead of ALLOW_GROW

 "git add -p" update.

 Will merge to 'master'.


* jt/lazy-fetch (2020-08-18) 7 commits
  (merged to 'next' on 2020-08-27 at 85f2319ba1)
 + fetch-pack: remove no_dependents code
 + promisor-remote: lazy-fetch objects in subprocess
 + fetch-pack: do not lazy-fetch during ref iteration
 + fetch: only populate existing_refs if needed
 + fetch: avoid reading submodule config until needed
 + fetch: allow refspecs specified through stdin
 + negotiator/noop: add noop fetch negotiator

 Updates to on-demand fetching code in lazily cloned repositories.

 Will merge to 'master'.


* jx/proc-receive-hook (2020-08-27) 10 commits
 - doc: add documentation for the proc-receive hook
 - transport: parse report options for tracking refs
 - t5411: test updates of remote-tracking branches
 - receive-pack: new config receive.procReceiveRefs
 - doc: add document for capability report-status-v2
 - New capability "report-status-v2" for git-push
 - receive-pack: feed report options to post-receive
 - receive-pack: add new proc-receive hook
 - t5411: add basic test cases for proc-receive hook
 - transport: not report a non-head push as a branch

 "git receive-pack" that accepts requests by "git push" learned to
 outsource most of the ref updates to the new "proc-receive" hook.

 Looking good.


* pw/rebase-i-more-options (2020-08-26) 6 commits
  (merged to 'next' on 2020-08-27 at c55cfeb247)
 + t3436: do not run git-merge-recursive in dashed form
  (merged to 'next' on 2020-08-21 at ade71fd49b)
 + rebase: add --reset-author-date
 + rebase -i: support --ignore-date
 + rebase -i: support --committer-date-is-author-date
 + am: stop exporting GIT_COMMITTER_DATE
 + rebase -i: add --ignore-whitespace flag

 "git rebase -i" learns a bit more options.

 Will merge to 'master'.


* jk/slimmed-down (2020-08-13) 5 commits
  (merged to 'next' on 2020-08-27 at bc8e9450c6)
 + drop vcs-svn experiment
 + make git-fast-import a builtin
 + make git-bugreport a builtin
 + make credential helpers builtins
 + Makefile: drop builtins from MSVC pdb list

 Trim an unused binary and turn a bunch of commands into built-in.

 Will merge to 'master'.


* ss/t7401-modernize (2020-08-21) 5 commits
  (merged to 'next' on 2020-08-27 at 516cba9c64)
 + t7401: add a NEEDSWORK
 + t7401: change indentation for enhanced readability
 + t7401: change syntax of test_i18ncmp calls for clarity
 + t7401: use 'short' instead of 'verify' and cut in rev-parse calls
 + t7401: modernize style

 Test clean-up.

 Will merge to 'master'.


* ds/maintenance-part-2 (2020-08-25) 8 commits
 - maintenance: add incremental-repack auto condition
 - maintenance: auto-size incremental-repack batch
 - maintenance: add incremental-repack task
 - midx: use start_delayed_progress()
 - midx: enable core.multiPackIndex by default
 - maintenance: create auto condition for loose-objects
 - maintenance: add loose-objects task
 - maintenance: add prefetch task
 (this branch uses ds/maintenance-part-1.)

 "git maintenance", an extended big brother of "git gc", continues
 to evolve.


* ss/submodule-summary-in-c (2020-08-12) 4 commits
  (merged to 'next' on 2020-08-17 at 9bc352cb70)
 + submodule: port submodule subcommand 'summary' from shell to C
 + t7421: introduce a test script for verifying 'summary' output
 + submodule: rename helper functions to avoid ambiguity
 + submodule: remove extra line feeds between callback struct and macro
 (this branch is used by ss/submodule-summary-in-c-fixes.)

 Yet another subcommand of "git submodule" is getting rewritten in C.

 Looking good.


* am/ci-wsfix (2020-08-21) 1 commit
  (merged to 'next' on 2020-08-24 at 8491e031f1)
 + ci: fix inconsistent indentation

 Aesthetic fix to a CI configuration file.

 Will merge to 'master'.


* ds/maintenance-part-1 (2020-08-25) 11 commits
 - maintenance: add trace2 regions for task execution
 - maintenance: add auto condition for commit-graph task
 - maintenance: use pointers to check --auto
 - maintenance: create maintenance.<task>.enabled config
 - maintenance: take a lock on the objects directory
 - maintenance: add --task option
 - maintenance: add commit-graph task
 - maintenance: initialize task array
 - maintenance: replace run_auto_gc()
 - maintenance: add --quiet option
 - maintenance: create basic maintenance runner
 (this branch is used by ds/maintenance-part-2.)

 A "git gc"'s big brother has been introduced to take care of more
 repository maintenance tasks, not limited to the object database
 cleaning.

 Comments?

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Jeff King]

On Thu, Aug 27, 2020 at 02:43:02PM -0700, Junio C Hamano wrote:

> * tb/repack-clearing-midx (2020-08-26) 1 commit
>   (merged to 'next' on 2020-08-27 at a465875cbb)
>  + builtin/repack.c: invalidate MIDX only when necessary
> 
>  When a packfile is removed by "git repack", multi-pack-index gets
>  cleared; the code was taught to do so less aggressively by first
>  checking if the midx actually refers to a pack that no longer
>  exists.
> 
>  Will merge to 'master'.

This seems to break t7700 when run with midx support:

  $ git checkout e08f7bb093
  HEAD is now at e08f7bb093 builtin/repack.c: invalidate MIDX only when necessary

  $ make && (cd t && GIT_TEST_MULTI_PACK_INDEX=1 ./t7700-repack.sh -i)
  [...]
  + git repack -a -d
  Enumerating objects: 10, done.
  Counting objects: 100% (10/10), done.
  Delta compression using up to 16 threads
  Compressing objects: 100% (5/5), done.
  Writing objects: 100% (10/10), done.
  Total 10 (delta 1), reused 10 (delta 1), pack-reused 0
  fatal: error preparing packfile from multi-pack-index
  error: last command exited with $?=128
  not ok 6 - packed obs in alt ODB are repacked when local repo has packs
  #	
  #		rm -f .git/objects/pack/* &&
  #		echo new_content >>file1 &&
  #		git add file1 &&
  #		test_tick &&
  #		git commit -m more_content &&
  #		git repack &&
  #		git repack -a -d &&
  #		test_no_missing_in_packs

I didn't look into whether it's a bug in the actual code, or just a
weird interaction with the way GIT_TEST_MULTI_PACK_INDEX triggers
git-repack to write a midx. But either way we should figure that out
before it graduates.

-Peff

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Thu, Aug 27, 2020 at 02:43:02PM -0700, Junio C Hamano wrote:
>
>> * tb/repack-clearing-midx (2020-08-26) 1 commit
>>   (merged to 'next' on 2020-08-27 at a465875cbb)
>>  + builtin/repack.c: invalidate MIDX only when necessary
>> 
>>  When a packfile is removed by "git repack", multi-pack-index gets
>>  cleared; the code was taught to do so less aggressively by first
>>  checking if the midx actually refers to a pack that no longer
>>  exists.
>> 
>>  Will merge to 'master'.
>
> This seems to break t7700 when run with midx support:
>
>   $ git checkout e08f7bb093
>   HEAD is now at e08f7bb093 builtin/repack.c: invalidate MIDX only when necessary
>
>   $ make && (cd t && GIT_TEST_MULTI_PACK_INDEX=1 ./t7700-repack.sh -i)
>   [...]
>   + git repack -a -d
>   Enumerating objects: 10, done.
>   Counting objects: 100% (10/10), done.
>   Delta compression using up to 16 threads
>   Compressing objects: 100% (5/5), done.
>   Writing objects: 100% (10/10), done.
>   Total 10 (delta 1), reused 10 (delta 1), pack-reused 0
>   fatal: error preparing packfile from multi-pack-index
>   error: last command exited with $?=128
>   not ok 6 - packed obs in alt ODB are repacked when local repo has packs
>   #	
>   #		rm -f .git/objects/pack/* &&
>   #		echo new_content >>file1 &&
>   #		git add file1 &&
>   #		test_tick &&
>   #		git commit -m more_content &&
>   #		git repack &&
>   #		git repack -a -d &&
>   #		test_no_missing_in_packs
>
> I didn't look into whether it's a bug in the actual code, or just a
> weird interaction with the way GIT_TEST_MULTI_PACK_INDEX triggers
> git-repack to write a midx. But either way we should figure that out
> before it graduates.

Thanks for stopping us ;-)

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Taylor Blau]

On Thu, Aug 27, 2020 at 04:36:43PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > On Thu, Aug 27, 2020 at 02:43:02PM -0700, Junio C Hamano wrote:
> >
> >> * tb/repack-clearing-midx (2020-08-26) 1 commit
> >>   (merged to 'next' on 2020-08-27 at a465875cbb)
> >>  + builtin/repack.c: invalidate MIDX only when necessary
> >>
> >>  When a packfile is removed by "git repack", multi-pack-index gets
> >>  cleared; the code was taught to do so less aggressively by first
> >>  checking if the midx actually refers to a pack that no longer
> >>  exists.
> >>
> >>  Will merge to 'master'.
> >
> > This seems to break t7700 when run with midx support:
> >
> >   $ git checkout e08f7bb093
> >   HEAD is now at e08f7bb093 builtin/repack.c: invalidate MIDX only when necessary
> >
> >   $ make && (cd t && GIT_TEST_MULTI_PACK_INDEX=1 ./t7700-repack.sh -i)
> >   [...]
> >   + git repack -a -d
> >   Enumerating objects: 10, done.
> >   Counting objects: 100% (10/10), done.
> >   Delta compression using up to 16 threads
> >   Compressing objects: 100% (5/5), done.
> >   Writing objects: 100% (10/10), done.
> >   Total 10 (delta 1), reused 10 (delta 1), pack-reused 0
> >   fatal: error preparing packfile from multi-pack-index
> >   error: last command exited with $?=128
> >   not ok 6 - packed obs in alt ODB are repacked when local repo has packs
> >   #
> >   #		rm -f .git/objects/pack/* &&
> >   #		echo new_content >>file1 &&
> >   #		git add file1 &&
> >   #		test_tick &&
> >   #		git commit -m more_content &&
> >   #		git repack &&
> >   #		git repack -a -d &&
> >   #		test_no_missing_in_packs
> >
> > I didn't look into whether it's a bug in the actual code, or just a
> > weird interaction with the way GIT_TEST_MULTI_PACK_INDEX triggers
> > git-repack to write a midx. But either way we should figure that out
> > before it graduates.
>
> Thanks for stopping us ;-)

Thanks indeed. I started looking into this tonight thinking that it'd be
an easy fix, but I think there is a deeper bug that is worth
investigating further.

Let's eject this for now. If the bug turns out to be easier than I
thought, I'll send the patch again, otherwise I'll send it with my
larger series when that's ready.


Thanks,
Taylor

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Elijah Newren]

On Thu, Aug 27, 2020 at 2:46 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> * mt/grep-sparse-checkout (2020-06-12) 6 commits
>  - config: add setting to ignore sparsity patterns in some cmds
>  - grep: honor sparse checkout patterns
>  - config: correctly read worktree configs in submodules
>  - t/helper/test-config: facilitate addition of new cli options
>  - t/helper/test-config: return exit codes consistently
>  - doc: grep: unify info on configuration variables
>
>  "git grep" has been tweaked to be limited to the sparse checkout
>  paths.
>
>  Review needed on 4/6; otherwise looking sane.
>  cf. <CABPp-BGdEyEeajYZj_rdxp=MyEQdszuyjVTax=hhYj3fOtRQUQ@mail.gmail.com>

Kinda disappointed to see this stalled out; especially after so much
work put into it.  This spawned lots of other side discussions and a
topic or two outside this series as well.  I really like the overall
result we came up with out of this series and would like to see it
land.  If we can't get more reviewers, maybe we just merge it down
anyway?  But, in an attempt to prod some review...

Jonathan: I pinged you in chat about this series some time ago and you
said you'd take a look and comment.  I can't find a reference
anywhere, but maybe you talked with Matheus in IRC somewhere?  Do you
recall?

Matheus: Do you have any outstanding items for this series or is it
good as far as you know?

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Jeff King]

On Thu, Aug 27, 2020 at 08:39:40PM -0400, Taylor Blau wrote:

> > >> * tb/repack-clearing-midx (2020-08-26) 1 commit
> > >>   (merged to 'next' on 2020-08-27 at a465875cbb)
> > >>  + builtin/repack.c: invalidate MIDX only when necessary
> [...]
> Thanks indeed. I started looking into this tonight thinking that it'd be
> an easy fix, but I think there is a deeper bug that is worth
> investigating further.

I imagine this is part of it:

diff --git a/builtin/repack.c b/builtin/repack.c
index f10f52779c..2cc05f968a 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -134,7 +134,7 @@ static void remove_redundant_pack(const char *dir_name, const char *base_name)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct multi_pack_index *m = get_multi_pack_index(the_repository);
-	strbuf_addf(&buf, "%s.pack", base_name);
+	strbuf_addf(&buf, "%s.idx", base_name);
 	if (m && midx_contains_pack(m, buf.buf))
 		clear_midx_file(the_repository);
 	strbuf_insertf(&buf, 0, "%s/", dir_name);

but maybe that is just the "easy" part you meant. Several tests still
seem to fail, which I guess is the "deeper" part. :)

If I'm understanding midx_contains_pack() correctly, then the code
looking for ".pack" could never have matched, and we would never have
deleted a midx here. Which makes me wonder why the "repack removes
multi-pack-index when deleting packs" test ever succeeded.

> Let's eject this for now. If the bug turns out to be easier than I
> thought, I'll send the patch again, otherwise I'll send it with my
> larger series when that's ready.

It's in next, so the preferred path forward is generally to do a fix on
top. Unless it's so unsalvageable that we have to revert, but I doubt
that here.

-Peff

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Jeff King]

On Fri, Aug 28, 2020 at 02:26:19AM -0400, Jeff King wrote:

> On Thu, Aug 27, 2020 at 08:39:40PM -0400, Taylor Blau wrote:
> 
> > > >> * tb/repack-clearing-midx (2020-08-26) 1 commit
> > > >>   (merged to 'next' on 2020-08-27 at a465875cbb)
> > > >>  + builtin/repack.c: invalidate MIDX only when necessary
> > [...]
> > Thanks indeed. I started looking into this tonight thinking that it'd be
> > an easy fix, but I think there is a deeper bug that is worth
> > investigating further.
> 
> I imagine this is part of it:
> 
> diff --git a/builtin/repack.c b/builtin/repack.c
> index f10f52779c..2cc05f968a 100644
> --- a/builtin/repack.c
> +++ b/builtin/repack.c
> @@ -134,7 +134,7 @@ static void remove_redundant_pack(const char *dir_name, const char *base_name)
>  {
>  	struct strbuf buf = STRBUF_INIT;
>  	struct multi_pack_index *m = get_multi_pack_index(the_repository);
> -	strbuf_addf(&buf, "%s.pack", base_name);
> +	strbuf_addf(&buf, "%s.idx", base_name);
>  	if (m && midx_contains_pack(m, buf.buf))
>  		clear_midx_file(the_repository);
>  	strbuf_insertf(&buf, 0, "%s/", dir_name);
> 
> but maybe that is just the "easy" part you meant. Several tests still
> seem to fail, which I guess is the "deeper" part. :)
> 
> If I'm understanding midx_contains_pack() correctly, then the code
> looking for ".pack" could never have matched, and we would never have
> deleted a midx here. Which makes me wonder why the "repack removes
> multi-pack-index when deleting packs" test ever succeeded.

Sorry, this is all nonsense.

I forgot about the hackery added in 013fd7ada3 (midx: check both pack
and index names for containment, 2019-04-05). And anyway, the patch
above is totally bogus because we need the ".pack" form in the buffer
when we call unlink_pack_path().

So there's definitely something more odd going on in that failing test.

-Peff

ds/maintenance-part-[1-2] (was: What's cooking in git.git (Aug 2020, #07; Thu, 27))

[Derrick Stolee]

On 8/27/2020 5:43 PM, Junio C Hamano wrote:
> * ds/maintenance-part-2 (2020-08-25) 8 commits
>  - maintenance: add incremental-repack auto condition
>  - maintenance: auto-size incremental-repack batch
>  - maintenance: add incremental-repack task
>  - midx: use start_delayed_progress()
>  - midx: enable core.multiPackIndex by default
>  - maintenance: create auto condition for loose-objects
>  - maintenance: add loose-objects task
>  - maintenance: add prefetch task
>  (this branch uses ds/maintenance-part-1.)
> 
>  "git maintenance", an extended big brother of "git gc", continues
>  to evolve.

> * ds/maintenance-part-1 (2020-08-25) 11 commits
>  - maintenance: add trace2 regions for task execution
>  - maintenance: add auto condition for commit-graph task
>  - maintenance: use pointers to check --auto
>  - maintenance: create maintenance.<task>.enabled config
>  - maintenance: take a lock on the objects directory
>  - maintenance: add --task option
>  - maintenance: add commit-graph task
>  - maintenance: initialize task array
>  - maintenance: replace run_auto_gc()
>  - maintenance: add --quiet option
>  - maintenance: create basic maintenance runner
>  (this branch is used by ds/maintenance-part-2.)
> 
>  A "git gc"'s big brother has been introduced to take care of more
>  repository maintenance tasks, not limited to the object database
>  cleaning.
> 
>  Comments?

Thanks for calling out a call for comments here.

I appreciate all of the comments on these series. It has
really improved the feature.

I'm interested to hear who still has this on their list
to review or if there are some remaining unresolved
issues.

I sent my "RFC v3" [1] for the background portion of
maintenance, and hope to send a non-RFC version next week
if there are no more radical shifts in the strategy. Please
take a look if you are interested.

[1] https://lore.kernel.org/git/pull.680.v3.git.1598629517.gitgitgadget@gmail.com/

Thanks,
-Stolee

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Taylor Blau]

On Fri, Aug 28, 2020 at 04:31:25AM -0400, Jeff King wrote:
> On Fri, Aug 28, 2020 at 02:26:19AM -0400, Jeff King wrote:
>
> > On Thu, Aug 27, 2020 at 08:39:40PM -0400, Taylor Blau wrote:
> >
> > If I'm understanding midx_contains_pack() correctly, then the code
> > looking for ".pack" could never have matched, and we would never have
> > deleted a midx here. Which makes me wonder why the "repack removes
> > multi-pack-index when deleting packs" test ever succeeded.
>
> Sorry, this is all nonsense.
>
> I forgot about the hackery added in 013fd7ada3 (midx: check both pack
> and index names for containment, 2019-04-05). And anyway, the patch
> above is totally bogus because we need the ".pack" form in the buffer
> when we call unlink_pack_path().

Yeah, 013fd7ada3 is definitely a hack, but what you have is definitely
incorrect.

> So there's definitely something more odd going on in that failing test.

It's the funky order that we load the MIDX chain in (which is that the
MIDX belonging to the deepest alternate shows up *first* as
'r->objects->multi_pack_index', and that the local MIDX shows up as the
last thing when traversing '->next').

I sent what I consider to be a little bit of a hack in [1], but it's
definitely enough to fix t7700.6.

> -Peff

Thanks for reporting it.

Taylor

[1]: https://lore.kernel.org/git/20200828180621.GA9036@nand.nand.local/T/#u

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Hariom verma]

Hi,

On Fri, Aug 28, 2020 at 3:14 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> --------------------------------------------------
> [Cooking]
>
> * hv/ref-filter-misc (2020-08-17) 9 commits
>   (merged to 'next' on 2020-08-27 at c015fa6b0f)
>  + ref-filter: add `sanitize` option for 'subject' atom
>  + format-support: move `format_sanitized_subject()` from pretty
>  + pretty: refactor `format_sanitized_subject()`
>  + ref-filter: add `short` modifier to 'parent' atom
>  + ref-filter: add `short` modifier to 'tree' atom
>  + ref-filter: rename `objectname` related functions and fields
>  + ref-filter: modify error messages in `grab_objectname()`
>  + ref-filter: refactor `grab_objectname()`
>  + ref-filter: support different email formats
>
>  The "--format=" option to the "for-each-ref" command and friends
>  learned a few more tricks, e.g. the ":short" suffix that applies to
>  "objectname" now also can be used for "parent", "tree", etc.
>
>  Will merge to 'master'.

I sent an updated version of the patch series addressing your comment
concerning new file format-support.{c,h}[1].

If you are still unsure about adding a new file, you might want to
take a look at the updated version of patch series:
https://public-inbox.org/git/pull.684.v4.git.1598046110.gitgitgadget@gmail.com/

Footnote:
[1]: https://public-inbox.org/git/xmqqlfid1305.fsf@gitster.c.googlers.com/

Thanks,
Hariom

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Junio C Hamano]

Hariom verma <hariom18599@gmail.com> writes:

>> * hv/ref-filter-misc (2020-08-17) 9 commits
>>   (merged to 'next' on 2020-08-27 at c015fa6b0f)
>>  + ref-filter: add `sanitize` option for 'subject' atom
>>  + format-support: move `format_sanitized_subject()` from pretty
>>  + pretty: refactor `format_sanitized_subject()`
>>  + ref-filter: add `short` modifier to 'parent' atom
>>  + ref-filter: add `short` modifier to 'tree' atom
>>  + ref-filter: rename `objectname` related functions and fields
>>  + ref-filter: modify error messages in `grab_objectname()`
>>  + ref-filter: refactor `grab_objectname()`
>>  + ref-filter: support different email formats
>>
>>  The "--format=" option to the "for-each-ref" command and friends
>>  learned a few more tricks, e.g. the ":short" suffix that applies to
>>  "objectname" now also can be used for "parent", "tree", etc.
>>
>>  Will merge to 'master'.
>
> I sent an updated version of the patch series addressing your comment
> concerning new file format-support.{c,h}[1].

Yikes, my mistake.  As long as you plan to vastly extend what would
be in format-support.[ch], I do not mind to have a pair of separate
files in the end, by the time when we have, say, unified "--format"
support for for-each-ref family (e.g. "%(token)") and log family
(e.g. "%x" fixed few letter combinations).  So the step that moves
some from pretty.c to format-support.c does not bother me all that
much.  It just felt unnecessary within the scope of this series.

But other stuff (like format-sanitized-subject having unnecessary
allocation and unnecessary special casing of LF) are worth fixing in
the version queued above.

Thanks for stopping me.

Let's revert the above out of 'next' and start afresh using v4.

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Matheus Tavares Bernardino]

Hi, Elijah

On Thu, Aug 27, 2020 at 11:16 PM Elijah Newren <newren@gmail.com> wrote:
>
> On Thu, Aug 27, 2020 at 2:46 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > * mt/grep-sparse-checkout (2020-06-12) 6 commits
> >  - config: add setting to ignore sparsity patterns in some cmds
> >  - grep: honor sparse checkout patterns
> >  - config: correctly read worktree configs in submodules
> >  - t/helper/test-config: facilitate addition of new cli options
> >  - t/helper/test-config: return exit codes consistently
> >  - doc: grep: unify info on configuration variables
> >
> >  "git grep" has been tweaked to be limited to the sparse checkout
> >  paths.
> >
> >  Review needed on 4/6; otherwise looking sane.
> >  cf. <CABPp-BGdEyEeajYZj_rdxp=MyEQdszuyjVTax=hhYj3fOtRQUQ@mail.gmail.com>
>
> Kinda disappointed to see this stalled out; especially after so much
> work put into it.  This spawned lots of other side discussions and a
> topic or two outside this series as well.  I really like the overall
> result we came up with out of this series and would like to see it
> land [...]

Yeah, I would also really like to see it land :) Thanks to your last
rounds of review, I think we got to a much better design regarding
each grep case.

>  [...] If we can't get more reviewers, maybe we just merge it down
> anyway?  But, in an attempt to prod some review...
>
> Jonathan: I pinged you in chat about this series some time ago and you
> said you'd take a look and comment.  I can't find a reference
> anywhere, but maybe you talked with Matheus in IRC somewhere?  Do you
> recall?
>
> Matheus: Do you have any outstanding items for this series or is it
> good as far as you know?

Hm, I don't have anything else planned to add.

Just one minor aesthetic consideration: should we perhaps eliminate
the sparse-checkout.c file (and the accompanying header) from the last
patch? This file contains only one function; It was created with the
idea of being later populated by another parallel series [1]. However,
if we only have this function for now, should we move it elsewhere?
I'm not sure where, though. Maybe to config.c, together with
git_config_get_index_threads() and others?

[1]: https://lore.kernel.org/git/2188577cd848d7cee77f06f1ad2b181864e5e36d.1588857462.git.gitgitgadget@gmail.com/

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Hariom verma]

Hi,

On Fri, Aug 28, 2020 at 3:14 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> [Cooking]
>
> * hv/ref-filter-trailers-atom-parsing-fix (2020-08-21) 2 commits
>   (merged to 'next' on 2020-08-24 at 79b27f3263)
>  + ref-filter: 'contents:trailers' show error if `:` is missing
>  + t6300: unify %(trailers) and %(contents:trailers) tests
>
>  The parser for "git for-each-ref --format=..." was too loose when
>  parsing the "%(trailers...)" atom, and forgot that "trailers" and
>  "trailers:<modifers>" are the only two allowed forms, which has
>  been corrected.
>
>  Will merge to 'master'.
>

After a discussion, we agreed on keeping the helper function instead
on duplicating code in "ref-filter: 'contents:trailers' show error if
`:` is missing"
There is a high possibility we might want to reuse helper in other places too.
Especially in the case of newly added "%(subject:sanitize)", if we
also want "%(contents:subject:sanitize)" to work.

Full discussion:
https://public-inbox.org/git/CA+CkUQ8Gst2RTaXY6t+ytWu_9Pu7eqnRYRrnawRwYd_NN=u0Lg@mail.gmail.com/

I'm about to send the updated patch series soon.

Thanks,
Hariom

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Hariom verma]

Hi,

On Sat, Aug 29, 2020 at 11:58 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Hariom verma <hariom18599@gmail.com> writes:
>
> > On Fri, Aug 28, 2020 at 3:14 AM Junio C Hamano <gitster@pobox.com> wrote:
> >>
> >> [Cooking]
> >>
> >> * hv/ref-filter-trailers-atom-parsing-fix (2020-08-21) 2 commits
> >>   (merged to 'next' on 2020-08-24 at 79b27f3263)
> >>  + ref-filter: 'contents:trailers' show error if `:` is missing
> >>  + t6300: unify %(trailers) and %(contents:trailers) tests
> >> ...
> >
> > After a discussion, we agreed on keeping the helper function instead
> > on duplicating code in "ref-filter: 'contents:trailers' show error if
> > `:` is missing"
> > There is a high possibility we might want to reuse helper in other places too.
> > Especially in the case of newly added "%(subject:sanitize)", if we
> > also want "%(contents:subject:sanitize)" to work.
> >
> > Full discussion:
> > https://public-inbox.org/git/CA+CkUQ8Gst2RTaXY6t+ytWu_9Pu7eqnRYRrnawRwYd_NN=u0Lg@mail.gmail.com/
> >
> > I'm about to send the updated patch series soon.
>
> IIRC, the code in question is good for the purpose of what already
> exists and is easier to follow without helper.  When we need to make
> the code more elaborabed and/or when we actually have the second
> callsite would be the ideal time to introduce such a helper as a
> preliminay clean-up patch early in such a follow-on series that
> would happen after the "fix" in question graduates, I would think.
>
> To be honest, I am not sure if we even need an incremental on top
> right now, unless we want to delay the two patches to fix real
> breakage above and make them wait for patches that adds features
> that require to call the same helper from elsewhere.
>

Yeah, I agree with you.
Let's not delay these 2 patches. Sorry for the noise though.

Thanks,
Hariom

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Hariom verma]

Hi,

On Sat, Aug 29, 2020 at 2:21 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> Hariom verma <hariom18599@gmail.com> writes:
>
> >> * hv/ref-filter-misc (2020-08-17) 9 commits
> >>   (merged to 'next' on 2020-08-27 at c015fa6b0f)
> >>  + ref-filter: add `sanitize` option for 'subject' atom
> >>  + format-support: move `format_sanitized_subject()` from pretty
> >>  + pretty: refactor `format_sanitized_subject()`
> >>  + ref-filter: add `short` modifier to 'parent' atom
> >>  + ref-filter: add `short` modifier to 'tree' atom
> >>  + ref-filter: rename `objectname` related functions and fields
> >>  + ref-filter: modify error messages in `grab_objectname()`
> >>  + ref-filter: refactor `grab_objectname()`
> >>  + ref-filter: support different email formats
> >>
> >>  The "--format=" option to the "for-each-ref" command and friends
> >>  learned a few more tricks, e.g. the ":short" suffix that applies to
> >>  "objectname" now also can be used for "parent", "tree", etc.
> >>
> >>  Will merge to 'master'.
> >
> > I sent an updated version of the patch series addressing your comment
> > concerning new file format-support.{c,h}[1].
>
> But other stuff (like format-sanitized-subject having unnecessary
> allocation and unnecessary special casing of LF) are worth fixing in
> the version queued above.

Yeah, I fixed that too in the v4 of the patch series.

> Let's revert the above out of 'next' and start afresh using v4.

Just saw v4 in the `seen` ;)

Thanks,
Hariom

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Junio C Hamano]

Hariom verma <hariom18599@gmail.com> writes:

> On Fri, Aug 28, 2020 at 3:14 AM Junio C Hamano <gitster@pobox.com> wrote:
>>
>> [Cooking]
>>
>> * hv/ref-filter-trailers-atom-parsing-fix (2020-08-21) 2 commits
>>   (merged to 'next' on 2020-08-24 at 79b27f3263)
>>  + ref-filter: 'contents:trailers' show error if `:` is missing
>>  + t6300: unify %(trailers) and %(contents:trailers) tests
>> ...
>
> After a discussion, we agreed on keeping the helper function instead
> on duplicating code in "ref-filter: 'contents:trailers' show error if
> `:` is missing"
> There is a high possibility we might want to reuse helper in other places too.
> Especially in the case of newly added "%(subject:sanitize)", if we
> also want "%(contents:subject:sanitize)" to work.
>
> Full discussion:
> https://public-inbox.org/git/CA+CkUQ8Gst2RTaXY6t+ytWu_9Pu7eqnRYRrnawRwYd_NN=u0Lg@mail.gmail.com/
>
> I'm about to send the updated patch series soon.

Days after the topic got merged to 'next' is not a time to send any
updated patch series.  It should come in the form of incremental
update on top of what is already there.

IIRC, the code in question is good for the purpose of what already
exists and is easier to follow without helper.  When we need to make
the code more elaborabed and/or when we actually have the second
callsite would be the ideal time to introduce such a helper as a
preliminay clean-up patch early in such a follow-on series that
would happen after the "fix" in question graduates, I would think.

To be honest, I am not sure if we even need an incremental on top
right now, unless we want to delay the two patches to fix real
breakage above and make them wait for patches that adds features
that require to call the same helper from elsewhere.

Thanks.

Re: What's cooking in git.git (Aug 2020, #07; Thu, 27)

[Junio C Hamano]

Hariom verma <hariom18599@gmail.com> writes:

> Yeah, I agree with you.
> Let's not delay these 2 patches. Sorry for the noise though.

Please do not be sorry, as you didn't raise any unnecessary noise.
Sanity checking each others' action is a good way to make sure we
collectively avoid mistakes, and you just did with the other topic
of yours yesterday, which was very appreciated.

Thanks.