What's cooking in git.git (Oct 2021, #02; Wed, 6)

What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Junio C Hamano]

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.

The eleventh batch is in.

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/repo-settings-cleanup (2021-09-22) 5 commits
  (merged to 'next' on 2021-09-28 at 43d70c31e1)
 + repository.h: don't use a mix of int and bitfields
 + repo-settings.c: simplify the setup
 + read-cache & fetch-negotiator: check "enum" values in switch()
 + environment.c: remove test-specific "ignore_untracked..." variable
 + wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c

 Code cleanup.


* ab/retire-decl-of-missing-unused-funcs (2021-10-01) 4 commits
  (merged to 'next' on 2021-10-03 at a49287eaa9)
 + config.h: remove unused git_config_get_untracked_cache() declaration
 + log-tree.h: remove unused function declarations
 + grep.h: remove unused grep_threads_ok() declaration
 + builtin.h: remove cmd_tar_tree() declaration

 Remove external declaration of functions that no longer exist.


* ab/retire-refs-unused-funcs (2021-09-28) 5 commits
  (merged to 'next' on 2021-10-03 at f91e74fa7d)
 + refs/ref-cache.[ch]: remove "incomplete" from create_dir_entry()
 + refs/ref-cache.c: remove "mkdir" parameter from find_containing_dir()
 + refs/ref-cache.[ch]: remove unused add_ref_entry()
 + refs/ref-cache.[ch]: remove unused remove_entry_from_dir()
 + refs.[ch]: remove unused ref_storage_backend_exists()

 Code cleanup.


* ab/retire-string-list-init (2021-09-28) 1 commit
  (merged to 'next' on 2021-10-03 at 4834949cc3)
 + string-list.[ch]: remove string_list_init() compatibility function

 Code cleanup.


* ew/midx-doc-update (2021-09-24) 1 commit
  (merged to 'next' on 2021-09-28 at f2cbe598eb)
 + doc/technical: update note about core.multiPackIndex

 Doc tweak.


* gc/doc-first-contribution-reroll (2021-09-22) 1 commit
  (merged to 'next' on 2021-09-29 at b7dea55eae)
 + MyFirstContribution: Document --range-diff option when writing v2

 Doc update.


* jk/grep-haystack-is-read-only (2021-09-22) 5 commits
  (merged to 'next' on 2021-09-28 at 1660a6be89)
 + grep: store grep_source buffer as const
 + grep: mark "haystack" buffers as const
 + grep: stop modifying buffer in grep_source_1()
 + grep: stop modifying buffer in show_line()
 + grep: stop modifying buffer in strip_timestamp
 (this branch is used by hm/paint-hits-in-log-grep.)

 Code clean-up in the "grep" machinery.


* jt/add-submodule-odb-clean-up (2021-09-09) 3 commits
  (merged to 'next' on 2021-09-28 at 4d843448be)
 + revision: remove "submodule" from opt struct
 + repository: support unabsorbed in repo_submodule_init
 + submodule: remove unnecessary unabsorbed fallback
 (this branch is used by jt/no-abuse-alternate-odb-for-submodules.)

 More code paths that use the hack to add submodule's object
 database to the set of alternate object store have been cleaned up.


* lh/systemd-timers (2021-09-27) 1 commit
  (merged to 'next' on 2021-10-03 at 81834609ea)
 + maintenance: fix test t7900-maintenance.sh

 Testfix.


* os/status-docfix (2021-09-28) 1 commit
  (merged to 'next' on 2021-10-03 at a13019916a)
 + doc: fix capitalization in "git status --porcelain=v2" description

 Docfix.


* pw/rebase-of-a-tag-fix (2021-09-22) 10 commits
  (merged to 'next' on 2021-09-28 at 980add2a67)
 + rebase: dereference tags
 + rebase: use lookup_commit_reference_by_name()
 + rebase: use our standard error return value
 + t3407: rework rebase --quit tests
 + t3407: strengthen rebase --abort tests
 + t3407: use test_path_is_missing
 + t3407: rename a variable
 + t3407: use test_cmp_rev
 + t3407: use test_commit
 + t3407: run tests in $TEST_DIRECTORY
 (this branch is used by pw/fix-some-issues-in-reset-head.)

 "git rebase <upstream> <tag>" failed when aborted in the middle, as
 it mistakenly tried to write the tag object instead of peeling it
 to HEAD.


* pw/rebase-reread-todo-after-editing (2021-09-24) 2 commits
  (merged to 'next' on 2021-09-28 at c67d5e383e)
 + rebase: fix todo-list rereading
 + sequencer.c: factor out a function

 The code to re-read the edited todo list in "git rebase -i" was
 made more robust.


* tb/commit-graph-usage-fix (2021-09-22) 2 commits
  (merged to 'next' on 2021-09-28 at f021339c39)
 + builtin/multi-pack-index.c: disable top-level --[no-]progress
 + builtin/commit-graph.c: don't accept common --[no-]progress

 Regression in "git commit-graph" command line parsing has been
 corrected.


* ws/refer-to-forkpoint-config-in-rebase-doc (2021-09-20) 1 commit
  (merged to 'next' on 2021-09-29 at 49181eaafb)
 + Document `rebase.forkpoint` in rebase man page

 Doc update.

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

* gc/use-repo-settings (2021-10-04) 3 commits
 - gc: perform incremental repack when implictly enabled
 - fsck: verify multi-pack-index when implictly enabled
 - fsck: verify commit graph when implicitly enabled

 It is wrong to read some settings directly from the config
 subsystem, as things like feature.experimental can affect their
 default values.

 Under review.
 cf. <70aca052-716f-40ed-47c4-1882fdbd221e@gmail.com>


* jh/perf-remove-test-times (2021-10-04) 1 commit
 - t/perf/perf-lib.sh: remove test_times.* at the end test_perf_()

 Perf test fix.

 Will merge to 'next'.

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

* ar/submodule-update (2021-09-20) 8 commits
 . submodule--helper: rename helper functions
 . submodule--helper: remove unused helpers
 . submodule--helper: remove update-clone subcommand
 . submodule: move core cmd_update() logic to C
 . 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.

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

* pw/fix-some-issues-in-reset-head (2021-10-06) 13 commits
 - sparse index: fix use-after-free bug in cache_tree_verify()
 - 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.


* rs/mergesort (2021-10-01) 9 commits
  (merged to 'next' on 2021-10-03 at 29a672574f)
 + mergesort: use ranks stack
 + p0071: test performance of llist_mergesort()
 + p0071: measure sorting of already sorted and reversed files
 + test-mergesort: add unriffle_skewed mode
 + test-mergesort: add unriffle mode
 + test-mergesort: add generate subcommand
 + test-mergesort: add test subcommand
 + test-mergesort: add sort subcommand
 + test-mergesort: use strbuf_getline()

 The mergesort implementation used to sort linked list has been
 optimized.

 Will merge to 'master'.


* bs/doc-blame-color-lines (2021-10-01) 2 commits
 - blame: document --color-* options
 - blame: describe default output format

 The "--color-lines" and "--color-by-age" options of "git blame"
 have been missing, which are now documented.

 Expecting a reroll.
 cf. <CAPig+cSWutBRQK+Qy=nkaDZRvy4trVNPuo+cF-quC2rBwNe2fw@mail.gmail.com>


* mr/bisect-in-c-4 (2021-10-01) 1 commit
  (merged to 'next' on 2021-10-06 at c13c14238d)
 + bisect--helper: add space between colon and following sentence

 Message fix.

 Will merge to 'master'.


* cm/save-restore-terminal (2021-10-06) 2 commits
 - editor: save and reset terminal after calling EDITOR
 - terminal: teach git how to save/restore its terminal settings

 An editor session launched during a Git operation (e.g. during 'git
 commit') can leave the terminal in a funny state.  The code path
 has updated to save the terminal state before, and restore it
 after, it spawns an editor.

 Will merge to 'next'.


* rs/p3400-lose-tac (2021-10-03) 1 commit
  (merged to 'next' on 2021-10-06 at 688dc7137c)
 + p3400: stop using tac(1)

 Test portability update.

 Will merge to 'master'.


* ja/doc-status-types-and-copies (2021-10-04) 4 commits
  (merged to 'next' on 2021-10-06 at 4de6571bf7)
 + Documentation/git-status: mention how to detect copies
 + Documentation/git-status: document porcelain status T (typechange)
 + Documentation/diff-format: state in which cases porcelain status is T
 + Documentation/git-status: remove impossible porcelain status DR and DC

 A few kinds of changes "git status" can show were not described.

 Will merge to 'master'.


* tb/aggregate-ignore-leading-whitespaces (2021-10-04) 1 commit
  (merged to 'next' on 2021-10-06 at 619a7db2d4)
 + t/perf/aggregate.perl: tolerate leading spaces

 Test portability update.

 Will merge to 'master'.


* ab/designated-initializers (2021-09-27) 5 commits
  (merged to 'next' on 2021-10-03 at 179f652de6)
 + cbtree.h: define cb_init() in terms of CBTREE_INIT
 + *.h: move some *_INIT to designated initializers
 + *.h _INIT macros: don't specify fields equal to 0
 + *.[ch] *_INIT macros: use { 0 } for a "zero out" idiom
 + submodule-config.h: remove unused SUBMODULE_INIT macro
 (this branch is used by ab/designated-initializers-more.)

 Code clean-up.

 Will merge to 'master'.


* ab/designated-initializers-more (2021-10-01) 6 commits
 - builtin/remote.c: add and use SHOW_INFO_INIT
 - builtin/remote.c: add and use a REF_STATES_INIT
 - urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT
 - builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro
 - daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro
 - Merge branch 'ab/designated-initializers' into ab/designated-initializers-more
 (this branch uses ab/designated-initializers.)

 Code clean-up.

 Will merge to 'next'.


* ab/parse-options-cleanup (2021-10-01) 11 commits
 - parse-options: change OPT_{SHORT,UNSET} to an enum
 - parse-options tests: test optname() output
 - parse-options.[ch]: make opt{bug,name}() "static"
 - commit-graph: stop using optname()
 - parse-options.c: move optname() earlier in the file
 - parse-options.h: make the "flags" in "struct option" an enum
 - parse-options.c: use exhaustive "case" arms for "enum parse_opt_type"
 - parse-options.c: use exhaustive "case" arms for "enum parse_opt_result"
 - parse-options.[ch]: consistently use "enum parse_opt_result"
 - parse-options.[ch]: consistently use "enum parse_opt_flags"
 - parse-options.h: move PARSE_OPT_SHELL_EVAL between enums

 Random changes to parse-options implementation.

 Will merge to 'next'?


* ab/retire-git-config-key-is-valid (2021-09-28) 1 commit
  (merged to 'next' on 2021-10-03 at fc7a0a55d1)
 + config.c: remove unused git_config_key_is_valid()

 Code cleanup.

 Will merge to 'master'.


* mt/grep-submodule-textconv (2021-09-29) 1 commit
  (merged to 'next' on 2021-10-06 at 1950944b8c)
 + grep: demonstrate bug with textconv attributes and submodules

 "git grep --recurse-submodules" takes trees and blobs from the
 submodule repository, but the textconv settings when processing a
 blob from the submodule is not taken from the submodule repository.
 A demonstration is added to demonstrate the issue, without fixing
 it.

 Will merge to 'master'.


* es/superproject-aware-submodules (2021-08-19) 5 commits
 - fixup! introduce submodule.superprojectGitDir record
 - 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).

 Brought back to 'seen' to see if it still plays well with the rest
 of 'seen', without the conflicting ar/submodule-update topic.


