{"thread":{"id":"56656","subject":"What's cooking in git.git (Oct 2021, #02; Wed, 6)","startedAt":"2021-10-07T00:24:23Z","lastAt":"2022-08-19T11:23:25Z","messageCount":24,"participants":["Junio C Hamano","Ævar Arnfjörð Bjarmason","Jeff King","Taylor Blau","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"438106","messageId":"xmqqfstdr8b5.fsf@gitster.g","threadId":"56656","inReplyTo":null,"subject":"What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-07T00:24:14Z","receivedAt":"2021-10-07T00:24:23Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Here are the topics that have been cooking in my tree. Commits\nprefixed with '+' are in 'next' (being in 'next' is a sign that a\ntopic is stable enough to be used and are candidate to be in a\nfuture release). Commits prefixed with '-' are only in 'seen',\nwhich means nothing more than that I have found them of interest for\nsome reason (like \"it may have hard-to-resolve conflicts with\nanother topic already in flight\" or \"this may turn out to be\nuseful\"). Do not read too much into a topic being in (or not in)\n'seen'. The ones marked with '.' do not appear in any of the\nintegration branches, but I am still holding onto them.\n\nThe eleventh batch is in.\n\nCopies of the source code to Git live in many repositories, and the\nfollowing is a list of the ones I push into or their mirrors. Some\nrepositories have only a subset of branches.\n\nWith maint, master, next, seen, todo:\n\n\tgit://git.kernel.org/pub/scm/git/git.git/\n\tgit://repo.or.cz/alt-git.git/\n\thttps://kernel.googlesource.com/pub/scm/git/git/\n\thttps://github.com/git/git/\n\thttps://gitlab.com/git-vcs/git/\n\nWith all the integration branches and topics broken out:\n\n\thttps://github.com/gitster/git/\n\nEven though the preformatted documentation in HTML and man format\nare not sources, they are published in these repositories for\nconvenience (replace \"htmldocs\" with \"manpages\" for the manual\npages):\n\n\tgit://git.kernel.org/pub/scm/git/git-htmldocs.git/\n\thttps://github.com/gitster/git-htmldocs.git/\n\nRelease tarballs are available at:\n\n\thttps://www.kernel.org/pub/software/scm/git/\n\n--------------------------------------------------\n[Graduated to 'master']\n\n* ab/repo-settings-cleanup (2021-09-22) 5 commits\n (merged to 'next' on 2021-09-28 at 43d70c31e1)\n + repository.h: don't use a mix of int and bitfields\n + repo-settings.c: simplify the setup\n + read-cache & fetch-negotiator: check \"enum\" values in switch()\n + environment.c: remove test-specific \"ignore_untracked...\" variable\n + wrapper.c: add x{un,}setenv(), and use xsetenv() in environment.c\n\n Code cleanup.\n\n\n* ab/retire-decl-of-missing-unused-funcs (2021-10-01) 4 commits\n (merged to 'next' on 2021-10-03 at a49287eaa9)\n + config.h: remove unused git_config_get_untracked_cache() declaration\n + log-tree.h: remove unused function declarations\n + grep.h: remove unused grep_threads_ok() declaration\n + builtin.h: remove cmd_tar_tree() declaration\n\n Remove external declaration of functions that no longer exist.\n\n\n* ab/retire-refs-unused-funcs (2021-09-28) 5 commits\n (merged to 'next' on 2021-10-03 at f91e74fa7d)\n + refs/ref-cache.[ch]: remove \"incomplete\" from create_dir_entry()\n + refs/ref-cache.c: remove \"mkdir\" parameter from find_containing_dir()\n + refs/ref-cache.[ch]: remove unused add_ref_entry()\n + refs/ref-cache.[ch]: remove unused remove_entry_from_dir()\n + refs.[ch]: remove unused ref_storage_backend_exists()\n\n Code cleanup.\n\n\n* ab/retire-string-list-init (2021-09-28) 1 commit\n (merged to 'next' on 2021-10-03 at 4834949cc3)\n + string-list.[ch]: remove string_list_init() compatibility function\n\n Code cleanup.\n\n\n* ew/midx-doc-update (2021-09-24) 1 commit\n (merged to 'next' on 2021-09-28 at f2cbe598eb)\n + doc/technical: update note about core.multiPackIndex\n\n Doc tweak.\n\n\n* gc/doc-first-contribution-reroll (2021-09-22) 1 commit\n (merged to 'next' on 2021-09-29 at b7dea55eae)\n + MyFirstContribution: Document --range-diff option when writing v2\n\n Doc update.\n\n\n* jk/grep-haystack-is-read-only (2021-09-22) 5 commits\n (merged to 'next' on 2021-09-28 at 1660a6be89)\n + grep: store grep_source buffer as const\n + grep: mark \"haystack\" buffers as const\n + grep: stop modifying buffer in grep_source_1()\n + grep: stop modifying buffer in show_line()\n + grep: stop modifying buffer in strip_timestamp\n (this branch is used by hm/paint-hits-in-log-grep.)\n\n Code clean-up in the \"grep\" machinery.\n\n\n* jt/add-submodule-odb-clean-up (2021-09-09) 3 commits\n (merged to 'next' on 2021-09-28 at 4d843448be)\n + revision: remove \"submodule\" from opt struct\n + repository: support unabsorbed in repo_submodule_init\n + submodule: remove unnecessary unabsorbed fallback\n (this branch is used by jt/no-abuse-alternate-odb-for-submodules.)\n\n More code paths that use the hack to add submodule's object\n database to the set of alternate object store have been cleaned up.\n\n\n* lh/systemd-timers (2021-09-27) 1 commit\n (merged to 'next' on 2021-10-03 at 81834609ea)\n + maintenance: fix test t7900-maintenance.sh\n\n Testfix.\n\n\n* os/status-docfix (2021-09-28) 1 commit\n (merged to 'next' on 2021-10-03 at a13019916a)\n + doc: fix capitalization in \"git status --porcelain=v2\" description\n\n Docfix.\n\n\n* pw/rebase-of-a-tag-fix (2021-09-22) 10 commits\n (merged to 'next' on 2021-09-28 at 980add2a67)\n + rebase: dereference tags\n + rebase: use lookup_commit_reference_by_name()\n + rebase: use our standard error return value\n + t3407: rework rebase --quit tests\n + t3407: strengthen rebase --abort tests\n + t3407: use test_path_is_missing\n + t3407: rename a variable\n + t3407: use test_cmp_rev\n + t3407: use test_commit\n + t3407: run tests in $TEST_DIRECTORY\n (this branch is used by pw/fix-some-issues-in-reset-head.)\n\n \"git rebase \" failed when aborted in the middle, as\n it mistakenly tried to write the tag object instead of peeling it\n to HEAD.\n\n\n* pw/rebase-reread-todo-after-editing (2021-09-24) 2 commits\n (merged to 'next' on 2021-09-28 at c67d5e383e)\n + rebase: fix todo-list rereading\n + sequencer.c: factor out a function\n\n The code to re-read the edited todo list in \"git rebase -i\" was\n made more robust.\n\n\n* tb/commit-graph-usage-fix (2021-09-22) 2 commits\n (merged to 'next' on 2021-09-28 at f021339c39)\n + builtin/multi-pack-index.c: disable top-level --[no-]progress\n + builtin/commit-graph.c: don't accept common --[no-]progress\n\n Regression in \"git commit-graph\" command line parsing has been\n corrected.\n\n\n* ws/refer-to-forkpoint-config-in-rebase-doc (2021-09-20) 1 commit\n (merged to 'next' on 2021-09-29 at 49181eaafb)\n + Document `rebase.forkpoint` in rebase man page\n\n Doc update.\n\n--------------------------------------------------\n[New Topics]\n\n* gc/use-repo-settings (2021-10-04) 3 commits\n - gc: perform incremental repack when implictly enabled\n - fsck: verify multi-pack-index when implictly enabled\n - fsck: verify commit graph when implicitly enabled\n\n It is wrong to read some settings directly from the config\n subsystem, as things like feature.experimental can affect their\n default values.\n\n Under review.\n cf. <70aca052-716f-40ed-47c4-1882fdbd221e@gmail.com>\n\n\n* jh/perf-remove-test-times (2021-10-04) 1 commit\n - t/perf/perf-lib.sh: remove test_times.* at the end test_perf_()\n\n Perf test fix.\n\n Will merge to 'next'.\n\n--------------------------------------------------\n[Stalled]\n\n* ar/submodule-update (2021-09-20) 8 commits\n . submodule--helper: rename helper functions\n . submodule--helper: remove unused helpers\n . submodule--helper: remove update-clone subcommand\n . submodule: move core cmd_update() logic to C\n . submodule--helper: refactor get_submodule_displaypath()\n . submodule--helper: rename helpers for update-clone\n . submodule--helper: get remote names from any repository\n . submodule--helper: split up ensure_core_worktree()\n\n Rewrite of \"git submodule update\" in C.\n\n Kicked out of 'seen' to make room for es/superproject-aware-submodules\n which is among the topics this topic stomps on.\n\n--------------------------------------------------\n[Cooking]\n\n* pw/fix-some-issues-in-reset-head (2021-10-06) 13 commits\n - sparse index: fix use-after-free bug in cache_tree_verify()\n - rebase -m: don't fork git checkout\n - rebase --apply: set ORIG_HEAD correctly\n - rebase --apply: fix reflog\n - reset_head(): take struct rebase_head_opts\n - rebase: cleanup reset_head() calls\n - reset_head(): make default_reflog_action optional\n - reset_head(): factor out ref updates\n - reset_head(): remove action parameter\n - reset_head(): don't run checkout hook if there is an error\n - reset_head(): fix checkout\n - rebase: factor out checkout for up to date branch\n - Merge branch 'pw/rebase-of-a-tag-fix' into pw/fix-some-issues-in-reset-head\n\n Fix \"some issues\" in a helper function reset_head().\n\n Expecting a reroll.\n Needs a lot better explanation, including what the issues are,\n which codepaths the helper is used and to do what, and tests to\n protect the fixes.\n\n\n* rs/mergesort (2021-10-01) 9 commits\n (merged to 'next' on 2021-10-03 at 29a672574f)\n + mergesort: use ranks stack\n + p0071: test performance of llist_mergesort()\n + p0071: measure sorting of already sorted and reversed files\n + test-mergesort: add unriffle_skewed mode\n + test-mergesort: add unriffle mode\n + test-mergesort: add generate subcommand\n + test-mergesort: add test subcommand\n + test-mergesort: add sort subcommand\n + test-mergesort: use strbuf_getline()\n\n The mergesort implementation used to sort linked list has been\n optimized.\n\n Will merge to 'master'.\n\n\n* bs/doc-blame-color-lines (2021-10-01) 2 commits\n - blame: document --color-* options\n - blame: describe default output format\n\n The \"--color-lines\" and \"--color-by-age\" options of \"git blame\"\n have been missing, which are now documented.\n\n Expecting a reroll.\n cf. \n\n\n* mr/bisect-in-c-4 (2021-10-01) 1 commit\n (merged to 'next' on 2021-10-06 at c13c14238d)\n + bisect--helper: add space between colon and following sentence\n\n Message fix.\n\n Will merge to 'master'.\n\n\n* cm/save-restore-terminal (2021-10-06) 2 commits\n - editor: save and reset terminal after calling EDITOR\n - terminal: teach git how to save/restore its terminal settings\n\n An editor session launched during a Git operation (e.g. during 'git\n commit') can leave the terminal in a funny state. The code path\n has updated to save the terminal state before, and restore it\n after, it spawns an editor.\n\n Will merge to 'next'.\n\n\n* rs/p3400-lose-tac (2021-10-03) 1 commit\n (merged to 'next' on 2021-10-06 at 688dc7137c)\n + p3400: stop using tac(1)\n\n Test portability update.\n\n Will merge to 'master'.\n\n\n* ja/doc-status-types-and-copies (2021-10-04) 4 commits\n (merged to 'next' on 2021-10-06 at 4de6571bf7)\n + Documentation/git-status: mention how to detect copies\n + Documentation/git-status: document porcelain status T (typechange)\n + Documentation/diff-format: state in which cases porcelain status is T\n + Documentation/git-status: remove impossible porcelain status DR and DC\n\n A few kinds of changes \"git status\" can show were not described.\n\n Will merge to 'master'.\n\n\n* tb/aggregate-ignore-leading-whitespaces (2021-10-04) 1 commit\n (merged to 'next' on 2021-10-06 at 619a7db2d4)\n + t/perf/aggregate.perl: tolerate leading spaces\n\n Test portability update.\n\n Will merge to 'master'.\n\n\n* ab/designated-initializers (2021-09-27) 5 commits\n (merged to 'next' on 2021-10-03 at 179f652de6)\n + cbtree.h: define cb_init() in terms of CBTREE_INIT\n + *.h: move some *_INIT to designated initializers\n + *.h _INIT macros: don't specify fields equal to 0\n + *.[ch] *_INIT macros: use { 0 } for a \"zero out\" idiom\n + submodule-config.h: remove unused SUBMODULE_INIT macro\n (this branch is used by ab/designated-initializers-more.)\n\n Code clean-up.\n\n Will merge to 'master'.\n\n\n* ab/designated-initializers-more (2021-10-01) 6 commits\n - builtin/remote.c: add and use SHOW_INFO_INIT\n - builtin/remote.c: add and use a REF_STATES_INIT\n - urlmatch.[ch]: add and use URLMATCH_CONFIG_INIT\n - builtin/blame.c: refactor commit_info_init() to COMMIT_INFO_INIT macro\n - daemon.c: refactor hostinfo_init() to HOSTINFO_INIT macro\n - Merge branch 'ab/designated-initializers' into ab/designated-initializers-more\n (this branch uses ab/designated-initializers.)\n\n Code clean-up.\n\n Will merge to 'next'.\n\n\n* ab/parse-options-cleanup (2021-10-01) 11 commits\n - parse-options: change OPT_{SHORT,UNSET} to an enum\n - parse-options tests: test optname() output\n - parse-options.[ch]: make opt{bug,name}() \"static\"\n - commit-graph: stop using optname()\n - parse-options.c: move optname() earlier in the file\n - parse-options.h: make the \"flags\" in \"struct option\" an enum\n - parse-options.c: use exhaustive \"case\" arms for \"enum parse_opt_type\"\n - parse-options.c: use exhaustive \"case\" arms for \"enum parse_opt_result\"\n - parse-options.[ch]: consistently use \"enum parse_opt_result\"\n - parse-options.[ch]: consistently use \"enum parse_opt_flags\"\n - parse-options.h: move PARSE_OPT_SHELL_EVAL between enums\n\n Random changes to parse-options implementation.\n\n Will merge to 'next'?\n\n\n* ab/retire-git-config-key-is-valid (2021-09-28) 1 commit\n (merged to 'next' on 2021-10-03 at fc7a0a55d1)\n + config.c: remove unused git_config_key_is_valid()\n\n Code cleanup.\n\n Will merge to 'master'.\n\n\n* mt/grep-submodule-textconv (2021-09-29) 1 commit\n (merged to 'next' on 2021-10-06 at 1950944b8c)\n + grep: demonstrate bug with textconv attributes and submodules\n\n \"git grep --recurse-submodules\" takes trees and blobs from the\n submodule repository, but the textconv settings when processing a\n blob from the submodule is not taken from the submodule repository.\n A demonstration is added to demonstrate the issue, without fixing\n it.\n\n Will merge to 'master'.\n\n\n* es/superproject-aware-submodules (2021-08-19) 5 commits\n - fixup! introduce submodule.superprojectGitDir record\n - submodule: record superproject gitdir during 'update'\n - submodule: record superproject gitdir during absorbgitdirs\n - introduce submodule.superprojectGitDir record\n - t7400-submodule-basic: modernize inspect() helper\n\n A configuration variable in a submodule points at the location of\n the superproject it is bound to (RFC).\n\n Brought back to 'seen' to see if it still plays well with the rest\n of 'seen', without the conflicting ar/submodule-update topic.\n\n\n* ab/fsck-unexpected-type (2021-10-01) 17 commits\n - fsck: report invalid object type-path combinations\n - fsck: don't hard die on invalid object types\n - object-file.c: stop dying in parse_loose_header()\n - object-file.c: return ULHR_TOO_LONG on \"header too long\"\n - object-file.c: use \"enum\" return type for unpack_loose_header()\n - object-file.c: simplify unpack_loose_short_header()\n - object-file.c: make parse_loose_header_extended() public\n - object-file.c: return -1, not \"status\" from unpack_loose_header()\n - object-file.c: don't set \"typep\" when returning non-zero\n - cat-file tests: test for current --allow-unknown-type behavior\n - cat-file tests: add corrupt loose object test\n - cat-file tests: test for missing/bogus object with -t, -s and -p\n - cat-file tests: move bogus_* variable declarations earlier\n - fsck tests: test for garbage appended to a loose object\n - fsck tests: test current hash/type mismatch behavior\n - fsck tests: refactor one test to use a sub-repo\n - fsck tests: add test for fsck-ing an unknown type\n\n \"git fsck\" has been taught to report mismatch between expected and\n actual types of an object better.\n\n Will merge to 'next'.\n\n\n* ab/config-based-hooks-1 (2021-09-27) 8 commits\n (merged to 'next' on 2021-10-06 at d05325ed35)\n + hook-list.h: add a generated list of hooks, like config-list.h\n + hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n + hook.c: add a hook_exists() wrapper and use it in bugreport.c\n + hook.[ch]: move find_hook() from run-command.c to hook.c\n + Makefile: remove an out-of-date comment\n + Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)\n + Makefile: stop hardcoding {command,config}-list.h\n + Makefile: mark \"check\" target as .PHONY\n\n Mostly preliminary clean-up in the hook API.\n\n Will merge to 'master'.\n\n\n* ab/http-pinned-public-key-mismatch (2021-09-27) 1 commit\n (merged to 'next' on 2021-10-03 at cd67328eed)\n + http: check CURLE_SSL_PINNEDPUBKEYNOTMATCH when emitting errors\n\n HTTPS error handling updates.\n\n Will merge to 'master'.\n\n\n* jk/ref-paranoia (2021-09-27) 16 commits\n (merged to 'next' on 2021-10-03 at 8c2cb6a3a6)\n + refs: drop \"broken\" flag from for_each_fullref_in()\n + ref-filter: drop broken-ref code entirely\n + ref-filter: stop setting FILTER_REFS_INCLUDE_BROKEN\n + repack, prune: drop GIT_REF_PARANOIA settings\n + refs: turn on GIT_REF_PARANOIA by default\n + refs: omit dangling symrefs when using GIT_REF_PARANOIA\n + refs: add DO_FOR_EACH_OMIT_DANGLING_SYMREFS flag\n + refs-internal.h: reorganize DO_FOR_EACH_* flag documentation\n + refs-internal.h: move DO_FOR_EACH_* flags next to each other\n + t5312: be more assertive about command failure\n + t5312: test non-destructive repack\n + t5312: create bogus ref as necessary\n + t5312: drop \"verbose\" helper\n + t5600: provide detached HEAD for corruption failures\n + t5516: don't use HEAD ref for invalid ref-deletion tests\n + t7900: clean up some more broken refs\n (this branch is used by jt/no-abuse-alternate-odb-for-submodules.)\n\n The ref iteration code used to optionally allow dangling refs to be\n shown, which has been tightened up.\n\n Will merge to 'master'.\n\n\n* js/win-lazyload-buildfix (2021-09-28) 3 commits\n (merged to 'next' on 2021-10-03 at 26802e5d73)\n + Makefile: restrict -Wpedantic and -Wno-pedantic-ms-format better\n + lazyload.h: use an even more generic function pointer than FARPROC\n + lazyload.h: fix warnings about mismatching function pointer types\n\n Compilation fix.\n\n Will merge to 'master'.\n\n\n* ab/make-sparse-for-real (2021-09-22) 1 commit\n (merged to 'next' on 2021-10-06 at 10e3c31d6a)\n + Makefile: make the \"sparse\" target non-.PHONY\n\n Prevent \"make sparse\" from running for the source files that\n haven't been modified.\n\n Will merge to 'master'.\n\n\n* jt/no-abuse-alternate-odb-for-submodules (2021-10-01) 10 commits\n - fixup! refs: plumb repo into ref stores\n - submodule: trace adding submodule ODB as alternate\n - submodule: pass repo to check_has_commit()\n - object-file: only register submodule ODB if needed\n - merge-{ort,recursive}: remove add_submodule_odb()\n - refs: peeling non-the_repository iterators is BUG\n - refs: teach arbitrary repo support to iterators\n - refs: plumb repo into ref stores\n - Merge branch 'jk/ref-paranoia' into jt/no-abuse-alternate-odb-for-submodules\n - Merge branch 'jt/add-submodule-odb-clean-up' into jt/no-abuse-alternate-odb-for-submodules\n (this branch uses jk/ref-paranoia.)\n\n Follow through the work to use the repo interface to access\n submodule objects in-process, instead of abusing the alternate\n object database interface.\n\n Expecting a reroll.\n\n\n* tp/send-email-completion (2021-09-22) 3 commits\n - send-email docs: add format-patch options\n - send-email: move bash completions to core script\n - send-email: terminate --git-completion-helper with LF\n\n The command line complation for \"git send-email\" options have been\n tweaked to make it easier to keep it in sync with the command itself.\n\n Stalled.\n cf. \n\n\n* hm/paint-hits-in-log-grep (2021-09-29) 3 commits\n - pretty: colorize pattern matches in commit messages\n - grep: refactor next_match() and match_one_pattern() for external use\n - Merge branch 'jk/grep-haystack-is-read-only' into hm/paint-hits-in-log-grep\n\n \"git log --grep=string --author=name\" learns to highlight hits just\n like \"git grep string\" does.\n\n Expecting a reroll.\n cf. <87v92bju64.fsf@evledraar.gmail.com>\n\n\n* da/difftool (2021-09-30) 6 commits\n (merged to 'next' on 2021-10-03 at 3ba0335e4e)\n + difftool: add a missing space to the run_dir_diff() comments\n + difftool: remove an unnecessary call to strbuf_release()\n + difftool: refactor dir-diff to write files using helper functions\n + difftool: create a tmpdir path without repeated slashes\n + Merge branch 'da/difftool-dir-diff-symlink-fix' into da/difftool\n + Merge branch 'ab/retire-option-argument' into da/difftool\n\n Code clean-up in \"git difftool\".\n\n Will merge to 'master'.\n\n\n* en/removing-untracked-fixes (2021-09-27) 12 commits\n (merged to 'next' on 2021-10-06 at fc4e387fda)\n + Documentation: call out commands that nuke untracked files/directories\n + Comment important codepaths regarding nuking untracked files/dirs\n + unpack-trees: avoid nuking untracked dir in way of locally deleted file\n + unpack-trees: avoid nuking untracked dir in way of unmerged file\n + Change unpack_trees' 'reset' flag into an enum\n + Remove ignored files by default when they are in the way\n + unpack-trees: make dir an internal-only struct\n + unpack-trees: introduce preserve_ignored to unpack_trees_options\n + read-tree, merge-recursive: overwrite ignored files by default\n + checkout, read-tree: fix leak of unpack_trees_options.dir\n + t2500: add various tests for nuking untracked files\n + Merge branch 'en/am-abort-fix' into en/removing-untracked-fixes\n\n Various fixes in code paths that move untracked files away to make room.\n\n Will merge to 'master'.\n\n\n* ks/submodule-add-message-fix (2021-09-20) 1 commit\n - submodule--helper: fix incorrect newlines in an error message\n\n Message regression fix.\n\n Waiting for a response.\n cf. \n\n\n* ns/batched-fsync (2021-10-04) 9 commits\n . core.fsyncobjectfiles: performance tests for add and stash\n . core.fsyncobjectfiles: tests for batch mode\n . unpack-objects: use the bulk-checkin infrastructure\n . update-index: use the bulk-checkin infrastructure\n . core.fsyncobjectfiles: add windows support for batch mode\n . core.fsyncobjectfiles: batched disk flushes\n . bulk-checkin: rename 'state' variable and separate 'plugged' boolean\n . tmp-objdir: disable ref updates when replacing the primary odb\n . tmp-objdir: new API for creating temporary writable databases\n\n The \"core.fsyncobjectfiles\" configuration variable can now be set\n to \"batch\" for improved performance.\n\n Under discussion.\n\n Handling of temporary object directory is worked out with the\n en/remerge-diff topic.\n\n\n* jh/builtin-fsmonitor-part1 (2021-09-20) 7 commits\n (merged to 'next' on 2021-10-06 at 021f633b9c)\n + t/helper/simple-ipc: convert test-simple-ipc to use start_bg_command\n + run-command: create start_bg_command\n + simple-ipc/ipc-win32: add Windows ACL to named pipe\n + simple-ipc/ipc-win32: add trace2 debugging\n + simple-ipc: move definition of ipc_active_state outside of ifdef\n + simple-ipc: preparations for supporting binary messages.\n + trace2: add trace2_child_ready() to report on background children\n\n Built-in fsmonitor (part 1).\n\n Will merge to 'master'.\n\n\n* ab/align-parse-options-help (2021-09-22) 4 commits\n (merged to 'next' on 2021-10-06 at e22da7ef85)\n + parse-options: properly align continued usage output\n + git rev-parse --parseopt tests: add more usagestr tests\n + send-pack: properly use parse_options() API for usage string\n + parse-options API users: align usage output in C-strings\n\n When \"git cmd -h\" shows more than one line of usage text (e.g.\n the cmd subcommand may take sub-sub-command), parse-options API\n learned to align these lines, even across i18n/l10n.\n\n Will merge to 'master'.\n\n\n* ab/help-config-vars (2021-09-23) 9 commits\n (merged to 'next' on 2021-10-06 at bf9538cfbd)\n + help: move column config discovery to help.c library\n + help / completion: make \"git help\" do the hard work\n + help tests: test --config-for-completion option & output\n + help: simplify by moving to OPT_CMDMODE()\n + help: correct logic error in combining --all and --guides\n + help: correct logic error in combining --all and --config\n + help tests: add test for --config output\n + help: correct usage & behavior of \"git help --guides\"\n + help: correct the usage string in -h and documentation\n\n Teach \"git help -c\" into helping the command line completion of\n configuration variables.\n\n Will merge to 'master'.\n\n\n* tb/repack-write-midx (2021-10-01) 9 commits\n (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n + builtin/repack.c: make largest pack preferred\n + builtin/repack.c: support writing a MIDX while repacking\n + builtin/repack.c: extract showing progress to a variable\n + builtin/repack.c: rename variables that deal with non-kept packs\n + builtin/repack.c: keep track of existing packs unconditionally\n + midx: preliminary support for `--refs-snapshot`\n + builtin/multi-pack-index.c: support `--stdin-packs` mode\n + midx: expose `write_midx_file_only()` publicly\n\n \"git repack\" has been taught to generate multi-pack reachability\n bitmaps.\n\n Will merge to 'master'.\n\n\n* ds/add-rm-with-sparse-index (2021-09-28) 13 commits\n (merged to 'next' on 2021-10-06 at 80a9cda797)\n + advice: update message to suggest '--sparse'\n + mv: refuse to move sparse paths\n + rm: skip sparse paths with missing SKIP_WORKTREE\n + rm: add --sparse option\n + add: update --renormalize to skip sparse paths\n + add: update --chmod to skip sparse paths\n + add: implement the --sparse option\n + add: skip tracked paths outside sparse-checkout cone\n + add: fail when adding an untracked sparse file\n + dir: fix pattern matching on dirs\n + dir: select directories correctly\n + t1092: behavior for adding sparse files\n + t3705: test that 'sparse_entry' is unstaged\n\n \"git add\", \"git mv\", and \"git rm\" have been adjusted to avoid\n updating paths outside of the sparse-checkout definition unless\n the user specifies a \"--sparse\" option.\n\n Will merge to 'master'.\n\n\n* tb/midx-write-propagate-namehash (2021-09-17) 7 commits\n (merged to 'next' on 2021-09-29 at 24732fcfc8)\n + t5326: test propagating hashcache values\n + p5326: generate pack bitmaps before writing the MIDX bitmap\n + p5326: don't set core.multiPackIndex unnecessarily\n + p5326: create missing 'perf-tag' tag\n + midx.c: respect 'pack.writeBitmapHashcache' when writing bitmaps\n + pack-bitmap.c: propagate namehash values from existing bitmaps\n + t/helper/test-bitmap.c: add 'dump-hashes' mode\n\n \"git multi-pack-index write --bitmap\" learns to propagate the\n hashcache from original bitmap to resulting bitmap.\n\n Will merge to 'master'.\n\n\n* en/zdiff3 (2021-09-20) 2 commits\n - update documentation for new zdiff3 conflictStyle\n - xdiff: implement a zealous diff3, or \"zdiff3\"\n\n \"Zealous diff3\" style of merge conflict presentation has been added.\n\n\n* js/scalar (2021-09-14) 15 commits\n - scalar: accept -C and -c options before the subcommand\n - scalar: implement the `version` command\n - scalar: implement the `delete` command\n - scalar: teach 'reconfigure' to optionally handle all registered enlistments\n - scalar: allow reconfiguring an existing enlistment\n - scalar: implement the `run` command\n - scalar: teach 'clone' to support the --single-branch option\n - scalar: implement the `clone` subcommand\n - scalar: implement 'scalar list'\n - scalar: let 'unregister' handle a deleted enlistment directory gracefully\n - scalar: 'unregister' stops background maintenance\n - scalar: 'register' sets recommended config and starts maintenance\n - scalar: create test infrastructure\n - scalar: start documenting the command\n - scalar: create a rudimentary executable\n\n Add pieces from \"scalar\" to contrib/.\n\n Waiting for a response.\n cf. \n\n\n* ab/sanitize-leak-ci (2021-09-23) 2 commits\n (merged to 'next' on 2021-10-03 at dcd62a3fc6)\n + tests: add a test mode for SANITIZE=leak, run it in CI\n + Makefile: add SANITIZE=leak flag to GIT-BUILD-OPTIONS\n\n CI learns to run the leak sanitizer builds.\n\n Will merge to 'master'.\n\n\n* ms/customizable-ident-expansion (2021-09-01) 1 commit\n - keyword expansion: make \"$Id$\" string configurable\n\n Instead of \"$Id$\", user-specified string (like $FreeBSD$) can be\n used as an in-blob placeholder for keyword expansion.\n\n\n* js/retire-preserve-merges (2021-09-07) 11 commits\n (merged to 'next' on 2021-09-10 at f645ffd7a3)\n + sequencer: restrict scope of a formerly public function\n + rebase: remove a no-longer-used function\n + rebase: stop mentioning the -p option in comments\n + rebase: remove obsolete code comment\n + rebase: drop the internal `rebase--interactive` command\n + git-svn: drop support for `--preserve-merges`\n + rebase: drop support for `--preserve-merges`\n + pull: remove support for `--rebase=preserve`\n + tests: stop testing `git rebase --preserve-merges`\n + remote: warn about unhandled branch..rebase values\n + t5520: do not use `pull.rebase=preserve`\n\n The \"--preserve-merges\" option of \"git rebase\" has been removed.\n\n Will cook in 'next'.\n\n\n* en/remerge-diff (2021-08-31) 7 commits\n - doc/diff-options: explain the new --remerge-diff option\n - show, log: provide a --remerge-diff capability\n - tmp-objdir: new API for creating and removing primary object dirs\n - merge-ort: capture and print ll-merge warnings in our preferred fashion\n - ll-merge: add API for capturing warnings in a strbuf instead of stderr\n - merge-ort: add ability to record conflict messages in a file\n - merge-ort: mark a few more conflict messages as omittable\n\n A new presentation for two-parent merge \"--remerge-diff\" can be\n used to show the difference between mechanical (and possibly\n conflicted) merge results and the recorded resolution.\n\n Expecting a reroll.\n\n Handling of temporary object directory is worked out with the\n ns/batched-fsync topic.\n\n\n* sg/test-split-index-fix (2021-09-07) 7 commits\n (merged to 'next' on 2021-09-29 at 661ae75778)\n + read-cache: fix GIT_TEST_SPLIT_INDEX\n + tests: disable GIT_TEST_SPLIT_INDEX for sparse index tests\n + read-cache: look for shared index files next to the index, too\n + t1600-index: disable GIT_TEST_SPLIT_INDEX\n + t1600-index: don't run git commands upstream of a pipe\n + t1600-index: remove unnecessary redirection\n + Merge branch 'ds/sparse-index-ignored-files' into sg/test-split-index-fix\n\n Test updates.\n\n Will merge to 'master'.\n\n\n* ab/refs-errno-cleanup (2021-08-25) 4 commits\n - refs: make errno output explicit for refs_resolve_ref_unsafe\n - refs: explicitly return failure_errno from parse_loose_ref_contents\n - branch tests: test for errno propagating on failing read\n - refs: add failure_errno to refs_read_raw_ref() signature\n\n The \"remainder\" of hn/refs-errno-cleanup topic.\n\n What's the status of this one? Meh?\n\n\n* ab/lib-subtest (2021-09-22) 9 commits\n (merged to 'next' on 2021-10-06 at e8fa261811)\n + test-lib tests: get rid of copy/pasted mock test code\n + test-lib tests: assert 1 exit code, not non-zero\n + test-lib tests: refactor common part of check_sub_test_lib_test*()\n + test-lib tests: avoid subshell for \"test_cmp\" for readability\n + test-lib tests: don't provide a description for the sub-tests\n + test-lib tests: split up \"write and run\" into two functions\n + test-lib tests: move \"run_sub_test\" to a new lib-subtest.sh\n + Merge branch 'ps/t0000-output-directory-fix' into ab/lib-subtest\n + Merge branch 'jk/t0000-subtests-fix' into ab/lib-subtest\n\n Updates to the tests in t0000 to test the test framework.\n\n Will merge to 'master'.\n\n\n* ab/only-single-progress-at-once (2021-09-22) 8 commits\n - progress.c: add & assert a \"global_progress\" variable\n - pack-bitmap-write.c: add a missing stop_progress()\n - progress.c: add temporary variable from progress struct\n - progress.c: stop eagerly fflush(stderr) when not a terminal\n - progress.c: call progress_interval() from progress_test_force_update()\n - progress.c: move signal handler functions lower\n - progress.c tests: test some invalid usage\n - progress.c tests: make start/stop verbs on stdin\n\n Further tweaks on progress API.\n\n\n* fs/ssh-signing (2021-09-10) 9 commits\n - ssh signing: test that gpg fails for unknown keys\n - ssh signing: tests for logs, tags & push certs\n - ssh signing: duplicate t7510 tests for commits\n - ssh signing: verify signatures using ssh-keygen\n - ssh signing: provide a textual signing_key_id\n - ssh signing: retrieve a default key from ssh-agent\n - ssh signing: add ssh key format and signing code\n - ssh signing: add test prereqs\n - ssh signing: preliminary refactoring and clean-up\n\n Use ssh public crypto for object and push-cert signing.\n\n Will merge to 'next'.\n\n\n* cf/fetch-set-upstream-while-detached (2021-07-06) 1 commit\n - fetch: fix segfault on --set-upstream while on a detached HEAD\n\n \"git fetch --set-upstream\" while on detached HEAD segfaulted\n instead of noticing that such an operation did not make sense.\n\n Expecting a reroll.\n cf. \n\n\n* pw/diff-color-moved-fix (2021-08-05) 13 commits\n - diff: drop unused options parameter from cmp_in_block_with_wsd()\n - diff --color-moved: intern strings\n - diff: use designated initializers for emitted_diff_symbol\n - diff --color-moved-ws=allow-indentation-change: improve hash lookups\n - diff --color-moved: stop clearing potential moved blocks\n - diff --color-moved: shrink potential moved blocks as we go\n - diff --color-moved: unify moved block growth functions\n - diff --color-moved: call comparison function directly\n - diff --color-moved-ws=allow-indentation-change: simplify and optimize\n - diff: simplify allow-indentation-change delta calculation\n - diff --color-moved: avoid false short line matches and bad zerba coloring\n - diff --color-moved=zebra: fix alternate coloring\n - diff --color-moved: add perf tests\n\n Originally merged to 'next' on 2021-08-05\n\n Long-overdue correctness and performance update to \"diff\n --color-moved\" feature.\n\n Expecting a reroll.\n cf. <8bec1a6d-5052-50c3-4100-e6348289d581@gmail.com>\n\n\n* hn/reftable (2021-10-01) 24 commits\n - reftable: avoid non portable compile time pointer to function\n - config.mak.uname: last release and snapshots of Minix still use zlib 1.2.3\n - fixup! reftable: implement stack, a mutable database of reftable files.\n - fixup! reftable: add a heap-based priority queue for reftable records\n - squash! reftable: reading/writing blocks\n - Add \"test-tool dump-reftable\" command.\n - reftable: add dump utility\n - reftable: implement stack, a mutable database of reftable files.\n - reftable: implement refname validation\n - reftable: add merged table view\n - reftable: add a heap-based priority queue for reftable records\n - reftable: reftable file level tests\n - reftable: read reftable files\n - reftable: generic interface to tables\n - reftable: write reftable files\n - reftable: a generic binary tree implementation\n - reftable: reading/writing blocks\n - Provide zlib's uncompress2 from compat/zlib-compat.c\n - reftable: (de)serialization for the polymorphic record type.\n - reftable: add blocksource, an abstraction for random access reads\n - reftable: utility functions\n - reftable: add error related functionality\n - reftable: RFC: add LICENSE\n - hash.h: provide constants for the hash IDs\n\n The \"reftable\" backend for the refs API, without integrating into\n the refs subsystem.\n\n Expecting a reroll.\n\n--------------------------------------------------\n[Discarded]\n\n* ao/p4-avoid-decoding (2021-04-12) 2 commits\n . git-p4: do not decode data from perforce by default\n . git-p4: avoid decoding more data from perforce\n\n \"git p4\" in Python-2 days used to accept a lot more kinds of data\n from Perforce server as uninterrupted byte sequence, but after\n switching to Python-3, too many things are expected to be in UTF-8,\n which broke traditional use cases.\n\n Have been stalled for too long.\n cf. <20210504220153.1d9f0cb2@ado-tr>\n\n\n* tv/p4-fallback-encoding (2021-04-30) 1 commit\n . git-p4: git-p4.fallbackEncoding to specify non UTF-8 charset\n\n \"git p4\" learns the fallbackEncoding configuration variable to\n safely accept changeset descriptions that aren't written in UTF-8.\n\n Have been stalled for too long.\n cf. \n\n\n* jh/builtin-fsmonitor (2021-09-03) 37 commits\n . fixup! fsmonitor--daemon: implement handle_client callback\n . SQUASH??? https://github.com/git/git/runs/3438543601?check_suite_focus=true#step:5:136\n . BANDAID: sparse fixes\n . t7527: test FS event reporing on MacOS WRT case and Unicode\n . fsmonitor: handle shortname for .git\n . t7527: test status with untracked-cache and fsmonitor--daemon\n . fsmonitor: force update index after large responses\n . fsmonitor: enhance existing comments\n . fsmonitor--daemon: use a cookie file to sync with file system\n . fsmonitor--daemon: periodically truncate list of modified files\n . t7527: create test for fsmonitor--daemon\n . t/perf/p7519: add fsmonitor--daemon test cases\n . t/perf: avoid copying builtin fsmonitor files into test repo\n . t/perf/p7519: speed up test using \"test-tool touch\"\n . t/helper/test-touch: add helper to touch a series of files\n . fsmonitor--daemon: implement handle_client callback\n . fsmonitor-fs-listen-macos: implement FSEvent listener on MacOS\n . fsmonitor-fs-listen-macos: add macos header files for FSEvent\n . fsmonitor-fs-listen-win32: implement FSMonitor backend on Windows\n . fsmonitor--daemon: create token-based changed path cache\n . fsmonitor--daemon: define token-ids\n . fsmonitor--daemon: add pathname classification\n . fsmonitor: do not try to operate on bare repos\n . fsmonitor--daemon: implement 'start' command\n . fsmonitor--daemon: implement 'run' command\n . fsmonitor-fs-listen-macos: stub in backend for MacOS\n . fsmonitor-fs-listen-win32: stub in backend for Windows\n . t/helper/fsmonitor-client: create IPC client to talk to FSMonitor Daemon\n . fsmonitor--daemon: implement 'stop' and 'status' commands\n . fsmonitor--daemon: add a built-in fsmonitor daemon\n . fsmonitor: use IPC to query the builtin FSMonitor daemon\n . fsmonitor: config settings are repository-specific\n . help: include fsmonitor--daemon feature flag in version info\n . fsmonitor-ipc: create client routines for git-fsmonitor--daemon\n . fsmonitor--daemon: update fsmonitor documentation\n . fsmonitor--daemon: man page\n . simple-ipc: preparations for supporting binary messages.\n\n An attempt to write and ship with a watchman equivalent tailored\n for our use.\n\n Will be rerolled in pieces.\n\n\n* ab/config-based-hooks-base (2021-09-09) 36 commits\n . hooks: fix a TOCTOU in \"did we run a hook?\" heuristic\n . receive-pack: convert receive hooks to hook.h\n . post-update: use hook.h library\n . receive-pack: convert 'update' hook to hook.h\n . hooks: allow callers to capture output\n . run-command: allow capturing of collated output\n . reference-transaction: use hook.h to run hooks\n . hook tests: use a modern style for \"pre-push\" tests\n . hook tests: test for exact \"pre-push\" hook input\n . transport: convert pre-push hook to hook.h\n . hook: convert 'post-rewrite' hook in sequencer.c to hook.h\n . hook: provide stdin by string_list or callback\n . run-command: add stdin callback for parallelization\n . am: convert 'post-rewrite' hook to hook.h\n . hook: support passing stdin to hooks\n . run-command: allow stdin for run_processes_parallel\n . run-command: remove old run_hook_{le,ve}() hook API\n . receive-pack: convert push-to-checkout hook to hook.h\n . read-cache: convert post-index-change to use hook.h\n . commit: convert {pre-commit,prepare-commit-msg} hook to hook.h\n . git-p4: use 'git hook' to run hooks\n . send-email: use 'git hook run' for 'sendemail-validate'\n . git hook run: add an --ignore-missing flag\n . merge: convert post-merge to use hook.h\n . hooks: convert 'post-checkout' hook to hook library\n . am: convert applypatch to use hook.h\n . rebase: convert pre-rebase to use hook.h\n . gc: use hook library for pre-auto-gc hook\n . hook: add 'run' subcommand\n . hook-list.h: add a generated list of hooks, like config-list.h\n . hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n . hook.c: add a hook_exists() wrapper and use it in bugreport.c\n . hook.[ch]: move find_hook() from run-command.c to hook.c\n . Makefile: remove an out-of-date comment\n . Makefile: stop hardcoding {command,config}-list.h\n . Makefile: mark \"check\" target as .PHONY\n (this branch is used by es/config-based-hooks.)\n\n Restructuring of (a subset of) Emily's config-based-hooks series,\n to demonstrate that a series can be presented as a more logical and\n focused progression.\n\n Will be rerolled in pieces.\n\n\n* es/config-based-hooks (2021-09-09) 6 commits\n . hook: allow out-of-repo 'git hook' invocations\n . hook: include hooks from the config\n . hook: introduce \"git hook list\"\n . hook: allow parallel hook execution\n . fixup! hook: run a list of hooks instead\n . hook: run a list of hooks instead\n (this branch uses ab/config-based-hooks-base.)\n\n Revamp the hooks subsystem to allow multiple of them to trigger\n upon the same event and control via the configuration variables.\n\n\n* cb/make-compdb-fix (2021-09-22) 1 commit\n . Makefile: avoid breaking compilation database generation with DEVELOPER\n\n Adjust to recent change to use -pedantic for developer builds.\n\n Replaced by the ab/make-compdb-fix topic that uses the same\n approach as the ab/auto-depend-with-pedantic topic.\n\n\n* ab/pack-objects-stdin (2021-07-09) 5 commits\n . pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS\n . pack-objects.c: do stdin parsing via revision.c's API\n . revision.[ch]: add a \"handle_stdin_line\" API\n . revision.h: refactor \"disable_stdin\" and \"read_from_stdin\"\n . upload-pack: run is_repository_shallow() before setup_revisions()\n\n Introduce handle_stdin_line callback to revision API and uses it.\n\n Retracted for now.\n\n\n* ah/unleak-revisions (2021-09-20) 2 commits\n . log: UNLEAK original pending objects\n . log: UNLEAK rev to silence a large number of leaks\n\n Mark a few structures with UNLEAK() to help leak detection CI jobs.\n\n Retracted.\n cf. <05754f9c-cd58-30f5-e2d3-58b9221d2770@ahunt.org>\n\n\n* rs/p5311-use-test-file-size (2021-10-03) 1 commit\n . p5311: handle spaces in wc(1) output\n\n Test portability update.\n\n The tb/aggregate-ignore-leading-whitespaces supersedes this topic.\n"},{"id":"438109","messageId":"871r4xio37.fsf@evledraar.gmail.com","threadId":"56656","inReplyTo":"xmqqfstdr8b5.fsf@gitster.g","subject":"ab/make-sparse-for-real","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-07T02:01:41Z","receivedAt":"2021-10-07T02:13:42Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 06 2021, Junio C Hamano wrote:\n\n> * ab/make-sparse-for-real (2021-09-22) 1 commit\n> (merged to 'next' on 2021-10-06 at 10e3c31d6a)\n> + Makefile: make the \"sparse\" target non-.PHONY\n>\n> Prevent \"make sparse\" from running for the source files that\n> haven't been modified.\n>\n> Will merge to 'master'.\n\nI see you merged down the v2 of this[1], not the v3[2], thanks!\n\nI'm personally happier with the v2, but the v3 is just as useful for my\npurposes, and I understood that you/Ramsey had some flows/finger memory\ndepending on the semantics of the existing \"sparse\" target.\n\nIf it was a mistake I'm happy to submit a version of the v3 on top\n(since it's in \"next\" already), or if it was intended I'm happy to leave\nit be. Just checking.\n\n1. https://lore.kernel.org/git/patch-v2-1.1-059829f2195-20210923T000654Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/patch-v3-1.1-b6ba99ca4cc-20210928T011319Z-avarab@gmail.com/\n"},{"id":"438110","messageId":"YV5aaD418SyZqS/1@coredump.intra.peff.net","threadId":"56656","inReplyTo":"xmqqfstdr8b5.fsf@gitster.g","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-07T02:24:40Z","receivedAt":"2021-10-07T02:24:43Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:\n\n> * tb/repack-write-midx (2021-10-01) 9 commits\n> (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n> + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n> + builtin/repack.c: make largest pack preferred\n> + builtin/repack.c: support writing a MIDX while repacking\n> + builtin/repack.c: extract showing progress to a variable\n> + builtin/repack.c: rename variables that deal with non-kept packs\n> + builtin/repack.c: keep track of existing packs unconditionally\n> + midx: preliminary support for `--refs-snapshot`\n> + builtin/multi-pack-index.c: support `--stdin-packs` mode\n> + midx: expose `write_midx_file_only()` publicly\n> \n> \"git repack\" has been taught to generate multi-pack reachability\n> bitmaps.\n> \n> Will merge to 'master'.\n\nSorry not to catch this before it hit 'next', but there's a small leak\nin the test helper. This patch can go on top to fix it.\n\n-- >8 --\nSubject: [PATCH] test-read-midx: fix leak of bitmap_index struct\n\nIn read_midx_preferred_pack(), we open the bitmap index but never free\nit. This isn't a big deal since this is just a test helper, and we exit\nimmediately after, but since we're trying to keep our leak-checking tidy\nnow, it's worth fixing.\n\nSigned-off-by: Jeff King \n---\n t/helper/test-read-midx.c | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\nindex 0038559129..9d6fa7a377 100644\n--- a/t/helper/test-read-midx.c\n+++ b/t/helper/test-read-midx.c\n@@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)\n \t\treturn 1;\n \n \tbitmap = prepare_bitmap_git(the_repository);\n-\tif (!(bitmap && bitmap_is_midx(bitmap)))\n+\tif (!bitmap)\n \t\treturn 1;\n-\n+\tif (!bitmap_is_midx(bitmap)) {\n+\t\tfree_bitmap_index(bitmap);\n+\t\treturn 1;\n+\t}\n \n \tprintf(\"%s\\n\", midx->pack_names[midx_preferred_pack(bitmap)]);\n+\tfree_bitmap_index(bitmap);\n \treturn 0;\n }\n \n-- \n2.33.0.1340.gfe2cb2531f\n\n"},{"id":"438111","messageId":"YV5dmkkuCqAY2qqG@coredump.intra.peff.net","threadId":"56656","inReplyTo":"YV5aaD418SyZqS/1@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-07T02:38:18Z","receivedAt":"2021-10-07T02:38:30Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:\n\n> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:\n> \n> > * tb/repack-write-midx (2021-10-01) 9 commits\n> > (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n> > + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n> > + builtin/repack.c: make largest pack preferred\n> > + builtin/repack.c: support writing a MIDX while repacking\n> > + builtin/repack.c: extract showing progress to a variable\n> > + builtin/repack.c: rename variables that deal with non-kept packs\n> > + builtin/repack.c: keep track of existing packs unconditionally\n> > + midx: preliminary support for `--refs-snapshot`\n> > + builtin/multi-pack-index.c: support `--stdin-packs` mode\n> > + midx: expose `write_midx_file_only()` publicly\n> > \n> > \"git repack\" has been taught to generate multi-pack reachability\n> > bitmaps.\n> > \n> > Will merge to 'master'.\n> \n> Sorry not to catch this before it hit 'next', but there's a small leak\n> in the test helper. This patch can go on top to fix it.\n\nThe reason for that is that I didn't find it by inspection; I've started\nrunning my personal builds through coverity. It wasn't too bad to set up\nwith a GitHub Action, like so:\n\n---\n .github/workflows/coverity.yml | 35 ++++++++++++++++++++++++++++++++++\n 1 file changed, 35 insertions(+)\n create mode 100644 .github/workflows/coverity.yml\n\ndiff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml\nnew file mode 100644\nindex 0000000000..bfd4dff275\n--- /dev/null\n+++ b/.github/workflows/coverity.yml\n@@ -0,0 +1,35 @@\n+name: coverity-scan\n+on: push\n+\n+jobs:\n+ latest:\n+ runs-on: ubuntu-latest\n+ steps:\n+ - uses: actions/checkout@v2\n+ - run: ci/install-dependencies.sh\n+ - name: Download Coverity Build Tool\n+ run: |\n+ wget -q https://scan.coverity.com/download/linux64 --post-data \"token=$TOKEN&project=peff/git\" -O cov-analysis-linux64.tar.gz\n+ mkdir cov-analysis-linux64\n+ tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64\n+ env:\n+ TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}\n+\n+ - name: Build with cov-build\n+ run: |\n+ export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH\n+ cov-build --dir cov-int make\n+\n+ - name: Submit the result to Coverity Scan\n+ run: |\n+ tar czvf git.tgz cov-int\n+ curl \\\n+ --form project=peff/git \\\n+ --form token=$TOKEN \\\n+ --form email=peff@peff.net \\\n+ --form file=@git.tgz \\\n+ --form version=$(git rev-parse HEAD) \\\n+ --form description=\"$(./git version)\" \\\n+ https://scan.coverity.com/builds?project=peff/git\n+ env:\n+ TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}\n\nIs there interest in having something like this in the main repo? We'd\nneed to tweak some values:\n\n - we have to send the project name (here peff/git); we can presumably\n get this on the fly from the Actions environment\n\n - any repo which wants to use this has to set up the secret token\n (COVERITY_SCAN_TOKEN here). That involves creating a coverity\n account, and then setting the token in the GitHub web interface.\n Presumably we'd just bail immediately if that token isn't set, so\n forks aside from git/git would have to enable it independently.\n\n - likewise it needs the email address for the coverity account. That\n could probably be set in the environment, too.\n\n - we'd probably want to only run it for integration branches, since\n coverity sets some limits on how often it runs. This could probably\n be set in another environment variable, so people could tweak it for\n their forks if they wanted to (or we could use the ci-config hacks,\n but I put those together mostly because these environment variables\n didn't exist back then; I suspect we could switch off of them now).\n\nThere are tons of existing warnings, many of which are false positives.\nBut it keeps track of which problems are new, and emails out a summary\nof only the new ones (which is how I saw the leak here, which just hit\nnext). I don't care all that much about leaks here (we have other\ntechniques for finding them), but when Stefan used to do regular\ncoverity builds in the past, it routinely found useful errors.\n\nIf we had it running on git/git, it's possible for people to subscribe\nto those notifications (or view them on the site; both require the\npeople to have coverity accounts, but they're free).\n\nThoughts?\n\n-Peff\n"},{"id":"438113","messageId":"87wnmph73b.fsf@evledraar.gmail.com","threadId":"56656","inReplyTo":"YV5aaD418SyZqS/1@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-07T02:57:10Z","receivedAt":"2021-10-07T03:00:48Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 06 2021, Jeff King wrote:\n\n> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:\n>\n>> * tb/repack-write-midx (2021-10-01) 9 commits\n>> (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n>> + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n>> + builtin/repack.c: make largest pack preferred\n>> + builtin/repack.c: support writing a MIDX while repacking\n>> + builtin/repack.c: extract showing progress to a variable\n>> + builtin/repack.c: rename variables that deal with non-kept packs\n>> + builtin/repack.c: keep track of existing packs unconditionally\n>> + midx: preliminary support for `--refs-snapshot`\n>> + builtin/multi-pack-index.c: support `--stdin-packs` mode\n>> + midx: expose `write_midx_file_only()` publicly\n>> \n>> \"git repack\" has been taught to generate multi-pack reachability\n>> bitmaps.\n>> \n>> Will merge to 'master'.\n>\n> Sorry not to catch this before it hit 'next', but there's a small leak\n> in the test helper. This patch can go on top to fix it.\n>\n> -- >8 --\n> Subject: [PATCH] test-read-midx: fix leak of bitmap_index struct\n>\n> In read_midx_preferred_pack(), we open the bitmap index but never free\n> it. This isn't a big deal since this is just a test helper, and we exit\n> immediately after, but since we're trying to keep our leak-checking tidy\n> now, it's worth fixing.\n>\n> Signed-off-by: Jeff King \n> ---\n> t/helper/test-read-midx.c | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\n> index 0038559129..9d6fa7a377 100644\n> --- a/t/helper/test-read-midx.c\n> +++ b/t/helper/test-read-midx.c\n> @@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)\n> \t\treturn 1;\n> \n> \tbitmap = prepare_bitmap_git(the_repository);\n> -\tif (!(bitmap && bitmap_is_midx(bitmap)))\n> +\tif (!bitmap)\n> \t\treturn 1;\n> -\n> +\tif (!bitmap_is_midx(bitmap)) {\n> +\t\tfree_bitmap_index(bitmap);\n> +\t\treturn 1;\n> +\t}\n> \n> \tprintf(\"%s\\n\", midx->pack_names[midx_preferred_pack(bitmap)]);\n> +\tfree_bitmap_index(bitmap);\n> \treturn 0;\n> }\n\nThanks, I think it's no big deal, those tests seem to leak a lot\nalready. Here's a patch that might be generally applicable and makes a\nfew more of its tests pass.\n\nThe s/free/free_chunkfile/g seems like a good bug fix, and that \"m =\nNULL\" pattern seems odd, don't we always want to free in that scenario?\nIt passes all tests...\n\nThis brings t5319-multi-pack-index.sh down from 64 to 57 failures in\n\"next\", I didn't try in combination with your patch.\n\ndiff --git a/midx.c b/midx.c\nindex 7e06e859756..d24b1e6a9d4 100644\n--- a/midx.c\n+++ b/midx.c\n@@ -179,12 +179,13 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local\n \ttrace2_data_intmax(\"midx\", the_repository, \"load/num_packs\", m->num_packs);\n \ttrace2_data_intmax(\"midx\", the_repository, \"load/num_objects\", m->num_objects);\n \n+\tfree_chunkfile(cf);\n \treturn m;\n \n cleanup_fail:\n \tfree(m);\n \tfree(midx_name);\n-\tfree(cf);\n+\tfree_chunkfile(cf);\n \tif (midx_map)\n \t\tmunmap(midx_map, midx_size);\n \tif (0 <= fd)\n@@ -1602,7 +1603,7 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\t * Remaining tests assume that we have objects, so we can\n \t\t * return here.\n \t\t */\n-\t\treturn verify_midx_error;\n+\t\tgoto cleanup;\n \t}\n \n \tif (flags & MIDX_PROGRESS)\n@@ -1679,8 +1680,9 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag\n \t\tmidx_display_sparse_progress(progress, i + 1);\n \t}\n \tstop_progress(&progress);\n-\n+cleanup:\n \tfree(pairs);\n+\tfree(m);\n \n \treturn verify_midx_error;\n }\n@@ -1927,11 +1929,9 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n \t}\n \n \tresult = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);\n-\tm = NULL;\n \n cleanup:\n-\tif (m)\n-\t\tclose_midx(m);\n+\tclose_midx(m);\n \tfree(include_pack);\n \treturn result;\n }\n"},{"id":"438118","messageId":"YV5voCafN8dLff/R@nand.local","threadId":"56656","inReplyTo":"YV5aaD418SyZqS/1@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-07T03:55:12Z","receivedAt":"2021-10-07T03:55:17Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:\n> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:\n>\n> > * tb/repack-write-midx (2021-10-01) 9 commits\n> > (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n> > + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n> > + builtin/repack.c: make largest pack preferred\n> > + builtin/repack.c: support writing a MIDX while repacking\n> > + builtin/repack.c: extract showing progress to a variable\n> > + builtin/repack.c: rename variables that deal with non-kept packs\n> > + builtin/repack.c: keep track of existing packs unconditionally\n> > + midx: preliminary support for `--refs-snapshot`\n> > + builtin/multi-pack-index.c: support `--stdin-packs` mode\n> > + midx: expose `write_midx_file_only()` publicly\n> >\n> > \"git repack\" has been taught to generate multi-pack reachability\n> > bitmaps.\n> >\n> > Will merge to 'master'.\n>\n> Sorry not to catch this before it hit 'next', but there's a small leak\n> in the test helper. This patch can go on top to fix it.\n\nThanks for taking a look and catching this. The fix you wrote looks\ngood to me. For what it's worth:\n\n Acked-by: Taylor Blau \n\nThanks,\nTaylor\n"},{"id":"438119","messageId":"YV5yi+AejPGO9qOi@nand.local","threadId":"56656","inReplyTo":"YV5dmkkuCqAY2qqG@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-07T04:07:39Z","receivedAt":"2021-10-07T04:07:44Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:\n> The reason for that is that I didn't find it by inspection; I've started\n> running my personal builds through coverity. It wasn't too bad to set up\n> with a GitHub Action, like so:\n>\n> [...]\n\nIt looks like this would never cause the build to fail, but is merely\nresponsible for sending any warnings off to Coverity's UI?\n\n> Is there interest in having something like this in the main repo? We'd\n> need to tweak some values:\n>\n> - we have to send the project name (here peff/git); we can presumably\n> get this on the fly from the Actions environment\n\nYes; searching through [1] it looks like that is called $GITHUB_REPOSITORY.\n\n[1]: https://docs.github.com/en/actions/learn-github-actions/environment-variables\n\n> - any repo which wants to use this has to set up the secret token\n> (COVERITY_SCAN_TOKEN here). That involves creating a coverity\n> account, and then setting the token in the GitHub web interface.\n> Presumably we'd just bail immediately if that token isn't set, so\n> forks aside from git/git would have to enable it independently.\n>\n> - likewise it needs the email address for the coverity account. That\n> could probably be set in the environment, too.\n\nThese both seem reasonable to me, too.\n\n> There are tons of existing warnings, many of which are false positives.\n> But it keeps track of which problems are new, and emails out a summary\n> of only the new ones (which is how I saw the leak here, which just hit\n> next). I don't care all that much about leaks here (we have other\n> techniques for finding them), but when Stefan used to do regular\n> coverity builds in the past, it routinely found useful errors.\n\nI'm generally pessimistic about tools like Coverity, but I share your\nexperience that Coverity warnings are actually pretty high quality. Or\nat least they have a high enough signal-to-noise ratio that it makes\nthem worth looking through.\n\nSo I would be happy to have forks of GitHub have fewer barriers to use\nthis tool.\n\nThanks,\nTaylor\n"},{"id":"438120","messageId":"YV50cEN6hdvNOrpE@nand.local","threadId":"56656","inReplyTo":"87wnmph73b.fsf@evledraar.gmail.com","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-07T04:15:44Z","receivedAt":"2021-10-07T04:21:47Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Oct 07, 2021 at 04:57:10AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> @@ -1927,11 +1929,9 @@ int midx_repack(struct repository *r, const char *object_dir, size_t batch_size,\n> \t}\n>\n> \tresult = write_midx_internal(object_dir, NULL, NULL, NULL, NULL, flags);\n> -\tm = NULL;\n\nThis was the only hunk that gave me a little bit of pause. But it is the\nright thing to be doing.\n\nWhen this code was originally written, write_midx_internal took a\npointer to a struct multi_pack_index, and sometimes called close_midx()\non that pointer. Calling close_midx() on the same pointer is UB, since\nclose_midx() frees the provided pointer.\n\nIn other words, when this code was originally written, the 'm' variable\nin midx_repack() could become stale.\n\nBut since f57a739691 (midx: avoid opening multiple MIDXs when writing,\n2021-09-01), we don't pass a pointer into write_midx_internal(), only\nthe path to an object directory. So we can always call close_midx()\nhere.\n\nThanks,\nTaylor\n"},{"id":"438126","messageId":"87sfxdgtpu.fsf@evledraar.gmail.com","threadId":"56656","inReplyTo":"YV5dmkkuCqAY2qqG@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-10-07T07:42:11Z","receivedAt":"2021-10-07T07:49:45Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Oct 06 2021, Jeff King wrote:\n\n> On Wed, Oct 06, 2021 at 10:24:40PM -0400, Jeff King wrote:\n>\n>> On Wed, Oct 06, 2021 at 05:24:14PM -0700, Junio C Hamano wrote:\n>> \n>> > * tb/repack-write-midx (2021-10-01) 9 commits\n>> > (merged to 'next' on 2021-10-06 at ccdd5aaf2a)\n>> > + builtin/repack.c: pass `--refs-snapshot` when writing bitmaps\n>> > + builtin/repack.c: make largest pack preferred\n>> > + builtin/repack.c: support writing a MIDX while repacking\n>> > + builtin/repack.c: extract showing progress to a variable\n>> > + builtin/repack.c: rename variables that deal with non-kept packs\n>> > + builtin/repack.c: keep track of existing packs unconditionally\n>> > + midx: preliminary support for `--refs-snapshot`\n>> > + builtin/multi-pack-index.c: support `--stdin-packs` mode\n>> > + midx: expose `write_midx_file_only()` publicly\n>> > \n>> > \"git repack\" has been taught to generate multi-pack reachability\n>> > bitmaps.\n>> > \n>> > Will merge to 'master'.\n>> \n>> Sorry not to catch this before it hit 'next', but there's a small leak\n>> in the test helper. This patch can go on top to fix it.\n>\n> The reason for that is that I didn't find it by inspection; I've started\n> running my personal builds through coverity. It wasn't too bad to set up\n> with a GitHub Action, like so:\n>\n> ---\n> .github/workflows/coverity.yml | 35 ++++++++++++++++++++++++++++++++++\n> 1 file changed, 35 insertions(+)\n> create mode 100644 .github/workflows/coverity.yml\n>\n> diff --git a/.github/workflows/coverity.yml b/.github/workflows/coverity.yml\n> new file mode 100644\n> index 0000000000..bfd4dff275\n> --- /dev/null\n> +++ b/.github/workflows/coverity.yml\n> @@ -0,0 +1,35 @@\n> +name: coverity-scan\n> +on: push\n> +\n> +jobs:\n> + latest:\n> + runs-on: ubuntu-latest\n> + steps:\n> + - uses: actions/checkout@v2\n> + - run: ci/install-dependencies.sh\n> + - name: Download Coverity Build Tool\n> + run: |\n> + wget -q https://scan.coverity.com/download/linux64 --post-data \"token=$TOKEN&project=peff/git\" -O cov-analysis-linux64.tar.gz\n\nInteresting mix...\n\n> + mkdir cov-analysis-linux64\n> + tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64\n> + env:\n> + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}\n> +\n> + - name: Build with cov-build\n> + run: |\n> + export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH\n> + cov-build --dir cov-int make\n> +\n> + - name: Submit the result to Coverity Scan\n> + run: |\n> + tar czvf git.tgz cov-int\n> + curl \\\n\n...of curl & wget :)\n\n> + --form project=peff/git \\\n> + --form token=$TOKEN \\\n> + --form email=peff@peff.net \\\n> + --form file=@git.tgz \\\n> + --form version=$(git rev-parse HEAD) \\\n> + --form description=\"$(./git version)\" \\\n> + https://scan.coverity.com/builds?project=peff/git\n> + env:\n> + TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}\n>\n> Is there interest in having something like this in the main repo? We'd\n> need to tweak some values:\n\nI'm very interested in it, it would be great to have more CI targets,\neven if optional.\n\n> - we have to send the project name (here peff/git); we can presumably\n> get this on the fly from the Actions envir\n\nOur $CI_REPO_SLUG I believe. See\nhttps://docs.github.com/en/actions/learn-github-actions/environment-variables#default-environment-variables\n& a grep for CI_REPO_SLUG in git.git.\n\n> - any repo which wants to use this has to set up the secret token\n> (COVERITY_SCAN_TOKEN here). That involves creating a coverity\n> account, and then setting the token in the GitHub web interface.\n> Presumably we'd just bail immediately if that token isn't set, so\n> forks aside from git/git would have to enable it independently.\n\nI tried creating one of these now, requested access at\nhttps://scan.coverity.com/projects/git & it's pending.\n\nMaybe I should have clicked the \"connect with GitHub\" at the beginning,\nbut it wanted (ro) ACL access to all organizations I was in, including\nprivate boards or something. So I went for the generic sign-up.\n\n> - likewise it needs the email address for the coverity account. That\n> could probably be set in the environment, too.\n>\n> - we'd probably want to only run it for integration branches, since\n> coverity sets some limits on how often it runs. This could probably\n> be set in another environment variable, so people could tweak it for\n> their forks if they wanted to (or we could use the ci-config hacks,\n> but I put those together mostly because these environment variables\n> didn't exist back then; I suspect we could switch off of them now).\n>\n> There are tons of existing warnings, many of which are false positives.\n> But it keeps track of which problems are new, and emails out a summary\n> of only the new ones (which is how I saw the leak here, which just hit\n> next). I don't care all that much about leaks here (we have other\n> techniques for finding them), but when Stefan used to do regular\n> coverity builds in the past, it routinely found useful errors.\n>\n> If we had it running on git/git, it's possible for people to subscribe\n> to those notifications (or view them on the site; both require the\n> people to have coverity accounts, but they're free).\n>\n> Thoughts?\n\nSounds good, I wonder if they (if contacted) provide upon request some\ncommunity-wide keys for projects such as git, so it would Just Work for\nforks without their owners needing to sign up themselves...\n"},{"id":"438185","messageId":"xmqq7deor9v8.fsf@gitster.g","threadId":"56656","inReplyTo":"YV5aaD418SyZqS/1@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-07T18:02:51Z","receivedAt":"2021-10-07T18:02:58Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> In read_midx_preferred_pack(), we open the bitmap index but never free\n> it. This isn't a big deal since this is just a test helper, and we exit\n> immediately after, but since we're trying to keep our leak-checking tidy\n> now, it's worth fixing.\n\nThanks. That sounds sensible.\n\n>\n> Signed-off-by: Jeff King \n> ---\n> t/helper/test-read-midx.c | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n>\n> diff --git a/t/helper/test-read-midx.c b/t/helper/test-read-midx.c\n> index 0038559129..9d6fa7a377 100644\n> --- a/t/helper/test-read-midx.c\n> +++ b/t/helper/test-read-midx.c\n> @@ -85,11 +85,15 @@ static int read_midx_preferred_pack(const char *object_dir)\n> \t\treturn 1;\n> \n> \tbitmap = prepare_bitmap_git(the_repository);\n> -\tif (!(bitmap && bitmap_is_midx(bitmap)))\n> +\tif (!bitmap)\n> \t\treturn 1;\n> -\n> +\tif (!bitmap_is_midx(bitmap)) {\n> +\t\tfree_bitmap_index(bitmap);\n> +\t\treturn 1;\n> +\t}\n> \n> \tprintf(\"%s\\n\", midx->pack_names[midx_preferred_pack(bitmap)]);\n> +\tfree_bitmap_index(bitmap);\n> \treturn 0;\n> }\n"},{"id":"438260","messageId":"YV/BMkZrj4xQyvUL@coredump.intra.peff.net","threadId":"56656","inReplyTo":"YV5yi+AejPGO9qOi@nand.local","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-08T03:55:30Z","receivedAt":"2021-10-08T03:55:35Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 07, 2021 at 12:07:39AM -0400, Taylor Blau wrote:\n\n> On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:\n> > The reason for that is that I didn't find it by inspection; I've started\n> > running my personal builds through coverity. It wasn't too bad to set up\n> > with a GitHub Action, like so:\n> >\n> > [...]\n> \n> It looks like this would never cause the build to fail, but is merely\n> responsible for sending any warnings off to Coverity's UI?\n\nSort of. They basically wrap the \"make\" invocation to intercept \"cc\". My\nunderstanding is that their faux-compiler is mostly about gathering data\nabout the code. That gets stuffed into a tarball and uploaded to their\nservers, where the real analysis happens.\n\nIt's very black-box, which I don't love. But in my experience it\nproduces by far the most useful static-analysis output of any tool I've\nseen.\n\n> > There are tons of existing warnings, many of which are false positives.\n> > But it keeps track of which problems are new, and emails out a summary\n> > of only the new ones (which is how I saw the leak here, which just hit\n> > next). I don't care all that much about leaks here (we have other\n> > techniques for finding them), but when Stefan used to do regular\n> > coverity builds in the past, it routinely found useful errors.\n> \n> I'm generally pessimistic about tools like Coverity, but I share your\n> experience that Coverity warnings are actually pretty high quality. Or\n> at least they have a high enough signal-to-noise ratio that it makes\n> them worth looking through.\n> \n> So I would be happy to have forks of GitHub have fewer barriers to use\n> this tool.\n\nOK. I'll see if I can clean up the patch a bit.\n\n-Peff\n"},{"id":"438265","messageId":"YV/EnxkTvWWZ6xVD@coredump.intra.peff.net","threadId":"56656","inReplyTo":"87sfxdgtpu.fsf@evledraar.gmail.com","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-08T04:10:07Z","receivedAt":"2021-10-08T04:10:10Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 07, 2021 at 09:42:11AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Interesting mix...\n> [...]\n> ...of curl & wget :)\n\nHeh, yeah, I noticed that, too. This is literally cut and paste from\ncoverity's \"here's how to automate a scan\" instructions, which yes, use\nboth tools. I don't think it's a big deal, but we could pretty easily\nswap the wget invocation for curl.\n\n> > - any repo which wants to use this has to set up the secret token\n> > (COVERITY_SCAN_TOKEN here). That involves creating a coverity\n> > account, and then setting the token in the GitHub web interface.\n> > Presumably we'd just bail immediately if that token isn't set, so\n> > forks aside from git/git would have to enable it independently.\n> \n> I tried creating one of these now, requested access at\n> https://scan.coverity.com/projects/git & it's pending.\n\nSo that isn't actually my project. ;)\n\nThat's the old dead one that Stefan used to do builds for. Now I do have\naccess to that and saw your request, but it's not actually receiving new\nbuilds.\n\nI probably _could_ upload to that one, of course, but the builds I'm\ndoing are of my private topics.\n\nMy thinking was that if Junio is OK with it, we'd have this in the main\ntree and the git/git ones would be the main builds.\n\n> Sounds good, I wonder if they (if contacted) provide upon request some\n> community-wide keys for projects such as git, so it would Just Work for\n> forks without their owners needing to sign up themselves...\n\nI get the impression that it's a fairly hefty CPU cost for something\nthey're offering for free, and they really don't want everybody's random\nfork doing analyses. I.e., they'd rather see open source projects set up\nan analysis project for their official tree and that's it.\n\nOTOH, another concern of theirs is that the results aren't disclosed\npublicly, since they may have security implications. But they've made it\neasy enough for people to submit their random GitHub repositories, which\ncan contain anybody's code, so it doesn't seem like much of a secret.\n\nSo I dunno. They're nice enough to offer the service for free, and I\nwant to respect their wishes. But I'm having a hard-time finding\ndocuments describing exactly what's OK and what's next with respect to\nforks.\n\nThey do have some limits posted here:\n\n https://scan.coverity.com/faq#frequency\n\nIt's on the order of 3 builds per day for a code base of our size. Which\nis plenty for our integration branches, but not enough to test every\ntopic branch. And though we could get around that with treating forks as\nseparate projects, I think that's violating the spirit of the limit.\n\n-Peff\n"},{"id":"438279","messageId":"nycvar.QRO.7.76.6.2110080946060.395@tvgsbejvaqbjf.bet","threadId":"56656","inReplyTo":"YV/BMkZrj4xQyvUL@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-08T07:51:33Z","receivedAt":"2021-10-08T07:51:49Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff & Taylor,\n\nOn Thu, 7 Oct 2021, Jeff King wrote:\n\n> On Thu, Oct 07, 2021 at 12:07:39AM -0400, Taylor Blau wrote:\n>\n> > On Wed, Oct 06, 2021 at 10:38:18PM -0400, Jeff King wrote:\n> > > The reason for that is that I didn't find it by inspection; I've started\n> > > running my personal builds through coverity. It wasn't too bad to set up\n> > > with a GitHub Action, like so:\n> > >\n> > > [...]\n> >\n> > It looks like this would never cause the build to fail, but is merely\n> > responsible for sending any warnings off to Coverity's UI?\n>\n> Sort of. They basically wrap the \"make\" invocation to intercept \"cc\". My\n> understanding is that their faux-compiler is mostly about gathering data\n> about the code. That gets stuffed into a tarball and uploaded to their\n> servers, where the real analysis happens.\n>\n> It's very black-box, which I don't love. But in my experience it\n> produces by far the most useful static-analysis output of any tool I've\n> seen.\n\nIt is pretty black box, but I have to disagree that the static analysis\noutput is very useful. The majority are false positives about\nstrbuf/strvec type usage of a static, fixed-size array that is dynamically\nreplaced by a dynamically-allocated array. Coverity misses that subtlety\nand reports out-of-bounds accesses.\n\nGranted, I worked around those (I thought) by using the\n`-DFLEX_ARRAY=65536` trick, but I guess that is either not working as\ndesigned, or it stopped working at some stage.\n\nFWIW I have set up an Azure Pipeline to keep Git for Windows' `main`\nbranch covered by Coverity:\n\nhttps://dev.azure.com/git-for-windows/git/_build?definitionId=35\n\nIt essentially calls into this scripted code:\nhttps://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n\nCiao,\nDscho\n"},{"id":"438336","messageId":"xmqqsfxbjncz.fsf@gitster.g","threadId":"56656","inReplyTo":"YV/EnxkTvWWZ6xVD@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-08T20:03:08Z","receivedAt":"2021-10-08T20:03:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> They do have some limits posted here:\n>\n> https://scan.coverity.com/faq#frequency\n>\n> It's on the order of 3 builds per day for a code base of our size. Which\n> is plenty for our integration branches, but not enough to test every\n> topic branch.\n\nI usually have at least two pushout of 'seen' (one with the full set\nof 'seen' including known-to-be-broken topic integrations, the other\nwith seen~$some_hopefully_small_number that I didn't see brekaage in\nmy local build), and then on graduation days 'next' and 'master' are\nalso updated, so 3 is cutting very close ;-)\n"},{"id":"438343","messageId":"YWCn4tpr9RWsRxvr@coredump.intra.peff.net","threadId":"56656","inReplyTo":"xmqqsfxbjncz.fsf@gitster.g","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-08T20:19:46Z","receivedAt":"2021-10-08T20:19:48Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 08, 2021 at 01:03:08PM -0700, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > They do have some limits posted here:\n> >\n> > https://scan.coverity.com/faq#frequency\n> >\n> > It's on the order of 3 builds per day for a code base of our size. Which\n> > is plenty for our integration branches, but not enough to test every\n> > topic branch.\n> \n> I usually have at least two pushout of 'seen' (one with the full set\n> of 'seen' including known-to-be-broken topic integrations, the other\n> with seen~$some_hopefully_small_number that I didn't see brekaage in\n> my local build), and then on graduation days 'next' and 'master' are\n> also updated, so 3 is cutting very close ;-)\n\nHmm, yeah. They say \"21 builds per week\", which would be plenty (you\ndon't push out integrations every day), but I think they may also have a\nper-day limit.\n\nI'm not sure what happens when you hit the limit. If it just silently\nskips the analysis, that's fine (we'll pick up the changes the next\nday). If it causes a CI failure that nags you, that is less good. But\nprobably something we could work around in the Actions commands.\n\nTBH, though, I think just building \"seen\" would be sufficient for this\nkind of analysis. Most CI is about \"make sure we pass the tests and do\nnot graduate topics if it doesn't\". This is much more about \"generate a\nset of possibly-bogus annotations for some human to look at\". Given the\ntopic branch flow you use, it's pretty unlikely for a problem to\n_appear_ in master or next. And even if they do, it will be detected the\nnext time seen is run.\n\n-Peff\n"},{"id":"438361","messageId":"YWC49+xCh+zum8Ms@coredump.intra.peff.net","threadId":"56656","inReplyTo":"nycvar.QRO.7.76.6.2110080946060.395@tvgsbejvaqbjf.bet","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-08T21:32:39Z","receivedAt":"2021-10-08T21:32:42Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:\n\n> > Sort of. They basically wrap the \"make\" invocation to intercept \"cc\". My\n> > understanding is that their faux-compiler is mostly about gathering data\n> > about the code. That gets stuffed into a tarball and uploaded to their\n> > servers, where the real analysis happens.\n> >\n> > It's very black-box, which I don't love. But in my experience it\n> > produces by far the most useful static-analysis output of any tool I've\n> > seen.\n> \n> It is pretty black box, but I have to disagree that the static analysis\n> output is very useful. The majority are false positives about\n> strbuf/strvec type usage of a static, fixed-size array that is dynamically\n> replaced by a dynamically-allocated array. Coverity misses that subtlety\n> and reports out-of-bounds accesses.\n\nYes, I remember skipping past quite a few of those.\n\nTo be clear, I don't claim that its output is amazing. Only that it has\nproduced actionable output on many occasions. Grepping commit messages\nfor \"Coverity\" turns up several hits (many from you :) ). Most of those\nare leak fixes, and I do think we have better options there. But I\nrecall it detecting some hard-to-find memory and logic errors, too.\n\n> Granted, I worked around those (I thought) by using the\n> `-DFLEX_ARRAY=65536` trick, but I guess that is either not working as\n> designed, or it stopped working at some stage.\n> \n> FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`\n> branch covered by Coverity:\n> \n> https://dev.azure.com/git-for-windows/git/_build?definitionId=35\n> \n> It essentially calls into this scripted code:\n> https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n\nDo you have any objection to adding something like the Action I showed\neariler? It would do nothing in git-for-windows/git unless you set up\nthe right environment, so there shouldn't be any downside.\n\nI admit I was not really planning to try to suppress the false positives\nas you've done here; my plan was to just keep an eye on the \"new\"\nentries (having already gone through the existing ones years ago).\n\n-Peff\n"},{"id":"438369","messageId":"xmqqpmsfi3i7.fsf@gitster.g","threadId":"56656","inReplyTo":"YWCn4tpr9RWsRxvr@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-08T21:57:20Z","receivedAt":"2021-10-08T21:57:24Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Fri, Oct 08, 2021 at 01:03:08PM -0700, Junio C Hamano wrote:\n>\n>> I usually have at least two pushout of 'seen' (one with the full set\n>> of 'seen' including known-to-be-broken topic integrations, the other\n>> with seen~$some_hopefully_small_number that I didn't see brekaage in\n>> my local build), and then on graduation days 'next' and 'master' are\n>> also updated, so 3 is cutting very close ;-)\n>\n> Hmm, yeah. They say \"21 builds per week\", which would be plenty (you\n> don't push out integrations every day),\n\n'seen' is pushed out every day (not on weekends), though.\n\n> I'm not sure what happens when you hit the limit. If it just silently\n> skips the analysis, that's fine (we'll pick up the changes the next\n> day). If it causes a CI failure that nags you, that is less good. But\n> probably something we could work around in the Actions commands.\n\nYup.\n"},{"id":"439094","messageId":"nycvar.QRO.7.76.6.2110201416390.56@tvgsbejvaqbjf.bet","threadId":"56656","inReplyTo":"YWC49+xCh+zum8Ms@coredump.intra.peff.net","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-10-20T12:27:30Z","receivedAt":"2021-10-20T12:27:42Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Fri, 8 Oct 2021, Jeff King wrote:\n\n> On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:\n>\n> > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`\n> > branch covered by Coverity:\n> >\n> > https://dev.azure.com/git-for-windows/git/_build?definitionId=35\n> >\n> > It essentially calls into this scripted code:\n> > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n>\n> Do you have any objection to adding something like the Action I showed\n> eariler? It would do nothing in git-for-windows/git unless you set up\n> the right environment, so there shouldn't be any downside.\n\nNo objection. I'd just ask to use `${{github.repository}}` instead of\nhard-coding `peff/git`, and to really not run the workflow unless\nconfigured. So something like this:\n\nname: coverity-scan\non:\n push:\n - master\n - next\n - seen\n\njobs:\n coverity:\n runs-on: ubuntu-latest\n env:\n COVERITY_SCAN_TOKEN: ${{ secrets.COVERITY_SCAN_TOKEN }}\n COVERITY_SCAN_EMAIL: ${{ secrets.COVERITY_SCAN_EMAIL }}\n if: env.COVERITY_SCAN_TOKEN != '' && env.COVERITY_SCAN_EMAIL != ''\n steps:\n - uses: actions/checkout@v2\n - run: ci/install-dependencies.sh\n - name: Download Coverity Build Tool\n run: |\n wget -q https://scan.coverity.com/download/linux64 --post-data \"token=$COVERITY_SCAN_TOKEN&project=$GITHUB_REPOSITORY\" -O cov-analysis-linux64.tar.gz\n mkdir cov-analysis-linux64\n tar xzf cov-analysis-linux64.tar.gz --strip 1 -C cov-analysis-linux64\n - name: Build with cov-build\n run: |\n export PATH=$(pwd)/cov-analysis-linux64/bin:$PATH\n cov-build --dir cov-int make\n - name: Submit the result to Coverity Scan\n run: |\n tar czvf git.tgz cov-int\n curl \\\n --form project=$GITHUB_REPOSITORY \\\n --form token=$COVERITY_SCAN_TOKEN \\\n --form email=$COVERITY_SCAN_EMAIL \\\n --form file=@git.tgz \\\n --form version=$(git rev-parse HEAD) \\\n --form description=\"$(./git version)\" \\\n https://scan.coverity.com/builds?project=$GITHUB_REPOSITORY\n\nNote the `jobs.coverity.if` attribute. This is what will let the entire\njob be skipped unless the secrets are set up.\n\nI am very much in favor of having this in git/git. Do you want to provide\nthe commit message, or do you want me to shepher this?\n\n> I admit I was not really planning to try to suppress the false positives\n> as you've done here; my plan was to just keep an eye on the \"new\"\n> entries (having already gone through the existing ones years ago).\n\nI think we will _have_ to suppress the false positives at some point, as\nsomething like 9 out of 10 new reports I receive are about these, and it\ntakes time to analyze & dismiss them. In general, I have no trouble\nfinding more fun things to do with my time.\n\nCiao,\nDscho\n"},{"id":"439108","messageId":"YXAn8d/Pwz/4ro5c@nand.local","threadId":"56656","inReplyTo":"nycvar.QRO.7.76.6.2110201416390.56@tvgsbejvaqbjf.bet","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2021-10-20T14:30:09Z","receivedAt":"2021-10-20T14:30:28Z","isPatch":false,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:\n> Hi Peff,\n>\n> On Fri, 8 Oct 2021, Jeff King wrote:\n>\n> > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:\n> >\n> > > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`\n> > > branch covered by Coverity:\n> > >\n> > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35\n> > >\n> > > It essentially calls into this scripted code:\n> > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n> >\n> > Do you have any objection to adding something like the Action I showed\n> > eariler? It would do nothing in git-for-windows/git unless you set up\n> > the right environment, so there shouldn't be any downside.\n>\n> No objection. I'd just ask to use `${{github.repository}}` instead of\n> hard-coding `peff/git`, and to really not run the workflow unless\n> configured. So something like this:\n>\n> [...]\n>\n> I am very much in favor of having this in git/git. Do you want to provide\n> the commit message, or do you want me to shepher this?\n\nThis all looks good to me. FWIW, I was planning on picking this up\nmyself, but I'm happy to have been beaten to it ;). The only thing\nmissing is having a project for git.git set up on Coverity's website.\n\nI'd be happy to look into that if you wanted to polish this up and\nsubmit it. We should be able to proceed independently, since this will\nall be a noop without setting the various Coverity secrets inside of\ngit/git.\n\nThanks,\nTaylor\n"},{"id":"439113","messageId":"xmqqh7dbrbvr.fsf@gitster.g","threadId":"56656","inReplyTo":"nycvar.QRO.7.76.6.2110201416390.56@tvgsbejvaqbjf.bet","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-10-20T14:47:52Z","receivedAt":"2021-10-20T14:47:57Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> No objection. I'd just ask to use `${{github.repository}}` instead of\n> hard-coding `peff/git`, and to really not run the workflow unless\n> configured. So something like this:\n> ...\n> Note the `jobs.coverity.if` attribute. This is what will let the entire\n> job be skipped unless the secrets are set up.\n\nIt would be nice if \"the entire job be skipped\" did not even make it\nappear in the https://github.com/git/git/actions list.\n\nI am assuming that your illustration is using the same trick as\ngit-l10n jobs, so the result will appear on the page in the same way\nas they appear, i.e. with white circle with a 45-degree bar in it?\n\nAs long as we do not give outside parties anything stronger than a\nread-only access, I have no objection to the plan, either.\n\nThanks.\n"},{"id":"439116","messageId":"YXBALFsf305U8mJs@coredump.intra.peff.net","threadId":"56656","inReplyTo":"nycvar.QRO.7.76.6.2110201416390.56@tvgsbejvaqbjf.bet","subject":"Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-10-20T16:13:32Z","receivedAt":"2021-10-20T16:13:36Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:\n\n> Hi Peff,\n> \n> On Fri, 8 Oct 2021, Jeff King wrote:\n> \n> > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:\n> >\n> > > FWIW I have set up an Azure Pipeline to keep Git for Windows' `main`\n> > > branch covered by Coverity:\n> > >\n> > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35\n> > >\n> > > It essentially calls into this scripted code:\n> > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n> >\n> > Do you have any objection to adding something like the Action I showed\n> > eariler? It would do nothing in git-for-windows/git unless you set up\n> > the right environment, so there shouldn't be any downside.\n> \n> No objection. I'd just ask to use `${{github.repository}}` instead of\n> hard-coding `peff/git`, and to really not run the workflow unless\n> configured. So something like this:\n\nYep, those were directions I was planning to take it.\n\n> I am very much in favor of having this in git/git. Do you want to provide\n> the commit message, or do you want me to shepher this?\n\nI'd be just as happy if you did (I hadn't even looked at it since my\nearlier email).\n\nIt sounds like Taylor is volunteering to set up the Coverity side for\ngit.git, and I can help him with getting those COVERITY_* variables into\nthe GitHub environment.\n\n> > I admit I was not really planning to try to suppress the false positives\n> > as you've done here; my plan was to just keep an eye on the \"new\"\n> > entries (having already gone through the existing ones years ago).\n> \n> I think we will _have_ to suppress the false positives at some point, as\n> something like 9 out of 10 new reports I receive are about these, and it\n> takes time to analyze & dismiss them. In general, I have no trouble\n> finding more fun things to do with my time.\n\nThe volume is low enough that I've been OK just manually skipping past\nthem. But if we have mitigations to make that happen automatically, I'm\nall in favor. We can do that incrementally on top, but the fact that\nyou've already figured out most of it means it may make sense to just do\nit from the start (and is another good reason for you to submit the\npatches).\n\nSorry, I didn't mean to make more work/patches for you. :)\n\n-Peff\n"},{"id":"461274","messageId":"2n33pn8n-7529-443q-rq7n-q12nrqrn8p27@tzk.qr","threadId":"56656","inReplyTo":"YXBALFsf305U8mJs@coredump.intra.peff.net","subject":"Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-16T09:05:48Z","receivedAt":"2022-08-16T11:11:13Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff & Taylor,\n\nOn Wed, 20 Oct 2021, Jeff King wrote:\n\n> On Wed, Oct 20, 2021 at 02:27:30PM +0200, Johannes Schindelin wrote:\n>\n> > On Fri, 8 Oct 2021, Jeff King wrote:\n> >\n> > > On Fri, Oct 08, 2021 at 09:51:33AM +0200, Johannes Schindelin wrote:\n> > >\n> > > > FWIW I have set up an Azure Pipeline to keep Git for Windows'\n> > > > `main` branch covered by Coverity:\n> > > >\n> > > > https://dev.azure.com/git-for-windows/git/_build?definitionId=35\n> > > >\n> > > > It essentially calls into this scripted code:\n> > > > https://github.com/git-for-windows/build-extra/blob/4676f286a1ec830a5038b32400808a353dc6c48d/please.sh#L1820-L1915\n> > >\n> > > Do you have any objection to adding something like the Action I\n> > > showed eariler? It would do nothing in git-for-windows/git unless\n> > > you set up the right environment, so there shouldn't be any\n> > > downside.\n> >\n> > No objection. I'd just ask to use `${{github.repository}}` instead of\n> > hard-coding `peff/git`, and to really not run the workflow unless\n> > configured. So something like this:\n>\n> Yep, those were directions I was planning to take it.\n>\n> > I am very much in favor of having this in git/git. Do you want to provide\n> > the commit message, or do you want me to shepher this?\n>\n> I'd be just as happy if you did (I hadn't even looked at it since my\n> earlier email).\n>\n> It sounds like Taylor is volunteering to set up the Coverity side for\n> git.git, and I can help him with getting those COVERITY_* variables into\n> the GitHub environment.\n\nGiven the challenges with Coverity (false positives, lack of support on\nSynopsys' side, severely limited access to the reports), and given the\nrenewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am\nin favor of abandoning the idea to integrate Coverity in our GitHub\nworkflow.\n\nRegarding CodeQL, I am still uncertain what level of integration we will\nend up with, and the contacts I am working with are currently all on\nvacation, but I am confident that we will have an easier time going\nforward with static analysis using CodeQL instead of Coverity.\n\nCiao,\nDscho\n"},{"id":"461329","messageId":"Yvw87DJOOu+/jG6o@coredump.intra.peff.net","threadId":"56656","inReplyTo":"2n33pn8n-7529-443q-rq7n-q12nrqrn8p27@tzk.qr","subject":"Re: Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-08-17T00:57:16Z","receivedAt":"2022-08-17T00:57:22Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Aug 16, 2022 at 11:05:48AM +0200, Johannes Schindelin wrote:\n\n> > It sounds like Taylor is volunteering to set up the Coverity side for\n> > git.git, and I can help him with getting those COVERITY_* variables into\n> > the GitHub environment.\n> \n> Given the challenges with Coverity (false positives, lack of support on\n> Synopsys' side, severely limited access to the reports), and given the\n> renewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am\n> in favor of abandoning the idea to integrate Coverity in our GitHub\n> workflow.\n> \n> Regarding CodeQL, I am still uncertain what level of integration we will\n> end up with, and the contacts I am working with are currently all on\n> vacation, but I am confident that we will have an easier time going\n> forward with static analysis using CodeQL instead of Coverity.\n\nOK. I haven't been that impressed with CodeQL for C so far, but it may\nbe getting better. I certainly would be happier with a system that made\nit easier to display and share reports.\n\nCoverity does have a lot of false positives, but I've at least been able\nto pick useful fixes out of them (especially because it is good about\nsaying \"here are 5 new things to look at\"). I've been continuing to\nbuild my private branch with it, so we'll see if it turns up anything\nuseful. I do agree that inflicting it on ordinary users may be\ncounter-productive (I often have to stare really hard to understand why\nits false positives are false, and that is not something I would wish\non, say, a GGG user).\n\n-Peff\n"},{"id":"461544","messageId":"3896n74p-0r16-866o-r668-70q6pos078n9@tzk.qr","threadId":"56656","inReplyTo":"Yvw87DJOOu+/jG6o@coredump.intra.peff.net","subject":"Re: Coverity, was Re: What's cooking in git.git (Oct 2021, #02; Wed, 6)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-08-19T11:22:56Z","receivedAt":"2022-08-19T11:23:25Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff,\n\nOn Tue, 16 Aug 2022, Jeff King wrote:\n\n> On Tue, Aug 16, 2022 at 11:05:48AM +0200, Johannes Schindelin wrote:\n>\n> > > It sounds like Taylor is volunteering to set up the Coverity side for\n> > > git.git, and I can help him with getting those COVERITY_* variables into\n> > > the GitHub environment.\n> >\n> > Given the challenges with Coverity (false positives, lack of support on\n> > Synopsys' side, severely limited access to the reports), and given the\n> > renewed efforts by OSTIF that focus not on Coverity but on CodeQL, I am\n> > in favor of abandoning the idea to integrate Coverity in our GitHub\n> > workflow.\n> >\n> > Regarding CodeQL, I am still uncertain what level of integration we will\n> > end up with, and the contacts I am working with are currently all on\n> > vacation, but I am confident that we will have an easier time going\n> > forward with static analysis using CodeQL instead of Coverity.\n>\n> OK. I haven't been that impressed with CodeQL for C so far, but it may\n> be getting better.\n\nIf your lack of being impressed stems from CodeQL's default suite catching\nvery, very few issues, I agree with you.\n\nThe reason why I am more hopeful about CodeQL than about Coverity is that\nI now have a contact to work with (although they are currently on\nvacation, so I get to work on other things for now, including `merge-ort`\nand trying to integrate `mimalloc` in Git for Windows). And that is one\nmore contact who is willing to work with me than I ever had on the\nCoverity side of things.\n\nAnd apparently CodeQL's default settings optimize for reducing false\npositives in an attempt to avoid scaring potential users away.\n\nBut I have credible assurances that CodeQL has many more checks in store\nthat simply need to be enabled in order to catch way more issues, at the\nexpense of risking more false positives.\n\n> I certainly would be happier with a system that made it easier to\n> display and share reports.\n>\n> Coverity does have a lot of false positives, but I've at least been able\n> to pick useful fixes out of them (especially because it is good about\n> saying \"here are 5 new things to look at\"). I've been continuing to\n> build my private branch with it, so we'll see if it turns up anything\n> useful. I do agree that inflicting it on ordinary users may be\n> counter-productive (I often have to stare really hard to understand why\n> its false positives are false, and that is not something I would wish\n> on, say, a GGG user).\n\nDon't get me wrong, I do not plan on dropping the Coverity builds of the\nGit for Windows project's `main` branch at\nhttps://dev.azure.com/git-for-windows/git/_build?definitionId=35.\n\nAs you probably recall, I specifically looked through the Coverity report\nin advance of v2.37.0 and tried to address some of the issues in\nhttps://lore.kernel.org/git/pull.1264.git.1655336146.gitgitgadget@gmail.com/,\nwith mixed results (mainly because of some time constraints on my side,\ncombined with your willingness to help fix some issues with my patches, as\nwell as René's, Taylor's and Junio's excellent assistance).\n\nHowever, the sheer amount of false positives, with intentional issues\nbeing a close second (in test helpers, we are not all that strict about\nreleasing memory, for example), makes it a tough sell to ask more than\njust a few very dedicated contributors to have a look at the reports.\n\nWith CodeQL, I am optimistic that we can get it to a point where the\nburden can be carried by a larger group of people, with the help of\nenthusiastic CodeQL experts, which also means that the reports have a\nbigger chance at making Git safer.\n\nCiao,\nDscho\n"}]}