What's cooking in git.git (May 2022, #07; Wed, 25)

What's cooking in git.git (May 2022, #07; Wed, 25)

[Junio C Hamano]

Here are the topics that have been cooking in my tree.  Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a
future release).  Commits prefixed with '-' are only in 'seen',
and aren't considered "accepted" at all.

This cycle will conclude in early July (https://tinyurl.com/gitCal);
we are in the week #5 of the cycle.

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

With maint, master, next, seen, todo:

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

With all the integration branches and topics broken out:

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

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

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

Release tarballs are available at:

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

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

* ab/commit-plug-leaks (2022-05-12) 1 commit
  (merged to 'next' on 2022-05-16 at 00bcda44af)
 + commit: fix "author_ident" leak

 Leakfix in the top-level called-once function.
 source: <xmqqzgjmcqlg.fsf@gitster.g>


* ab/valgrind-fixes (2022-05-12) 4 commits
  (merged to 'next' on 2022-05-16 at 75d760528f)
 + commit-graph.c: don't assume that stat() succeeds
 + object-file: fix a unpack_loose_header() regression in 3b6a8db3b03
 + log test: skip a failing mkstemp() test under valgrind
 + tests: using custom GIT_EXEC_PATH breaks --valgrind tests
 (this branch is used by ds/object-file-unpack-loose-header-fix.)

 A bit of test framework fixes with a few fixes to issues found by
 valgrind.
 source: <20220512223218.237544-1-gitster@pobox.com>


* ep/coverage-report-wants-test-to-have-run (2022-04-13) 1 commit
  (merged to 'next' on 2022-05-18 at e4a51b0867)
 + Makefile: add a prerequisite to the coverage-report target

 "make coverage-report" without first running "make coverage" did
 not produce any meaningful result, which has been corrected.
 source: <20220414022513.31465-1-gitter.spiros@gmail.com>


* jc/archive-add-file-normalize-mode (2022-05-12) 1 commit
  (merged to 'next' on 2022-05-16 at 265bb02f2a)
 + archive: do not let on-disk mode leak to zip archives

 "git archive --add-file=<path>" picked up the raw permission bits
 from the path and propagated to zip output in some cases, without
 normalization, which has been corrected (tar output did not have
 this issue).
 source: <xmqqmtfme8v6.fsf@gitster.g>


* jc/avoid-redundant-submodule-fetch (2022-05-18) 1 commit
  (merged to 'next' on 2022-05-19 at 11dabe678b)
 + fetch: do not run a redundant fetch from submodule

 "git fetch --recurse-submodules" from multiple remotes (either from
 a remote group, or "--all") used to make one extra "git fetch" in
 the submodules, which has been corrected.
 source: <xmqqk0alyqyj.fsf_-_@gitster.g>


* jc/show-branch-g-current (2022-04-21) 1 commit
  (merged to 'next' on 2022-05-18 at 870a1a1a71)
 + show-branch: -g and --current are incompatible

 The "--current" option of "git show-branch" should have been made
 incompatible with the "--reflog" mode, but this was not enforced,
 which has been corrected.
 source: <xmqqh76mf7s4.fsf_-_@gitster.g>


* jt/fetch-peek-optional-section (2022-05-16) 1 commit
  (merged to 'next' on 2022-05-18 at bc876bd2cf)
 + fetch-pack: make unexpected peek result non-fatal

 "git fetch" unnecessarily failed when an unexpected optional
 section appeared in the output, which has been corrected.
 source: <20220516110221.3490982-1-jonathantanmy@google.com>


* os/fetch-check-not-current-branch (2022-05-16) 1 commit
  (merged to 'next' on 2022-05-19 at f48dca6322)
 + fetch: limit shared symref check only for local branches

 The way "git fetch" without "--update-head-ok" ensures that HEAD in
 no worktree points at any ref being updated was too wasteful, which
 has been optimized a bit.
 source: <pull.1266.v2.git.git.1652690501963.gitgitgadget@gmail.com>


* pb/ggg-in-mfc-doc (2022-05-12) 5 commits
  (merged to 'next' on 2022-05-19 at e1c148492f)
 + MyFirstContribution: drop PR description for GGG single-patch contributions
 + MyFirstContribution: reference "The cover letter" in GitGitGadget section
 + MyFirstContribution: reference "The cover letter" in "Preparing Email"
 + MyFirstContribution: add standalone section on cover letter
 + MyFirstContribution: add "Anatomy of a Patch Series" section

 Documentation update.
 source: <pull.1226.v3.git.1652399017.gitgitgadget@gmail.com>


* tb/receive-pack-code-cleanup (2022-05-18) 1 commit
  (merged to 'next' on 2022-05-19 at 756d64f5a2)
 + builtin/receive-pack.c: remove redundant 'if'

 Code clean-up.
 source: <d22f2ca975778d594449857d64be8cd8c0d4a327.1652905549.git.me@ttaylorr.com>

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

* ds/bundle-uri-more (2022-05-20) 24 commits
 - t5601: basic bundle URI tests
 - clone: unbundle the advertised bundles
 - bundle-uri: download bundles from an advertised list
 - bundle-uri: allow relative URLs in bundle lists
 - bundle-uri client: add boolean transfer.bundleURI setting
 - bundle-uri: serve URI advertisement from bundle.* config
 - bundle-uri client: add "git ls-remote-bundle-uri"
 - bundle-uri client: add minimal NOOP client
 - protocol v2: add server-side "bundle-uri" skeleton
 - bundle-uri: fetch a list of bundles
 - bundle-uri: parse bundle list in config format
 - bundle-uri: limit recursion depth for bundle lists
 - bundle-uri: unit test "key=value" parsing
 - bundle-uri: create "key=value" line parsing
 - bundle-uri: create base key-value pair parsing
 - bundle-uri: create bundle_list struct and helpers
 - clone: --bundle-uri cannot be combined with --depth
 - clone: add --bundle-uri option
 - fetch: add 'refs/bundle/' to log.excludeDecoration
 - fetch: add --bundle-uri option
 - bundle-uri: add support for http(s):// and file://
 - bundle-uri: create basic file-copy logic
 - remote-curl: add 'get' capability
 - docs: document bundle URI standard
 (this branch uses ds/bundle-uri.)

 source: <pull.1234.git.1653072042.gitgitgadget@gmail.com>
 source: <pull.1233.git.1652731865.gitgitgadget@gmail.com>


* jc/revert-show-parent-info (2022-05-23) 1 commit
 - revert: optionally refer to commit in the "reference" format

 source: <xmqqsfp2b30k.fsf@gitster.g>


* jc/http-clear-finished-pointer (2022-05-24) 1 commit
 - http.c: clear the 'finished' member once we are done with it

 Meant to go with js/ci-gcc-12-fixes

 Will merge to 'next'?
 source: <xmqqczgqjr8y.fsf_-_@gitster.g>


* js/ci-gcc-12-fixes (2022-05-24) 3 commits
 - dir.c: avoid "exceeds maximum object size" error with GCC v12.x
 - nedmalloc: avoid new compile error
 - compat/win32/syslog: fix use-after-realloc

 Fixes real problems noticed by gcc 12 and works around false
 positives.

 Will merge to 'next'?
 source: <pull.1238.git.1653351786.gitgitgadget@gmail.com>


* kl/setup-in-unreadable-worktree (2022-05-24) 1 commit
 - setup: don't die if realpath(3) fails on getcwd(3)

 Disable the "do not remove the directory the user started Git in"
 logic when Git cannot tell where that directory is.  Earlier we
 refused to run in such a case.

 Will merge to 'next'.
 source: <8b20840014d214023c50ee62439147f798e6f9cc.1653419993.git.kevin@kevinlocke.name>


* pb/use-freebsd-12.3-in-cirrus-ci (2022-05-25) 1 commit
 - ci: update Cirrus-CI image to FreeBSD 12.3

 Update the version of FreeBSD image used in Cirrus CI.

 Will merge to 'next'.
 source: <20220525125112.86954-1-levraiphilippeblain@gmail.com>


* yw/cmake-updates (2022-05-24) 3 commits
 - cmake: remove (_)UNICODE def on Windows in CMakeLists.txt
 - cmake: add pcre2 support
 - cmake: fix CMakeLists.txt on Linux

 CMake updates.

 Will merge to 'next'?
 source: <pull.1267.v2.git.git.1653374328.gitgitgadget@gmail.com>


* sg/build-gitweb (2022-05-25) 1 commit
 - Makefile: build 'gitweb' in the default target

 "make all" should but didn't build "gitweb".

 Will merge to 'next'.
 source: <20220525205651.825669-1-szeder.dev@gmail.com>

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

* en/merge-tree (2022-02-23) 13 commits
 - git-merge-tree.txt: add a section on potentional usage mistakes
 - merge-tree: add a --allow-unrelated-histories flag
 - merge-tree: allow `ls-files -u` style info to be NUL terminated
 - merge-tree: provide easy access to `ls-files -u` style info
 - merge-tree: provide a list of which files have conflicts
 - merge-ort: provide a merge_get_conflicted_files() helper function
 - merge-tree: support including merge messages in output
 - merge-ort: split out a separate display_update_messages() function
 - merge-tree: implement real merges
 - merge-tree: add option parsing and initial shell for real merge function
 - merge-tree: move logic for existing merge into new function
 - merge-tree: rename merge_trees() to trivial_merge_trees()
 - Merge branch 'en/remerge-diff' into en/merge-trees

 A new command is introduced that takes two commits and computes a
 tree that would be contained in the resulting merge commit, if the
 histories leading to these two commits were to be merged, and is
 added as a new mode of "git merge-tree" subcommand.

 On hold.
 cf. <CABPp-BGZ7OAYRR5YKRsxJSo-C=ho+qcNAkqwkim8CkhCfCeHsA@mail.gmail.com>
 source: <pull.1122.v6.git.1645602413.gitgitgadget@gmail.com>


* ab/ci-github-workflow-markup (2022-05-22) 12 commits
 . fixup! ci: make it easier to find failed tests' logs in the GitHub workflow
 . ci: call `finalize_test_case_output` a little later
 . ci: use `--github-workflow-markup` in the GitHub workflow
 . ci: optionally mark up output in the GitHub workflow
 . test(junit): avoid line feeds in XML attributes
 . tests: refactor --write-junit-xml code
 . ci: make it easier to find failed tests' logs in the GitHub workflow
 . CI: stop setting FAILED_TEST_ARTIFACTS N times
 . CI: don't include "test-results/" in ci/print-test-failures.sh output
 . CI: add --exit-code to ci/print-test-failures.sh
 . CI: don't "cd" in ci/print-test-failures.sh
 . Merge branch 'ab/ci-setup-simplify' into ab/ci-github-workflow-markup
 (this branch uses ab/ci-setup-simplify.)

 Build a moral equivalent of js/ci-github-workflow-markup on top of
 ab/ci-setup-simplify.
 source: <RFC-cover-v5-00.10-00000000000-20220421T183001Z-avarab@gmail.com>


* ab/ci-setup-simplify (2022-04-21) 29 commits
 . CI: make it easy to use ci/*.sh outside of CI
 . CI: don't use "set -x" in "ci/lib.sh" output
 . CI: set PYTHON_PATH setting for osx-{clang,gcc} into "$jobname" case
 . CI: set SANITIZE=leak in MAKEFLAGS directly
 . CI: set CC in MAKEFLAGS directly, don't add it to the environment
 . CI: add more variables to MAKEFLAGS, except under vs-build
 . CI: narrow down variable definitions in --build and --test
 . CI: only invoke ci/lib.sh as "steps" in main.yml
 . CI: pre-select test slice in Windows & VS tests
 . ci/run-test-slice.sh: replace shelling out with "echo"
 . CI: move "env" definitions into ci/lib.sh
 . CI: combine ci/install{,-docker}-dependencies.sh
 . CI: split up and reduce "ci/test-documentation.sh"
 . CI: invoke "make artifacts-tar" directly in windows-build
 . CI: check ignored unignored build artifacts in "win[+VS] build" too
 . CI: make ci/{lib,install-dependencies}.sh POSIX-compatible
 . CI: remove "run-build-and-tests.sh", run "make [test]" directly
 . CI: export variables via a wrapper
 . CI: consistently use "export" in ci/lib.sh
 . CI: move p4 and git-lfs variables to ci/install-dependencies.sh
 . CI: have "static-analysis" run "check-builtins", not "documentation"
 . CI: have "static-analysis" run a "make ci-static-analysis" target
 . CI: don't have "git grep" invoke a pager in tree content check
 . CI/lib.sh: stop adding leading whitespace to $MAKEFLAGS
 . CI: remove unused Azure ci/* code
 . CI: remove dead "tree skipping" code
 . CI: remove more dead Travis CI support
 . CI: make "$jobname" explicit, remove fallback
 . CI: run "set -ex" early in ci/lib.sh
 (this branch is used by ab/ci-github-workflow-markup.)

 Drive more actions done in CI via the Makefile instead of shell
 commands sprinkled in .github/workflows/main.yml
 source: <cover-v5-00.29-00000000000-20220421T181526Z-avarab@gmail.com>


* et/xdiff-indirection (2022-02-17) 1 commit
 - xdiff: provide indirection to git functions

 Insert a layer of preprocessor macros for common functions in xdiff
 codebase.

 Will discard, as it has been stalled for way too long.
 cf. <xmqqbkyudb8n.fsf@gitster.g>
 source: <20220217225408.GB7@edef91d97c94>


* bc/stash-export (2022-04-08) 4 commits
 - builtin/stash: provide a way to import stashes from a ref
 - builtin/stash: provide a way to export stashes to a ref
 - builtin/stash: factor out revision parsing into a function
 - object-name: make get_oid quietly return an error

 A mechanism to export and import stash entries to and from a normal
 commit to transfer it across repositories has been introduced.

 Expecting a reroll.
 cf. <YnL2d4Vr9Vr7W4Hj@camp.crustytoothpaste.net>
 source: <20220407215352.3491567-1-sandals@crustytoothpaste.net>


* js/wait-or-whine-can-fail (2022-04-28) 1 commit
 - run-command: don't spam trace2_child_exit()

 We used to log an error return from wait_or_whine() as process
 termination of the waited child, which was incorrect.

 Needs clarifying "in rare cases".
 source: <4616d09ffa632bd2c9e308a713c4bdf2a1328c3c.1651179450.git.steadmon@google.com>


* dl/prompt-pick-fix (2022-03-25) 1 commit
 . git-prompt: fix sequencer/todo detection

 Fix shell prompt script (in contrib/) for those who set
 rebase.abbreviateCommands; we failed to recognize that we were in a
 multi-step cherry-pick session.

 Will discard, as it has been stalled for way too long.
 cf. <xmqqwngdzque.fsf@gitster.g>
 source: <20220325145301.3370-1-danny0838@gmail.com>


* es/superproject-aware-submodules (2022-03-09) 3 commits
 . rev-parse: short-circuit superproject worktree when config unset
 . introduce submodule.hasSuperproject record
 . t7400-submodule-basic: modernize inspect() helper

 A configuration variable in a repository tells if it is (or is not)
 a submodule of a superproject.

 Will discard, as it has been stalled for way too long.
 cf. <kl6l4k45s7cb.fsf@chooglen-macbookpro.roam.corp.google.com>
 source: <20220310004423.2627181-1-emilyshaffer@google.com>


* cw/remote-object-info (2022-05-06) 11 commits
 - SQUASH??? coccicheck
 - SQUASH??? ensure that coccicheck is happy
 - SQUASH??? compilation fix
 - cat-file: add --batch-command remote-object-info command
 - cat-file: move parse_cmd and DEFAULT_FORMAT up
 - transport: add object-info fallback to fetch
 - transport: add client side capability to request object-info
 - object-info: send attribute packet regardless of object ids
 - object-store: add function to free object_info contents
 - fetch-pack: move fetch default settings
 - fetch-pack: refactor packet writing

 A client component to talk with the object-info endpoint.

 Expecting a reroll.
 source: <20220502170904.2770649-1-calvinwan@google.com>

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

* js/ci-github-workflow-markup (2022-05-21) 12 commits
 - ci: call `finalize_test_case_output` a little later
 - ci(github): mention where the full logs can be found
 - ci: use `--github-workflow-markup` in the GitHub workflow
 - ci(github): avoid printing test case preamble twice
 - ci(github): skip the logs of the successful test cases
 - ci: optionally mark up output in the GitHub workflow
 - ci/run-build-and-tests: add some structure to the GitHub workflow output
 - ci: make it easier to find failed tests' logs in the GitHub workflow
 - ci/run-build-and-tests: take a more high-level view
 - test(junit): avoid line feeds in XML attributes
 - tests: refactor --write-junit-xml code
 - ci: fix code style

 Update the GitHub workflow support to make it quicker to get to the
 failing test.

 Will merge to 'next'?
 source: <pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>


* js/bisect-in-c (2022-05-21) 15 commits
 - bisect: no longer try to clean up left-over `.git/head-name` files
 - bisect: remove Cogito-related code
 - Turn `git bisect` into a full built-in
 - bisect: teach the `bisect--helper` command to show the correct usage strings
 - bisect: move even the command-line parsing to `bisect--helper`
 - bisect--helper: return only correct exit codes in `cmd_*()`
 - bisect--helper: move the `BISECT_STATE` case to the end
 - bisect--helper: make `--bisect-state` optional
 - bisect--helper: align the sub-command order with git-bisect.sh
 - bisect--helper: using `--bisect-state` without an argument is a bug
 - bisect--helper: really retire `--bisect-autostart`
 - bisect--helper: really retire --bisect-next-check
 - bisect--helper: retire the --no-log option
 - bisect: avoid double-quoting when printing the failed command
 - bisect run: fix the error message

 Final bits of "git bisect.sh" have been rewritten in C.

 Will merge to 'next'?
 source: <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>


* cb/path-owner-check-with-sudo-plus (2022-05-12) 1 commit
 - git-compat-util: allow root to access both SUDO_UID and root owned
 (this branch uses cb/path-owner-check-with-sudo.)

 "sudo git foo" used to consider a repository owned by the original
 user a safe one to access; it now also considers a repository owned
 by root a safe one, too (after all, if an attacker can craft a
 malicious repository owned by root, the box is 0wned already).
 source: <20220513010020.55361-1-carenas@gmail.com>


* jc/t6424-failing-merge-preserve-local-changes (2022-05-19) 1 commit
  (merged to 'next' on 2022-05-23 at 849cf6f24c)
 + t6424: make sure a failed merge preserves local changes

 The tests that ensured merges stop when interfering local changes
 are present did not make sure that local changes are preserved; now
 they do.

 Will merge to 'master'.
 source: <xmqqbkvtnyae.fsf@gitster.g>


* tb/geom-repack-with-keep-and-max (2022-05-20) 3 commits
 - builtin/repack.c: ensure that `names` is sorted
 - t7703: demonstrate object corruption with pack.packSizeLimit
 - repack: respect --keep-pack with geometric repack

 source: <cover.1653073280.git.me@ttaylorr.com>


* ab/hooks-regression-fix (2022-05-18) 8 commits
 - hook API: fix v2.36.0 regression: hooks should be connected to a TTY
 - hook API: don't redundantly re-set "no_stdin" and "stdout_to_stderr"
 - hook tests: fix redirection logic error in 96e7225b310
 - run-command: add an "ungroup" option to run_process_parallel()
 - run-command.c: add an initializer for "struct parallel_processes"
 - run-command tests: test stdout of run_command_parallel()
 - run-command API: use "opts" struct for run_processes_parallel{,_tr2}()
 - run-command tests: change if/if/... to if/else if/else

 In Git 2.36 we revamped the way how hooks are invoked.  One change
 that is end-user visible is that the output of a hook is no longer
 directly connected to the standard output of "git" that spawns the
 hook, which was noticed post release.  This is getting corrected.
 source: <cover-v2-0.8-00000000000-20220518T195858Z-avarab@gmail.com>


* tb/midx-race-in-pack-objects (2022-05-24) 4 commits
 - builtin/pack-objects.c: ensure pack validity from MIDX bitmap objects
 - builtin/pack-objects.c: ensure included `--stdin-packs` exist
 - builtin/pack-objects.c: avoid redundant NULL check
 - pack-bitmap.c: check preferred pack validity when opening MIDX bitmap

 The multi-pack-index code did not protect the packfile it is going
 to depend on from getting removed while in use, which has been
 corrected.

 Will merge to 'next'?
 source: <cover.1653418457.git.me@ttaylorr.com>


* ds/bundle-uri (2022-05-16) 8 commits
  (merged to 'next' on 2022-05-25 at 43b1b9092c)
 + bundle.h: make "fd" version of read_bundle_header() public
 + remote: allow relative_url() to return an absolute url
 + remote: move relative_url()
 + http: make http_get_file() external
 + fetch-pack: move --keep=* option filling to a function
 + fetch-pack: add a deref_without_lazy_fetch_extended()
 + dir API: add a generalized path_match_flags() function
 + connect.c: refactor sending of agent & object-format
 (this branch is used by ds/bundle-uri-more.)

 Preliminary code refactoring around transport and bundle code.

 Will merge to 'master'.
 source: <pull.1233.git.1652731865.gitgitgadget@gmail.com>


* ds/sparse-sparse-checkout (2022-05-23) 10 commits
 - sparse-checkout: integrate with sparse index
 - p2000: add test for 'git sparse-checkout [add|set]'
 - sparse-index: complete partial expansion
 - sparse-index: partially expand directories
 - sparse-checkout: --no-sparse-index needs a full index
 - cache-tree: implement cache_tree_find_path()
 - sparse-index: introduce partially-sparse indexes
 - sparse-index: create expand_index()
 - t1092: stress test 'git sparse-checkout set'
 - t1092: refactor 'sparse-index contents' test

 "sparse-checkout" learns to work well with the sparse-index
 feature.

 Will merge to 'next'?
 source: <pull.1208.v3.git.1653313726.gitgitgadget@gmail.com>


* gc/bare-repo-discovery (2022-05-16) 3 commits
 - SQUASH??? move new test to t0035
 - setup.c: learn discovery.bareRepository=cwd
 - setup.c: make bare repo discovery optional

 Introduce a discovery.barerepository configuration variable that
 allows users to forbid discovery of bare repositories.

 Expecting a reroll.
 source: <pull.1261.v2.git.git.1652485058.gitgitgadget@gmail.com>


* ds/object-file-unpack-loose-header-fix (2022-05-16) 1 commit
 - object-file: convert 'switch' back to 'if'

 Coding style fix.

 Will merge to 'next'.
 source: <377be0e9-8a0f-4a86-0a66-3b08c0284dae@github.com>


* gg/worktree-from-the-above (2022-05-20) 3 commits
 - dir: minor refactoring / clean-up
 - dir: cache git_dir's realpath
 - dir: traverse into repository

 With a non-bare repository, with core.worktree pointing at a
 directory that has the repository as its subdirectory, regressed in
 Git 2.27 days.

 Needs review.
 source: <20220520192840.8942-1-ggossdev@gmail.com>


* ac/remote-v-with-object-list-filters (2022-05-09) 1 commit
  (merged to 'next' on 2022-05-20 at 8d2dc10d8f)
 + builtin/remote.c: teach `-v` to list filters for promisor remotes

 "git remote -v" now shows the list-objects-filter used during
 fetching from the remote, if available.

 Will merge to 'master'.
 source: <pull.1227.v4.git.1652095969026.gitgitgadget@gmail.com>


* cc/http-curlopt-resolve (2022-05-16) 1 commit
  (merged to 'next' on 2022-05-20 at 80a07dc7de)
 + http: add custom hostname to IP address resolutions

 With the new http.curloptResolve configuration, the CURLOPT_RESOLVE
 mechanism that allows cURL based applications to use pre-resolved
 IP addresses for the requests is exposed to the scripts.

 Will merge to 'master'.
 source: <20220516083851.202057-1-chriscool@tuxfamily.org>


* jx/l10n-workflow-change (2022-05-23) 8 commits
 - l10n: Document the new l10n workflow
 - Makefile: add "po-init" rule to initialize po/XX.po
 - Makefile: add "po-update" rule to update po/XX.po
 - po/git.pot: don't check in result of "make pot"
 - i18n CI: stop allowing non-ASCII source messages in po/git.pot
 - Makefile: have "make pot" not "reset --hard"
 - Makefile: generate "po/git.pot" from stable LOCALIZED_C
 - Makefile: sort "po/git.pot" by file location

 A workflow change for translators are being proposed.

 Will merge to 'next'?
 source: <20220523012531.4505-1-worldhello.net@gmail.com>


* cb/path-owner-check-with-sudo (2022-05-12) 3 commits
  (merged to 'next' on 2022-05-19 at d282e56560)
 + t0034: add negative tests and allow git init to mostly work under sudo
 + git-compat-util: avoid failing dir ownership checks if running privileged
 + t: regression git needs safe.directory when using sudo
 (this branch is used by cb/path-owner-check-with-sudo-plus.)

 With a recent update to refuse access to repositories of other
 people by default, "sudo make install" and "sudo git describe"
 stopped working.  This series intends to loosen it while keeping
 the safety.

 Will merge to 'master'.
 source: <20220513010020.55361-1-carenas@gmail.com>


* cg/tools-for-git-doc (2022-04-21) 1 commit
  (merged to 'next' on 2022-05-19 at e6b6309afb)
 + Documentation/ToolsForGit.txt: Tools for developing Git

 A new doc that lists tips for tools to work with Git's codebase.

 Will merge to 'master'.
 source: <20220421084515.21236-2-cogoni.guillaume@gmail.com>


* ar/send-email-confirm-by-default (2022-04-22) 1 commit
 - send-email: always confirm by default

 "git send-email" is changed so that by default it asks for
 confirmation before sending each message out.

 Will discard.

 I wanted to like this, and had it in the version of Git I use
 myself for daily work, but the prompting turned out to be somewhat
 distracting.

 Thoughts?
 source: <20220422083629.1404989-1-hi@alyssa.is>


* ab/env-array (2022-05-20) 4 commits
 - run-command API users: use "env" not "env_array" in comments & names
 - cocci: remove env_array -> env migration
 - run-command API: rename "env_array" to "env"
 - cocci: add a rename of "struct child_process"'s "env_array" to "env"

 Rename .env_array member to .env in the child_process structure.

 Expecting a (hopefully final) reroll, before merging it to 'next'.
 cf. <xmqqilq0jhk2.fsf@gitster.g>
 source: <cover-v2-0.4-00000000000-20220520T072122Z-avarab@gmail.com>


* ab/plug-leak-in-revisions (2022-04-13) 28 commits
 - revisions API: add a TODO for diff_free(&revs->diffopt)
 - revisions API: have release_revisions() release "topo_walk_info"
 - revisions API: have release_revisions() release "date_mode"
 - revisions API: call diff_free(&revs->pruning) in revisions_release()
 - revisions API: release "reflog_info" in release revisions()
 - revisions API: clear "boundary_commits" in release_revisions()
 - revisions API: have release_revisions() release "prune_data"
 - revisions API: have release_revisions() release "grep_filter"
 - revisions API: have release_revisions() release "filter"
 - revisions API: have release_revisions() release "cmdline"
 - revisions API: have release_revisions() release "mailmap"
 - revisions API: have release_revisions() release "commits"
 - revisions API users: use release_revisions() for "prune_data" users
 - revisions API users: use release_revisions() with UNLEAK()
 - revisions API users: use release_revisions() in builtin/log.c
 - revisions API users: use release_revisions() in http-push.c
 - revisions API users: add "goto cleanup" for release_revisions()
 - stash: always have the owner of "stash_info" free it
 - revisions API users: use release_revisions() needing REV_INFO_INIT
 - revision.[ch]: document and move code declared around "init"
 - revisions API users: add straightforward release_revisions()
 - revision.[ch]: provide and start using a release_revisions()
 - cocci: add and apply free_commit_list() rules
 - format-patch: don't leak "extra_headers" or "ref_message_ids"
 - string_list API users: use string_list_init_{no,}dup
 - blame: use "goto cleanup" for cleanup_scoreboard()
 - t/helper/test-fast-rebase.c: don't leak "struct strbuf"
 - Merge branch 'ds/partial-bundle-more' into ab/plug-leak-in-revisions

 Plug the memory leaks from the trickiest API of all, the revision
 walker.

 Will merge to 'next'?
 source: <cover-v6-00.27-00000000000-20220413T195935Z-avarab@gmail.com>


* ns/batch-fsync (2022-04-06) 13 commits
  (merged to 'next' on 2022-05-23 at 379d8bd500)
 + core.fsyncmethod: performance tests for batch mode
 + t/perf: add iteration setup mechanism to perf-lib
 + core.fsyncmethod: tests for batch mode
 + test-lib-functions: add parsing helpers for ls-files and ls-tree
 + core.fsync: use batch mode and sync loose objects by default on Windows
 + unpack-objects: use the bulk-checkin infrastructure
 + update-index: use the bulk-checkin infrastructure
 + builtin/add: add ODB transaction around add_files_to_cache
 + cache-tree: use ODB transaction around writing a tree
 + core.fsyncmethod: batched disk flushes for loose-objects
 + bulk-checkin: rebrand plug/unplug APIs as 'odb transactions'
 + bulk-checkin: rename 'state' variable and separate 'plugged' boolean
 + Merge branch 'ns/core-fsyncmethod' into ns/batch-fsync

 Introduce a filesystem-dependent mechanism to optimize the way the
 bits for many loose object files are ensured to hit the disk
 platter.

 Will merge to 'master'.
 source: <pull.1134.v5.git.1648616734.gitgitgadget@gmail.com>


* en/sparse-cone-becomes-default (2022-04-21) 9 commits
  (merged to 'next' on 2022-05-13 at c168eb55cf)
 + Documentation: some sparsity wording clarifications
 + git-sparse-checkout.txt: mark non-cone mode as deprecated
 + git-sparse-checkout.txt: flesh out pattern set sections a bit
 + git-sparse-checkout.txt: add a new EXAMPLES section
 + git-sparse-checkout.txt: shuffle some sections and mark as internal
 + git-sparse-checkout.txt: update docs for deprecation of 'init'
 + git-sparse-checkout.txt: wording updates for the cone mode default
 + sparse-checkout: make --cone the default
 + tests: stop assuming --no-cone is the default mode for sparse-checkout

 Deprecate non-cone mode of the sparse-checkout feature.

 Will cook in 'next' til 06-03 and then merge to 'master'.
 source: <pull.1148.v3.git.1650594746.gitgitgadget@gmail.com>


* tb/cruft-packs (2022-05-25) 18 commits
 - sha1-file.c: don't freshen cruft packs
 - builtin/gc.c: conditionally avoid pruning objects via loose
 - builtin/repack.c: add cruft packs to MIDX during geometric repack
 - builtin/repack.c: use named flags for existing_packs
 - builtin/repack.c: allow configuring cruft pack generation
 - builtin/repack.c: support generating a cruft pack
 - builtin/pack-objects.c: --cruft with expiration
 - reachable: report precise timestamps from objects in cruft packs
 - reachable: add options to add_unseen_recent_objects_to_traversal
 - builtin/pack-objects.c: --cruft without expiration
 - builtin/pack-objects.c: return from create_object_entry()
 - t/helper: add 'pack-mtimes' test-tool
 - pack-mtimes: support writing pack .mtimes files
 - chunk-format.h: extract oid_version()
 - pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles'
 - fixup! pack-mtimes: support reading .mtimes files
 - pack-mtimes: support reading .mtimes files
 - Documentation/technical: add cruft-packs.txt

 A mechanism to pack unreachable objects into a "cruft pack",
 instead of ejecting them into loose form to be reclaimed later, has
 been introduced.

 Will merge to 'next' after squashing fixup! in???
 source: <cover.1653088640.git.me@ttaylorr.com>


* tk/simple-autosetupmerge (2022-04-29) 3 commits
  (merged to 'next' on 2022-05-19 at 9666852f1e)
 + push: new config option "push.autoSetupRemote" supports "simple" push
 + push: default to single remote even when not named origin
 + branch: new autosetupmerge option 'simple' for matching branches

 "git -c branch.autosetupmerge=simple branch $A $B" will set the $B
 as $A's upstream only when $A and $B shares the same name, and "git
 -c push.default=simple" on branch $A would push to update the
 branch $A at the remote $B came from.  Also more places use the
 sole remote, if exists, before defaulting to 'origin'.

 Will merge to 'master'.
 source: <pull.1161.v5.git.1651226206.gitgitgadget@gmail.com>


* jh/builtin-fsmonitor-part3 (2022-05-25) 31 commits
 - t7527: improve implicit shutdown testing in fsmonitor--daemon
 - fsmonitor--daemon: allow --super-prefix argument
 - t7527: test Unicode NFC/NFD handling on MacOS
 - t/lib-unicode-nfc-nfd: helper prereqs for testing unicode nfc/nfd
 - t/helper/hexdump: add helper to print hexdump of stdin
 - fsmonitor: on macOS also emit NFC spelling for NFD pathname
 - t7527: test FSMonitor on case insensitive+preserving file system
 - fsmonitor: never set CE_FSMONITOR_VALID on submodules
 - t/perf/p7527: add perf test for builtin FSMonitor
 - t7527: FSMonitor tests for directory moves
 - fsmonitor: optimize processing of directory events
 - fsm-listen-darwin: shutdown daemon if worktree root is moved/renamed
 - fsm-health-win32: force shutdown daemon if worktree root moves
 - fsm-health-win32: add polling framework to monitor daemon health
 - fsmonitor--daemon: stub in health thread
 - fsmonitor--daemon: rename listener thread related variables
 - fsmonitor--daemon: prepare for adding health thread
 - fsmonitor--daemon: cd out of worktree root
 - fsm-listen-darwin: ignore FSEvents caused by xattr changes on macOS
 - unpack-trees: initialize fsmonitor_has_run_once in o->result
 - fsmonitor-settings: NTFS and FAT32 on MacOS are incompatible
 - fsmonitor-settings: remote repos on Windows are incompatible
 - fsmonitor-settings: remote repos on macOS are incompatible
 - fsmonitor-settings: stub in macOS-specific incompatibility checking
 - fsmonitor-settings: VFS for Git virtual repos are incompatible
 - fsmonitor-settings: stub in Win32-specific incompatibility checking
 - fsmonitor-settings: bare repos are incompatible with FSMonitor
 - t/helper/fsmonitor-client: create stress test
 - t7527: test FSMonitor on repos with Unicode root paths
 - fsm-listen-win32: handle shortnames
 - Merge branch 'jh/builtin-fsmonitor-part2' into jh/builtin-fsmonitor-part3

 More fsmonitor--daemon.

 Expecting further work.
 Breaks its own t7527?
 source: <pull.1143.v8.git.1653490852.gitgitgadget@gmail.com>


* js/scalar-diagnose (2022-05-25) 9 commits
 - fixup! archive --add-virtual-file: allow paths containing colons
 - fixup! archive: optionally add "virtual" files
 - scalar: teach `diagnose` to gather loose objects information
 - scalar: teach `diagnose` to gather packfile info
 - scalar diagnose: include disk space information
 - Implement `scalar diagnose`
 - scalar: validate the optional enlistment argument
 - archive --add-virtual-file: allow paths containing colons
 - archive: optionally add "virtual" files

 Implementation of "scalar diagnose" subcommand.

 Will merge to 'next'?
 source: <pull.1128.v6.git.1653145696.gitgitgadget@gmail.com>


* js/use-builtin-add-i (2021-12-01) 2 commits
  (merged to 'next' on 2022-05-23 at a6434bc6f7)
 + add -i: default to the built-in implementation
 + t2016: require the PERL prereq only when necessary

 "git add -i" was rewritten in C some time ago and has been in
 testing; the reimplementation is now exposed to general public by
 default.

 Will merge to 'master'.
 source: <pull.1087.git.1638281655.gitgitgadget@gmail.com>

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Victoria Dye]

Junio C Hamano wrote:
> * js/ci-github-workflow-markup (2022-05-21) 12 commits
>  - ci: call `finalize_test_case_output` a little later
>  - ci(github): mention where the full logs can be found
>  - ci: use `--github-workflow-markup` in the GitHub workflow
>  - ci(github): avoid printing test case preamble twice
>  - ci(github): skip the logs of the successful test cases
>  - ci: optionally mark up output in the GitHub workflow
>  - ci/run-build-and-tests: add some structure to the GitHub workflow output
>  - ci: make it easier to find failed tests' logs in the GitHub workflow
>  - ci/run-build-and-tests: take a more high-level view
>  - test(junit): avoid line feeds in XML attributes
>  - tests: refactor --write-junit-xml code
>  - ci: fix code style
> 
>  Update the GitHub workflow support to make it quicker to get to the
>  failing test.
> 
>  Will merge to 'next'?
>  source: <pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>
> 

The latest version of this nicely addressed the feedback I originally had,
particularly in improving page loading time. It's still slower than before
this series, but IMO it's manageable (especially taking into account the
improved information accessibility). 

I don't see (or have) any other unaddressed concerns, so I'm in favor of
moving it to 'next'.

> * ds/sparse-sparse-checkout (2022-05-23) 10 commits
>  - sparse-checkout: integrate with sparse index
>  - p2000: add test for 'git sparse-checkout [add|set]'
>  - sparse-index: complete partial expansion
>  - sparse-index: partially expand directories
>  - sparse-checkout: --no-sparse-index needs a full index
>  - cache-tree: implement cache_tree_find_path()
>  - sparse-index: introduce partially-sparse indexes
>  - sparse-index: create expand_index()
>  - t1092: stress test 'git sparse-checkout set'
>  - t1092: refactor 'sparse-index contents' test
> 
>  "sparse-checkout" learns to work well with the sparse-index
>  feature.
> 
>  Will merge to 'next'?
>  source: <pull.1208.v3.git.1653313726.gitgitgadget@gmail.com>
> 

Likewise here - V2 handled all of my nits, and V3 made some helpful
improvements to variable naming & documentation. There's nothing else I'm
waiting for on this, so I'd be happy to see it merge to 'next'.

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Junio C Hamano]

Victoria Dye <vdye@github.com> writes:

>> * js/ci-github-workflow-markup (2022-05-21) 12 commits
>>  - ci: call `finalize_test_case_output` a little later
>>  - ci(github): mention where the full logs can be found
>>  - ci: use `--github-workflow-markup` in the GitHub workflow
>>  - ci(github): avoid printing test case preamble twice
>>  - ci(github): skip the logs of the successful test cases
>>  - ci: optionally mark up output in the GitHub workflow
>>  - ci/run-build-and-tests: add some structure to the GitHub workflow output
>>  - ci: make it easier to find failed tests' logs in the GitHub workflow
>>  - ci/run-build-and-tests: take a more high-level view
>>  - test(junit): avoid line feeds in XML attributes
>>  - tests: refactor --write-junit-xml code
>>  - ci: fix code style
>> 
>>  Update the GitHub workflow support to make it quicker to get to the
>>  failing test.
>> 
>>  Will merge to 'next'?
>>  source: <pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>
>
> The latest version of this nicely addressed the feedback I originally had,
> particularly in improving page loading time. It's still slower than before
> this series, but IMO it's manageable (especially taking into account the
> improved information accessibility). 
>
> I don't see (or have) any other unaddressed concerns, so I'm in favor of
> moving it to 'next'.

I see Ævar sent another reroll of "rebuild the base" and "then
rebase a (hopefully) equivalent of this series on top", but I think
it is unhealthy to keep doing that.  Does the latest "rebuild the
base" part look unsalvageably and fundamentally bad that it is not
worth our time to consider joining forces to polish it sufficiently
and then build this on top?

If that is the case, then I am OK to merge this to 'next' to cast it
in stone, and then the let "rebuild the base" part once die, to be
reborn as many "tweak the way things work to (clarify|optimize) X"
follow-up topics.

Thanks.

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> Victoria Dye <vdye@github.com> writes:
>
>>> * js/ci-github-workflow-markup (2022-05-21) 12 commits
>>>  - ci: call `finalize_test_case_output` a little later
>>>  - ci(github): mention where the full logs can be found
>>>  - ci: use `--github-workflow-markup` in the GitHub workflow
>>>  - ci(github): avoid printing test case preamble twice
>>>  - ci(github): skip the logs of the successful test cases
>>>  - ci: optionally mark up output in the GitHub workflow
>>>  - ci/run-build-and-tests: add some structure to the GitHub workflow output
>>>  - ci: make it easier to find failed tests' logs in the GitHub workflow
>>>  - ci/run-build-and-tests: take a more high-level view
>>>  - test(junit): avoid line feeds in XML attributes
>>>  - tests: refactor --write-junit-xml code
>>>  - ci: fix code style
>>> 
>>>  Update the GitHub workflow support to make it quicker to get to the
>>>  failing test.
>>> 
>>>  Will merge to 'next'?
>>>  source: <pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>
>>
>> The latest version of this nicely addressed the feedback I originally had,
>> particularly in improving page loading time. It's still slower than before
>> this series, but IMO it's manageable (especially taking into account the
>> improved information accessibility). 
>>
>> I don't see (or have) any other unaddressed concerns, so I'm in favor of
>> moving it to 'next'.
>
> I see Ævar sent another reroll of "rebuild the base" and "then
> rebase a (hopefully) equivalent of this series on top", but I think
> it is unhealthy to keep doing that.

I'd understood per
https://lore.kernel.org/git/xmqqwnecqdti.fsf@gitster.g/ that you were
mulling over whether to take my version of js/ci-github-workflow-markup
or not, and you rightly pointed out some bugs in (at the time) the
latest version.

So I re-rolled the two:

  https://lore.kernel.org/git/cover-v6-00.29-00000000000-20220525T094123Z-avarab@gmail.com/
  https://lore.kernel.org/git/cover-v6-00.14-00000000000-20220525T100743Z-avarab@gmail.com/

As the latter of the two notes there are some outstanding bugs that
hadn't been noted before in the v3 version of
js/ci-github-workflow-markup that you currently have queued.

Victoria: This includes a bug in your
https://lore.kernel.org/git/patch-v6-10.14-90a152d79f9-20220525T100743Z-avarab@gmail.com/
where a patch that aims to change the *.markup output alters the *.out
output, i.e. the "raw log" output now omits output that you were trying
to omit from the *.markup output.

But as the performance numbers on my version notes the new GitHub
foldable output seems to be around 20% faster on top of my "base" topic,
the reason being that structurally un-folding "make", "make test"
etc. to the "step" levels is asking the already overworked GitHub CI
JavaScript to do less work.

And while it leaves the new output Johannes is proposing on by default,
there's now a facility to disable it:
https://lore.kernel.org/git/patch-v6-14.14-0b02b186c87-20220525T100743Z-avarab@gmail.com/

Which at least for the time being is something I'm going to use until I
can figure out how to speed it up & get the now-missing parts of the raw
log output back,

> Does the latest "rebuild the base" part look unsalvageably and
> fundamentally bad that it is not worth our time to consider joining
> forces to polish it sufficiently and then build this on top?

I think per [1] that Johannes hasn't looked at the "base" topic in any
detail out of a belief that the two approaches aren't complimentary,
which I think the ~20% improvement in performance (per my re-roll, sent
since then) shows that it's worthwhile to combine the two.

I use CI quite heavily, apparently in a way that neither you nor
Johannes use, because even with that improvement I really don't find the
new output worthwhile. I.e. I can open/search the raw logs and sometimes
even figure out what broke exactly while the new UX will still be at the
spinning loading icon.

But as noted above my latest re-roll provides an out for that, you can
just change your ci-config to use the old output if you'd like, for that
and reasons I've noted before I'd really prefer to see the combination
of the two land.

Thanks!

tb/cruft-packs (was Re: What's cooking in git.git (May 2022, #07; Wed, 25))

[Taylor Blau]

On Thu, May 26, 2022 at 01:41:25AM -0700, Junio C Hamano wrote:
> * tb/cruft-packs (2022-05-25) 18 commits
>  - sha1-file.c: don't freshen cruft packs
>  - builtin/gc.c: conditionally avoid pruning objects via loose
>  - builtin/repack.c: add cruft packs to MIDX during geometric repack
>  - builtin/repack.c: use named flags for existing_packs
>  - builtin/repack.c: allow configuring cruft pack generation
>  - builtin/repack.c: support generating a cruft pack
>  - builtin/pack-objects.c: --cruft with expiration
>  - reachable: report precise timestamps from objects in cruft packs
>  - reachable: add options to add_unseen_recent_objects_to_traversal
>  - builtin/pack-objects.c: --cruft without expiration
>  - builtin/pack-objects.c: return from create_object_entry()
>  - t/helper: add 'pack-mtimes' test-tool
>  - pack-mtimes: support writing pack .mtimes files
>  - chunk-format.h: extract oid_version()
>  - pack-write: pass 'struct packing_data' to 'stage_tmp_packfiles'
>  - fixup! pack-mtimes: support reading .mtimes files
>  - pack-mtimes: support reading .mtimes files
>  - Documentation/technical: add cruft-packs.txt
>
>  A mechanism to pack unreachable objects into a "cruft pack",
>  instead of ejecting them into loose form to be reclaimed later, has
>  been introduced.
>
>  Will merge to 'next' after squashing fixup! in???
>  source: <cover.1653088640.git.me@ttaylorr.com>

I think this is ready. This topic has been thoroughly reviewed, and the
outstanding topics:

  - user-facing documentation cautioning about mixed-version cruft GCs
    (similar to what I added in Documentation/technical/cruft-packs.txt
    in v4)

  - 64-bit timestamps, if desired

Can easily be done on top. The first one is trivial, and I'll send
some patches before getting too close to the -rc phase. The latter is
more substantial, but isn't a requirement for merging this (and the
format is extensible so this could be done on top if there is
significant interest).

Thanks,
Taylor

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Taylor Blau]

On Thu, May 26, 2022 at 01:41:25AM -0700, Junio C Hamano wrote:
> * tb/geom-repack-with-keep-and-max (2022-05-20) 3 commits
>  - builtin/repack.c: ensure that `names` is sorted
>  - t7703: demonstrate object corruption with pack.packSizeLimit
>  - repack: respect --keep-pack with geometric repack
>
>  source: <cover.1653073280.git.me@ttaylorr.com>

Stolee took a look at this and left a positive review. Victoria and I
wrote these patches together, so I assume/hope she's on board, too ;).

I think these are both ready to merge.

Thanks,
Taylor

tb/midx-race-in-pack-objects (was Re: What's cooking in git.git (May 2022, #07; Wed, 25))

[Taylor Blau]

On Thu, May 26, 2022 at 01:41:25AM -0700, Junio C Hamano wrote:
> * tb/midx-race-in-pack-objects (2022-05-24) 4 commits
>  - builtin/pack-objects.c: ensure pack validity from MIDX bitmap objects
>  - builtin/pack-objects.c: ensure included `--stdin-packs` exist
>  - builtin/pack-objects.c: avoid redundant NULL check
>  - pack-bitmap.c: check preferred pack validity when opening MIDX bitmap
>
>  The multi-pack-index code did not protect the packfile it is going
>  to depend on from getting removed while in use, which has been
>  corrected.
>
>  Will merge to 'next'?
>  source: <cover.1653418457.git.me@ttaylorr.com>

I think these are in good shape, but I'd like for Victoria to take a
look at the version on the list here.

We are running these patches at GitHub, and they have definitively
closed the race we were investigating here. But the commit messages
changed slightly when going upstream, so I'd appreciate Victoria's (or
anybody else who is interested) eyes on these.

Thanks,
Taylor

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Victoria Dye]

Junio C Hamano wrote:
> Victoria Dye <vdye@github.com> writes:
> 
>>> * js/ci-github-workflow-markup (2022-05-21) 12 commits
>>>  - ci: call `finalize_test_case_output` a little later
>>>  - ci(github): mention where the full logs can be found
>>>  - ci: use `--github-workflow-markup` in the GitHub workflow
>>>  - ci(github): avoid printing test case preamble twice
>>>  - ci(github): skip the logs of the successful test cases
>>>  - ci: optionally mark up output in the GitHub workflow
>>>  - ci/run-build-and-tests: add some structure to the GitHub workflow output
>>>  - ci: make it easier to find failed tests' logs in the GitHub workflow
>>>  - ci/run-build-and-tests: take a more high-level view
>>>  - test(junit): avoid line feeds in XML attributes
>>>  - tests: refactor --write-junit-xml code
>>>  - ci: fix code style
>>>
>>>  Update the GitHub workflow support to make it quicker to get to the
>>>  failing test.
>>>
>>>  Will merge to 'next'?
>>>  source: <pull.1117.v3.git.1653171536.gitgitgadget@gmail.com>
>>
>> The latest version of this nicely addressed the feedback I originally had,
>> particularly in improving page loading time. It's still slower than before
>> this series, but IMO it's manageable (especially taking into account the
>> improved information accessibility). 
>>
>> I don't see (or have) any other unaddressed concerns, so I'm in favor of
>> moving it to 'next'.
> 
> I see Ævar sent another reroll of "rebuild the base" and "then
> rebase a (hopefully) equivalent of this series on top", but I think
> it is unhealthy to keep doing that.  Does the latest "rebuild the
> base" part look unsalvageably and fundamentally bad that it is not
> worth our time to consider joining forces to polish it sufficiently
> and then build this on top?
> 

My impression of 'ab/ci-setup-simplify' is that its core changes are either
unrelated to, or at least not mutually exclusive with, the
'js/ci-github-workflow-markup' series. While Ævar has sent an example/RFC
with one series rebased on top of the other, I don't see the two as
inextricably linked, or even really comparable. Because of that, I don't
think it would be fair to either series if we continued to hold up *both*
because of different levels of consensus, review, etc.

As for my thoughts on each series, I do still think
'js/ci-github-workflow-markup' is ready for 'next' (for the reasons I sent
earlier). I think 'ab/ci-setup-simplify' needs more review - especially
given its length and the variety of changes - to ensure it doesn't introduce
regressions or hurt developer quality-of-life. I've personally had a
difficult time making sense of the series enough to review it, so I can't
confidently judge it one way or another myself.

> If that is the case, then I am OK to merge this to 'next' to cast it
> in stone, and then the let "rebuild the base" part once die, to be
> reborn as many "tweak the way things work to (clarify|optimize) X"
> follow-up topics.
> 

I'm not sure 'ab/ci-setup-simplify' would need to "die", more that it would
be adjusted to rebase on top of an updated 'next' (including
'js/ci-github-workflow-markup'). That said, a re-sent version focusing on
its own optimizations/improvements (rather than a comparisons against an IMO
largely unrelated series) would almost certainly benefit both the series and
its readers.

> Thanks.

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Victoria Dye wrote:

> I'm not sure 'ab/ci-setup-simplify' would need to "die", more that it
> would be adjusted to rebase on top of an updated 'next' (including
> 'js/ci-github-workflow-markup'). That said, a re-sent version focusing
> on its own optimizations/improvements (rather than a comparisons
> against an IMO largely unrelated series) would almost certainly
> benefit both the series and[...]

We probably just crossed E-Mails, but it would be good to know if your
assessment that they're "largely unrelated" is before or after seeing
the numbers I noted at
https://lore.kernel.org/git/220526.86leuo5f2g.gmgdl@evledraar.gmail.com/

I.e. that it improves the performance of js/ci-github-workflow-markup by
~20%. That's the reason I originally looked at combining the two,
because that series was doing a lot of work to re-implement features we
could get "natively" in the GitHub CI if we just split certain things by
steps, instead of dividing those same things with the "group" output.

There's a few other things where they combine
nicely. E.g. js/ci-github-workflow-markup contains its own mini-version
re-invention of ci/print-test-failures.sh, in the combined version it
can just use that (adjusted) script to get the same output.

sg/build-gitweb (was: What's cooking in git.git (May 2022, #07; Wed, 25))

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> * sg/build-gitweb (2022-05-25) 1 commit
>  - Makefile: build 'gitweb' in the default target
>
>  "make all" should but didn't build "gitweb".
>
>  Will merge to 'next'.
>  source: <20220525205651.825669-1-szeder.dev@gmail.com>

Did you see the discussion about what that costs us in terms of noop
"make" performance? See
https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/
& https://lore.kernel.org/git/Yo8y3AHWa3PChLwd@coredump.intra.peff.net/

jc/http-clear-finished-pointer (was: What's cooking in git.git (May 2022, #07; Wed, 25))

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> * jc/http-clear-finished-pointer (2022-05-24) 1 commit
>  - http.c: clear the 'finished' member once we are done with it
>
>  Meant to go with js/ci-gcc-12-fixes
>
>  Will merge to 'next'?
>  source: <xmqqczgqjr8y.fsf_-_@gitster.g>

The end of the proposed commit message says:

    [...]Clear the finished member before the control leaves the
    function, which has a side effect of unconfusing compilers like
    recent GCC 12 that is over-eager to warn against such an assignment.

I cannot reproduce this suppressing the warning as noted in past
exchanges, it's not affected by this "clear if we set it" pattern. It
needs to be unconditionally cleared.

So I don't think this actively makes anything worse, but I don't think
it's doing what you're expecting it to do either.

js/bisect-in-c (was: What's cooking in git.git (May 2022, #07; Wed, 25))

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> * js/bisect-in-c (2022-05-21) 15 commits
>  - bisect: no longer try to clean up left-over `.git/head-name` files
>  - bisect: remove Cogito-related code
>  - Turn `git bisect` into a full built-in
>  - bisect: teach the `bisect--helper` command to show the correct usage strings
>  - bisect: move even the command-line parsing to `bisect--helper`
>  - bisect--helper: return only correct exit codes in `cmd_*()`
>  - bisect--helper: move the `BISECT_STATE` case to the end
>  - bisect--helper: make `--bisect-state` optional
>  - bisect--helper: align the sub-command order with git-bisect.sh
>  - bisect--helper: using `--bisect-state` without an argument is a bug
>  - bisect--helper: really retire `--bisect-autostart`
>  - bisect--helper: really retire --bisect-next-check
>  - bisect--helper: retire the --no-log option
>  - bisect: avoid double-quoting when printing the failed command
>  - bisect run: fix the error message
>
>  Final bits of "git bisect.sh" have been rewritten in C.
>
>  Will merge to 'next'?
>  source: <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>

This topic has outstanding regressions in CLI parsing. I.e. we'll now
offer to start bisection where we previously errored out on invalid
command usage. See my replies in that thread.

jx/l10n-workflow-change (was: What's cooking in git.git (May 2022, #07; Wed, 25))

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> * jx/l10n-workflow-change (2022-05-23) 8 commits
>  - l10n: Document the new l10n workflow
>  - Makefile: add "po-init" rule to initialize po/XX.po
>  - Makefile: add "po-update" rule to update po/XX.po
>  - po/git.pot: don't check in result of "make pot"
>  - i18n CI: stop allowing non-ASCII source messages in po/git.pot
>  - Makefile: have "make pot" not "reset --hard"
>  - Makefile: generate "po/git.pot" from stable LOCALIZED_C
>  - Makefile: sort "po/git.pot" by file location
>
>  A workflow change for translators are being proposed.
>
>  Will merge to 'next'?
>  source: <20220523012531.4505-1-worldhello.net@gmail.com>

I'm happy with the current (posted today) version of it, and think it
should advance.

Re: sg/build-gitweb

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, May 26 2022, Junio C Hamano wrote:
>
>> * sg/build-gitweb (2022-05-25) 1 commit
>>  - Makefile: build 'gitweb' in the default target
>>
>>  "make all" should but didn't build "gitweb".
>>
>>  Will merge to 'next'.
>>  source: <20220525205651.825669-1-szeder.dev@gmail.com>
>
> Did you see the discussion about what that costs us in terms of noop
> "make" performance? See
> https://lore.kernel.org/git/220526.86k0a96sv2.gmgdl@evledraar.gmail.com/
> & https://lore.kernel.org/git/Yo8y3AHWa3PChLwd@coredump.intra.peff.net/

I've already marked it with

 * sg/build-gitweb (2022-05-25) 1 commit
  - Makefile: build 'gitweb' in the default target
 
  "make all" should but didn't build "gitweb".
 
- Will merge to 'next'.
+ Will discard.
+ cf. <220526.86k0a96sv2.gmgdl@evledraar.gmail.com>
+ cf. <Yo8y3AHWa3PChLwd@coredump.intra.peff.net>
  source: <20220525205651.825669-1-szeder.dev@gmail.com>

Thanks for a reminder.  More eyes help reduce mistakes.

Re* jc/http-clear-finished-pointer

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, May 26 2022, Junio C Hamano wrote:
>
>> * jc/http-clear-finished-pointer (2022-05-24) 1 commit
>>  - http.c: clear the 'finished' member once we are done with it
>>
>>  Meant to go with js/ci-gcc-12-fixes
>>
>>  Will merge to 'next'?
>>  source: <xmqqczgqjr8y.fsf_-_@gitster.g>
>
> The end of the proposed commit message says:
>
>     [...]Clear the finished member before the control leaves the
>     function, which has a side effect of unconfusing compilers like
>     recent GCC 12 that is over-eager to warn against such an assignment.
>
> I cannot reproduce this suppressing the warning as noted in past
> exchanges, it's not affected by this "clear if we set it" pattern. It
> needs to be unconditionally cleared.

Interesting.  I still have conditional clearing in the tree, though
I was reasonably sure I got rid of the conditional and made it
always clear, when I rewrote that part of the log message.  After
all, I ran "commit --amend" so that I do not forget the issue after
sending https://lore.kernel.org/git/xmqqleurlt31.fsf@gitster.g/ X-<.

Thanks for catching.  What is queued is not what I intended to
queue.

But there is one thing that is puzzling.  Ever since this, together
with the three patches from Dscho for gcc12, got included in 'seen',
the branch started passing the Windows build that used to complain
and did not work, so at least with the version of gcc12 used over
there, it apparently is sufficient to clear only when we are
responsible for placing an address that is about to become invalid,
while leaving the pointer we didn't stuff in unmodified.

As far as I understand, with the most recent analysis by Dscho on
the http-push codepath, we can return to the loop while the slot is
holding a different request that is unrelated to ours that has
already finished without recursively calling run_active_slot(), and
with the current *(slot->finished)=1 trick, it will successfully
notify our loop that our request is done, even though slot->in_use
is set to true back again when it happens.  But by definition, at
that point, slot->finished is not used by anybody (obviously not by
us, but also not by the request that is currently using the slot,
because it hasn't used run_active_slot() and slot->finished is not
touched by it), so it is safe to unconditionally clear the member.

----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
Subject: [PATCH v3] http.c: clear the 'finished' member once we are done with it

In http.c, the run_active_slot() function allows the given "slot" to
make progress by calling step_active_slots() in a loop repeatedly,
and the loop is not left until the request held in the slot
completes.

Ages ago, we used to use the slot->in_use member to get out of the
loop, which misbehaved when the request in "slot" completes (at
which time, the result of the request is copied away from the slot,
and the in_use member is cleared, making the slot ready to be
reused), and the "slot" gets reused to service a different request
(at which time, the "slot" becomes in_use again, even though it is
for a different request).  The loop terminating condition mistakenly
thought that the original request has yet to be completed.

Today's code, after baa7b67d (HTTP slot reuse fixes, 2006-03-10)
fixed this issue, uses a separate "slot->finished" member that is
set in run_active_slot() to point to an on-stack variable, and the
code that completes the request in finish_active_slot() clears the
on-stack variable via the pointer to signal that the particular
request held by the slot has completed.  It also clears the in_use
member (as before that fix), so that the slot itself can safely be
reused for an unrelated request.

One thing that is not quite clean in this arrangement is that,
unless the slot gets reused, at which point the finished member is
reset to NULL, the member keeps the value of &finished, which
becomes a dangling pointer into the stack when run_active_slot()
returns.  Clear the finished member before the control leaves the
function, which has a side effect of unconfusing compilers like
recent GCC 12 that is over-eager to warn against such an assignment.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 http.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/http.c b/http.c
index 229da4d148..9a98372f74 100644
--- a/http.c
+++ b/http.c
@@ -1367,6 +1367,32 @@ void run_active_slot(struct active_request_slot *slot)
 			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
 		}
 	}
+
+	/*
+	 * The value of slot->finished we set before the loop was used
+	 * to set our "finished" variable when our request completed.
+	 *
+	 * 1. The slot may not have been reused for another requst
+	 *    yet, in which case it still has &finished.
+	 *
+	 * 2. The slot may already be in-use to serve another request,
+	 *    which can further be divided into two cases:
+	 *
+	 * (a) If call run_active_slot() hasn't been called for that
+	 *     other request, slot->finished may still have the
+	 *     address of our &finished.
+	 *
+	 * (b) If the request did call run_active_slot(), then the
+	 *     call would have updated slot->finished at the beginning
+	 *     of this function, and with the clearing of the member
+	 *     below, we would find that slot->finished is now NULL.
+	 *
+	 * In all cases, slot->finished has no useful information to
+	 * anybody at this point.  Some compilers warn us for
+	 * attempting to smuggle a pointer that is about to become
+	 * invalid, i.e. &finished.  We clear it here to assure them.
+	 */
+	slot->finished = NULL;
 }
 
 static void release_active_slot(struct active_request_slot *slot)