* ab/fsck-unexpected-type (2021-10-01) 17 commits
 - fsck: report invalid object type-path combinations
 - fsck: don't hard die on invalid object types
 - object-file.c: stop dying in parse_loose_header()
 - object-file.c: return ULHR_TOO_LONG on "header too long"
 - object-file.c: use "enum" return type for unpack_loose_header()
 - object-file.c: simplify unpack_loose_short_header()
 - object-file.c: make parse_loose_header_extended() public
 - object-file.c: return -1, not "status" from unpack_loose_header()
 - object-file.c: don't set "typep" when returning non-zero
 - cat-file tests: test for current --allow-unknown-type behavior
 - cat-file tests: add corrupt loose object test
 - cat-file tests: test for missing/bogus object with -t, -s and -p
 - cat-file tests: move bogus_* variable declarations earlier
 - fsck tests: test for garbage appended to a loose object
 - fsck tests: test current hash/type mismatch behavior
 - fsck tests: refactor one test to use a sub-repo
 - fsck tests: add test for fsck-ing an unknown type

 "git fsck" has been taught to report mismatch between expected and
 actual types of an object better.

 Will merge to 'next'.


* ab/config-based-hooks-1 (2021-09-27) 8 commits
  (merged to 'next' on 2021-10-06 at d05325ed35)
 + hook-list.h: add a generated list of hooks, like config-list.h
 + hook.c users: use "hook_exists()" instead of "find_hook()"
 + hook.c: add a hook_exists() wrapper and use it in bugreport.c
 + hook.[ch]: move find_hook() from run-command.c to hook.c
 + Makefile: remove an out-of-date comment
 + Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H)
 + Makefile: stop hardcoding {command,config}-list.h
 + Makefile: mark "check" target as .PHONY

 Mostly preliminary clean-up in the hook API.

 Will merge to 'master'.


* ab/http-pinned-public-key-mismatch (2021-09-27) 1 commit
  (merged to 'next' on 2021-10-03 at cd67328eed)
 + http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors

 HTTPS error handling updates.

 Will merge to 'master'.


* jk/ref-paranoia (2021-09-27) 16 commits
  (merged to 'next' on 2021-10-03 at 8c2cb6a3a6)
 + refs: drop "broken" flag from for_each_fullref_in()
 + ref-filter: drop broken-ref code entirely
 + ref-filter: stop setting FILTER_REFS_INCLUDE_BROKEN
 + repack, prune: drop GIT_REF_PARANOIA settings
 + refs: turn on GIT_REF_PARANOIA by default
 + refs: omit dangling symrefs when using GIT_REF_PARANOIA
 + refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flag
 + refs-internal.h: reorganize DO_FOR_EACH_* flag documentation
 + refs-internal.h: move DO_FOR_EACH_* flags next to each other
 + t5312: be more assertive about command failure
 + t5312: test non-destructive repack
 + t5312: create bogus ref as necessary
 + t5312: drop "verbose" helper
 + t5600: provide detached HEAD for corruption failures
 + t5516: don't use HEAD ref for invalid ref-deletion tests
 + t7900: clean up some more broken refs
 (this branch is used by jt/no-abuse-alternate-odb-for-submodules.)

 The ref iteration code used to optionally allow dangling refs to be
 shown, which has been tightened up.

 Will merge to 'master'.


* js/win-lazyload-buildfix (2021-09-28) 3 commits
  (merged to 'next' on 2021-10-03 at 26802e5d73)
 + Makefile: restrict -Wpedantic and -Wno-pedantic-ms-format better
 + lazyload.h: use an even more generic function pointer than FARPROC
 + lazyload.h: fix warnings about mismatching function pointer types

 Compilation fix.

 Will merge to 'master'.


* ab/make-sparse-for-real (2021-09-22) 1 commit
  (merged to 'next' on 2021-10-06 at 10e3c31d6a)
 + Makefile: make the "sparse" target non-.PHONY

 Prevent "make sparse" from running for the source files that
 haven't been modified.

 Will merge to 'master'.


* jt/no-abuse-alternate-odb-for-submodules (2021-10-01) 10 commits
 - fixup! refs: plumb repo into ref stores
 - submodule: trace adding submodule ODB as alternate
 - submodule: pass repo to check_has_commit()
 - object-file: only register submodule ODB if needed
 - merge-{ort,recursive}: remove add_submodule_odb()
 - refs: peeling non-the_repository iterators is BUG
 - refs: teach arbitrary repo support to iterators
 - refs: plumb repo into ref stores
 - Merge branch 'jk/ref-paranoia' into jt/no-abuse-alternate-odb-for-submodules
 - Merge branch 'jt/add-submodule-odb-clean-up' into jt/no-abuse-alternate-odb-for-submodules
 (this branch uses jk/ref-paranoia.)

 Follow through the work to use the repo interface to access
 submodule objects in-process, instead of abusing the alternate
 object database interface.

 Expecting a reroll.


* tp/send-email-completion (2021-09-22) 3 commits
 - send-email docs: add format-patch options
 - send-email: move bash completions to core script
 - send-email: terminate --git-completion-helper with LF

 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.

 Stalled.
 cf. <YU6+BWC+xvGJP3b0@carlos-mbp.lan>


* hm/paint-hits-in-log-grep (2021-09-29) 3 commits
 - pretty: colorize pattern matches in commit messages
 - grep: refactor next_match() and match_one_pattern() for external use
 - Merge branch 'jk/grep-haystack-is-read-only' into hm/paint-hits-in-log-grep

 "git log --grep=string --author=name" learns to highlight hits just
 like "git grep string" does.

 Expecting a reroll.
 cf. <87v92bju64.fsf@evledraar.gmail.com>


* da/difftool (2021-09-30) 6 commits
  (merged to 'next' on 2021-10-03 at 3ba0335e4e)
 + difftool: add a missing space to the run_dir_diff() comments
 + difftool: remove an unnecessary call to strbuf_release()
 + difftool: refactor dir-diff to write files using helper functions
 + difftool: create a tmpdir path without repeated slashes
 + Merge branch 'da/difftool-dir-diff-symlink-fix' into da/difftool
 + Merge branch 'ab/retire-option-argument' into da/difftool

 Code clean-up in "git difftool".

 Will merge to 'master'.


* en/removing-untracked-fixes (2021-09-27) 12 commits
  (merged to 'next' on 2021-10-06 at fc4e387fda)
 + Documentation: call out commands that nuke untracked files/directories
 + Comment important codepaths regarding nuking untracked files/dirs
 + unpack-trees: avoid nuking untracked dir in way of locally deleted file
 + unpack-trees: avoid nuking untracked dir in way of unmerged file
 + Change unpack_trees' 'reset' flag into an enum
 + Remove ignored files by default when they are in the way
 + unpack-trees: make dir an internal-only struct
 + unpack-trees: introduce preserve_ignored to unpack_trees_options
 + read-tree, merge-recursive: overwrite ignored files by default
 + checkout, read-tree: fix leak of unpack_trees_options.dir
 + t2500: add various tests for nuking untracked files
 + Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes

 Various fixes in code paths that move untracked files away to make room.

 Will merge to 'master'.


* ks/submodule-add-message-fix (2021-09-20) 1 commit
 - submodule--helper: fix incorrect newlines in an error message

 Message regression fix.

 Waiting for a response.
 cf. <m27df9lvm1.fsf@gmail.com>


* ns/batched-fsync (2021-10-04) 9 commits
 . 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
 . tmp-objdir: disable ref updates when replacing the primary odb
 . tmp-objdir: new API for creating temporary writable databases

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

 Under discussion.

 Handling of temporary object directory is worked out with the
 en/remerge-diff topic.