-- 
2.36.1-306-g0dbcc0e187

Re: js/bisect-in-c

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> On Thu, May 26 2022, Junio C Hamano wrote:
>
>> * js/bisect-in-c (2022-05-21) 15 commits
>>  - bisect: no longer try to clean up left-over `.git/head-name` files
>>  - bisect: remove Cogito-related code
>>  - Turn `git bisect` into a full built-in
>>  - bisect: teach the `bisect--helper` command to show the correct usage strings
>>  - bisect: move even the command-line parsing to `bisect--helper`
>>  - bisect--helper: return only correct exit codes in `cmd_*()`
>>  - bisect--helper: move the `BISECT_STATE` case to the end
>>  - bisect--helper: make `--bisect-state` optional
>>  - bisect--helper: align the sub-command order with git-bisect.sh
>>  - bisect--helper: using `--bisect-state` without an argument is a bug
>>  - bisect--helper: really retire `--bisect-autostart`
>>  - bisect--helper: really retire --bisect-next-check
>>  - bisect--helper: retire the --no-log option
>>  - bisect: avoid double-quoting when printing the failed command
>>  - bisect run: fix the error message
>>
>>  Final bits of "git bisect.sh" have been rewritten in C.
>>
>>  Will merge to 'next'?
>>  source: <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>
>
> This topic has outstanding regressions in CLI parsing. I.e. we'll now
> offer to start bisection where we previously errored out on invalid
> command usage. See my replies in that thread.