* jh/builtin-fsmonitor-part1 (2021-09-20) 7 commits
  (merged to 'next' on 2021-10-06 at 021f633b9c)
 + t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command
 + run-command: create start_bg_command
 + simple-ipc/ipc-win32: add Windows ACL to named pipe
 + simple-ipc/ipc-win32: add trace2 debugging
 + simple-ipc: move definition of ipc_active_state outside of ifdef
 + simple-ipc: preparations for supporting binary messages.
 + trace2: add trace2_child_ready() to report on background children

 Built-in fsmonitor (part 1).

 Will merge to 'master'.


* ab/align-parse-options-help (2021-09-22) 4 commits
  (merged to 'next' on 2021-10-06 at e22da7ef85)
 + parse-options: properly align continued usage output
 + git rev-parse --parseopt tests: add more usagestr tests
 + send-pack: properly use parse_options() API for usage string
 + parse-options API users: align usage output in C-strings

 When "git cmd -h" shows more than one line of usage text (e.g.
 the cmd subcommand may take sub-sub-command), parse-options API
 learned to align these lines, even across i18n/l10n.

 Will merge to 'master'.


* ab/help-config-vars (2021-09-23) 9 commits
  (merged to 'next' on 2021-10-06 at bf9538cfbd)
 + help: move column config discovery to help.c library
 + help / completion: make "git help" do the hard work
 + help tests: test --config-for-completion option & output
 + help: simplify by moving to OPT_CMDMODE()
 + help: correct logic error in combining --all and --guides
 + help: correct logic error in combining --all and --config
 + help tests: add test for --config output
 + help: correct usage & behavior of "git help --guides"
 + help: correct the usage string in -h and documentation

 Teach "git help -c" into helping the command line completion of
 configuration variables.

 Will merge to 'master'.


* tb/repack-write-midx (2021-10-01) 9 commits
  (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
 + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
 + builtin/repack.c: make largest pack preferred
 + builtin/repack.c: support writing a MIDX while repacking
 + builtin/repack.c: extract showing progress to a variable
 + builtin/repack.c: rename variables that deal with non-kept packs
 + builtin/repack.c: keep track of existing packs unconditionally
 + midx: preliminary support for `--refs-snapshot`
 + builtin/multi-pack-index.c: support `--stdin-packs` mode
 + midx: expose `write_midx_file_only()` publicly

 "git repack" has been taught to generate multi-pack reachability
 bitmaps.

 Will merge to 'master'.


* ds/add-rm-with-sparse-index (2021-09-28) 13 commits
  (merged to 'next' on 2021-10-06 at 80a9cda797)
 + advice: update message to suggest '--sparse'
 + mv: refuse to move sparse paths
 + rm: skip sparse paths with missing SKIP_WORKTREE
 + rm: add --sparse option
 + add: update --renormalize to skip sparse paths
 + add: update --chmod to skip sparse paths
 + add: implement the --sparse option
 + add: skip tracked paths outside sparse-checkout cone
 + add: fail when adding an untracked sparse file
 + dir: fix pattern matching on dirs
 + dir: select directories correctly
 + t1092: behavior for adding sparse files
 + t3705: test that 'sparse_entry' is unstaged

 "git add", "git mv", and "git rm" have been adjusted to avoid
 updating paths outside of the sparse-checkout definition unless
 the user specifies a "--sparse" option.

 Will merge to 'master'.


* tb/midx-write-propagate-namehash (2021-09-17) 7 commits
  (merged to 'next' on 2021-09-29 at 24732fcfc8)
 + t5326: test propagating hashcache values
 + p5326: generate pack bitmaps before writing the MIDX bitmap
 + p5326: don't set core.multiPackIndex unnecessarily
 + p5326: create missing 'perf-tag' tag
 + midx.c: respect 'pack.writeBitmapHashcache' when writing bitmaps
 + pack-bitmap.c: propagate namehash values from existing bitmaps
 + t/helper/test-bitmap.c: add 'dump-hashes' mode

 "git multi-pack-index write --bitmap" learns to propagate the
 hashcache from original bitmap to resulting bitmap.

 Will merge to 'master'.


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


* js/scalar (2021-09-14) 15 commits
 - scalar: accept -C and -c options before the subcommand
 - 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
 - scalar: create test infrastructure
 - scalar: start documenting the command
 - scalar: create a rudimentary executable

 Add pieces from "scalar" to contrib/.

 Waiting for a response.
 cf. <pull.1005.v4.git.1631630356.gitgitgadget@gmail.com>


* ab/sanitize-leak-ci (2021-09-23) 2 commits
  (merged to 'next' on 2021-10-03 at dcd62a3fc6)
 + tests: add a test mode for SANITIZE=leak, run it in CI
 + Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS

 CI learns to run the leak sanitizer builds.

 Will merge to 'master'.


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


* js/retire-preserve-merges (2021-09-07) 11 commits
  (merged to 'next' on 2021-09-10 at f645ffd7a3)
 + sequencer: restrict scope of a formerly public function
 + rebase: remove a no-longer-used function
 + rebase: stop mentioning the -p option in comments
 + rebase: remove obsolete code comment
 + rebase: drop the internal `rebase--interactive` command
 + git-svn: drop support for `--preserve-merges`
 + rebase: drop support for `--preserve-merges`
 + pull: remove support for `--rebase=preserve`
 + tests: stop testing `git rebase --preserve-merges`
 + remote: warn about unhandled branch.<name>.rebase values
 + t5520: do not use `pull.rebase=preserve`

 The "--preserve-merges" option of "git rebase" has been removed.

 Will cook in 'next'.


* en/remerge-diff (2021-08-31) 7 commits
 - doc/diff-options: explain the new --remerge-diff option
 - show, log: provide a --remerge-diff capability
 - tmp-objdir: new API for creating and removing primary object dirs
 - 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

 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.

 Expecting a reroll.

 Handling of temporary object directory is worked out with the
 ns/batched-fsync topic.


* sg/test-split-index-fix (2021-09-07) 7 commits
  (merged to 'next' on 2021-09-29 at 661ae75778)
 + read-cache: fix GIT_TEST_SPLIT_INDEX
 + tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests
 + read-cache: look for shared index files next to the index, too
 + t1600-index: disable GIT_TEST_SPLIT_INDEX
 + t1600-index: don't run git commands upstream of a pipe
 + t1600-index: remove unnecessary redirection
 + Merge branch 'ds/sparse-index-ignored-files' into sg/test-split-index-fix

 Test updates.

 Will merge to 'master'.


* ab/refs-errno-cleanup (2021-08-25) 4 commits
 - refs: make errno output explicit for refs_resolve_ref_unsafe
 - refs: explicitly return failure_errno from parse_loose_ref_contents
 - branch tests: test for errno propagating on failing read
 - refs: add failure_errno to refs_read_raw_ref() signature

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

 What's the status of this one?  Meh?


* ab/lib-subtest (2021-09-22) 9 commits
  (merged to 'next' on 2021-10-06 at e8fa261811)
 + test-lib tests: get rid of copy/pasted mock test code
 + test-lib tests: assert 1 exit code, not non-zero
 + test-lib tests: refactor common part of check_sub_test_lib_test*()
 + test-lib tests: avoid subshell for "test_cmp" for readability
 + test-lib tests: don't provide a description for the sub-tests
 + test-lib tests: split up "write and run" into two functions
 + test-lib tests: move "run_sub_test" to a new lib-subtest.sh
 + Merge branch 'ps/t0000-output-directory-fix' into ab/lib-subtest
 + Merge branch 'jk/t0000-subtests-fix' into ab/lib-subtest

 Updates to the tests in t0000 to test the test framework.

 Will merge to 'master'.


* ab/only-single-progress-at-once (2021-09-22) 8 commits
 - progress.c: add & assert a "global_progress" variable
 - pack-bitmap-write.c: add a missing stop_progress()
 - progress.c: add temporary variable from progress struct
 - progress.c: stop eagerly fflush(stderr) when not a terminal
 - progress.c: call progress_interval() from progress_test_force_update()
 - progress.c: move signal handler functions lower
 - progress.c tests: test some invalid usage
 - progress.c tests: make start/stop verbs on stdin

 Further tweaks on progress API.


* fs/ssh-signing (2021-09-10) 9 commits
 - ssh signing: test that gpg fails for unknown keys
 - ssh signing: tests for logs, tags & push certs
 - ssh signing: duplicate t7510 tests for commits
 - ssh signing: verify signatures using ssh-keygen
 - ssh signing: provide a textual signing_key_id
 - ssh signing: retrieve a default key from ssh-agent
 - ssh signing: add ssh key format and signing code
 - ssh signing: add test prereqs
 - ssh signing: preliminary refactoring and clean-up

 Use ssh public crypto for object and push-cert signing.

 Will merge to 'next'.


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

 Expecting a reroll.
 cf. <xmqqsg0ri5mq.fsf@gitster.g>


* pw/diff-color-moved-fix (2021-08-05) 13 commits
 - diff: drop unused options parameter from cmp_in_block_with_wsd()
 - 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: add perf tests

 Originally merged to 'next' on 2021-08-05

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

 Expecting a reroll.
 cf. <8bec1a6d-5052-50c3-4100-e6348289d581@gmail.com>


* hn/reftable (2021-10-01) 24 commits
 - reftable: avoid non portable compile time pointer to function
 - config.mak.uname: last release and snapshots of Minix still use zlib 1.2.3
 - fixup! reftable: implement stack, a mutable database of reftable files.
 - fixup! reftable: add a heap-based priority queue for reftable records
 - squash! reftable: reading/writing blocks
 - 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: RFC: add LICENSE
 - hash.h: provide constants for the hash IDs

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

 Expecting a reroll.

--------------------------------------------------
[Discarded]

* ao/p4-avoid-decoding (2021-04-12) 2 commits
 . git-p4: do not decode data from perforce by default
 . git-p4: avoid decoding more data from perforce

 "git p4" in Python-2 days used to accept a lot more kinds of data
 from Perforce server as uninterrupted byte sequence, but after
 switching to Python-3, too many things are expected to be in UTF-8,
 which broke traditional use cases.

 Have been stalled for too long.
 cf. <20210504220153.1d9f0cb2@ado-tr>


* tv/p4-fallback-encoding (2021-04-30) 1 commit
 . git-p4: git-p4.fallbackEncoding to specify non UTF-8 charset

 "git p4" learns the fallbackEncoding configuration variable to
 safely accept changeset descriptions that aren't written in UTF-8.

 Have been stalled for too long.
 cf. <CAKu1iLUaLuAZWqjNK4tfhhR=YaSt4MdQ+90ZY-JcEh_SeHyYCw@mail.gmail.com>


* jh/builtin-fsmonitor (2021-09-03) 37 commits
 . fixup! fsmonitor--daemon: implement handle_client callback
 . SQUASH??? https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136
 . BANDAID: sparse fixes
 . t7527: test FS event reporing on MacOS WRT case and Unicode
 . fsmonitor: handle shortname for .git
 . t7527: test status with untracked-cache and fsmonitor--daemon
 . fsmonitor: force update index after large responses
 . fsmonitor: enhance existing comments
 . fsmonitor--daemon: use a cookie file to sync with file system
 . fsmonitor--daemon: periodically truncate list of modified files
 . t7527: create test for fsmonitor--daemon
 . t/perf/p7519: add fsmonitor--daemon test cases
 . t/perf: avoid copying builtin fsmonitor files into test repo
 . t/perf/p7519: speed up test using "test-tool touch"
 . t/helper/test-touch: add helper to touch a series of files
 . fsmonitor--daemon: implement handle_client callback
 . fsmonitor-fs-listen-macos: implement FSEvent listener on MacOS
 . fsmonitor-fs-listen-macos: add macos header files for FSEvent
 . fsmonitor-fs-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: do not try to operate on bare repos
 . fsmonitor--daemon: implement 'start' command
 . fsmonitor--daemon: implement 'run' command
 . fsmonitor-fs-listen-macos: stub in backend for MacOS
 . fsmonitor-fs-listen-win32: stub in backend for Windows
 . t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon
 . fsmonitor--daemon: implement 'stop' and 'status' commands
 . fsmonitor--daemon: add a built-in fsmonitor daemon
 . fsmonitor: use IPC to query the builtin FSMonitor daemon
 . fsmonitor: config settings are repository-specific
 . help: include fsmonitor--daemon feature flag in version info
 . fsmonitor-ipc: create client routines for git-fsmonitor--daemon
 . fsmonitor--daemon: update fsmonitor documentation
 . fsmonitor--daemon: man page
 . simple-ipc: preparations for supporting binary messages.

 An attempt to write and ship with a watchman equivalent tailored
 for our use.

 Will be rerolled in pieces.


* ab/config-based-hooks-base (2021-09-09) 36 commits
 . hooks: fix a TOCTOU in "did we run a hook?" heuristic
 . receive-pack: convert receive hooks to hook.h
 . post-update: use hook.h library
 . receive-pack: convert 'update' hook to hook.h
 . hooks: allow callers to capture output
 . run-command: allow capturing of collated output
 . reference-transaction: use hook.h to run hooks
 . hook tests: use a modern style for "pre-push" tests
 . hook tests: test for exact "pre-push" hook input
 . transport: convert pre-push hook to hook.h
 . hook: convert 'post-rewrite' hook in sequencer.c to hook.h
 . hook: provide stdin by string_list or callback
 . run-command: add stdin callback for parallelization
 . am: convert 'post-rewrite' hook to hook.h
 . hook: support passing stdin to hooks
 . run-command: allow stdin for run_processes_parallel
 . 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
 . merge: convert post-merge to use hook.h
 . hooks: convert 'post-checkout' hook to hook library
 . am: convert applypatch to use hook.h
 . rebase: convert pre-rebase to use hook.h
 . gc: use hook library for pre-auto-gc hook
 . hook: add 'run' subcommand
 . hook-list.h: add a generated list of hooks, like config-list.h
 . hook.c users: use "hook_exists()" instead of "find_hook()"
 . hook.c: add a hook_exists() wrapper and use it in bugreport.c
 . hook.[ch]: move find_hook() from run-command.c to hook.c
 . Makefile: remove an out-of-date comment
 . Makefile: stop hardcoding {command,config}-list.h
 . Makefile: mark "check" target as .PHONY
 (this branch is used by es/config-based-hooks.)

 Restructuring of (a subset of) Emily's config-based-hooks series,
 to demonstrate that a series can be presented as a more logical and
 focused progression.

 Will be rerolled in pieces.


* es/config-based-hooks (2021-09-09) 6 commits
 . hook: allow out-of-repo 'git hook' invocations
 . hook: include hooks from the config
 . hook: introduce "git hook list"
 . hook: allow parallel hook execution
 . fixup! hook: run a list of hooks instead
 . hook: run a list of hooks instead
 (this branch uses ab/config-based-hooks-base.)

 Revamp the hooks subsystem to allow multiple of them to trigger
 upon the same event and control via the configuration variables.


* cb/make-compdb-fix (2021-09-22) 1 commit
 . Makefile: avoid breaking compilation database generation with DEVELOPER

 Adjust to recent change to use -pedantic for developer builds.

 Replaced by the ab/make-compdb-fix topic that uses the same
 approach as the ab/auto-depend-with-pedantic topic.


* ab/pack-objects-stdin (2021-07-09) 5 commits
 . pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
 . pack-objects.c: do stdin parsing via revision.c's API
 . revision.[ch]: add a "handle_stdin_line" API
 . revision.h: refactor "disable_stdin" and "read_from_stdin"
 . upload-pack: run is_repository_shallow() before setup_revisions()

 Introduce handle_stdin_line callback to revision API and uses it.

 Retracted for now.


* ah/unleak-revisions (2021-09-20) 2 commits
 . log: UNLEAK original pending objects
 . log: UNLEAK rev to silence a large number of leaks

 Mark a few structures with UNLEAK() to help leak detection CI jobs.

 Retracted.
 cf. <05754f9c-cd58-30f5-e2d3-58b9221d2770@ahunt.org>


* rs/p5311-use-test-file-size (2021-10-03) 1 commit
 . p5311: handle spaces in wc(1) output

 Test portability update.

 The tb/aggregate-ignore-leading-whitespaces supersedes this topic.

ab/make-sparse-for-real

[Ævar Arnfjörð Bjarmason]

On Wed, Oct 06 2021, Junio C Hamano wrote:

> * ab/make-sparse-for-real (2021-09-22) 1 commit
>   (merged to 'next' on 2021-10-06 at 10e3c31d6a)
>  + Makefile: make the "sparse" target non-.PHONY
>
>  Prevent "make sparse" from running for the source files that
>  haven't been modified.
>
>  Will merge to 'master'.

I see you merged down the v2 of this[1], not the v3[2], thanks!

I'm personally happier with the v2, but the v3 is just as useful for my
purposes, and I understood that you/Ramsey had some flows/finger memory
depending on the semantics of the existing "sparse" target.

If it was a mistake I'm happy to submit a version of the v3 on top
(since it's in "next" already), or if it was intended I'm happy to leave
it be. Just checking.

1. https://lore.kernel.org/git/patch-v2-1.1-059829f2195-20210923T000654Z-avarab@gmail.com/
2. https://lore.kernel.org/git/patch-v3-1.1-b6ba99ca4cc-20210928T011319Z-avarab@gmail.com/

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:

> * tb/repack-write-midx (2021-10-01) 9 commits
>   (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
>  + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
>  + builtin/repack.c: make largest pack preferred
>  + builtin/repack.c: support writing a MIDX while repacking
>  + builtin/repack.c: extract showing progress to a variable
>  + builtin/repack.c: rename variables that deal with non-kept packs
>  + builtin/repack.c: keep track of existing packs unconditionally
>  + midx: preliminary support for `--refs-snapshot`
>  + builtin/multi-pack-index.c: support `--stdin-packs` mode
>  + midx: expose `write_midx_file_only()` publicly
> 
>  "git repack" has been taught to generate multi-pack reachability
>  bitmaps.
> 
>  Will merge to 'master'.

Sorry not to catch this before it hit 'next', but there's a small leak
in the test helper. This patch can go on top to fix it.

-- >8 --
Subject: [PATCH] test-read-midx: fix leak of bitmap_index struct

In read_midx_preferred_pack(), we open the bitmap index but never free
it. This isn't a big deal since this is just a test helper, and we exit
immediately after, but since we're trying to keep our leak-checking tidy
now, it's worth fixing.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/helper/test-read-midx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
index 0038559129..9d6fa7a377 100644
--- a/t/helper/test-read-midx.c
+++ b/t/helper/test-read-midx.c
@@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)
 		return 1;
 
 	bitmap = prepare_bitmap_git(the_repository);
-	if (!(bitmap && bitmap_is_midx(bitmap)))
+	if (!bitmap)
 		return 1;
-
+	if (!bitmap_is_midx(bitmap)) {
+		free_bitmap_index(bitmap);
+		return 1;
+	}
 
 	printf("%s\n", midx->pack_names[midx_preferred_pack(bitmap)]);
+	free_bitmap_index(bitmap);
 	return 0;
 }
 
-- 
2.33.0.1340.gfe2cb2531f

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:

> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:
> 
> > * tb/repack-write-midx (2021-10-01) 9 commits
> >   (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
> >  + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
> >  + builtin/repack.c: make largest pack preferred
> >  + builtin/repack.c: support writing a MIDX while repacking
> >  + builtin/repack.c: extract showing progress to a variable
> >  + builtin/repack.c: rename variables that deal with non-kept packs
> >  + builtin/repack.c: keep track of existing packs unconditionally
> >  + midx: preliminary support for `--refs-snapshot`
> >  + builtin/multi-pack-index.c: support `--stdin-packs` mode
> >  + midx: expose `write_midx_file_only()` publicly
> > 
> >  "git repack" has been taught to generate multi-pack reachability
> >  bitmaps.
> > 
> >  Will merge to 'master'.
> 
> Sorry not to catch this before it hit 'next', but there's a small leak
> in the test helper. This patch can go on top to fix it.

The reason for that is that I didn't find it by inspection; I've started
running my personal builds through coverity. It wasn't too bad to set up
with a GitHub Action, like so:

---
 .github/workflows/coverity.yml | 35 ++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)
 create mode 100644 .github/workflows/coverity.yml

diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
new file mode 100644
index 0000000000..bfd4dff275
--- /dev/null
+++ b/.github/workflows/coverity.yml
@@ -0,0 +1,35 @@
+name: coverity-scan
+on: push
+
+jobs:
+  latest:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - run: ci/install-dependencies.sh
+      - name: Download Coverity Build Tool
+        run: |
+          wget -q https://scan.coverity.com/download/linux64 --post-data "token=$TOKEN&project=peff/git" -O cov-analysis-linux64.tar.gz
+          mkdir cov-analysis-linux64
+          tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64
+        env:
+          TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}
+
+      - name: Build with cov-build
+        run: |
+          export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH
+          cov-build --dir cov-int make
+
+      - name: Submit the result to Coverity Scan
+        run: |
+          tar czvf git.tgz cov-int
+          curl \
+            --form project=peff/git \
+            --form token=$TOKEN \
+            --form email=peff@peff.net \
+            --form file=@git.tgz \
+            --form version=$(git rev-parse HEAD) \
+            --form description="$(./git version)" \
+            https://scan.coverity.com/builds?project=peff/git
+        env:
+          TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}