Pointers?

Re: js/bisect-in-c

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

>>>  Final bits of "git bisect.sh" have been rewritten in C.
>>>
>>>  Will merge to 'next'?
>>>  source: <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>
>>
>> This topic has outstanding regressions in CLI parsing. I.e. we'll now
>> offer to start bisection where we previously errored out on invalid
>> command usage. See my replies in that thread.
>
> Pointers?

I guess you meant this one?

https://lore.kernel.org/git/220521.86zgjazuy4.gmgdl@evledraar.gmail.com/

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Junio C Hamano]

Taylor Blau <me@ttaylorr.com> writes:

> On Thu, May 26, 2022 at 01:41:25AM -0700, Junio C Hamano wrote:
>> * tb/geom-repack-with-keep-and-max (2022-05-20) 3 commits
>>  - builtin/repack.c: ensure that `names` is sorted
>>  - t7703: demonstrate object corruption with pack.packSizeLimit
>>  - repack: respect --keep-pack with geometric repack
>>
>>  source: <cover.1653073280.git.me@ttaylorr.com>
>
> Stolee took a look at this and left a positive review. Victoria and I
> wrote these patches together, so I assume/hope she's on board, too ;).

Yup, I did find these patches are OK, too.

Thanks.

Re: What's cooking in git.git (May 2022, #07; Wed, 25)

[Junio C Hamano]

Victoria Dye <vdye@github.com> writes:

>> If that is the case, then I am OK to merge this to 'next' to cast it
>> in stone, and then the let "rebuild the base" part once die, to be
>> reborn as many "tweak the way things work to (clarify|optimize) X"
>> follow-up topics.
>> 
> I'm not sure 'ab/ci-setup-simplify' would need to "die", more that it would
> be adjusted to rebase on top of an updated 'next' (including
> 'js/ci-github-workflow-markup').

Yeah, that is much closer to what I meant to say.  The one that
wants to build directly on top of 'master' would need to be retired,
because Dscho's series makes quite a lot of overlapping changes, and
it no longer would be "GitHub workflow is built on top of this
rearchitected foundation" anymore.

> That said, a re-sent version focusing on
> its own optimizations/improvements (rather than a comparisons against an IMO
> largely unrelated series) would almost certainly benefit both the series and
> its readers.

Thanks.

Re: js/bisect-in-c

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
>
>>>>  Final bits of "git bisect.sh" have been rewritten in C.
>>>>
>>>>  Will merge to 'next'?
>>>>  source: <pull.1132.v3.git.1653144546.gitgitgadget@gmail.com>
>>>
>>> This topic has outstanding regressions in CLI parsing. I.e. we'll now
>>> offer to start bisection where we previously errored out on invalid
>>> command usage. See my replies in that thread.
>>
>> Pointers?
>
> I guess you meant this one?
>
> https://lore.kernel.org/git/220521.86zgjazuy4.gmgdl@evledraar.gmail.com/