Is there interest in having something like this in the main repo? We'd
need to tweak some values:

  - we have to send the project name (here peff/git); we can presumably
    get this on the fly from the Actions environment

  - any repo which wants to use this has to set up the secret token
    (COVERITY_SCAN_TOKEN here). That involves creating a coverity
    account, and then setting the token in the GitHub web interface.
    Presumably we'd just bail immediately if that token isn't set, so
    forks aside from git/git would have to enable it independently.

  - likewise it needs the email address for the coverity account. That
    could probably be set in the environment, too.

  - we'd probably want to only run it for integration branches, since
    coverity sets some limits on how often it runs. This could probably
    be set in another environment variable, so people could tweak it for
    their forks if they wanted to (or we could use the ci-config hacks,
    but I put those together mostly because these environment variables
    didn't exist back then; I suspect we could switch off of them now).

There are tons of existing warnings, many of which are false positives.
But it keeps track of which problems are new, and emails out a summary
of only the new ones (which is how I saw the leak here, which just hit
next). I don't care all that much about leaks here (we have other
techniques for finding them), but when Stefan used to do regular
coverity builds in the past, it routinely found useful errors.

If we had it running on git/git, it's possible for people to subscribe
to those notifications (or view them on the site; both require the
people to have coverity accounts, but they're free).

Thoughts?

-Peff

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Ævar Arnfjörð Bjarmason]

On Wed, Oct 06 2021, Jeff King wrote:

> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:
>
>> * tb/repack-write-midx (2021-10-01) 9 commits
>>   (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
>>  + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
>>  + builtin/repack.c: make largest pack preferred
>>  + builtin/repack.c: support writing a MIDX while repacking
>>  + builtin/repack.c: extract showing progress to a variable
>>  + builtin/repack.c: rename variables that deal with non-kept packs
>>  + builtin/repack.c: keep track of existing packs unconditionally
>>  + midx: preliminary support for `--refs-snapshot`
>>  + builtin/multi-pack-index.c: support `--stdin-packs` mode
>>  + midx: expose `write_midx_file_only()` publicly
>> 
>>  "git repack" has been taught to generate multi-pack reachability
>>  bitmaps.
>> 
>>  Will merge to 'master'.
>
> Sorry not to catch this before it hit 'next', but there's a small leak
> in the test helper. This patch can go on top to fix it.
>
> -- >8 --
> Subject: [PATCH] test-read-midx: fix leak of bitmap_index struct
>
> In read_midx_preferred_pack(), we open the bitmap index but never free
> it. This isn't a big deal since this is just a test helper, and we exit
> immediately after, but since we're trying to keep our leak-checking tidy
> now, it's worth fixing.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/helper/test-read-midx.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
> index 0038559129..9d6fa7a377 100644
> --- a/t/helper/test-read-midx.c
> +++ b/t/helper/test-read-midx.c
> @@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)
>  		return 1;
>  
>  	bitmap = prepare_bitmap_git(the_repository);
> -	if (!(bitmap && bitmap_is_midx(bitmap)))
> +	if (!bitmap)
>  		return 1;
> -
> +	if (!bitmap_is_midx(bitmap)) {
> +		free_bitmap_index(bitmap);
> +		return 1;
> +	}
>  
>  	printf("%s\n", midx->pack_names[midx_preferred_pack(bitmap)]);
> +	free_bitmap_index(bitmap);
>  	return 0;
>  }

Thanks, I think it's no big deal, those tests seem to leak a lot
already. Here's a patch that might be generally applicable and makes a
few more of its tests pass.

The s/free/free_chunkfile/g seems like a good bug fix, and that "m =
NULL" pattern seems odd, don't we always want to free in that scenario?
It passes all tests...

This brings t5319-multi-pack-index.sh down from 64 to 57 failures in
"next", I didn't try in combination with your patch.

diff --git a/midx.c b/midx.c
index 7e06e859756..d24b1e6a9d4 100644
--- a/midx.c
+++ b/midx.c
@@ -179,12 +179,13 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
 	trace2_data_intmax("midx", the_repository, "load/num_packs", m->num_packs);
 	trace2_data_intmax("midx", the_repository, "load/num_objects", m->num_objects);
 
+	free_chunkfile(cf);
 	return m;
 
 cleanup_fail:
 	free(m);
 	free(midx_name);
-	free(cf);
+	free_chunkfile(cf);
 	if (midx_map)
 		munmap(midx_map, midx_size);
 	if (0 <= fd)
@@ -1602,7 +1603,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
 		 * Remaining tests assume that we have objects, so we can
 		 * return here.
 		 */
-		return verify_midx_error;
+		goto cleanup;
 	}
 
 	if (flags & MIDX_PROGRESS)
@@ -1679,8 +1680,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
 		midx_display_sparse_progress(progress, i + 1);
 	}
 	stop_progress(&progress);
-
+cleanup:
 	free(pairs);
+	free(m);
 
 	return verify_midx_error;
 }
@@ -1927,11 +1929,9 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
 	}
 
 	result = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);
-	m = NULL;
 
 cleanup:
-	if (m)
-		close_midx(m);
+	close_midx(m);
 	free(include_pack);
 	return result;
 }

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Taylor Blau]

On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:
> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:
>
> > * tb/repack-write-midx (2021-10-01) 9 commits
> >   (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
> >  + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
> >  + builtin/repack.c: make largest pack preferred
> >  + builtin/repack.c: support writing a MIDX while repacking
> >  + builtin/repack.c: extract showing progress to a variable
> >  + builtin/repack.c: rename variables that deal with non-kept packs
> >  + builtin/repack.c: keep track of existing packs unconditionally
> >  + midx: preliminary support for `--refs-snapshot`
> >  + builtin/multi-pack-index.c: support `--stdin-packs` mode
> >  + midx: expose `write_midx_file_only()` publicly
> >
> >  "git repack" has been taught to generate multi-pack reachability
> >  bitmaps.
> >
> >  Will merge to 'master'.
>
> Sorry not to catch this before it hit 'next', but there's a small leak
> in the test helper. This patch can go on top to fix it.

Thanks for taking a look and catching this. The fix you wrote looks
good to me. For what it's worth:

    Acked-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Taylor Blau]

On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:
> The reason for that is that I didn't find it by inspection; I've started
> running my personal builds through coverity. It wasn't too bad to set up
> with a GitHub Action, like so:
>
> [...]

It looks like this would never cause the build to fail, but is merely
responsible for sending any warnings off to Coverity's UI?

> Is there interest in having something like this in the main repo? We'd
> need to tweak some values:
>
>   - we have to send the project name (here peff/git); we can presumably
>     get this on the fly from the Actions environment

Yes; searching through [1] it looks like that is called $GITHUB_REPOSITORY.

[1]: https://docs.github.com/en/actions/learn-github-actions/environment-variables

>   - any repo which wants to use this has to set up the secret token
>     (COVERITY_SCAN_TOKEN here). That involves creating a coverity
>     account, and then setting the token in the GitHub web interface.
>     Presumably we'd just bail immediately if that token isn't set, so
>     forks aside from git/git would have to enable it independently.
>
>   - likewise it needs the email address for the coverity account. That
>     could probably be set in the environment, too.