Yes, sorry about the lack of specific reference upthread.

As noted after in
https://lore.kernel.org/git/220523.865ylwzgji.gmgdl@evledraar.gmail.com/
we're rewriting code in this case that doesn't have test coverage to
catch these regressions, so more than just a narrow bugfix I'd really
like to see a re-roll that gives some confidence that there aren't other
such issues by first adding missing test coverage.

Re: Re* jc/http-clear-finished-pointer

[Johannes Schindelin]

Hi Junio,

On Thu, 26 May 2022, Junio C Hamano wrote:

> diff --git a/http.c b/http.c
> index 229da4d148..9a98372f74 100644
> --- a/http.c
> +++ b/http.c
> @@ -1367,6 +1367,32 @@ void run_active_slot(struct active_request_slot *slot)
>  			select(max_fd+1, &readfds, &writefds, &excfds, &select_timeout);
>  		}
>  	}
> +
> +	/*
> +	 * The value of slot->finished we set before the loop was used
> +	 * to set our "finished" variable when our request completed.
> +	 *
> +	 * 1. The slot may not have been reused for another requst
> +	 *    yet, in which case it still has &finished.
> +	 *
> +	 * 2. The slot may already be in-use to serve another request,
> +	 *    which can further be divided into two cases:
> +	 *
> +	 * (a) If call run_active_slot() hasn't been called for that
> +	 *     other request, slot->finished may still have the
> +	 *     address of our &finished.
> +	 *
> +	 * (b) If the request did call run_active_slot(), then the
> +	 *     call would have updated slot->finished at the beginning
> +	 *     of this function, and with the clearing of the member
> +	 *     below, we would find that slot->finished is now NULL.
> +	 *
> +	 * In all cases, slot->finished has no useful information to
> +	 * anybody at this point.  Some compilers warn us for
> +	 * attempting to smuggle a pointer that is about to become
> +	 * invalid, i.e. &finished.  We clear it here to assure them.
> +	 */
> +	slot->finished = NULL;
>  }
>
>  static void release_active_slot(struct active_request_slot *slot)
> --
> 2.36.1-306-g0dbcc0e187

I just verified that there is currently no other location in Git's code
that assigns a non-NULL value to `slot->finished` than
`run_active_slot()`. Otherwise we would potentially overwrite the value
here (which is why I preferred the conditional assignment, which does not
shut up GCC though). So for now, this solution is safe.

Having said that, it is quite puzzling that GCC thinks it is safe to
assign a local variable's pointer to a struct that is then accessed
outside the current file. This would make it easy to copy and use the
pointer well after the function scope was left. This is _not_ the case in
Git's source code, but GCC seems that this isn't possible by
(mis-)interpreting the final `slot->finished = NULL` to mean that the
`slot->finished = &finished` was safe (because it clearly isn't). In GCC's
defense, there is probably a lot of code out there that would no longer
compile if they truly enforced the new `-Wdangling-pointer` rule
correctly.

With all that, here is my ACK.

Ciao,
Dscho

Re: Re* jc/http-clear-finished-pointer

[Junio C Hamano]

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

This analysis and explanation was actually still too conservative.

>> +	/*
>> +	 * The value of slot->finished we set before the loop was used
>> +	 * to set our "finished" variable when our request completed.
>> +	 *
>> +	 * 1. The slot may not have been reused for another requst
>> +	 *    yet, in which case it still has &finished.
>> +	 *
>> +	 * 2. The slot may already be in-use to serve another request,
>> +	 *    which can further be divided into two cases:
>> +	 *
>> +	 * (a) If call run_active_slot() hasn't been called for that
>> +	 *     other request, slot->finished may still have the
>> +	 *     address of our &finished.
>> +	 *
>> +	 * (b) If the request did call run_active_slot(), then the
>> +	 *     call would have updated slot->finished at the beginning
>> +	 *     of this function, and with the clearing of the member
>> +	 *     below, we would find that slot->finished is now NULL.

Given that there are only two places that assign to slot->finished
(one is at the beginning of run_active_slot(), the other is when
get_active_slot() either allocates a new one or decides to reuse the
one that finish_active_slot() has marked to be no longer in use),
all in "reused" cases, not just 2-(b), but even in case 2-(a),
slot->finished should be NULL.

>> +	 * In all cases, slot->finished has no useful information to
>> +	 * anybody at this point.  Some compilers warn us for
>> +	 * attempting to smuggle a pointer that is about to become
>> +	 * invalid, i.e. &finished.  We clear it here to assure them.
>> +	 */
>> +	slot->finished = NULL;
>>  }