These both seem reasonable to me, too.

> There are tons of existing warnings, many of which are false positives.
> But it keeps track of which problems are new, and emails out a summary
> of only the new ones (which is how I saw the leak here, which just hit
> next). I don't care all that much about leaks here (we have other
> techniques for finding them), but when Stefan used to do regular
> coverity builds in the past, it routinely found useful errors.

I'm generally pessimistic about tools like Coverity, but I share your
experience that Coverity warnings are actually pretty high quality. Or
at least they have a high enough signal-to-noise ratio that it makes
them worth looking through.

So I would be happy to have forks of GitHub have fewer barriers to use
this tool.

Thanks,
Taylor

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Taylor Blau]

On Thu, Oct 07, 2021 at 04:57:10AM +0200, Ævar Arnfjörð Bjarmason wrote:
> @@ -1927,11 +1929,9 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,
>  	}
>
>  	result = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);
> -	m = NULL;

This was the only hunk that gave me a little bit of pause. But it is the
right thing to be doing.

When this code was originally written, write_midx_internal took a
pointer to a struct multi_pack_index, and sometimes called close_midx()
on that pointer. Calling close_midx() on the same pointer is UB, since
close_midx() frees the provided pointer.

In other words, when this code was originally written, the 'm' variable
in midx_repack() could become stale.

But since f57a739691 (midx: avoid opening multiple MIDXs when writing,
2021-09-01), we don't pass a pointer into write_midx_internal(), only
the path to an object directory. So we can always call close_midx()
here.

Thanks,
Taylor

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Ævar Arnfjörð Bjarmason]

On Wed, Oct 06 2021, Jeff King wrote:

> On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:
>
>> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:
>> 
>> > * tb/repack-write-midx (2021-10-01) 9 commits
>> >   (merged to 'next' on 2021-10-06 at ccdd5aaf2a)
>> >  + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps
>> >  + builtin/repack.c: make largest pack preferred
>> >  + builtin/repack.c: support writing a MIDX while repacking
>> >  + builtin/repack.c: extract showing progress to a variable
>> >  + builtin/repack.c: rename variables that deal with non-kept packs
>> >  + builtin/repack.c: keep track of existing packs unconditionally
>> >  + midx: preliminary support for `--refs-snapshot`
>> >  + builtin/multi-pack-index.c: support `--stdin-packs` mode
>> >  + midx: expose `write_midx_file_only()` publicly
>> > 
>> >  "git repack" has been taught to generate multi-pack reachability
>> >  bitmaps.
>> > 
>> >  Will merge to 'master'.
>> 
>> Sorry not to catch this before it hit 'next', but there's a small leak
>> in the test helper. This patch can go on top to fix it.
>
> The reason for that is that I didn't find it by inspection; I've started
> running my personal builds through coverity. It wasn't too bad to set up
> with a GitHub Action, like so:
>
> ---
>  .github/workflows/coverity.yml | 35 ++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
>  create mode 100644 .github/workflows/coverity.yml
>
> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml
> new file mode 100644
> index 0000000000..bfd4dff275
> --- /dev/null
> +++ b/.github/workflows/coverity.yml
> @@ -0,0 +1,35 @@
> +name: coverity-scan
> +on: push
> +
> +jobs:
> +  latest:
> +    runs-on: ubuntu-latest
> +    steps:
> +      - uses: actions/checkout@v2
> +      - run: ci/install-dependencies.sh
> +      - name: Download Coverity Build Tool
> +        run: |
> +          wget -q https://scan.coverity.com/download/linux64 --post-data "token=$TOKEN&project=peff/git" -O cov-analysis-linux64.tar.gz

Interesting mix...

> +          mkdir cov-analysis-linux64
> +          tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64
> +        env:
> +          TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}
> +
> +      - name: Build with cov-build
> +        run: |
> +          export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH
> +          cov-build --dir cov-int make
> +
> +      - name: Submit the result to Coverity Scan
> +        run: |
> +          tar czvf git.tgz cov-int
> +          curl \

...of curl & wget :)

> +            --form project=peff/git \
> +            --form token=$TOKEN \
> +            --form email=peff@peff.net \
> +            --form file=@git.tgz \
> +            --form version=$(git rev-parse HEAD) \
> +            --form description="$(./git version)" \
> +            https://scan.coverity.com/builds?project=peff/git
> +        env:
> +          TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}
>
> Is there interest in having something like this in the main repo? We'd
> need to tweak some values:

I'm very interested in it, it would be great to have more CI targets,
even if optional.

>   - we have to send the project name (here peff/git); we can presumably
>     get this on the fly from the Actions envir

Our $CI_REPO_SLUG I believe. See
https://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables
& a grep for CI_REPO_SLUG in git.git.

>   - any repo which wants to use this has to set up the secret token
>     (COVERITY_SCAN_TOKEN here). That involves creating a coverity
>     account, and then setting the token in the GitHub web interface.
>     Presumably we'd just bail immediately if that token isn't set, so
>     forks aside from git/git would have to enable it independently.

I tried creating one of these now, requested access at
https://scan.coverity.com/projects/git & it's pending.

Maybe I should have clicked the "connect with GitHub" at the beginning,
but it wanted (ro) ACL access to all organizations I was in, including
private boards or something. So I went for the generic sign-up.

>   - likewise it needs the email address for the coverity account. That
>     could probably be set in the environment, too.
>
>   - we'd probably want to only run it for integration branches, since
>     coverity sets some limits on how often it runs. This could probably
>     be set in another environment variable, so people could tweak it for
>     their forks if they wanted to (or we could use the ci-config hacks,
>     but I put those together mostly because these environment variables
>     didn't exist back then; I suspect we could switch off of them now).
>
> There are tons of existing warnings, many of which are false positives.
> But it keeps track of which problems are new, and emails out a summary
> of only the new ones (which is how I saw the leak here, which just hit
> next). I don't care all that much about leaks here (we have other
> techniques for finding them), but when Stefan used to do regular
> coverity builds in the past, it routinely found useful errors.
>
> If we had it running on git/git, it's possible for people to subscribe
> to those notifications (or view them on the site; both require the
> people to have coverity accounts, but they're free).
>
> Thoughts?

Sounds good, I wonder if they (if contacted) provide upon request some
community-wide keys for projects such as git, so it would Just Work for
forks without their owners needing to sign up themselves...

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> In read_midx_preferred_pack(), we open the bitmap index but never free
> it. This isn't a big deal since this is just a test helper, and we exit
> immediately after, but since we're trying to keep our leak-checking tidy
> now, it's worth fixing.

Thanks.  That sounds sensible.

>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  t/helper/test-read-midx.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c
> index 0038559129..9d6fa7a377 100644
> --- a/t/helper/test-read-midx.c
> +++ b/t/helper/test-read-midx.c
> @@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)
>  		return 1;
>  
>  	bitmap = prepare_bitmap_git(the_repository);
> -	if (!(bitmap && bitmap_is_midx(bitmap)))
> +	if (!bitmap)
>  		return 1;
> -
> +	if (!bitmap_is_midx(bitmap)) {
> +		free_bitmap_index(bitmap);
> +		return 1;
> +	}
>  
>  	printf("%s\n", midx->pack_names[midx_preferred_pack(bitmap)]);
> +	free_bitmap_index(bitmap);
>  	return 0;
>  }

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Thu, Oct 07, 2021 at 12:07:39AM -0400, Taylor Blau wrote:

> On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:
> > The reason for that is that I didn't find it by inspection; I've started
> > running my personal builds through coverity. It wasn't too bad to set up
> > with a GitHub Action, like so:
> >
> > [...]
> 
> It looks like this would never cause the build to fail, but is merely
> responsible for sending any warnings off to Coverity's UI?

Sort of. They basically wrap the "make" invocation to intercept "cc". My
understanding is that their faux-compiler is mostly about gathering data
about the code. That gets stuffed into a tarball and uploaded to their
servers, where the real analysis happens.

It's very black-box, which I don't love. But in my experience it
produces by far the most useful static-analysis output of any tool I've
seen.

> > There are tons of existing warnings, many of which are false positives.
> > But it keeps track of which problems are new, and emails out a summary
> > of only the new ones (which is how I saw the leak here, which just hit
> > next). I don't care all that much about leaks here (we have other
> > techniques for finding them), but when Stefan used to do regular
> > coverity builds in the past, it routinely found useful errors.
> 
> I'm generally pessimistic about tools like Coverity, but I share your
> experience that Coverity warnings are actually pretty high quality. Or
> at least they have a high enough signal-to-noise ratio that it makes
> them worth looking through.
> 
> So I would be happy to have forks of GitHub have fewer barriers to use
> this tool.

OK. I'll see if I can clean up the patch a bit.

-Peff

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Thu, Oct 07, 2021 at 09:42:11AM +0200, Ævar Arnfjörð Bjarmason wrote:

> Interesting mix...
> [...]
> ...of curl & wget :)

Heh, yeah, I noticed that, too. This is literally cut and paste from
coverity's "here's how to automate a scan" instructions, which yes, use
both tools. I don't think it's a big deal, but we could pretty easily
swap the wget invocation for curl.

> >   - any repo which wants to use this has to set up the secret token
> >     (COVERITY_SCAN_TOKEN here). That involves creating a coverity
> >     account, and then setting the token in the GitHub web interface.
> >     Presumably we'd just bail immediately if that token isn't set, so
> >     forks aside from git/git would have to enable it independently.
> 
> I tried creating one of these now, requested access at
> https://scan.coverity.com/projects/git & it's pending.

So that isn't actually my project. ;)

That's the old dead one that Stefan used to do builds for. Now I do have
access to that and saw your request, but it's not actually receiving new
builds.

I probably _could_ upload to that one, of course, but the builds I'm
doing are of my private topics.

My thinking was that if Junio is OK with it, we'd have this in the main
tree and the git/git ones would be the main builds.

> Sounds good, I wonder if they (if contacted) provide upon request some
> community-wide keys for projects such as git, so it would Just Work for
> forks without their owners needing to sign up themselves...

I get the impression that it's a fairly hefty CPU cost for something
they're offering for free, and they really don't want everybody's random
fork doing analyses. I.e., they'd rather see open source projects set up
an analysis project for their official tree and that's it.

OTOH, another concern of theirs is that the results aren't disclosed
publicly, since they may have security implications. But they've made it
easy enough for people to submit their random GitHub repositories, which
can contain anybody's code, so it doesn't seem like much of a secret.

So I dunno. They're nice enough to offer the service for free, and I
want to respect their wishes. But I'm having a hard-time finding
documents describing exactly what's OK and what's next with respect to
forks.

They do have some limits posted here:

  https://scan.coverity.com/faq#frequency

It's on the order of 3 builds per day for a code base of our size. Which
is plenty for our integration branches, but not enough to test every
topic branch. And though we could get around that with treating forks as
separate projects, I think that's violating the spirit of the limit.

-Peff

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Johannes Schindelin]

Hi Peff & Taylor,

On Thu, 7 Oct 2021, Jeff King wrote:

> On Thu, Oct 07, 2021 at 12:07:39AM -0400, Taylor Blau wrote:
>
> > On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:
> > > The reason for that is that I didn't find it by inspection; I've started
> > > running my personal builds through coverity. It wasn't too bad to set up
> > > with a GitHub Action, like so:
> > >
> > > [...]
> >
> > It looks like this would never cause the build to fail, but is merely
> > responsible for sending any warnings off to Coverity's UI?
>
> Sort of. They basically wrap the "make" invocation to intercept "cc". My
> understanding is that their faux-compiler is mostly about gathering data
> about the code. That gets stuffed into a tarball and uploaded to their
> servers, where the real analysis happens.
>
> It's very black-box, which I don't love. But in my experience it
> produces by far the most useful static-analysis output of any tool I've
> seen.

It is pretty black box, but I have to disagree that the static analysis
output is very useful. The majority are false positives about
strbuf/strvec type usage of a static, fixed-size array that is dynamically
replaced by a dynamically-allocated array. Coverity misses that subtlety
and reports out-of-bounds accesses.

Granted, I worked around those (I thought) by using the
`-DFLEX_ARRAY=65536` trick, but I guess that is either not working as
designed, or it stopped working at some stage.

FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`
branch covered by Coverity:

https://dev.azure.com/git-for-windows/git/_build?definitionId=35

It essentially calls into this scripted code:
https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915

Ciao,
Dscho

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> They do have some limits posted here:
>
>   https://scan.coverity.com/faq#frequency
>
> It's on the order of 3 builds per day for a code base of our size. Which
> is plenty for our integration branches, but not enough to test every
> topic branch.

I usually have at least two pushout of 'seen' (one with the full set
of 'seen' including known-to-be-broken topic integrations, the other
with seen~$some_hopefully_small_number that I didn't see brekaage in
my local build), and then on graduation days 'next' and 'master' are
also updated, so 3 is cutting very close ;-)

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Fri, Oct 08, 2021 at 01:03:08PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > They do have some limits posted here:
> >
> >   https://scan.coverity.com/faq#frequency
> >
> > It's on the order of 3 builds per day for a code base of our size. Which
> > is plenty for our integration branches, but not enough to test every
> > topic branch.
> 
> I usually have at least two pushout of 'seen' (one with the full set
> of 'seen' including known-to-be-broken topic integrations, the other
> with seen~$some_hopefully_small_number that I didn't see brekaage in
> my local build), and then on graduation days 'next' and 'master' are
> also updated, so 3 is cutting very close ;-)

Hmm, yeah. They say "21 builds per week", which would be plenty (you
don't push out integrations every day), but I think they may also have a
per-day limit.

I'm not sure what happens when you hit the limit. If it just silently
skips the analysis, that's fine (we'll pick up the changes the next
day). If it causes a CI failure that nags you, that is less good. But
probably something we could work around in the Actions commands.

TBH, though, I think just building "seen" would be sufficient for this
kind of analysis. Most CI is about "make sure we pass the tests and do
not graduate topics if it doesn't". This is much more about "generate a
set of possibly-bogus annotations for some human to look at". Given the
topic branch flow you use, it's pretty unlikely for a problem to
_appear_ in master or next. And even if they do, it will be detected the
next time seen is run.

-Peff

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:

> > Sort of. They basically wrap the "make" invocation to intercept "cc". My
> > understanding is that their faux-compiler is mostly about gathering data
> > about the code. That gets stuffed into a tarball and uploaded to their
> > servers, where the real analysis happens.
> >
> > It's very black-box, which I don't love. But in my experience it
> > produces by far the most useful static-analysis output of any tool I've
> > seen.
> 
> It is pretty black box, but I have to disagree that the static analysis
> output is very useful. The majority are false positives about
> strbuf/strvec type usage of a static, fixed-size array that is dynamically
> replaced by a dynamically-allocated array. Coverity misses that subtlety
> and reports out-of-bounds accesses.

Yes, I remember skipping past quite a few of those.

To be clear, I don't claim that its output is amazing. Only that it has
produced actionable output on many occasions. Grepping commit messages
for "Coverity" turns up several hits (many from you :) ). Most of those
are leak fixes, and I do think we have better options there. But I
recall it detecting some hard-to-find memory and logic errors, too.

> Granted, I worked around those (I thought) by using the
> `-DFLEX_ARRAY=65536` trick, but I guess that is either not working as
> designed, or it stopped working at some stage.
> 
> FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`
> branch covered by Coverity:
> 
> https://dev.azure.com/git-for-windows/git/_build?definitionId=35
> 
> It essentially calls into this scripted code:
> https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915

Do you have any objection to adding something like the Action I showed
eariler? It would do nothing in git-for-windows/git unless you set up
the right environment, so there shouldn't be any downside.

I admit I was not really planning to try to suppress the false positives
as you've done here; my plan was to just keep an eye on the "new"
entries (having already gone through the existing ones years ago).

-Peff

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Oct 08, 2021 at 01:03:08PM -0700, Junio C Hamano wrote:
>
>> I usually have at least two pushout of 'seen' (one with the full set
>> of 'seen' including known-to-be-broken topic integrations, the other
>> with seen~$some_hopefully_small_number that I didn't see brekaage in
>> my local build), and then on graduation days 'next' and 'master' are
>> also updated, so 3 is cutting very close ;-)
>
> Hmm, yeah. They say "21 builds per week", which would be plenty (you
> don't push out integrations every day),

'seen' is pushed out every day (not on weekends), though.

> I'm not sure what happens when you hit the limit. If it just silently
> skips the analysis, that's fine (we'll pick up the changes the next
> day). If it causes a CI failure that nags you, that is less good. But
> probably something we could work around in the Actions commands.

Yup.

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Johannes Schindelin]

Hi Peff,

On Fri, 8 Oct 2021, Jeff King wrote:

> On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:
>
> > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`
> > branch covered by Coverity:
> >
> > https://dev.azure.com/git-for-windows/git/_build?definitionId=35
> >
> > It essentially calls into this scripted code:
> > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915
>
> Do you have any objection to adding something like the Action I showed
> eariler? It would do nothing in git-for-windows/git unless you set up
> the right environment, so there shouldn't be any downside.

No objection. I'd just ask to use `${{github.repository}}` instead of
hard-coding `peff/git`, and to really not run the workflow unless
configured. So something like this:

name: coverity-scan
on:
  push:
    - master
    - next
    - seen

jobs:
  coverity:
    runs-on: ubuntu-latest
    env:
      COVERITY_SCAN_TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}
      COVERITY_SCAN_EMAIL: ${{ secrets.COVERITY_SCAN_EMAIL }}
    if: env.COVERITY_SCAN_TOKEN != '' && env.COVERITY_SCAN_EMAIL != ''
    steps:
      - uses: actions/checkout@v2
      - run: ci/install-dependencies.sh
      - name: Download Coverity Build Tool
        run: |
          wget -q https://scan.coverity.com/download/linux64 --post-data "token=$COVERITY_SCAN_TOKEN&project=$GITHUB_REPOSITORY" -O cov-analysis-linux64.tar.gz
          mkdir cov-analysis-linux64
          tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64
      - name: Build with cov-build
        run: |
          export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH
          cov-build --dir cov-int make
      - name: Submit the result to Coverity Scan
        run: |
          tar czvf git.tgz cov-int
          curl \
            --form project=$GITHUB_REPOSITORY \
            --form token=$COVERITY_SCAN_TOKEN \
            --form email=$COVERITY_SCAN_EMAIL \
            --form file=@git.tgz \
            --form version=$(git rev-parse HEAD) \
            --form description="$(./git version)" \
            https://scan.coverity.com/builds?project=$GITHUB_REPOSITORY

Note the `jobs.coverity.if` attribute. This is what will let the entire
job be skipped unless the secrets are set up.

I am very much in favor of having this in git/git. Do you want to provide
the commit message, or do you want me to shepher this?

> I admit I was not really planning to try to suppress the false positives
> as you've done here; my plan was to just keep an eye on the "new"
> entries (having already gone through the existing ones years ago).

I think we will _have_ to suppress the false positives at some point, as
something like 9 out of 10 new reports I receive are about these, and it
takes time to analyze & dismiss them. In general, I have no trouble
finding more fun things to do with my time.

Ciao,
Dscho

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Taylor Blau]

On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:
> Hi Peff,
>
> On Fri, 8 Oct 2021, Jeff King wrote:
>
> > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:
> >
> > > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`
> > > branch covered by Coverity:
> > >
> > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35
> > >
> > > It essentially calls into this scripted code:
> > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915
> >
> > Do you have any objection to adding something like the Action I showed
> > eariler? It would do nothing in git-for-windows/git unless you set up
> > the right environment, so there shouldn't be any downside.
>
> No objection. I'd just ask to use `${{github.repository}}` instead of
> hard-coding `peff/git`, and to really not run the workflow unless
> configured. So something like this:
>
> [...]
>
> I am very much in favor of having this in git/git. Do you want to provide
> the commit message, or do you want me to shepher this?