The conclusion is still valid.  The slot->finished member will not
have any useful information when we get here.  It either keeps the
value &finished we stored if the slot wasn't reused, or it has NULL
if it was reused.

> Having said that, it is quite puzzling that GCC thinks it is safe to
> assign a local variable's pointer to a struct that is then accessed
> outside the current file. This would make it easy to copy and use the
> pointer well after the function scope was left. This is _not_ the case in
> Git's source code, but GCC seems that this isn't possible by
> (mis-)interpreting the final `slot->finished = NULL` to mean that the
> `slot->finished = &finished` was safe (because it clearly isn't).

I do not think it is compiler's job to prevent us from assigning to
slot->finished, in fear of somebody downstream copying the pointer
away to a global variable that can be accessed long after we return.

We can store a pointer to an object we malloc() in a member of a
struct, pass the struct to somebody else, who might copy the member
away in a global variable for their own use later, but when they
give control back to us, we may free() the object via the member in
the struct.  The compiler may see that we free() the pointer we stored
in the struct, but it would not warn us against doing so.

> With all that, here is my ACK.

Thanks.

Re: Re* jc/http-clear-finished-pointer

[Ævar Arnfjörð Bjarmason]

On Thu, May 26 2022, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> On Thu, May 26 2022, Junio C Hamano wrote:
>>
>>> * jc/http-clear-finished-pointer (2022-05-24) 1 commit
>>>  - http.c: clear the 'finished' member once we are done with it
>>>
>>>  Meant to go with js/ci-gcc-12-fixes
>>>
>>>  Will merge to 'next'?
>>>  source: <xmqqczgqjr8y.fsf_-_@gitster.g>
>>
>> The end of the proposed commit message says:
>>
>>     [...]Clear the finished member before the control leaves the
>>     function, which has a side effect of unconfusing compilers like
>>     recent GCC 12 that is over-eager to warn against such an assignment.
>>
>> I cannot reproduce this suppressing the warning as noted in past
>> exchanges, it's not affected by this "clear if we set it" pattern. It
>> needs to be unconditionally cleared.
>
> Interesting.  I still have conditional clearing in the tree, though
> I was reasonably sure I got rid of the conditional and made it
> always clear, when I rewrote that part of the log message.  After
> all, I ran "commit --amend" so that I do not forget the issue after
> sending https://lore.kernel.org/git/xmqqleurlt31.fsf@gitster.g/ X-<.
>
> Thanks for catching.  What is queued is not what I intended to
> queue.
>
> But there is one thing that is puzzling.  Ever since this, together
> with the three patches from Dscho for gcc12, got included in 'seen',
> the branch started passing the Windows build that used to complain
> and did not work, so at least with the version of gcc12 used over
> there, it apparently is sufficient to clear only when we are
> responsible for placing an address that is about to become invalid,
> while leaving the pointer we didn't stuff in unmodified.

I didn't find what specific build(s) you were referring to, but perhaps
this is due to an interaction with 9c539d1027d (config.mak.dev:
alternative workaround to gcc 12 warning in http.c, 2022-04-15)?
I.e. with DEVELOPER=1 we'd already get past the warning with
gcc+Windows, presumably (but I haven't confirmed whether the
detect-compiler etc. works the same there).

> As far as I understand, with the most recent analysis by Dscho on
> the http-push codepath, we can return to the loop while the slot is
> holding a different request that is unrelated to ours that has
> already finished without recursively calling run_active_slot(), and
> with the current *(slot->finished)=1 trick, it will successfully
> notify our loop that our request is done, even though slot->in_use
> is set to true back again when it happens.  But by definition, at
> that point, slot->finished is not used by anybody (obviously not by
> us, but also not by the request that is currently using the slot,
> because it hasn't used run_active_slot() and slot->finished is not
> touched by it), so it is safe to unconditionally clear the member.
>
> ----- >8 --------- >8 --------- >8 --------- >8 --------- >8 -----
> Subject: [PATCH v3] http.c: clear the 'finished' member once we are done with it

I see this is in "next" already, as a follow-up we should have a revert
of 9c539d1027d (config.mak.dev: alternative workaround to gcc 12 warning
in http.c, 2022-04-15) which this patch makes redundant.

Re: Re* jc/http-clear-finished-pointer

[Junio C Hamano]

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

> ... as a follow-up we should have a revert
> of 9c539d1027d (config.mak.dev: alternative workaround to gcc 12 warning
> in http.c, 2022-04-15) which this patch makes redundant.

It is a good point that 9c539d10 (config.mak.dev: alternative
workaround to gcc 12 warning in http.c, 2022-04-15) hides problems.
Unfortunatelly https://github.com/git/git/runs/6665825113 dies early
without reaching the point where it touches http.c X-< so we cannot
tell.

I am OK to do a revert of the redundant one on 'next', which should
give us a confirmation that the particular version of GCC12 is OK
with the code we happen to have and letting its -Wdangling-pointer
trigger would not harm us with any other false positives.  Then we
can do the same on the 'master' front with that revert with Dscho's
other three GCC12 fixes (they are fixes, like the "realloc then use"
one) graduating first (which should break http.c) and then the patch
in this thread as separate steps, if we really wanted to.

Thanks.