This all looks good to me. FWIW, I was planning on picking this up
myself, but I'm happy to have been beaten to it ;). The only thing
missing is having a project for git.git set up on Coverity's website.

I'd be happy to look into that if you wanted to polish this up and
submit it. We should be able to proceed independently, since this will
all be a noop without setting the various Coverity secrets inside of
git/git.

Thanks,
Taylor

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> No objection. I'd just ask to use `${{github.repository}}` instead of
> hard-coding `peff/git`, and to really not run the workflow unless
> configured. So something like this:
> ...
> Note the `jobs.coverity.if` attribute. This is what will let the entire
> job be skipped unless the secrets are set up.

It would be nice if "the entire job be skipped" did not even make it
appear in the https://github.com/git/git/actions list.

I am assuming that your illustration is using the same trick as
git-l10n jobs, so the result will appear on the page in the same way
as they appear, i.e. with white circle with a 45-degree bar in it?

As long as we do not give outside parties anything stronger than a
read-only access, I have no objection to the plan, either.

Thanks.

Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:

> Hi Peff,
> 
> On Fri, 8 Oct 2021, Jeff King wrote:
> 
> > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:
> >
> > > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`
> > > branch covered by Coverity:
> > >
> > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35
> > >
> > > It essentially calls into this scripted code:
> > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915
> >
> > Do you have any objection to adding something like the Action I showed
> > eariler? It would do nothing in git-for-windows/git unless you set up
> > the right environment, so there shouldn't be any downside.
> 
> No objection. I'd just ask to use `${{github.repository}}` instead of
> hard-coding `peff/git`, and to really not run the workflow unless
> configured. So something like this:

Yep, those were directions I was planning to take it.

> I am very much in favor of having this in git/git. Do you want to provide
> the commit message, or do you want me to shepher this?

I'd be just as happy if you did (I hadn't even looked at it since my
earlier email).

It sounds like Taylor is volunteering to set up the Coverity side for
git.git, and I can help him with getting those COVERITY_* variables into
the GitHub environment.

> > I admit I was not really planning to try to suppress the false positives
> > as you've done here; my plan was to just keep an eye on the "new"
> > entries (having already gone through the existing ones years ago).
> 
> I think we will _have_ to suppress the false positives at some point, as
> something like 9 out of 10 new reports I receive are about these, and it
> takes time to analyze & dismiss them. In general, I have no trouble
> finding more fun things to do with my time.

The volume is low enough that I've been OK just manually skipping past
them. But if we have mitigations to make that happen automatically, I'm
all in favor. We can do that incrementally on top, but the fact that
you've already figured out most of it means it may make sense to just do
it from the start (and is another good reason for you to submit the
patches).

Sorry, I didn't mean to make more work/patches for you. :)

-Peff

Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Johannes Schindelin]

Hi Peff & Taylor,

On Wed, 20 Oct 2021, Jeff King wrote:

> On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:
>
> > On Fri, 8 Oct 2021, Jeff King wrote:
> >
> > > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:
> > >
> > > > FWIW I have set up an Azure Pipeline to keep Git for Windows'
> > > > `main` branch covered by Coverity:
> > > >
> > > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35
> > > >
> > > > It essentially calls into this scripted code:
> > > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915
> > >
> > > Do you have any objection to adding something like the Action I
> > > showed eariler? It would do nothing in git-for-windows/git unless
> > > you set up the right environment, so there shouldn't be any
> > > downside.
> >
> > No objection. I'd just ask to use `${{github.repository}}` instead of
> > hard-coding `peff/git`, and to really not run the workflow unless
> > configured. So something like this:
>
> Yep, those were directions I was planning to take it.
>
> > I am very much in favor of having this in git/git. Do you want to provide
> > the commit message, or do you want me to shepher this?
>
> I'd be just as happy if you did (I hadn't even looked at it since my
> earlier email).
>
> It sounds like Taylor is volunteering to set up the Coverity side for
> git.git, and I can help him with getting those COVERITY_* variables into
> the GitHub environment.

Given the challenges with Coverity (false positives, lack of support on
Synopsys' side, severely limited access to the reports), and given the
renewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am
in favor of abandoning the idea to integrate Coverity in our GitHub
workflow.

Regarding CodeQL, I am still uncertain what level of integration we will
end up with, and the contacts I am working with are currently all on
vacation, but I am confident that we will have an easier time going
forward with static analysis using CodeQL instead of Coverity.

Ciao,
Dscho

Re: Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Jeff King]

On Tue, Aug 16, 2022 at 11:05:48AM +0200, Johannes Schindelin wrote:

> > It sounds like Taylor is volunteering to set up the Coverity side for
> > git.git, and I can help him with getting those COVERITY_* variables into
> > the GitHub environment.
> 
> Given the challenges with Coverity (false positives, lack of support on
> Synopsys' side, severely limited access to the reports), and given the
> renewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am
> in favor of abandoning the idea to integrate Coverity in our GitHub
> workflow.
> 
> Regarding CodeQL, I am still uncertain what level of integration we will
> end up with, and the contacts I am working with are currently all on
> vacation, but I am confident that we will have an easier time going
> forward with static analysis using CodeQL instead of Coverity.

OK. I haven't been that impressed with CodeQL for C so far, but it may
be getting better. I certainly would be happier with a system that made
it easier to display and share reports.

Coverity does have a lot of false positives, but I've at least been able
to pick useful fixes out of them (especially because it is good about
saying "here are 5 new things to look at"). I've been continuing to
build my private branch with it, so we'll see if it turns up anything
useful. I do agree that inflicting it on ordinary users may be
counter-productive (I often have to stare really hard to understand why
its false positives are false, and that is not something I would wish
on, say, a GGG user).

-Peff

Re: Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)

[Johannes Schindelin]

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

Hi Peff,

On Tue, 16 Aug 2022, Jeff King wrote:

> On Tue, Aug 16, 2022 at 11:05:48AM +0200, Johannes Schindelin wrote:
>
> > > It sounds like Taylor is volunteering to set up the Coverity side for
> > > git.git, and I can help him with getting those COVERITY_* variables into
> > > the GitHub environment.
> >
> > Given the challenges with Coverity (false positives, lack of support on
> > Synopsys' side, severely limited access to the reports), and given the
> > renewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am
> > in favor of abandoning the idea to integrate Coverity in our GitHub
> > workflow.
> >
> > Regarding CodeQL, I am still uncertain what level of integration we will
> > end up with, and the contacts I am working with are currently all on
> > vacation, but I am confident that we will have an easier time going
> > forward with static analysis using CodeQL instead of Coverity.
>
> OK. I haven't been that impressed with CodeQL for C so far, but it may
> be getting better.

If your lack of being impressed stems from CodeQL's default suite catching
very, very few issues, I agree with you.

The reason why I am more hopeful about CodeQL than about Coverity is that
I now have a contact to work with (although they are currently on
vacation, so I get to work on other things for now, including `merge-ort`
and trying to integrate `mimalloc` in Git for Windows). And that is one
more contact who is willing to work with me than I ever had on the
Coverity side of things.

And apparently CodeQL's default settings optimize for reducing false
positives in an attempt to avoid scaring potential users away.

But I have credible assurances that CodeQL has many more checks in store
that simply need to be enabled in order to catch way more issues, at the
expense of risking more false positives.

> I certainly would be happier with a system that made it easier to
> display and share reports.
>
> Coverity does have a lot of false positives, but I've at least been able
> to pick useful fixes out of them (especially because it is good about
> saying "here are 5 new things to look at"). I've been continuing to
> build my private branch with it, so we'll see if it turns up anything
> useful. I do agree that inflicting it on ordinary users may be
> counter-productive (I often have to stare really hard to understand why
> its false positives are false, and that is not something I would wish
> on, say, a GGG user).

Don't get me wrong, I do not plan on dropping the Coverity builds of the
Git for Windows project's `main` branch at
https://dev.azure.com/git-for-windows/git/_build?definitionId=35.

As you probably recall, I specifically looked through the Coverity report
in advance of v2.37.0 and tried to address some of the issues in
https://lore.kernel.org/git/pull.1264.git.1655336146.gitgitgadget@gmail.com/,
with mixed results (mainly because of some time constraints on my side,
combined with your willingness to help fix some issues with my patches, as
well as René's, Taylor's and Junio's excellent assistance).

However, the sheer amount of false positives, with intentional issues
being a close second (in test helpers, we are not all that strict about
releasing memory, for example), makes it a tough sell to ask more than
just a few very dedicated contributors to have a look at the reports.

With CodeQL, I am optimistic that we can get it to a point where the
burden can be carried by a larger group of people, with the help of
enthusiastic CodeQL experts, which also means that the reports have a
bigger chance at making Git safer.

Ciao,
Dscho