{"thread":{"id":"58869","subject":"What's cooking in git.git (Nov 2022, #07; Tue, 29)","startedAt":"2022-11-29T09:40:09Z","lastAt":"2022-12-09T14:08:42Z","messageCount":43,"participants":["Junio C Hamano","Glen Choo","Derrick Stolee","Phillip Wood","Ævar Arnfjörð Bjarmason","Jeff King","Taylor Blau","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"468161","messageId":"xmqqsfi22j67.fsf@gitster.g","threadId":"58869","inReplyTo":null,"subject":"What's cooking in git.git (Nov 2022, #07; Tue, 29)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-29T09:40:00Z","receivedAt":"2022-11-29T09:40:09Z","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 future\nrelease). Commits prefixed with '-' are only in 'seen', and aren't\nconsidered \"accepted\" at all. A topic without enough support may be\ndiscarded after a long period of no activity.\n\nThe preview release -rc0 for this cycle has been tagged.\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/doc-synopsis-and-cmd-usage (2022-11-27) 1 commit\n (merged to 'next' on 2022-11-28 at a517ea2f95)\n + i18n: fix command template placeholder format\n\n Doc and message fix.\n source: \n\n\n* ab/fewer-the-index-macros (2022-11-21) 11 commits\n (merged to 'next' on 2022-11-23 at de20206cac)\n + cocci: apply \"pending\" index-compatibility to some \"builtin/*.c\"\n + cache.h & test-tool.h: add & use \"USE_THE_INDEX_VARIABLE\"\n + {builtin/*,repository}.c: add & use \"USE_THE_INDEX_VARIABLE\"\n + cocci: apply \"pending\" index-compatibility to \"t/helper/*.c\"\n + cocci & cache.h: apply variable section of \"pending\" index-compatibility\n + cocci & cache.h: apply a selection of \"pending\" index-compatibility\n + cocci: add a index-compatibility.pending.cocci\n + read-cache API & users: make discard_index() return void\n + cocci & cache.h: remove rarely used \"the_index\" compat macros\n + builtin/{grep,log}.: don't define \"USE_THE_INDEX_COMPATIBILITY_MACROS\"\n + cache.h: remove unused \"the_index\" compat macros\n\n Progress on removing 'the_index' convenience wrappers.\n source: \n\n\n* ah/chainlint-cpuinfo-parse-fix (2022-11-23) 1 commit\n (merged to 'next' on 2022-11-28 at 1e51eafde5)\n + chainlint.pl: fix /proc/cpuinfo regexp\n\n The format of a line in /proc/cpuinfo that describes a CPU on s390x\n looked different from everybody else, and the code in chainlint.pl\n failed to parse it.\n source: \n\n\n* es/locate-httpd-module-location-in-test (2022-11-22) 1 commit\n (merged to 'next' on 2022-11-23 at dfa19a744f)\n + lib-httpd: extend module location auto-detection\n\n Add one more candidate directory that may house httpd modules while\n running tests.\n source: \n\n\n* ew/prune-with-missing-objects-pack (2022-11-21) 1 commit\n (merged to 'next' on 2022-11-23 at bd328c5d01)\n + prune: quiet ENOENT on missing directories\n\n \"git prune\" may try to iterate over .git/objects/pack for trash\n files to remove in it, and loudly fail when the directory is\n missing, which is not necessary. The command has been taught to\n ignore such a failure.\n source: <20221119201213.2398081-1-e@80x24.org>\n\n\n* gc/resolve-alternate-symlinks (2022-11-25) 1 commit\n (merged to 'next' on 2022-11-28 at 509d2005aa)\n + object-file: use real paths when adding alternates\n\n Resolve symbolic links when processing the locations of alternate\n object stores, since failing to do so can lead to confusing and buggy\n behavior.\n source: \n\n\n* jh/trace2-timers-and-counters (2022-11-25) 1 commit\n (merged to 'next' on 2022-11-28 at 2fa64103b7)\n + trace2 tests: guard pthread test with \"PTHREAD\"\n\n Test fix.\n source: \n\n\n* jk/parse-object-type-mismatch (2022-11-22) 3 commits\n (merged to 'next' on 2022-11-22 at 69dc60536b)\n + parse_object(): simplify blob conditional\n (merged to 'next' on 2022-11-18 at 1ee133a089)\n + parse_object(): check on-disk type of suspected blob\n + parse_object(): drop extra \"has\" check before checking object type\n (this branch is used by ab/tag-object-type-errors.)\n\n `parse_object()` hardening when checking for the existence of a\n suspected blob object.\n source: \n\n\n* jx/ci-ubuntu-fix (2022-11-27) 4 commits\n (merged to 'next' on 2022-11-28 at 2eaa5b6c61)\n + ci: install python on ubuntu\n + ci: use the same version of p4 on both Linux and macOS\n + ci: remove the pipe after \"p4 -V\" to catch errors\n + github-actions: run gcc-8 on ubuntu-20.04 image\n\n Adjust the GitHub CI to newer ubuntu release.\n source: <20221124153934.12470-1-worldhello.net@gmail.com>\n\n\n* km/merge-recursive-typofix (2022-11-27) 1 commit\n (merged to 'next' on 2022-11-28 at cb6c488e9c)\n + merge-recursive: fix variable typo in error message\n\n Fix an old typo in an error message.\n source: <20221125173745.738643-1-kyle@kyleam.com>\n\n\n* pw/config-int-parse-fixes (2022-11-09) 3 commits\n (merged to 'next' on 2022-11-23 at 06ee2fff7b)\n + git_parse_signed(): avoid integer overflow\n + config: require at least one digit when parsing numbers\n + git_parse_unsigned: reject negative values\n\n Assorted fixes of parsing end-user input as integers.\n source: \n\n\n* rs/list-objects-filter-leakfix (2022-11-21) 1 commit\n (merged to 'next' on 2022-11-23 at 1e148fc7d2)\n + list-objects-filter: plug combine_filter_data leak\n\n Leakfix.\n source: \n\n\n* sg/plug-line-log-leaks (2022-11-02) 3 commits\n (merged to 'next' on 2022-11-23 at 5d2e1b065b)\n + diff.c: use diff_free_queue()\n + line-log: free the diff queues' arrays when processing merge commits\n + line-log: free diff queue when processing non-merge commits\n\n A handful of leaks in the line-log machinery have been plugged.\n source: <20221102220142.574890-1-szeder.dev@gmail.com>\n\n\n* zk/push-use-bitmaps (2022-11-22) 1 commit\n (merged to 'next' on 2022-11-23 at 2ce20bcaf5)\n + t5516: fail to run in verbose mode\n\n Test fix.\n source: <20221121134040.12260-1-worldhello.net@gmail.com>\n\n--------------------------------------------------\n[New Topics]\n\n* rr/status-untracked-advice (2022-11-25) 1 commit\n - status: modernize git-status \"slow untracked files\" advice\n\n The advice message given by \"git status\" when it takes long time to\n enumerate untracked paths has been updated.\n\n Will merge to 'next'?\n source: \n\n\n* sa/git-var-empty (2022-11-27) 2 commits\n - var: allow GIT_EDITOR to return null\n - var: do not print usage() with a correct invocation\n\n \"git var UNKNOWN_VARIABLE\" and \"git var VARIABLE\" with the variable\n given an empty value used to behave identically. Now the latter\n just gives an empty output, while the former still gives an error\n message.\n\n Will merge to 'next'?\n source: \n\n\n* jx/t1301-updates (2022-11-29) 3 commits\n - t1301: do not change $CWD in \"shared=all\" test case\n - t1301: use test_when_finished for cleanup\n - t1301: fix wrong template dir for git-init\n\n Test updates.\n\n Seems to break CI.\n source: <20221127145130.16155-1-worldhello.net@gmail.com>\n\n\n* km/send-email-with-v-reroll-count (2022-11-27) 1 commit\n - send-email: relay '-v N' to format-patch\n\n \"git send-email -v 3\" used to be expanded to \"git send-email\n --validate 3\" when the user meant to pass them down to\n \"format-patch\", which has been corrected.\n\n Seems to break CI.\n source: <87edtp5uws.fsf@kyleam.com>\n\n\n* ps/gnumake-4.4-fix (2022-11-28) 1 commit\n (merged to 'next' on 2022-11-29 at 1151bc06fc)\n + Makefile: avoid multiple patterns when recipes generate one file\n\n Adjust Makefile for GNU make 4.4\n\n Will cook in 'next'.\n source: <20221127224251.2508200-2-psmith@gnu.org>\n\n\n* so/diff-merges-more (2022-11-28) 5 commits\n - diff-merges: issue warning on lone '-m' option\n - diff-merges: support list of values for --diff-merges\n - diff-merges: implement log.diffMergesForce config\n - diff-merges: implement log.diffMerges-m-imply-p config\n - diff-merges: implement [no-]hide option and log.diffMergesHide config\n\n Assorted updates to \"--diff-merges=X\" option.\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3560918726\n source: <20221127093721.31012-1-sorganov@gmail.com>\n\n--------------------------------------------------\n[Stalled]\n\n* ms/sendemail-validate-headers (2022-11-11) 1 commit\n . Expose header information to git-send-email's sendemail-validate hook\n\n Expecting a reroll.\n Appears to break t9001 completely?\n source: <20221111194223.644845-2-michael.strawbridge@amd.com>\n\n\n* hl/archive-recursive (2022-10-19) 10 commits\n . fixup! archive: add tests for git archive --recurse-submodules\n . archive: add tests for git archive --recurse-submodules\n . archive: add --recurse-submodules to git-archive command\n . archive: remove global repository from archive_args\n . archive: pass repo objects to write_archive handlers\n . tree: add repository parameter to read_tree_fn_t\n . tree: handle submodule case for read_tree_at properly\n . tree: increase test coverage for tree.c\n . tree: update cases to use repo_ tree methods\n . tree: do not use the_repository for tree traversal methods.\n\n \"git archive\" has been taught \"--recurse-submodules\" option to\n create a tarball that includes contents from submodules.\n\n Expecting a reroll.\n Seems to break win+VS test(8).\n cf. https://github.com/git/git/actions/runs/3293333066 whose only\n difference from https://github.com/git/git/actions/runs/3293553109\n is the inclusion of this topic.\n source: \n\n\n* pw/test-todo (2022-10-06) 3 commits\n . test_todo: allow [verbose] test as the command\n . test_todo: allow [!] grep as the command\n . tests: add test_todo() to mark known breakages\n\n RFC for test framework improvement.\n\n Needs review.\n source: \n\n\n* cw/submodule-status-in-parallel (2022-11-08) 6 commits\n - diff-lib: parallelize run_diff_files for submodules\n - diff-lib: refactor match_stat_with_submodule\n - submodule: move status parsing into function\n - submodule: strbuf variable rename\n - run-command: add duplicate_output_fn to run_processes_parallel_opts\n - Merge branch 'ab/run-hook-api-cleanup' into cw/submodule-status-in-parallel\n\n Allow the internal \"diff-files\" engine to run \"how has this\n submodule changed?\" in parallel to speed up \"git status\".\n\n Waiting for review.\n source: <20221020232532.1128326-1-calvinwan@google.com>\n\n\n* js/bisect-in-c (2022-08-30) 17 commits\n . bisect: no longer try to clean up left-over `.git/head-name` files\n . bisect: remove Cogito-related code\n . Turn `git bisect` into a full built-in\n . bisect: move even the command-line parsing to `bisect--helper`\n . bisect--helper: make `state` optional\n . bisect--helper: calling `bisect_state()` without an argument is a bug\n . bisect: avoid double-quoting when printing the failed command\n . bisect run: fix the error message\n . bisect: verify that a bogus option won't try to start a bisection\n . bisect--helper: migrate to OPT_SUBCOMMAND()\n . bisect--helper: make the order consistently `argc, argv`\n . bisect--helper: make `terms` an explicit singleton\n . bisect--helper: simplify exit code computation\n . bisect--helper: really retire `--bisect-autostart`\n . bisect--helper: really retire --bisect-next-check\n . bisect--helper: retire the --no-log option\n . Merge branch 'sg/parse-options-subcommand' into js/bisect-in-c\n\n Final bits of \"git bisect.sh\" have been rewritten in C.\n\n Temporarily ejected from 'seen'. Waiting for a response on how this\n topic interacts with 'dd/git-bisect-builtin'.\n cf. \n source: \n\n\n* od/ci-use-checkout-v3-when-applicable (2022-10-10) 2 commits\n . ci(main): linux32 uses actions/checkout@v2\n . ci(main): upgrade actions/checkout to v3\n\n Attempt to update GitHub CI to use actions/checkout@v3\n\n Expecting a reroll.\n Seems to break the CI completely.\n source: \n\n\n* ed/fsmonitor-inotify (2022-11-25) 6 commits\n - fsmonitor: update doc for Linux\n - fsmonitor: test updates\n - fsmonitor: enable fsmonitor for Linux\n - fsmonitor: implement filesystem change listener for Linux\n - fsmonitor: determine if filesystem is local or remote\n - fsmonitor: prepare to share code between Mac OS and Linux\n\n Bundled fsmonitor for Linux using inotify API.\n\n Needs review on the updated round.\n source: \n\n\n* ag/merge-strategies-in-c (2022-08-10) 14 commits\n . sequencer: use the \"octopus\" strategy without forking\n . sequencer: use the \"resolve\" strategy without forking\n . merge: use the \"octopus\" strategy without forking\n . merge: use the \"resolve\" strategy without forking\n . merge-octopus: rewrite in C\n . merge-recursive: move better_branch_name() to merge.c\n . merge-resolve: rewrite in C\n . merge-one-file: rewrite in C\n . update-index: move add_cacheinfo() to read-cache.c\n . merge-index: add a new way to invoke `git-merge-one-file'\n . merge-index: drop the index\n . merge-index: libify merge_one_path() and merge_all()\n . t6060: add tests for removed files\n . t6060: modify multiple files to expose a possible issue with merge-index\n\n An attempt to rewrite remaining merge strategies from shell to C.\n\n Needs more work.\n At the minimum, we should lose 11/14 and possibly 08/14.\n cf. \n source: <20220809185429.20098-1-alban.gruin@gmail.com>\n\n\n* es/doc-creation-factor-fix (2022-07-28) 2 commits\n . range-diff: clarify --creation-factor=\n . format-patch: clarify --creation-factor=\n\n Expecting a reroll by somebody more familiar with the logic\n cf. \n source: <7229p500-p2r4-on87-6802-8o90s36rr3s4@tzk.qr>\n\n\n* cw/remote-object-info (2022-08-13) 7 commits\n . SQUASH???\n . cat-file: add remote-object-info to batch-command\n . transport: add client support for object-info\n . serve: advertise object-info feature\n . protocol-caps: initialization bug fix\n . fetch-pack: move fetch initialization\n . fetch-pack: refactor packet writing\n\n A client component to talk with the object-info endpoint.\n\n Expecting a reroll.\n Under SANITIZE=address, t1006-cat-file.sh finds a breakage.\n cf. <20220728230210.2952731-1-calvinwan@google.com>\n cf. \n source: <20220728230210.2952731-1-calvinwan@google.com>\n\n--------------------------------------------------\n[Cooking]\n\n* ab/config-multi-and-nonbool (2022-11-27) 9 commits\n - for-each-repo: with bad config, don't conflate and \n - config API: add \"string\" version of *_value_multi(), fix segfaults\n - config API users: test for *_get_value_multi() segfaults\n - for-each-repo: error on bad --config\n - config API: have *_multi() return an \"int\" and take a \"dest\"\n - versioncmp.c: refactor config reading next commit\n - config tests: add \"NULL\" tests for *_get_value_multi()\n - config tests: cover blind spots in git_die_config() tests\n - for-each-repo tests: test bad --config keys\n\n Assorted config API updates.\n\n Waiting for review.\n source: \n\n\n* yn/git-jump-emacs (2022-11-27) 3 commits\n (merged to 'next' on 2022-11-29 at d0960938a0)\n + git-jump: invoke emacs/emacsclient\n + git-jump: move valid-mode check earlier\n + git-jump: add an optional argument '--stdout'\n\n \"git jump\" (in contrib/) learned to present the \"quickfix list\" to\n its standard output (instead of letting it consumed by the editor\n it invokes), and learned to also drive emacs/emacsclient.\n\n Will cook in 'next'.\n source: \n\n\n* sa/cat-file-mailmap--batch-check (2022-11-21) 2 commits\n - cat-file: add mailmap support to --batch-check option\n - cat-file: add mailmap support to -s option\n\n 'cat-file' gains mailmap support for its '--batch-check' and '-s'\n options.\n\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3560918726\n source: <20221120074852.121346-1-siddharthasthana31@gmail.com>\n\n\n* ab/tag-object-type-errors (2022-11-22) 5 commits\n - tag: don't emit potentially incorrect \"object is a X, not a Y\"\n - tag: don't misreport type of tagged objects in errors\n - object tests: add test for unexpected objects in tags\n - object-file.c: free the \"t.tag\" in check_tag()\n - Merge branch 'jk/parse-object-type-mismatch' into ab/tag-object-type-errors\n\n Hardening checks around mismatched object types when one of those\n objects is a tag.\n\n Needs review.\n source: \n\n\n* js/range-diff-mbox (2022-11-23) 1 commit\n - range-diff: support reading mbox files\n\n 'git range-diff' gained support for reading either side from an .mbox\n file instead of a revision range.\n\n Expecting review responses.\n source: \n\n\n* rj/branch-copy-and-rename (2022-11-17) 2 commits\n - branch: clear target branch configuration before copying or renaming\n - branch: force-copy a branch to itself via @{-1} is a no-op\n\n Fix a pair of bugs in 'git branch'.\n\n Waiting for review discussion to settle.\n source: \n\n\n* tl/ls-tree--pattern (2022-11-17) 6 commits\n - ls-tree: introduce '--pattern' option\n - ls-tree: introduce 'match_pattern()' function\n - ls-tree: improving cohension in the print code\n - ls-tree: optimize params of 'show_tree_common_default_long()'\n - t3104: remove shift code in 'test_ls_tree_format'\n - ls-tree: cleanup the redundant SPACE\n\n A synonym for \"ls-tree | grep \", \"ls-tree\n --pattern=\" was introduced.\n\n Waiting for review response, but leaning negative.\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3560918726\n source: <20221117113023.65865-1-tenglong.tl@alibaba-inc.com>\n\n\n* tr/am--no-verify (2022-11-29) 1 commit\n - am: Allow passing --no-verify flag\n\n Conditionally skip the pre-applypatch and applypatch-msg hooks when\n applying patches with 'git am'.\n\n Expecting a (hopefully final) reroll.\n source: <20221128174825.1510407-1-thierry.reding@gmail.com>\n\n\n* ab/remove--super-prefix (2022-11-21) 12 commits\n . fetch: rename \"--submodule-prefix\" to \"--super-prefix\"\n . read-tree: add \"--super-prefix\" option, eliminate global\n . submodule--helper: convert \"{update,clone}\" to their own \"--super-prefix\"\n . submodule--helper: convert \"status\" to its own \"--super-prefix\"\n . submodule--helper: convert \"sync\" to its own \"--super-prefix\"\n . submodule--helper: convert \"foreach\" to its own \"--super-prefix\"\n . submodule--helper: don't use global --super-prefix in \"absorbgitdirs\"\n . submodule.c & submodule--helper: pass along \"super_prefix\" param\n . read-tree + fetch tests: test failing \"--super-prefix\" interaction\n . Merge branch 'ab/submodule-no-abspath' into ab/remove--super-prefix\n (merged to 'next' on 2022-11-18 at 34d0accc7b)\n + submodule--helper absorbgitdirs: no abspaths in \"Migrating git...\"\n . Merge branch 'ab/submodule-helper-prep-only' into ab/remove--super-prefix\n\n Remove the top-level `--super-prefix` option.\n source: \n\n\n* ab/submodule-no-abspath (2022-11-23) 2 commits\n (merged to 'next' on 2022-11-23 at 97b6096e7c)\n + submodule absorbgitdirs: use relative and paths\n (merged to 'next' on 2022-11-18 at 34d0accc7b)\n + submodule--helper absorbgitdirs: no abspaths in \"Migrating git...\"\n\n Remove an absolute path in the \"Migrating git directory\" message.\n\n Will cook in 'next', but leaning negative.\n source: \n source: \n\n\n* ew/format-patch-mboxrd (2022-11-14) 1 commit\n - format-patch: add --mboxrd alias for --pretty=mboxrd\n\n Teach `format-patch` a convenient alias for `--pretty=mboxrd`.\n\n Waiting for discussion to settle.\n source: <20221114094114.18986-1-e@80x24.org>\n\n\n* js/drop-mingw-test-cmp (2022-11-14) 2 commits\n - tests(mingw): avoid very slow `mingw_test_cmp`\n - t0021: use Windows-friendly `pwd`\n\n Use `git diff --no-index` as a test_cmp on Windows.\n\n Waiting for review response.\n source: \n\n\n* mc/switch-advice (2022-11-09) 1 commit\n - po: use `switch` over `checkout` in error message\n\n Use 'switch' instead of 'checkout' in an error message.\n\n Waiting for review response.\n source: \n\n\n* rs/multi-filter-args (2022-11-21) 3 commits\n - Revert \"pack-objects: lazily set up \"struct rev_info\", don't leak\"\n - t5317: demonstrate failure to handle multiple --filter options\n - t5317: stop losing return codes of git ls-files\n\n Fix a bug where `pack-objects` would not respect multiple `--filter`\n arguments when invoked directly.\n source: \n\n\n* ab/various-leak-fixes (2022-11-21) 16 commits\n (merged to 'next' on 2022-11-21 at eff484a27c)\n + built-ins: use free() not UNLEAK() if trivial, rm dead code\n + revert: fix parse_options_concat() leak\n + cherry-pick: free \"struct replay_opts\" members\n + rebase: don't leak on \"--abort\"\n + connected.c: free the \"struct packed_git\"\n + sequencer.c: fix \"opts->strategy\" leak in read_strategy_opts()\n + ls-files: fix a --with-tree memory leak\n + revision API: call graph_clear() in release_revisions()\n + unpack-file: fix ancient leak in create_temp_file()\n + built-ins & libs & helpers: add/move destructors, fix leaks\n + dir.c: free \"ident\" and \"exclude_per_dir\" in \"struct untracked_cache\"\n + read-cache.c: clear and free \"sparse_checkout_patterns\"\n + commit: discard partial cache before (re-)reading it\n + {reset,merge}: call discard_index() before returning\n + tests: mark tests as passing with SANITIZE=leak\n + Merge branch 'pw/rebase-no-reflog-action' into ab/various-leak-fixes\n\n Various leak fixes.\n\n Will cook in 'next'.\n source: \n\n\n* aw/complete-case-insensitive (2022-11-07) 2 commits\n - completion: add case-insensitive match of pseudorefs\n - completion: add optional ignore-case when matching refs\n\n Introduce a case insensitive mode to the Bash completion helpers.\n\n Waiting for review.\n Seems to break CI.\n source: \n\n\n* dd/git-bisect-builtin (2022-11-15) 13 commits\n (merged to 'next' on 2022-11-15 at e16e754058)\n + bisect; remove unused \"git-bisect.sh\" and \".gitignore\" entry\n (merged to 'next' on 2022-11-14 at fc304fb52f)\n + Turn `git bisect` into a full built-in\n + bisect--helper: log: allow arbitrary number of arguments\n + bisect--helper: handle states directly\n + bisect--helper: emit usage for \"git bisect\"\n + bisect test: test exit codes on bad usage\n + bisect--helper: identify as bisect when report error\n + bisect-run: verify_good: account for non-negative exit status\n + bisect run: keep some of the post-v2.30.0 output\n + bisect: fix output regressions in v2.30.0\n + bisect: refactor bisect_run() to match CodingGuidelines\n + bisect tests: test for v2.30.0 \"bisect run\" regressions\n + Merge branch 'dd/bisect-helper-subcommand' into dd/git-bisect-builtin\n\n `git bisect` becomes a builtin.\n\n Will cook in 'next'.\n source: \n\n\n* ds/packed-refs-v2 (2022-11-07) 30 commits\n - refs: skip hashing when writing packed-refs v2\n - p1401: create performance test for ref operations\n - ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some builds\n - t*: skip packed-refs v2 over http tests\n - t3210: require packed-refs v1 for some tests\n - t5502: add PACKED_REFS_V1 prerequisite\n - t5312: allow packed-refs v2 format\n - t1409: test with packed-refs v2\n - packed-backend: create GIT_TEST_PACKED_REFS_VERSION\n - packed-refs: write prefix chunks\n - packed-refs: read optional prefix chunks\n - packed-refs: read file format v2\n - packed-refs: write file format version 2\n - packed-backend: create shell of v2 writes\n - config: add config values for packed-refs v2\n - packed-backend: create abstraction for writing refs\n - packed-backend: extract iterator/updates merge\n - packed-backend: extract add_write_error()\n - refs: extract packfile format to new file\n - chunk-format: parse trailing table of contents\n - chunk-format: allow trailing table of contents\n - chunk-format: store chunk offset during write\n - chunk-format: document trailing table of contents\n - chunk-format: number of chunks is optional\n - refs: allow loose files without packed-refs\n - repository: wire ref extensions to ref backends\n - config: fix multi-level bulleted list\n - extensions: add refFormat extension\n - read-cache: add index.computeHash config option\n - hashfile: allow skipping the hash function\n\n Waiting for review.\n source: \n\n\n* ja/worktree-orphan (2022-11-10) 2 commits\n - worktree add: add --orphan flag\n - worktree add: Include -B in usage docs\n\n 'git worktree add' learned how to create a worktree based on an\n orphaned branch with `--orphan`.\n\n Expecting another round?\n source: <20221110233137.10414-1-jacobabel@nullpo.dev>\n\n\n* tb/ci-concurrency (2022-11-08) 1 commit\n - ci: avoid unnecessary builds\n\n Avoid unnecessary builds in CI, with settings configured in\n ci-config.\n\n Waiting for review.\n source: \n\n\n* tl/notes--blankline (2022-11-09) 5 commits\n - notes.c: introduce \"--no-blank-line\" option\n - notes.c: provide tips when target and append note are both empty\n - notes.c: drop unreachable code in 'append_edit()'\n - notes.c: cleanup for \"designated init\" and \"char ptr init\"\n - notes.c: cleanup 'strbuf_grow' call in 'append_edit'\n\n 'git notes append' was taught '--[no-]blank-line' to conditionally\n add a LF between a new and existing note.\n\n Expecting a reroll.\n source: \n\n\n* ds/bundle-uri-4 (2022-11-16) 9 commits\n - clone: unbundle the advertised bundles\n - bundle-uri: download bundles from an advertised list\n - bundle-uri: allow relative URLs in bundle lists\n - strbuf: introduce strbuf_strip_file_from_path()\n - bundle-uri client: add boolean transfer.bundleURI setting\n - bundle-uri: serve bundle.* keys from config\n - bundle-uri client: add helper for testing server\n - bundle-uri client: add minimal NOOP client\n - protocol v2: add server-side \"bundle-uri\" skeleton\n\n Bundle URIs part 4.\n\n Waiting for review.\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3560918726\n source: \n\n\n* tl/pack-bitmap-absolute-paths (2022-11-29) 4 commits\n - pack-bitmap.c: trace bitmap ignore logs when midx-bitmap is found\n - pack-bitmap.c: break out of the bitmap loop early if not tracing\n (merged to 'next' on 2022-11-14 at 34eb0ea05a)\n + pack-bitmap.c: avoid exposing absolute paths\n + pack-bitmap.c: remove unnecessary \"open_pack_index()\" calls\n\n The pack-bitmap machinery is taught to log the paths of redundant\n bitmap(s) to trace2 instead of stderr.\n\n Will merge to 'next'.\n source: \n\n\n* ab/cmake-nix-and-ci (2022-11-04) 14 commits\n (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n + cmake: copy over git-p4.py for t983[56] perforce test\n + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n + cmake: increase test timeout on Windows only\n + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n + Makefile + cmake: use environment, not GIT-BUILD-DIR\n + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n + cmake: don't copy chainlint.pl to build directory\n + cmake: update instructions for portable CMakeLists.txt\n + cmake: use \"-S\" and \"-B\" to specify source and build directories\n + cmake: don't invoke msgfmt with --statistics\n\n Fix assorted issues with CTest on *nix machines.\n\n Will cook in 'next'.\n source: \n\n\n* ab/make-bin-wrappers (2022-10-31) 4 commits\n . Makefile: simplify $(test_bindir_programs) rule by splitting it up\n . Makefile: rename \"test_bindir_programs\" variable, pre-declare\n . Makefile: define \"TEST_{PROGRAM,OBJS}\" variables earlier\n . Makefile: factor sed-powered '#!/bin/sh' munging into a variable\n\n Resolve issues with the bin-wrappers/% rules where \"make\n bin-wrappers/git\" would generate the script but not \"git\" itself.\n\n Waiting for review discussion to settle, but leaning negative.\n source: \n\n\n* kz/merge-tree-merge-base (2022-11-25) 3 commits\n (merged to 'next' on 2022-11-25 at 298ca8e2e8)\n + docs: fix description of the `--merge-base` option\n (merged to 'next' on 2022-11-14 at 76d48ae21f)\n + merge-tree.c: allow specifying the merge-base when --stdin is passed\n + merge-tree.c: add --merge-base= option\n\n \"merge-tree\" learns a new `--merge-base` option.\n\n Will cook in 'next'.\n source: \n source: \n\n\n* po/pretty-hard-trunc (2022-11-13) 1 commit\n - pretty-formats: add hard truncation, without ellipsis, options\n\n Add a new pretty format which truncates without ellipsis.\n\n Waiting for review.\n source: <20221112143616.1429-1-philipoakley@iee.email>\n\n\n* rr/long-status-advice (2022-11-15) 1 commit\n - status: long status advice adapted to recent capabilities\n\n The advice message emitted by a slow \"status\" run is amended to\n mention fsmonitor.\n\n Waiting for reviewer feedback on the updated round.\n source: \n\n\n* gc/submodule-clone-update-with-branches (2022-10-30) 8 commits\n - clone, submodule update: create and check out branches\n - submodule--helper: remove update_data.suboid\n - submodule update: refactor update targets\n - submodule: return target of submodule symref\n - t5617: drop references to remote-tracking branches\n - submodule--helper clone: create named branch\n - repo-settings: add submodule_propagate_branches\n - clone: teach --detach option\n\n \"git clone --recurse-submodules\" and \"git submodule update\" learns\n to honor the \"propagete branches\" option.\n\n Waiting for review on the updated round.\n source: \n\n\n* cc/filtered-repack (2022-11-23) 2 commits\n - repack: add --filter= option\n - pack-objects: allow --filter without --stdout\n\n \"git repack\" learns to discard objects that ought to be retrievable\n again from the promissor remote.\n\n Needs review.\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3560918726\n source: <20221122175150.366828-1-christian.couder@gmail.com>\n\n\n* mc/credential-helper-auth-headers (2022-11-02) 11 commits\n - t5556: add HTTP authentication tests\n - test-http-server: add simple authentication\n - test-http-server: pass Git requests to http-backend\n - test-http-server: add HTTP request parsing\n - test-http-server: add HTTP error response function\n - test-http-server: add stub HTTP server test helper\n - http: set specific auth scheme depending on credential\n - http: move proactive auth to first slot creation\n - http: store all request headers on active_request_slot\n - credential: add WWW-Authenticate header to cred requests\n - http: read HTTP WWW-Authenticate response headers\n\n Extending credential helper protocol.\n\n Needs review.\n Seems to break CI.\n cf. https://github.com/git/git/actions/runs/3562942886/jobs/5985179202\n source: \n"},{"id":"468202","messageId":"kl6lsfi1d1tf.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58869","inReplyTo":"xmqqsfi22j67.fsf@gitster.g","subject":"ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-11-29T18:59:24Z","receivedAt":"2022-11-29T19:01:23Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> Here are the topics that have been cooking in my tree. Commits\n> prefixed with '+' are in 'next' (being in 'next' is a sign that a\n> topic is stable enough to be used and are candidate to be in a future\n> release). Commits prefixed with '-' are only in 'seen', and aren't\n> considered \"accepted\" at all. A topic without enough support may be\n> discarded after a long period of no activity.\n>\n> The preview release -rc0 for this cycle has been tagged.\n\n[...]\n\n> * ab/remove--super-prefix (2022-11-21) 12 commits\n> . fetch: rename \"--submodule-prefix\" to \"--super-prefix\"\n> . read-tree: add \"--super-prefix\" option, eliminate global\n> . submodule--helper: convert \"{update,clone}\" to their own \"--super-prefix\"\n> . submodule--helper: convert \"status\" to its own \"--super-prefix\"\n> . submodule--helper: convert \"sync\" to its own \"--super-prefix\"\n> . submodule--helper: convert \"foreach\" to its own \"--super-prefix\"\n> . submodule--helper: don't use global --super-prefix in \"absorbgitdirs\"\n> . submodule.c & submodule--helper: pass along \"super_prefix\" param\n> . read-tree + fetch tests: test failing \"--super-prefix\" interaction\n> . Merge branch 'ab/submodule-no-abspath' into ab/remove--super-prefix\n> (merged to 'next' on 2022-11-18 at 34d0accc7b)\n> + submodule--helper absorbgitdirs: no abspaths in \"Migrating git...\"\n> . Merge branch 'ab/submodule-helper-prep-only' into ab/remove--super-prefix\n>\n> Remove the top-level `--super-prefix` option.\n> source: \n\nHm, it looks like ab/remove--super-prefix missed the preview release..\nPer the discussion ending at [1] I think my one-patch fix to \"git\nfetch\" [2] should have made it into the release (it's pretty low-risk\nand doesn't introduce too much churn to ab/remove--super-prefix). Is it\ntoo late for that?\n\n[1] https://lore.kernel.org/git/221117.86y1s9h2q5.gmgdl@evledraar.gmail.com\n[2] https://lore.kernel.org/git/pull.1378.git.git.1668210935360.gitgitgadget@gmail.com\n"},{"id":"468204","messageId":"kl6lpmd5d1e8.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58869","inReplyTo":"xmqqsfi22j67.fsf@gitster.g","subject":"Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-11-29T19:08:31Z","receivedAt":"2022-11-29T19:08:38Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits\n> - clone, submodule update: create and check out branches\n> - submodule--helper: remove update_data.suboid\n> - submodule update: refactor update targets\n> - submodule: return target of submodule symref\n> - t5617: drop references to remote-tracking branches\n> - submodule--helper clone: create named branch\n> - repo-settings: add submodule_propagate_branches\n> - clone: teach --detach option\n>\n> \"git clone --recurse-submodules\" and \"git submodule update\" learns\n> to honor the \"propagete branches\" option.\n>\n> Waiting for review on the updated round.\n> source: \n\nJonathan left adequate feedback on the updated round, and we had decided\non a direction for the reroll in [1].\n\nHowever, Calvin is also looking at how we could parallelize worktree\nupdates to speed up \"git clone --recurse-submodules\", so the Google\nfolks are taking an even bigger step back to figure out how worktree\nupdating should look, which will probably end in a different approach\nfrom [1], but it should answer the questions on that thread about \"git\ncheckout\" with branches.\n\n[1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com\n"},{"id":"468208","messageId":"b223202b-0ae2-bdb5-e9d6-21a6321a9e31@github.com","threadId":"58869","inReplyTo":"xmqqsfi22j67.fsf@gitster.g","subject":"ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-11-29T21:16:04Z","receivedAt":"2022-11-29T21:16:13Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/29/2022 4:40 AM, Junio C Hamano wrote:\n\n> * ds/bundle-uri-4 (2022-11-16) 9 commits\n> - clone: unbundle the advertised bundles\n> - bundle-uri: download bundles from an advertised list\n> - bundle-uri: allow relative URLs in bundle lists\n> - strbuf: introduce strbuf_strip_file_from_path()\n> - bundle-uri client: add boolean transfer.bundleURI setting\n> - bundle-uri: serve bundle.* keys from config\n> - bundle-uri client: add helper for testing server\n> - bundle-uri client: add minimal NOOP client\n> - protocol v2: add server-side \"bundle-uri\" skeleton\n> \n> Bundle URIs part 4.\n> \n> Waiting for review.\n> Seems to break CI.\n> cf. https://github.com/git/git/actions/runs/3560918726\n> source: \n\nThanks for pointing this out. I'll be sure to fix the relevant\nfailures in linux-TEST-vars [1] before the next version.\n\n[1] https://github.com/git/git/actions/runs/3560918726/jobs/5981349269#step:4:1854\n\nThanks,\n-Stolee\n"},{"id":"468222","messageId":"xmqqilix150o.fsf@gitster.g","threadId":"58869","inReplyTo":"kl6lsfi1d1tf.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-30T03:43:19Z","receivedAt":"2022-11-30T03:44:39Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Glen Choo writes:\n\n> Hm, it looks like ab/remove--super-prefix missed the preview release..\n> Per the discussion ending at [1] I think my one-patch fix to \"git\n> fetch\" [2] should have made it into the release (it's pretty low-risk\n> and doesn't introduce too much churn to ab/remove--super-prefix). Is it\n> too late for that?\n\nNobody seemed to have commented on [2]. Is this fixing recent\nregressions, or is it more like addressing an \"if it hurts, do not\ndo it then\" problem?\n\nThe fact alone that these questions need to be asked _now_ is a good\nindication that it is way too late for this cycle, I would have to\nsay.\n\nThanks.\n\n\n"},{"id":"468223","messageId":"xmqqedtl14xr.fsf@gitster.g","threadId":"58869","inReplyTo":"kl6lpmd5d1e8.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-11-30T03:45:04Z","receivedAt":"2022-11-30T03:46:41Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Glen Choo writes:\n\n> Junio C Hamano writes:\n>\n>> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits\n>> - clone, submodule update: create and check out branches\n>> - submodule--helper: remove update_data.suboid\n>> - submodule update: refactor update targets\n>> - submodule: return target of submodule symref\n>> - t5617: drop references to remote-tracking branches\n>> - submodule--helper clone: create named branch\n>> - repo-settings: add submodule_propagate_branches\n>> - clone: teach --detach option\n>>\n>> \"git clone --recurse-submodules\" and \"git submodule update\" learns\n>> to honor the \"propagete branches\" option.\n>>\n>> Waiting for review on the updated round.\n>> source: \n>\n> Jonathan left adequate feedback on the updated round, and we had decided\n> on a direction for the reroll in [1].\n>\n> However, Calvin is also looking at how we could parallelize worktree\n> updates to speed up \"git clone --recurse-submodules\", so the Google\n> folks are taking an even bigger step back to figure out how worktree\n> updating should look, which will probably end in a different approach\n> from [1], but it should answer the questions on that thread about \"git\n> checkout\" with branches.\n>\n> [1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com\n\nThanks for a status report.\n\nSo it sounds more like \"discard for now\" to expect a fresh restart.\n\n\n"},{"id":"468236","messageId":"544fff8a-7d56-57a1-00a3-d1a9302e227c@dunelm.org.uk","threadId":"58869","inReplyTo":"xmqqsfi22j67.fsf@gitster.g","subject":"ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-30T09:57:29Z","receivedAt":"2022-11-30T09:57:37Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 29/11/2022 09:40, Junio C Hamano wrote:\n> * ab/cmake-nix-and-ci (2022-11-04) 14 commits\n> (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n> + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n> + cmake: copy over git-p4.py for t983[56] perforce test\n> + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n> + cmake: increase test timeout on Windows only\n> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n> + Makefile + cmake: use environment, not GIT-BUILD-DIR\n> + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n> + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n> + cmake: don't copy chainlint.pl to build directory\n> + cmake: update instructions for portable CMakeLists.txt\n> + cmake: use \"-S\" and \"-B\" to specify source and build directories\n> + cmake: don't invoke msgfmt with --statistics\n> \n> Fix assorted issues with CTest on *nix machines.\n\nIf that's all this series did then I think it would be fine. However it \nalso makes changes to test-lib.sh to hard code the build directory in an \nattempt to remove GIT-BUILD-DIR. I'm not convinced that is an \nimprovement on the status quo. As I mentioned previously [1] I think the \nnon-*nix related patches could do with a review from the windows folks \nbefore this hits master.\n\nBest Wishes\n\nPhillip\n\n[1] \nhttps://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/\n\n> Will cook in 'next'.\n> source: \n"},{"id":"468238","messageId":"b50066fb-e958-baa8-2cd9-6e7f964f7e30@dunelm.org.uk","threadId":"58869","inReplyTo":"xmqqsfi22j67.fsf@gitster.g","subject":"Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-11-30T10:02:26Z","receivedAt":"2022-11-30T10:02:34Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 29/11/2022 09:40, Junio C Hamano wrote:\n> * pw/test-todo (2022-10-06) 3 commits\n> . test_todo: allow [verbose] test as the command\n> . test_todo: allow [!] grep as the command\n> . tests: add test_todo() to mark known breakages\n> \n> RFC for test framework improvement.\n> \n> Needs review.\n> source: \n\nI'm planning to re-roll this but I probably wont have time before the \nnew year. If anyone has time to take a look before that would be really \nhelpful. Ævar has given some useful feedback but it would be good to \nknow if others think this is useful.\n\nBest Wishes\n\nPhillip\n"},{"id":"468241","messageId":"221130.86h6ygiuc0.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"544fff8a-7d56-57a1-00a3-d1a9302e227c@dunelm.org.uk","subject":"Re: ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-30T10:16:40Z","receivedAt":"2022-11-30T10:56:55Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 30 2022, Phillip Wood wrote:\n\n> Hi Junio\n\nHi, and thanks for your review of this series.\n\n> On 29/11/2022 09:40, Junio C Hamano wrote:\n>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits\n>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n>> + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n>> + cmake: copy over git-p4.py for t983[56] perforce test\n>> + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n>> + cmake: increase test timeout on Windows only\n>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n>> + Makefile + cmake: use environment, not GIT-BUILD-DIR\n>> + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n>> + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n>> + cmake: don't copy chainlint.pl to build directory\n>> + cmake: update instructions for portable CMakeLists.txt\n>> + cmake: use \"-S\" and \"-B\" to specify source and build directories\n>> + cmake: don't invoke msgfmt with --statistics\n>> Fix assorted issues with CTest on *nix machines.\n>\n> If that's all this series did then I think it would be fine. However\n> it also makes changes to test-lib.sh to hard code the build directory\n> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an \n> improvement on the status quo.\n\nI think the series as it stands addresses those concerns. In particular\nbuilding outside of contrib/buildsystems/out works, just as before:\n\n\tcmake -S contrib/buildsystems -B /tmp/git-build -DCMAKE_BUILD_TYPE=Debug &&\n make -C /tmp/git-build &&\n ctest --test-dir /tmp/git-build -R t0001\n\nPer [1] and [2] which added the \"ctest\" support that's the use-case for\nthis part of the build: running the tests with ctest, which works as\nbefore with the default or custom directories.\n\nPerhaps the reason this has been a sticking point for you is that in\nsummarizing this, Johannes's [3] didn't make that distinction between\nrunning the tests with \"ctest\" and running them manually by entering the\n\"t/\" directory after the build. I.e.:\n\n\t(cd t && ./t0001-init.sh)\n\nIt's only that part which acts differently in this series. I.e. if you\nwere to build in /tmp/git-build this would no longer find your built\nassets:\n\n\t$ ./t0001-init.sh\n\terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n\nIf you just leave it at the default of \"contrib/buildsystems/out\" it'll\nwork:\n\n\t(cd t && ./t0001-init.sh)\n\tok 1 [...]\n\nI think my [4] convincingly makes the case that nobody will\ncare. I.e. as the [5] it links to the use-case for running the test\nafter the build without ctest was (\"[...]\" insert is mine):\n\n\t[To build and test with VS] open the worktree as a folder, and\n\tVisual Studio will find the `CMakeLists.txt` file and\n\tautomatically generate the project files.\n\nI.e. we want to support the user who builds with that method, and runs\nthe tests manually. I think you're worrying about an edge case that\nnobody's using in practice.\n\n> As I mentioned previously [1] I think\n> the non-*nix related patches could do with a review from the windows\n> folks before this hits master.\n\nI'd welcome another review of it, but at this point it's not for lack of\nwaiting for interest from the CC'd Windows people.\n\nPer the above I don't think any special Windows knowledge is really\nneeded, just a reading of the above history & use-cases.\n\nAll of which I've been careful not to break, and which you can now\nsimply test on *nix with this series, so no Windows-specific testing is\nneeded anymore for this concern you're raising.\n\n*If* we have someone that's been using this on Windows and e.g. building\nthis in /tmp/git-build (or whatever the Windows equivalent) with a\ncustom recipe all they'll need to have it work as before is:\n\n\tGIT_TEST_BUILD_DIR=/tmp/git-build ./t0001-init.sh\n\nI think nobody's straying off the golden path to do that, but if they\nare doing so and building in some custom directory they're already\ntweaking, just setting an environment variable doesn't seem like a big\nimposition.\n\nThe flip-side of that trade-off is (on both Windows and *nix) that the\nexisting way to support the use-case has unintended side-effects, which\nthe series improves:\n\n* When we pick up the not-a-Makefile tree implicitly like this we'll\n now emit a message telling you what git we're implicitly using.\n\n* We no longer have edge cases where you can e.g. build with make, then\n cmake, then run some \"make\" target that won't go through the path to\n remove GIT-BUILD-DIR (e.g. changing \"git.c\" and \"make git\").\n\n Then when you run the tests you'll end up with a different git running\n it than what you'd expect, i.e. the old stale cmake one.\n\nEven that hypothetical user who's going to need to set\n\"GIT_TEST_BUILD_DIR\" would benefit from that, as they'd no longer\naccidentally flip-flop between the two if they ran \"make\" and wiped away\nthe rather fragile link between the source directory & the linked-to\ncmake build directory.\n\n> [1]\n> https://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/\n>\n>> Will cook in 'next'.\n>> source: \n\n1. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26)\n2. 7f5397a07c6 (cmake: support for testing git when building out of the\n source tree, 2020-06-26)\n3. ee9e66e4e76 (cmake: avoid editing t/test-lib.sh, 2022-10-18)\n4. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,\n 2022-11-03)\n5. 3eccc7b99d4 (cmake: ignore files generated by CMake as run in Visual\n Studio, 2020-09-25)\n"},{"id":"468265","messageId":"kl6lk03cqpqx.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58869","inReplyTo":"xmqqedtl14xr.fsf@gitster.g","subject":"Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-11-30T18:08:38Z","receivedAt":"2022-11-30T18:08:43Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> Glen Choo writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits\n>>> - clone, submodule update: create and check out branches\n>>> - submodule--helper: remove update_data.suboid\n>>> - submodule update: refactor update targets\n>>> - submodule: return target of submodule symref\n>>> - t5617: drop references to remote-tracking branches\n>>> - submodule--helper clone: create named branch\n>>> - repo-settings: add submodule_propagate_branches\n>>> - clone: teach --detach option\n>>>\n>>> \"git clone --recurse-submodules\" and \"git submodule update\" learns\n>>> to honor the \"propagete branches\" option.\n>>>\n>>> Waiting for review on the updated round.\n>>> source: \n>>\n>> Jonathan left adequate feedback on the updated round, and we had decided\n>> on a direction for the reroll in [1].\n>>\n>> However, Calvin is also looking at how we could parallelize worktree\n>> updates to speed up \"git clone --recurse-submodules\", so the Google\n>> folks are taking an even bigger step back to figure out how worktree\n>> updating should look, which will probably end in a different approach\n>> from [1], but it should answer the questions on that thread about \"git\n>> checkout\" with branches.\n>>\n>> [1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com\n>\n> Thanks for a status report.\n>\n> So it sounds more like \"discard for now\" to expect a fresh restart.\n\nYes, that is accurate :)\n"},{"id":"468267","messageId":"kl6lh6ygqphu.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58869","inReplyTo":"xmqqilix150o.fsf@gitster.g","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-11-30T18:14:05Z","receivedAt":"2022-11-30T18:15:38Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Junio C Hamano writes:\n\n> Glen Choo writes:\n>\n>> Hm, it looks like ab/remove--super-prefix missed the preview release..\n>> Per the discussion ending at [1] I think my one-patch fix to \"git\n>> fetch\" [2] should have made it into the release (it's pretty low-risk\n>> and doesn't introduce too much churn to ab/remove--super-prefix). Is it\n>> too late for that?\n>\n> Nobody seemed to have commented on [2]. Is this fixing recent\n> regressions, or is it more like addressing an \"if it hurts, do not\n> do it then\" problem?\n\nÆvar did comment on the patch in [2], but unfortunately it happened on\nthe thread ending at [1] (and others), so it's not easy to follow.\n\nIt's solidly in the latter category. I don't think this has ever worked.\nc.f. https://lore.kernel.org/git/kl6lsfiivcau.fsf@chooglen-macbookpro.roam.corp.google.com/\n\n> The fact alone that these questions need to be asked _now_ is a good\n> indication that it is way too late for this cycle, I would have to\n> say.\n\nAt any rate, we shouldn't be rushing review, so this is fair (though\nunfortunate). Let's continue counting on ab/remove--super-prefix and\nignoring my one patch, then.\n\n>\n> Thanks.\n"},{"id":"468277","messageId":"221130.864jugi59l.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"kl6lh6ygqphu.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-11-30T19:43:10Z","receivedAt":"2022-11-30T19:58:21Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Nov 30 2022, Glen Choo wrote:\n\n> Junio C Hamano writes:\n>\n>> Glen Choo writes:\n>>\n>>> Hm, it looks like ab/remove--super-prefix missed the preview release..\n>>> Per the discussion ending at [1] I think my one-patch fix to \"git\n>>> fetch\" [2] should have made it into the release (it's pretty low-risk\n>>> and doesn't introduce too much churn to ab/remove--super-prefix). Is it\n>>> too late for that?\n>>\n>> Nobody seemed to have commented on [2]. Is this fixing recent\n>> regressions, or is it more like addressing an \"if it hurts, do not\n>> do it then\" problem?\n>\n> Ævar did comment on the patch in [2], but unfortunately it happened on\n> the thread ending at [1] (and others), so it's not easy to follow.\n>\n> It's solidly in the latter category. I don't think this has ever worked.\n> c.f. https://lore.kernel.org/git/kl6lsfiivcau.fsf@chooglen-macbookpro.roam.corp.google.com/\n>\n>> The fact alone that these questions need to be asked _now_ is a good\n>> indication that it is way too late for this cycle, I would have to\n>> say.\n>\n> At any rate, we shouldn't be rushing review, so this is fair (though\n> unfortunate). Let's continue counting on ab/remove--super-prefix and\n> ignoring my one patch, then.\n\nFor my part I was waiting to see what Junio would do with\n\"ab/submodule-no-abspath\", which is already in \"next\". Depending on\nwhether it's ejected or not I'd need to re-roll\n\"ab/remove--super-prefix\" on top of a new \"master\", as it extends the\ntests it added.\n\nYou noted in [1] that you strongly preferred seeing\n\"ab/submodule-no-abspath\" ejected. I think you're right that the output\nis a bit weird, but:\n\nA. I think it's mainly odd/unintuitive for the recursive cases, I think\n outside of our own test suite absorbing repositories recursively\n almost never happens.\n\nB. I think it's an improvement in the output compared to the absolute\n paths we have now, especially for the common case of non-recursive.\n\nC. Changing it made it easier to test it, which is how it ended up as a\n supposedly quick prerequisite for \"ab/remove--super-prefix\": It's\n otherwise changing a test blindspot.\n\nD. As you note in [1] the data we'd need to pass around to make it\n sensible (maybe it should always be consistent with \"git mv -v\"?)\n would require passing more state around, some of which is tricky.\n\nI'd prefer to just have it graduate as-is, and build\n\"ab/remove--super-prefix\" on top. We can always further tweak the output\nlater.\n\nBut if you & Junio feel otherwise I think the best way forward would be\nto eject both topics, and I'd submit a re-rolled\n\"ab/remove--super-prefix\".\n\nEither would work as a way forward. Just let me know what you both\nprefer.\n\n1. https://lore.kernel.org/git/kl6l7czmec10.fsf@chooglen-macbookpro.roam.corp.google.com/\n"},{"id":"468301","messageId":"xmqq8rjru2da.fsf@gitster.g","threadId":"58869","inReplyTo":"221130.864jugi59l.gmgdl@evledraar.gmail.com","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-01T05:20:01Z","receivedAt":"2022-12-01T05:20:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> For my part I was waiting to see what Junio would do with\n> \"ab/submodule-no-abspath\", which is already in \"next\".\n\nI have no strong opinion on it myself, but have been carrying the\n\"leaning negative\" label from Taylor, so the default action would be\nto eject when 'next' rewinds.\n\n\n"},{"id":"468320","messageId":"663b93ef-0c89-a5f6-1069-b4be97915d20@dunelm.org.uk","threadId":"58869","inReplyTo":"221130.86h6ygiuc0.gmgdl@evledraar.gmail.com","subject":"Re: ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-01T14:23:01Z","receivedAt":"2022-12-01T14:23:31Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Nov 30 2022, Phillip Wood wrote:\n> \n>> Hi Junio\n> \n> Hi, and thanks for your review of this series.\n> \n>> On 29/11/2022 09:40, Junio C Hamano wrote:\n>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits\n>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n>>> + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n>>> + cmake: copy over git-p4.py for t983[56] perforce test\n>>> + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n>>> + cmake: increase test timeout on Windows only\n>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR\n>>> + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n>>> + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n>>> + cmake: don't copy chainlint.pl to build directory\n>>> + cmake: update instructions for portable CMakeLists.txt\n>>> + cmake: use \"-S\" and \"-B\" to specify source and build directories\n>>> + cmake: don't invoke msgfmt with --statistics\n>>> Fix assorted issues with CTest on *nix machines.\n\nJunio please drop this series when you rebuild next as it breaks \nmanually running individual test scripts when building with Visual Studio.\n\n>> If that's all this series did then I think it would be fine. However\n>> it also makes changes to test-lib.sh to hard code the build directory\n>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an\n>> improvement on the status quo.\n> \n> I think the series as it stands addresses those concerns. In particular\n> building outside of contrib/buildsystems/out works, just as before:\n> \n> \tcmake -S contrib/buildsystems -B /tmp/git-build -DCMAKE_BUILD_TYPE=Debug &&\n> make -C /tmp/git-build &&\n> ctest --test-dir /tmp/git-build -R t0001\n> \n> Per [1] and [2] which added the \"ctest\" support that's the use-case for\n> this part of the build: running the tests with ctest, which works as\n> before with the default or custom directories.\n> \n> Perhaps the reason this has been a sticking point for you is that in\n> summarizing this, Johannes's [3] didn't make that distinction between\n> running the tests with \"ctest\" and running them manually by entering the\n> \"t/\" directory after the build. I.e.:\n\nIn other words Johannes thinks both are equally important. The windows \nbuild has always supported running the tests manually from /t and he \nquite reasonable wants that to continue working.\n\n> \t(cd t && ./t0001-init.sh)\n> \n> It's only that part which acts differently in this series. I.e. if you\n> were to build in /tmp/git-build this would no longer find your built\n> assets:\n> \n> \t$ ./t0001-init.sh\n> \terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n> \n> If you just leave it at the default of \"contrib/buildsystems/out\" it'll\n> work:\n> \n> \t(cd t && ./t0001-init.sh)\n> \tok 1 [...]\n> \n> I think my [4] convincingly makes the case that nobody will\n> care. I.e. as the [5] it links to the use-case for running the test\n> after the build without ctest was (\"[...]\" insert is mine):\n> \n> \t[To build and test with VS] open the worktree as a folder, and\n> \tVisual Studio will find the `CMakeLists.txt` file and\n> \tautomatically generate the project files.\n> \n> I.e. we want to support the user who builds with that method, and runs\n> the tests manually. I think you're worrying about an edge case that\n> nobody's using in practice.\n\nYou seem to be assuming that Visual Studio creates its build artifacts \nin contrib/buildsystems/out based on a gitignore rule. Given the rule \nignores _all_ subdirectories below contrib/buildsystems/out that is a \nbig assumption. Despite me repeatedly raising concerns about the hard \ncoded build directory you do not seem to have checked exactly where \nVisual Studio creates its build artifacts. This morning I installed \nVisual Studio to check this and discovered the build is in a \nsubdirectory below contrib/buildsystems/out so this series will break \nmanual test runs for anyone building git using the recommend method. I \nfind it rather frustrating that you argue below that Windows specific \nknowledge and testing are not required when you're altering the Windows \nbuild.\n\nBest Wishes\n\nPhillip\n\n>> As I mentioned previously [1] I think\n>> the non-*nix related patches could do with a review from the windows\n>> folks before this hits master.\n> \n> I'd welcome another review of it, but at this point it's not for lack of\n> waiting for interest from the CC'd Windows people.\n> \n> Per the above I don't think any special Windows knowledge is really\n> needed, just a reading of the above history & use-cases.\n> \n> All of which I've been careful not to break, and which you can now\n> simply test on *nix with this series, so no Windows-specific testing is\n> needed anymore for this concern you're raising.\n> \n> *If* we have someone that's been using this on Windows and e.g. building\n> this in /tmp/git-build (or whatever the Windows equivalent) with a\n> custom recipe all they'll need to have it work as before is:\n> \n> \tGIT_TEST_BUILD_DIR=/tmp/git-build ./t0001-init.sh\n> \n> I think nobody's straying off the golden path to do that, but if they\n> are doing so and building in some custom directory they're already\n> tweaking, just setting an environment variable doesn't seem like a big\n> imposition.\n> \n> The flip-side of that trade-off is (on both Windows and *nix) that the\n> existing way to support the use-case has unintended side-effects, which\n> the series improves:\n> \n> * When we pick up the not-a-Makefile tree implicitly like this we'll\n> now emit a message telling you what git we're implicitly using.\n> \n> * We no longer have edge cases where you can e.g. build with make, then\n> cmake, then run some \"make\" target that won't go through the path to\n> remove GIT-BUILD-DIR (e.g. changing \"git.c\" and \"make git\").\n> \n> Then when you run the tests you'll end up with a different git running\n> it than what you'd expect, i.e. the old stale cmake one.\n> \n> Even that hypothetical user who's going to need to set\n> \"GIT_TEST_BUILD_DIR\" would benefit from that, as they'd no longer\n> accidentally flip-flop between the two if they ran \"make\" and wiped away\n> the rather fragile link between the source directory & the linked-to\n> cmake build directory.\n> \n>> [1]\n>> https://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/\n>>\n>>> Will cook in 'next'.\n>>> source: \n> \n> 1. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26)\n> 2. 7f5397a07c6 (cmake: support for testing git when building out of the\n> source tree, 2020-06-26)\n> 3. ee9e66e4e76 (cmake: avoid editing t/test-lib.sh, 2022-10-18)\n> 4. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,\n> 2022-11-03)\n> 5. 3eccc7b99d4 (cmake: ignore files generated by CMake as run in Visual\n> Studio, 2020-09-25)\n"},{"id":"468324","messageId":"81dbae6e-b198-c056-5107-99c88ce77f94@github.com","threadId":"58869","inReplyTo":"b223202b-0ae2-bdb5-e9d6-21a6321a9e31@github.com","subject":"Re: ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Derrick Stolee","fromEmail":"derrickstolee@github.com","sentAt":"2022-12-01T15:06:50Z","receivedAt":"2022-12-01T15:07:29Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/29/2022 4:16 PM, Derrick Stolee wrote:\n> On 11/29/2022 4:40 AM, Junio C Hamano wrote:\n> \n>> * ds/bundle-uri-4 (2022-11-16) 9 commits\n>> - clone: unbundle the advertised bundles\n>> - bundle-uri: download bundles from an advertised list\n>> - bundle-uri: allow relative URLs in bundle lists\n>> - strbuf: introduce strbuf_strip_file_from_path()\n>> - bundle-uri client: add boolean transfer.bundleURI setting\n>> - bundle-uri: serve bundle.* keys from config\n>> - bundle-uri client: add helper for testing server\n>> - bundle-uri client: add minimal NOOP client\n>> - protocol v2: add server-side \"bundle-uri\" skeleton\n>>\n>> Bundle URIs part 4.\n>>\n>> Waiting for review.\n>> Seems to break CI.\n>> cf. https://github.com/git/git/actions/runs/3560918726\n>> source: \n> \n> Thanks for pointing this out. I'll be sure to fix the relevant\n> failures in linux-TEST-vars [1] before the next version.\n> \n> [1] https://github.com/git/git/actions/runs/3560918726/jobs/5981349269#step:4:1854\n\nI have discovered that the problem is due to the test variable\nGIT_TEST_PACKED_REFS_VERSION=2, so instead it's probably best\nto eject ds/packed-refs-v2 from 'seen' instead of ds/bundle-uri-4.\n\nThanks,\n-Stolee\n"},{"id":"468331","messageId":"patch-1.1-f27d8bd4491-20221201T162451Z-avarab@gmail.com","threadId":"58869","inReplyTo":"663b93ef-0c89-a5f6-1069-b4be97915d20@dunelm.org.uk","subject":"[PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-01T16:39:28Z","receivedAt":"2022-12-01T16:40:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in [1] and discover git built by cmake in subdirs of\n\"contrib/buildsystems/out\", in addition to the \"out\" directory itself.\n\nAs noted in [2] the default for Visual Studio is to use\n\"out/build/\", where \"\" is a default configuration\nname. We might be able to make this deterministic in the future with a\n\"CMakePresets.json\", but that facility is newer than our oldest\nsupported CMake and VS version.\n\n1. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,\n 2022-11-03)\n2. https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nOn Thu, Dec 01 2022, Phillip Wood wrote:\n\n> On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:\n>> On Wed, Nov 30 2022, Phillip Wood wrote:\n>> \n>>> Hi Junio\n>> Hi, and thanks for your review of this series.\n>> \n>>> On 29/11/2022 09:40, Junio C Hamano wrote:\n>>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits\n>>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n>>>> + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n>>>> + cmake: copy over git-p4.py for t983[56] perforce test\n>>>> + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n>>>> + cmake: increase test timeout on Windows only\n>>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n>>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR\n>>>> + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n>>>> + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n>>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n>>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n>>>> + cmake: don't copy chainlint.pl to build directory\n>>>> + cmake: update instructions for portable CMakeLists.txt\n>>>> + cmake: use \"-S\" and \"-B\" to specify source and build directories\n>>>> + cmake: don't invoke msgfmt with --statistics\n>>>> Fix assorted issues with CTest on *nix machines.\n>\n> Junio please drop this series when you rebuild next as it breaks\n> manually running individual test scripts when building with Visual\n> Studio.\n\nI think the issue you've spotted is easily fixed on top. See below.\n\n>>> If that's all this series did then I think it would be fine. However\n>>> it also makes changes to test-lib.sh to hard code the build directory\n>>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an\n>>> improvement on the status quo.\n>> I think the series as it stands addresses those concerns. In\n>> particular\n>> building outside of contrib/buildsystems/out works, just as before:\n>> \tcmake -S contrib/buildsystems -B /tmp/git-build\n>> -DCMAKE_BUILD_TYPE=Debug &&\n>> make -C /tmp/git-build &&\n>> ctest --test-dir /tmp/git-build -R t0001\n>> Per [1] and [2] which added the \"ctest\" support that's the use-case\n>> for\n>> this part of the build: running the tests with ctest, which works as\n>> before with the default or custom directories.\n>> Perhaps the reason this has been a sticking point for you is that in\n>> summarizing this, Johannes's [3] didn't make that distinction between\n>> running the tests with \"ctest\" and running them manually by entering the\n>> \"t/\" directory after the build. I.e.:\n>\n> In other words Johannes thinks both are equally important. The windows\n> build has always supported running the tests manually from /t and he \n> quite reasonable wants that to continue working.\n\nYes, that should work. Now, clearly I missed that VS doesn't use \"out\"\nby default, but a subdirectory, which the below patch fixes.\n\nBut I think we can still draw a distinction between anything under\n\"out\" and arbitrary user-supplied directories, which can possibly be\nlocated outside of the source tree.\n\n>> \t(cd t && ./t0001-init.sh)\n>> It's only that part which acts differently in this series. I.e. if\n>> you\n>> were to build in /tmp/git-build this would no longer find your built\n>> assets:\n>> \t$ ./t0001-init.sh\n>> \terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n>> If you just leave it at the default of \"contrib/buildsystems/out\"\n>> it'll\n>> work:\n>> \t(cd t && ./t0001-init.sh)\n>> \tok 1 [...]\n>> I think my [4] convincingly makes the case that nobody will\n>> care. I.e. as the [5] it links to the use-case for running the test\n>> after the build without ctest was (\"[...]\" insert is mine):\n>> \t[To build and test with VS] open the worktree as a folder, and\n>> \tVisual Studio will find the `CMakeLists.txt` file and\n>> \tautomatically generate the project files.\n>> I.e. we want to support the user who builds with that method, and\n>> runs\n>> the tests manually. I think you're worrying about an edge case that\n>> nobody's using in practice.\n>\n> You seem to be assuming that Visual Studio creates its build artifacts\n> in contrib/buildsystems/out based on a gitignore rule. Given the rule \n> ignores _all_ subdirectories below contrib/buildsystems/out that is a\n> big assumption. Despite me repeatedly raising concerns about the hard \n> coded build directory you do not seem to have checked exactly where\n> Visual Studio creates its build artifacts.\n\nI did check, but I got it wrong from reading the docs & commit message.\n\nI don't have a local Windows or VS setup. Thanks for testing it.\n\n> This morning I installed \n> Visual Studio to check this and discovered the build is in a\n> subdirectory below contrib/buildsystems/out so this series will break \n> manual test runs for anyone building git using the recommend method. I\n> find it rather frustrating that you argue below that Windows specific \n> knowledge and testing are not required when you're altering the\n> Windows build.\n\nI was saying that you could round-trip test the auto-discovery of the\nbuild directory on *nix.\n\nNow, clearly I missed the \"in a subdir of out\" case. But that's\nseparate from whether you can test that case cross-platform. With the\nbelow patch this works:\n\n\tcmake -S contrib/buildsystems -B contrib/buildsystems/out/a/b/c -DCMAKE_BUILD_TYPE=Debug &&\n\tmake -C contrib/buildsystems/out/a/b/c &&\n\t(cd t && ./t0071-sort.sh)\n\nBut did this work for you before, and does it work on \"master\"? I\nthink this never worked out of the box until my series. I.e. if you do\nthat on v2.38.0 (before \"js/cmake-updates\") you'll get:\n\n\t$ ./t0071-sort.sh \n\terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n\nThe reason is that before \"js/cmake-updates\" the part of the cmake\nrecipe that established the ling between the test-lib.sh and the\ncmake-built tree was contingent on running ctest. E.g.:\n\n\tctest --test-dir contrib/buildsystems/out/a/b/c -R t0071\n\nOnce you did that it would patch your t/test-lib.sh:\n\t\n\tdiff --git a/t/test-lib.sh b/t/test-lib.sh\n\tindex a65df2fd220..70b0a633e4c 100644\n\t--- a/t/test-lib.sh\n\t+++ b/t/test-lib.sh\n\t@@ -44,1 +44,1 @@ fi\n\t-GIT_BUILD_DIR=\"${TEST_DIRECTORY%/t}\"\n\t+GIT_BUILD_DIR=\"$TEST_DIRECTORY/../contrib/buildsystems/out/a/b/c\"\n\nLikewise, with \"js/cmake-updates\" it also doesn't work after you\n*build*. I. with it it would create a \"GIT-BUILD-DIR\" file, but only\nonce ctest is run. I.e.:\n\n\t(cd t && ./t0071-sort.sh);\n\tfile GIT-BUILD-DIR;\n\tctest --test-dir contrib/buildsystems/out/a/b/c -R t0071\n\tfile GIT-BUILD-DIR;\n\t(cd t && ./t0071-sort.sh)\n\nWould emit:\n\n\terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n\tGIT-BUILD-DIR: cannot open `GIT-BUILD-DIR' (No such file or directory)\n\t\n\tGIT-BUILD-DIR: ASCII text, with no line terminators\n\t\n\nIt's only with my series that it started working wihout having to run\n\"ctest\". With the below the test-lib.sh will optimistically find the\n\"git\" in \"contrib/buildsystems/out\".\n\nDoes the VS integration run the equivalent of \"ctest\" by default?\n\n t/test-lib.sh | 8 +++++---\n 1 file changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex ce319c9963e..9c63ee428f2 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -57,9 +57,11 @@ if test -n \"$GIT_TEST_BUILD_DIR\"\n then\n \tGIT_BUILD_DIR=\"$GIT_TEST_BUILD_DIR\"\n elif ! test -x \"$GIT_BUILD_DIR/git\" &&\n- test -x \"$GIT_BUILD_DIR/contrib/buildsystems/out/git\"\n+ test -d \"$GIT_BUILD_DIR/contrib/buildsystems/out\"\n then\n-\tGIT_BUILD_DIR=\"$GIT_SOURCE_DIR/contrib/buildsystems/out\"\n+\tGIT_BUILD_OPTIONS=\"$(find \"$GIT_BUILD_DIR/contrib/buildsystems/out\" \\\n+\t\t-type f -name 'GIT-BUILD-OPTIONS')\"\n+\tGIT_BUILD_DIR=\"${GIT_BUILD_OPTIONS%/GIT-BUILD-OPTIONS}\"\n \tGIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT=t\n \n \t# On Windows, we must convert Windows paths lest they contain a colon\n@@ -1646,7 +1648,7 @@ remove_trash_directory \"$TRASH_DIRECTORY\" || {\n # anything using lib-subtest.sh\n if test -n \"$GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT\" && test -t 1\n then\n-\tsay \"setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git\"\n+\tsay \"setup: had no ../git, but found & used cmake built git in '$GIT_BUILD_DIR/git'\"\n fi\n \n remove_trash=t\n-- \n2.39.0.rc1.974.g67e2c53d827\n\n"},{"id":"468332","messageId":"9c6400ce-6f71-6f1c-dd7e-719c7181fe83@dunelm.org.uk","threadId":"58869","inReplyTo":"patch-1.1-f27d8bd4491-20221201T162451Z-avarab@gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-01T16:48:37Z","receivedAt":"2022-12-01T16:48:58Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 01/12/2022 16:39, Ævar Arnfjörð Bjarmason wrote:\n> Fix a regression in [1] and discover git built by cmake in subdirs of\n> \"contrib/buildsystems/out\", in addition to the \"out\" directory itself.\n\nHow do you propose to find the most recent build? There are different \ndirectories for different architectures and different directories for \ndebug and release builds.\n\nHard coding the build directory is a bad idea and should be dropped.\n\nBest Wishes\n\nPhillip\n\n\n> As noted in [2] the default for Visual Studio is to use\n> \"out/build/\", where \"\" is a default configuration\n> name. We might be able to make this deterministic in the future with a\n> \"CMakePresets.json\", but that facility is newer than our oldest\n> supported CMake and VS version.\n> \n> 1. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,\n> 2022-11-03)\n> 2. https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason \n> ---\n> \n> On Thu, Dec 01 2022, Phillip Wood wrote:\n> \n>> On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:\n>>> On Wed, Nov 30 2022, Phillip Wood wrote:\n>>>\n>>>> Hi Junio\n>>> Hi, and thanks for your review of this series.\n>>>\n>>>> On 29/11/2022 09:40, Junio C Hamano wrote:\n>>>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits\n>>>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)\n>>>>> + CI: add a \"linux-cmake-test\" to run cmake & ctest on linux\n>>>>> + cmake: copy over git-p4.py for t983[56] perforce test\n>>>>> + cmake: only look for \"sh\" in \"C:/Program Files\" on Windows\n>>>>> + cmake: increase test timeout on Windows only\n>>>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults\n>>>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR\n>>>>> + test-lib.sh: support a \"GIT_TEST_BUILD_DIR\"\n>>>>> + cmake: set \"USE_LIBPCRE2\" in \"GIT-BUILD-OPTIONS\" for test-lib.sh\n>>>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable\n>>>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4\n>>>>> + cmake: don't copy chainlint.pl to build directory\n>>>>> + cmake: update instructions for portable CMakeLists.txt\n>>>>> + cmake: use \"-S\" and \"-B\" to specify source and build directories\n>>>>> + cmake: don't invoke msgfmt with --statistics\n>>>>> Fix assorted issues with CTest on *nix machines.\n>>\n>> Junio please drop this series when you rebuild next as it breaks\n>> manually running individual test scripts when building with Visual\n>> Studio.\n> \n> I think the issue you've spotted is easily fixed on top. See below.\n> \n>>>> If that's all this series did then I think it would be fine. However\n>>>> it also makes changes to test-lib.sh to hard code the build directory\n>>>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an\n>>>> improvement on the status quo.\n>>> I think the series as it stands addresses those concerns. In\n>>> particular\n>>> building outside of contrib/buildsystems/out works, just as before:\n>>> \tcmake -S contrib/buildsystems -B /tmp/git-build\n>>> -DCMAKE_BUILD_TYPE=Debug &&\n>>> make -C /tmp/git-build &&\n>>> ctest --test-dir /tmp/git-build -R t0001\n>>> Per [1] and [2] which added the \"ctest\" support that's the use-case\n>>> for\n>>> this part of the build: running the tests with ctest, which works as\n>>> before with the default or custom directories.\n>>> Perhaps the reason this has been a sticking point for you is that in\n>>> summarizing this, Johannes's [3] didn't make that distinction between\n>>> running the tests with \"ctest\" and running them manually by entering the\n>>> \"t/\" directory after the build. I.e.:\n>>\n>> In other words Johannes thinks both are equally important. The windows\n>> build has always supported running the tests manually from /t and he\n>> quite reasonable wants that to continue working.\n> \n> Yes, that should work. Now, clearly I missed that VS doesn't use \"out\"\n> by default, but a subdirectory, which the below patch fixes.\n> \n> But I think we can still draw a distinction between anything under\n> \"out\" and arbitrary user-supplied directories, which can possibly be\n> located outside of the source tree.\n> \n>>> \t(cd t && ./t0001-init.sh)\n>>> It's only that part which acts differently in this series. I.e. if\n>>> you\n>>> were to build in /tmp/git-build this would no longer find your built\n>>> assets:\n>>> \t$ ./t0001-init.sh\n>>> \terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n>>> If you just leave it at the default of \"contrib/buildsystems/out\"\n>>> it'll\n>>> work:\n>>> \t(cd t && ./t0001-init.sh)\n>>> \tok 1 [...]\n>>> I think my [4] convincingly makes the case that nobody will\n>>> care. I.e. as the [5] it links to the use-case for running the test\n>>> after the build without ctest was (\"[...]\" insert is mine):\n>>> \t[To build and test with VS] open the worktree as a folder, and\n>>> \tVisual Studio will find the `CMakeLists.txt` file and\n>>> \tautomatically generate the project files.\n>>> I.e. we want to support the user who builds with that method, and\n>>> runs\n>>> the tests manually. I think you're worrying about an edge case that\n>>> nobody's using in practice.\n>>\n>> You seem to be assuming that Visual Studio creates its build artifacts\n>> in contrib/buildsystems/out based on a gitignore rule. Given the rule\n>> ignores _all_ subdirectories below contrib/buildsystems/out that is a\n>> big assumption. Despite me repeatedly raising concerns about the hard\n>> coded build directory you do not seem to have checked exactly where\n>> Visual Studio creates its build artifacts.\n> \n> I did check, but I got it wrong from reading the docs & commit message.\n> \n> I don't have a local Windows or VS setup. Thanks for testing it.\n> \n>> This morning I installed\n>> Visual Studio to check this and discovered the build is in a\n>> subdirectory below contrib/buildsystems/out so this series will break\n>> manual test runs for anyone building git using the recommend method. I\n>> find it rather frustrating that you argue below that Windows specific\n>> knowledge and testing are not required when you're altering the\n>> Windows build.\n> \n> I was saying that you could round-trip test the auto-discovery of the\n> build directory on *nix.\n> \n> Now, clearly I missed the \"in a subdir of out\" case. But that's\n> separate from whether you can test that case cross-platform. With the\n> below patch this works:\n> \n> \tcmake -S contrib/buildsystems -B contrib/buildsystems/out/a/b/c -DCMAKE_BUILD_TYPE=Debug &&\n> \tmake -C contrib/buildsystems/out/a/b/c &&\n> \t(cd t && ./t0071-sort.sh)\n> \n> But did this work for you before, and does it work on \"master\"? I\n> think this never worked out of the box until my series. I.e. if you do\n> that on v2.38.0 (before \"js/cmake-updates\") you'll get:\n> \n> \t$ ./t0071-sort.sh\n> \terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n> \n> The reason is that before \"js/cmake-updates\" the part of the cmake\n> recipe that established the ling between the test-lib.sh and the\n> cmake-built tree was contingent on running ctest. E.g.:\n> \n> \tctest --test-dir contrib/buildsystems/out/a/b/c -R t0071\n> \n> Once you did that it would patch your t/test-lib.sh:\n> \t\n> \tdiff --git a/t/test-lib.sh b/t/test-lib.sh\n> \tindex a65df2fd220..70b0a633e4c 100644\n> \t--- a/t/test-lib.sh\n> \t+++ b/t/test-lib.sh\n> \t@@ -44,1 +44,1 @@ fi\n> \t-GIT_BUILD_DIR=\"${TEST_DIRECTORY%/t}\"\n> \t+GIT_BUILD_DIR=\"$TEST_DIRECTORY/../contrib/buildsystems/out/a/b/c\"\n> \n> Likewise, with \"js/cmake-updates\" it also doesn't work after you\n> *build*. I. with it it would create a \"GIT-BUILD-DIR\" file, but only\n> once ctest is run. I.e.:\n> \n> \t(cd t && ./t0071-sort.sh);\n> \tfile GIT-BUILD-DIR;\n> \tctest --test-dir contrib/buildsystems/out/a/b/c -R t0071\n> \tfile GIT-BUILD-DIR;\n> \t(cd t && ./t0071-sort.sh)\n> \n> Would emit:\n> \n> \terror: GIT-BUILD-OPTIONS missing (has Git been built?).\n> \tGIT-BUILD-DIR: cannot open `GIT-BUILD-DIR' (No such file or directory)\n> \t\n> \tGIT-BUILD-DIR: ASCII text, with no line terminators\n> \t\n> \n> It's only with my series that it started working wihout having to run\n> \"ctest\". With the below the test-lib.sh will optimistically find the\n> \"git\" in \"contrib/buildsystems/out\".\n> \n> Does the VS integration run the equivalent of \"ctest\" by default?\n> \n> t/test-lib.sh | 8 +++++---\n> 1 file changed, 5 insertions(+), 3 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index ce319c9963e..9c63ee428f2 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -57,9 +57,11 @@ if test -n \"$GIT_TEST_BUILD_DIR\"\n> then\n> \tGIT_BUILD_DIR=\"$GIT_TEST_BUILD_DIR\"\n> elif ! test -x \"$GIT_BUILD_DIR/git\" &&\n> - test -x \"$GIT_BUILD_DIR/contrib/buildsystems/out/git\"\n> + test -d \"$GIT_BUILD_DIR/contrib/buildsystems/out\"\n> then\n> -\tGIT_BUILD_DIR=\"$GIT_SOURCE_DIR/contrib/buildsystems/out\"\n> +\tGIT_BUILD_OPTIONS=\"$(find \"$GIT_BUILD_DIR/contrib/buildsystems/out\" \\\n> +\t\t-type f -name 'GIT-BUILD-OPTIONS')\"\n> +\tGIT_BUILD_DIR=\"${GIT_BUILD_OPTIONS%/GIT-BUILD-OPTIONS}\"\n> \tGIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT=t\n> \n> \t# On Windows, we must convert Windows paths lest they contain a colon\n> @@ -1646,7 +1648,7 @@ remove_trash_directory \"$TRASH_DIRECTORY\" || {\n> # anything using lib-subtest.sh\n> if test -n \"$GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT\" && test -t 1\n> then\n> -\tsay \"setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git\"\n> +\tsay \"setup: had no ../git, but found & used cmake built git in '$GIT_BUILD_DIR/git'\"\n> fi\n> \n> remove_trash=t\n"},{"id":"468334","messageId":"221201.86mt87gi50.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"9c6400ce-6f71-6f1c-dd7e-719c7181fe83@dunelm.org.uk","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-01T17:13:58Z","receivedAt":"2022-12-01T17:15:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 01 2022, Phillip Wood wrote:\n\n> On 01/12/2022 16:39, Ævar Arnfjörð Bjarmason wrote:\n>> Fix a regression in [1] and discover git built by cmake in subdirs of\n>> \"contrib/buildsystems/out\", in addition to the \"out\" directory itself.\n>\n> How do you propose to find the most recent build? There are different\n> directories for different architectures and different directories for \n> debug and release builds.\n\nI'm not proposing to do that, besides, if you built locally for\ndifferent architectures running the tests would just fail, wouldn't it?\n\n> Hard coding the build directory is a bad idea and should be dropped.\n\nI think it's a better idea to target the common case of a user building\n& being able to run the tests form t/, rather than requiring\nbootstrapping that process by running \"ctest\".\n\nDid you test that case on v2.38.0 and/or master? I.e. is VS running that\nctest portion automatically?\n"},{"id":"468335","messageId":"kl6la647ow7e.fsf@chooglen-macbookpro.roam.corp.google.com","threadId":"58869","inReplyTo":"221130.864jugi59l.gmgdl@evledraar.gmail.com","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-12-01T17:44:21Z","receivedAt":"2022-12-01T17:45:04Z","isPatch":false,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> For my part I was waiting to see what Junio would do with\n> \"ab/submodule-no-abspath\", which is already in \"next\". Depending on\n> whether it's ejected or not I'd need to re-roll\n> \"ab/remove--super-prefix\" on top of a new \"master\", as it extends the\n> tests it added.\n\nThanks for the status update!\n\n> You noted in [1] that you strongly preferred seeing\n> \"ab/submodule-no-abspath\" ejected. I think you're right that the output\n> is a bit weird, but:\n>\n> A. I think it's mainly odd/unintuitive for the recursive cases, I think\n> outside of our own test suite absorbing repositories recursively\n> almost never happens.\n\nFrankly, I find it odd in the non-recursive case too e.g. in one of the\nnon-recursive tests, we have:\n\n Migrating git directory of '\\''sub1'\\'' from '\\''sub1/.git'\\'' to '\\''../../.git/modules/sub1'\\''\n\nwhere all 3 paths are relative, the first two share the same base but\nnot the last one. I don't think a casual reader can easily tell that the\nlast one should be relative to the second one.\n\n> B. I think it's an improvement in the output compared to the absolute\n> paths we have now, especially for the common case of non-recursive.\n\nFor the reason above, this doesn't feel like an improvement to me :/\n\n> C. Changing it made it easier to test it, which is how it ended up as a\n> supposedly quick prerequisite for \"ab/remove--super-prefix\": It's\n> otherwise changing a test blindspot.\n\nWith abspaths, couldn't we test this with $PWD?\n\n> D. As you note in [1] the data we'd need to pass around to make it\n> sensible (maybe it should always be consistent with \"git mv -v\"?)\n> would require passing more state around, some of which is tricky.\n\nYeah I think this a good to have in the long run, but let's punt on it\nfor now.\n\n> I'd prefer to just have it graduate as-is, and build\n> \"ab/remove--super-prefix\" on top. We can always further tweak the output\n> later.\n>\n> But if you & Junio feel otherwise I think the best way forward would be\n> to eject both topics, and I'd submit a re-rolled\n> \"ab/remove--super-prefix\".\n\nA re-rolled \"ab/remove--super-prefix\" makes sense to me. Sorry again for\nnot voicing my thoughts sooner and saving us from this churn :(\n\n>\n> Either would work as a way forward. Just let me know what you both\n> prefer.\n>\n> 1. https://lore.kernel.org/git/kl6l7czmec10.fsf@chooglen-macbookpro.roam.corp.google.com/\n"},{"id":"468353","messageId":"xmqqmt86stm3.fsf@gitster.g","threadId":"58869","inReplyTo":"kl6la647ow7e.fsf@chooglen-macbookpro.roam.corp.google.com","subject":"Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-01T21:26:44Z","receivedAt":"2022-12-01T21:26:49Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Glen Choo writes:\n\n> Frankly, I find it odd in the non-recursive case too e.g. in one of the\n> non-recursive tests, we have:\n>\n> Migrating git directory of '\\''sub1'\\'' from '\\''sub1/.git'\\'' to '\\''../../.git/modules/sub1'\\''\n>\n> where all 3 paths are relative, the first two share the same base but\n> not the last one. I don't think a casual reader can easily tell that the\n> last one should be relative to the second one.\n\nThat's a good example, although I can understand why a developer\n(not a user) might choose to show the last one relative to the one\nthat comes before it.\n\nThanks, both.\n"},{"id":"468366","messageId":"xmqq5yeuspam.fsf@gitster.g","threadId":"58869","inReplyTo":"patch-1.1-f27d8bd4491-20221201T162451Z-avarab@gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-01T23:00:01Z","receivedAt":"2022-12-01T23:01:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> Junio please drop this series when you rebuild next as it breaks\n>> manually running individual test scripts when building with Visual\n>> Studio.\n>\n> I think the issue you've spotted is easily fixed on top. See below.\n\nSmells more like papering over than fixed, but let's see how folks\nwho need cmake/ctest feel about it.\n\nLet's mark the series never to graduate to 'master' for now,\noptionally revert it out of 'next'.\n\n Phillip, you asked about rebuilding 'next', which would not\n happen until 2.39.0 final---did you mean reverting the topic out\n of 'next'? Do you need 'next' without this topic, not just\n 'master'?\n\nI'll then wait for something both camps (you and folks on Visual\nStudio?) can agree on to requeue.\n\nThanks.\n"},{"id":"468384","messageId":"xmqqedtir6rt.fsf@gitster.g","threadId":"58869","inReplyTo":"81dbae6e-b198-c056-5107-99c88ce77f94@github.com","subject":"Re: ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-02T00:25:26Z","receivedAt":"2022-12-02T00:30:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> I have discovered that the problem is due to the test variable\n> GIT_TEST_PACKED_REFS_VERSION=2, so instead it's probably best\n> to eject ds/packed-refs-v2 from 'seen' instead of ds/bundle-uri-4.\n\nThanks for digging. I've shuffled the order of topics in 'seen'.\n"},{"id":"468436","messageId":"87f22a55-ee84-2f76-7b9b-924a97f44f89@dunelm.org.uk","threadId":"58869","inReplyTo":"xmqq5yeuspam.fsf@gitster.g","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-02T15:14:15Z","receivedAt":"2022-12-02T15:14:23Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio\n\nOn 01/12/2022 23:00, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> \n>>> Junio please drop this series when you rebuild next as it breaks\n>>> manually running individual test scripts when building with Visual\n>>> Studio.\n>>\n>> I think the issue you've spotted is easily fixed on top. See below.\n> \n> Smells more like papering over than fixed, but let's see how folks\n> who need cmake/ctest feel about it.\n\nAs MSVC uses different directories for debug and release builds there \ncan be more than one build directory. I don't think selecting one of \nthem at random using 'find' is a good idea.\n\n> Let's mark the series never to graduate to 'master' for now,\n> optionally revert it out of 'next'.\n> \n> Phillip, you asked about rebuilding 'next', which would not\n> happen until 2.39.0 final---did you mean reverting the topic out\n> of 'next'? Do you need 'next' without this topic, not just\n> 'master'?\n\nI don't mind waiting but I'm not a Windows user. I only tested this \ntopic under Windows because I knew Ævar had not and a quick web search \nfor \"MSVC CMake\" made me worry it was broken.\n\nI'm afraid I wont be spending anymore time on this topic. I had hoped \nthat having the CMake build work under Linux would help developers avoid \nbreaking it. However I'm concerned that if developers do not appreciate \nthat there are differences between the Linux and Windows builds it will \nactually create a false sense of security and be used as an excuse not \nto properly test under Windows[1]. Recent events have confirmed my view \nthat changes like this need the attention of someone with experience of \nWindows development and given that yesterday was the first time I'd used \nMSVC since about 1994 I do not fit that description.\n\nIn addition to the breakage I reported yesterday 623fde1438 (cmake: \nchmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4, 2022-11-03) \ncauses CMake older that 3.19 to error out when run from MSVC because \nchmod does not exist on Windows. Also when running 'ctest' on \"next\" I \nsee tests failing because they cannot find 'test-tool' (I haven't tried \nrunning the failing tests manually)\n\nBest Wishes\n\nPhillip\n\n[1] While our CI helps the MSVC job runs CMake manually, performs an \nin-tree build and does not use ctest. In contrast a user running the \nMSVC GUI does not run CMake themselves, ends up with an out-of-tree \nbuild and runs the tests with ctest.\n\n> I'll then wait for something both camps (you and folks on Visual\n> Studio?) can agree on to requeue.\n> \n> Thanks.\n"},{"id":"468450","messageId":"221202.86sfhxg2ng.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"87f22a55-ee84-2f76-7b9b-924a97f44f89@dunelm.org.uk","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-02T16:40:34Z","receivedAt":"2022-12-02T17:02:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 02 2022, Phillip Wood wrote:\n\n> On 01/12/2022 23:00, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>> \n>>>> Junio please drop this series when you rebuild next as it breaks\n>>>> manually running individual test scripts when building with Visual\n>>>> Studio.\n>>>\n>>> I think the issue you've spotted is easily fixed on top. See below.\n>> Smells more like papering over than fixed, but let's see how folks\n>> who need cmake/ctest feel about it.\n>\n> As MSVC uses different directories for debug and release builds there\n> can be more than one build directory. I don't think selecting one of \n> them at random using 'find' is a good idea.\n\nI think the latest iteration submitted today should fix that issue:\nhttps://lore.kernel.org/git/cover-v5-00.15-00000000000-20221202T110947Z-avarab@gmail.com/\n\n>> Let's mark the series never to graduate to 'master' for now,\n>> optionally revert it out of 'next'.\n>> Phillip, you asked about rebuilding 'next', which would not\n>> happen until 2.39.0 final---did you mean reverting the topic out\n>> of 'next'? Do you need 'next' without this topic, not just\n>> 'master'?\n>\n> I don't mind waiting but I'm not a Windows user. I only tested this\n> topic under Windows because I knew Ævar had not and a quick web search \n> for \"MSVC CMake\" made me worry it was broken.\n>\n> I'm afraid I wont be spending anymore time on this topic. I had hoped\n> that having the CMake build work under Linux would help developers\n> avoid breaking it. However I'm concerned that if developers do not\n> appreciate that there are differences between the Linux and Windows\n> builds it will actually create a false sense of security and be used\n> as an excuse not to properly test under Windows[1]. Recent events have\n> confirmed my view that changes like this need the attention of someone\n> with experience of Windows development and given that yesterday was\n> the first time I'd used MSVC since about 1994 I do not fit that\n> description.\n> [...]\n> [1] While our CI helps the MSVC job runs CMake manually, performs an\n> in-tree build and does not use ctest. In contrast a user running the \n> MSVC GUI does not run CMake themselves, ends up with an out-of-tree\n> build and runs the tests with ctest.\n\nI don't run Windows by choice, and I'm not interested in running a\npropriterary IDE (VS) either.\n\nThe main reason I'm working on this series is that while we as a project\nare happy to support proprietary OS's, it hasn't been a requirement for\nparticipation that you need to buy a copy of Windows, OSX, AIX, HP/UX or\nwhatever to submit patches.\n\nOf course we have platform-specific code. but this CMake component is\nunique in how invasive it is.\n\nIt's easy to e.g. stay away from the OSX-specific code in\ncompat/fsmonitor/*darwin*.[ch], or generally speaking the\nWindows-specific C code.\n\nBut for CMake it's become a hard requirenment for many changes, even\nthough it's a contrib/ component.\n\nNow, I'm not looking to get rid of it, it's clearly useful, particularly\nwith MSVC (or so I've gathered).\n\nBut I'd also like some future where any time I and others who don't use\nWindows need to patch certain parts of the Makefile that we don't need\nto spend a long time bouncing thigs against the Windows CI, but can run\nthis component other platforms..\n\nE.g. I think for cfe853e66be (hook-list.h: add a generated list of\nhooks, like config-list.h, 2021-09-26) I spent at least 3-4 hours on\nwhat should have been a 5-10 minute task of making the relatively minor\nchange to generate hook-list.h. It was a push, wait 30-60 minutes, find\nsome minor (e.g. syntax) issue, rinse & repeat.\n\nNow, clearly the outstanding issues with (if any) need to be fixed, and\nthanks for sticking with testing it for so long. But hopefully the above\ngives you some background.\n\n> In addition to the breakage I reported yesterday 623fde1438 (cmake:\n> chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4, 2022-11-03) \n> causes CMake older that 3.19 to error out when run from MSVC because\n> chmod does not exist on Windows. Also when running 'ctest' on \"next\" I \n> see tests failing because they cannot find 'test-tool' (I haven't\n> tried running the failing tests manually)\n\nThat's odd, do you happen to have some output from \"ctest\" for that?\n\nOur \"cmake\" build already invokes shell scripts which rely on \"grep\",\n\"sed\" etc, and the test suite itself invokes \"chmod\", I just understand\nthat it's a noop wrapper on Windows.\n\nSo I can also re-roll and just ifdef that part awa on win32, but I\nwonder what's going on there. I wonder if it's because we'd need to\nspawn it via \"/bin/sh\" (but just skippin it seems better).\n\nOne of the things I didn't change with this series is how \"test-tool\" is\nhandled. I.e. it's there before in the generated \"t/helper\" directory,\nand in the generated \"bin-wrappers/\".\n\nWe also find it when running the tests with Windows in GitHub CI.\n\nDid you try on v2.38.0 and/or \"master\" as well?\n"},{"id":"468471","messageId":"Y4qF3iHW2s+I0yNe@coredump.intra.peff.net","threadId":"58869","inReplyTo":"221202.86sfhxg2ng.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-02T23:10:22Z","receivedAt":"2022-12-02T23:10:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 02, 2022 at 05:40:34PM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > [1] While our CI helps the MSVC job runs CMake manually, performs an\n> > in-tree build and does not use ctest. In contrast a user running the \n> > MSVC GUI does not run CMake themselves, ends up with an out-of-tree\n> > build and runs the tests with ctest.\n> \n> I don't run Windows by choice, and I'm not interested in running a\n> propriterary IDE (VS) either.\n> \n> The main reason I'm working on this series is that while we as a project\n> are happy to support proprietary OS's, it hasn't been a requirement for\n> participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or\n> whatever to submit patches.\n> \n> Of course we have platform-specific code. but this CMake component is\n> unique in how invasive it is.\n> \n> It's easy to e.g. stay away from the OSX-specific code in\n> compat/fsmonitor/*darwin*.[ch], or generally speaking the\n> Windows-specific C code.\n> \n> But for CMake it's become a hard requirenment for many changes, even\n> though it's a contrib/ component.\n\nI have similar feelings to you here. Back when cmake support was\nintroduced, I explicitly wanted it to be something for people who cared\nabout it, but that wouldn't bother people who didn't use it:\n\n https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n\nI stand by that sentiment, but it seems to have crept up as a required\nthing to deal with, and that is mostly because of CI. Using cmake in CI\nis good for telling developers when a change they make has broken cmake.\nBut it also makes cmake their problem, and not the folks interested in\ncmake.\n\nNow maybe attitudes have changed, and I am out of date, and cmake\nsupport is considered mature and really important (or maybe nobody even\nagreed with me back then ;) ). But if not, should we consider softening\nthe CI output so that cmake failures aren't \"real\" failures? That seems\ndrastic and mean, and I don't like it. But it's the root of the issue,\nIMHO.\n\nAs a side note, this isn't the only such instance of this problem. Two\nother things to think about:\n\n - You mentioned darwin fsmonitor code. And it's true that you can\n largely ignore it if you don't touch it. But every once in a while\n you get bit by it (e.g., enabling a new compiler warning which\n triggers in code you don't compile on your platform, and now you\n have to guess-and-check the fix with CI). This sucks, but is kind of\n inevitable on a cross-platform system. I think the issue with cmake\n is that because it's basically duplicating/wrapping the Makefile, it\n _feels_ unnecessary to people on platforms with working make, and\n triggers more frequently (because changes to the rest of the build\n system may break cmake in subtle ways).\n\n - I'd actually put the leak-checking CI in the same boat. It's a good\n goal, and one I hope we work towards. But it feels like the current\n state is not very mature, and people often end up wrestling with CI\n to deal with failures that they didn't even introduce (e.g., adding\n a new test that happens to run a Git program that has an existing\n leak, and now you are on the hook for figuring out why the existing\n \"passes leaks\" annotation is wrong).\n\n My original hope is that we would introduce leak-checking tooling\n that people interested in leaks could use, and other people could\n ignore until we reached a leak-free state. Because it's in CI it\n means that people get notified of new leaks in code they write\n (which is good, and helps people interested in leaks), but it also\n means they have to deal with the immature state.\n\nI'm not necessarily proposing to drop the leaks CI job here. I'm mostly\nphilosophizing about the greater problem. In the early days of Git, the\ncross-platform testing philosophy was: somebody who cares will test on\nthat platform and write a patch. If they don't, how important could it\nbe? With CI that happens automatically and it becomes everybody's\nproblem, which is a blessing and a curse.\n\n-Peff\n"},{"id":"468478","messageId":"xmqqv8mtthn4.fsf@gitster.g","threadId":"58869","inReplyTo":"Y4qF3iHW2s+I0yNe@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-03T01:12:15Z","receivedAt":"2022-12-03T01:12:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I have similar feelings to you here. Back when cmake support was\n> introduced, I explicitly wanted it to be something for people who cared\n> about it, but that wouldn't bother people who didn't use it:\n>\n> https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n>\n> I stand by that sentiment, but it seems to have crept up as a required\n> thing to deal with, and that is mostly because of CI. Using cmake in CI\n> is good for telling developers when a change they make has broken cmake.\n> But it also makes cmake their problem, and not the folks interested in\n> cmake.\n>\n> Now maybe attitudes have changed, and I am out of date, and cmake\n> support is considered mature and really important (or maybe nobody even\n> agreed with me back then ;) ). But if not, should we consider softening\n> the CI output so that cmake failures aren't \"real\" failures? That seems\n> drastic and mean, and I don't like it. But it's the root of the issue,\n> IMHO.\n\nIt makes the two of us (or three couning Ævar?). \n\n> - I'd actually put the leak-checking CI in the same boat. It's a good\n> goal, and one I hope we work towards. But it feels like the current\n> state is not very mature, and people often end up wrestling with CI\n> to deal with failures that they didn't even introduce (e.g., adding\n> a new test that happens to run a Git program that has an existing\n> leak, and now you are on the hook for figuring out why the existing\n> \"passes leaks\" annotation is wrong).\n\nHear, hear.\n\n> I'm not necessarily proposing to drop the leaks CI job here. I'm mostly\n> philosophizing about the greater problem. In the early days of Git, the\n> cross-platform testing philosophy was: somebody who cares will test on\n> that platform and write a patch. If they don't, how important could it\n> be? With CI that happens automatically and it becomes everybody's\n> problem, which is a blessing and a curse.\n\nTrue.\n"},{"id":"468479","messageId":"221203.86pmd1dyqn.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"Y4qF3iHW2s+I0yNe@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-03T01:41:04Z","receivedAt":"2022-12-03T02:09:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 02 2022, Jeff King wrote:\n\n> On Fri, Dec 02, 2022 at 05:40:34PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > [1] While our CI helps the MSVC job runs CMake manually, performs an\n>> > in-tree build and does not use ctest. In contrast a user running the \n>> > MSVC GUI does not run CMake themselves, ends up with an out-of-tree\n>> > build and runs the tests with ctest.\n>> \n>> I don't run Windows by choice, and I'm not interested in running a\n>> propriterary IDE (VS) either.\n>> \n>> The main reason I'm working on this series is that while we as a project\n>> are happy to support proprietary OS's, it hasn't been a requirement for\n>> participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or\n>> whatever to submit patches.\n>> \n>> Of course we have platform-specific code. but this CMake component is\n>> unique in how invasive it is.\n>> \n>> It's easy to e.g. stay away from the OSX-specific code in\n>> compat/fsmonitor/*darwin*.[ch], or generally speaking the\n>> Windows-specific C code.\n>> \n>> But for CMake it's become a hard requirenment for many changes, even\n>> though it's a contrib/ component.\n>\n> I have similar feelings to you here. Back when cmake support was\n> introduced, I explicitly wanted it to be something for people who cared\n> about it, but that wouldn't bother people who didn't use it:\n>\n> https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n>\n> I stand by that sentiment, but it seems to have crept up as a required\n> thing to deal with, and that is mostly because of CI. Using cmake in CI\n> is good for telling developers when a change they make has broken cmake.\n> But it also makes cmake their problem, and not the folks interested in\n> cmake.\n\nThat's a bit of a pain, but I don't think the main problem is its\nintegration with CI. It's that there doesn't really seem to be an\ninterest in its active maintenance & review from its supposed main\ntarget audience.\n\nCase in point this \"ab/cmake-nix-and-ci\" topic: It's been queued for\naround 2 months now.\n\nI don't use Windows or VS. I'd just like it not to take half may day to\nthrow things at Windows CI whenever some obscure aspect of cmake breaks.\n\nThe only person who's been showing it any notable review interest\n(Phillip) apparently hasn't used Windows actively since the mid-90's :)\n\nWhich is a very different situation than if it were still something that\nbroke CI, but there was someone (or more than one person) active on list\nwilling to test patches to get it working, was familiar with the cmake\nlanguage & could help write the cmake version of Makefile changes when\nthose were needed etc.\n\n> Now maybe attitudes have changed, and I am out of date, and cmake\n> support is considered mature and really important (or maybe nobody even\n> agreed with me back then ;) ). But if not, should we consider softening\n> the CI output so that cmake failures aren't \"real\" failures? That seems\n> drastic and mean, and I don't like it. But it's the root of the issue,\n> IMHO.\n\nYeah, maybe. Maybe if we broke it we'd get people showing up to maintain\nit again :)\n\nI do think the series I've got here is the most practical way forward at\nthis point (any outstanding issues aside). I.e. it's usually fairly\neasy-ish to amend the cmake recipe.\n\nIt's just been taking *ages* because it's been a dumpste fire outside\nof Windows, so if you don't have that OS available to you you need to\nwait for the CI. Being able to run it in seconds on *nix really helps..\n\n> As a side note, this isn't the only such instance of this problem. Two\n> other things to think about:\n>\n> - You mentioned darwin fsmonitor code. And it's true that you can\n> largely ignore it if you don't touch it. But every once in a while\n> you get bit by it (e.g., enabling a new compiler warning which\n> triggers in code you don't compile on your platform, and now you\n> have to guess-and-check the fix with CI). This sucks, but is kind of\n> inevitable on a cross-platform system. I think the issue with cmake\n> is that because it's basically duplicating/wrapping the Makefile, it\n> _feels_ unnecessary to people on platforms with working make, and\n> triggers more frequently (because changes to the rest of the build\n> system may break cmake in subtle ways).\n\nYeah, we'll always have some cross-platform pain.\n\nBut e.g. \"chmod +x\" just works in the Makefile, including when we run it\non Windows. And I've run it on Windows CI. But just upthread of here\nPhillip is reporting that it doesn't work from the context of the CMake\nrecipe.\n\nI've been throwing some things at Windows CI, but I'm pretty stumped as\nto what that might be.\n\nSome warning on Mac OS X is trivial by comparison :)\n\n> - I'd actually put the leak-checking CI in the same boat. It's a good\n> goal, and one I hope we work towards. But it feels like the current\n> state is not very mature, and people often end up wrestling with CI\n> to deal with failures that they didn't even introduce (e.g., adding\n> a new test that happens to run a Git program that has an existing\n> leak, and now you are on the hook for figuring out why the existing\n> \"passes leaks\" annotation is wrong).\n>\n> My original hope is that we would introduce leak-checking tooling\n> that people interested in leaks could use, and other people could\n> ignore until we reached a leak-free state. Because it's in CI it\n> means that people get notified of new leaks in code they write\n> (which is good, and helps people interested in leaks), but it also\n> means they have to deal with the immature state.\n>\n> I'm not necessarily proposing to drop the leaks CI job here. I'm mostly\n> philosophizing about the greater problem. In the early days of Git, the\n> cross-platform testing philosophy was: somebody who cares will test on\n> that platform and write a patch. If they don't, how important could it\n> be? With CI that happens automatically and it becomes everybody's\n> problem, which is a blessing and a curse.\n\nThat's a definitely a bit of an irresistible digression :)\n\nFirst, I think we can agree that however frustrating that's been (and\nsorry!) it would be a lot worse if my average response time to the leak\ntesting breaking something was measured in months :)\n\nI do think that whatever issues we've had with it (and in retrospect I'd\ndo some of it differently) that it's a lot more mature these days than\nyou might remember.\n\nI've been making an effort to specifically address the sorts of leaks\nthat would cause that sort of pain, e.g. adding an unsespecting \"git\nlog\" to an existing test file and the like.\n\nI have a report I generate [1] of the outstanding leaks, sorted by\nde-duping stack traces, and an estimate of how many tests would start\npassing if a combination of leaks were solved[1].\n\nOn the one hand it's ~60k lines of scary stack traces, but on the other\nhand it used to be easily 3-4x that (I can't recall the exact size\noffhand).\n\nBut more importantly while we do have some widespread leaks still, it\nused to be the case that e.g. some leaks would show up in 50-100 test\nfiles.\n\nNow the #1 leak by number of times it's seen in test files happens in\njust 12 filess, and there's a very long tail of leaks that only happen\nin one test somewhere. I.e. are specific to their own area.\n\nJust anecdotally I think it's had sort of the opposite problem recently,\nthat it's getting good enough to start flagging about new leaks that\nreally are specific to newly queued topics. E.g. [2] and [3] are two\nrecent examples.\n\nBut in both cases there seems to have been an understandable assumption\nthat it was probably just \"linux-leaks\" being noisy, due to the job\ncrying wolf in the past.\n\n1. https://vm.nix.is/~avar/noindex/2022-12-03-git-leak-report.txt\n2. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/\n3. https://lore.kernel.org/git/2488058d-dc59-e8c1-0611-fbcaeb083d73@web.de/\n"},{"id":"468509","messageId":"Y422mm4Bcu0BWPZt@coredump.intra.peff.net","threadId":"58869","inReplyTo":"221203.86pmd1dyqn.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-05T09:15:06Z","receivedAt":"2022-12-05T09:15:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > I have similar feelings to you here. Back when cmake support was\n> > introduced, I explicitly wanted it to be something for people who cared\n> > about it, but that wouldn't bother people who didn't use it:\n> >\n> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n> >\n> > I stand by that sentiment, but it seems to have crept up as a required\n> > thing to deal with, and that is mostly because of CI. Using cmake in CI\n> > is good for telling developers when a change they make has broken cmake.\n> > But it also makes cmake their problem, and not the folks interested in\n> > cmake.\n> \n> That's a bit of a pain, but I don't think the main problem is its\n> integration with CI. It's that there doesn't really seem to be an\n> interest in its active maintenance & review from its supposed main\n> target audience.\n\nYeah, there are two issues:\n\n 1. People who don't care about cmake having to think about cmake at\n all.\n\n 2. People trying to fix cmake and not getting traction.\n\nMy pain has usually been (1), but you were nice enough here to make it\nto (2). :)\n\n> > Now maybe attitudes have changed, and I am out of date, and cmake\n> > support is considered mature and really important (or maybe nobody even\n> > agreed with me back then ;) ). But if not, should we consider softening\n> > the CI output so that cmake failures aren't \"real\" failures? That seems\n> > drastic and mean, and I don't like it. But it's the root of the issue,\n> > IMHO.\n> \n> Yeah, maybe. Maybe if we broke it we'd get people showing up to maintain\n> it again :)\n\nBy the way, I looked in the archive, and me complaining about cmake has\ncome up once or twice. Johannes offered some helpful guidance on the\nvalue of running the vsbuild tests and how we might work around them:\n\n https://lore.kernel.org/git/nycvar.QRO.7.76.6.2008141352430.54@tvgsbejvaqbjf.bet/\n\n> > As a side note, this isn't the only such instance of this problem. Two\n> > other things to think about:\n> >\n> > - You mentioned darwin fsmonitor code. And it's true that you can\n> > largely ignore it if you don't touch it. But every once in a while\n> > you get bit by it (e.g., enabling a new compiler warning which\n> > triggers in code you don't compile on your platform, and now you\n> > have to guess-and-check the fix with CI). This sucks, but is kind of\n> > inevitable on a cross-platform system. I think the issue with cmake\n> > is that because it's basically duplicating/wrapping the Makefile, it\n> > _feels_ unnecessary to people on platforms with working make, and\n> > triggers more frequently (because changes to the rest of the build\n> > system may break cmake in subtle ways).\n> \n> Yeah, we'll always have some cross-platform pain.\n> \n> But e.g. \"chmod +x\" just works in the Makefile, including when we run it\n> on Windows. And I've run it on Windows CI. But just upthread of here\n> Phillip is reporting that it doesn't work from the context of the CMake\n> recipe.\n> \n> I've been throwing some things at Windows CI, but I'm pretty stumped as\n> to what that might be.\n> \n> Some warning on Mac OS X is trivial by comparison :)\n\nThat was just an example, of course. :) I have also done this kind of\n\"guess and check\" with the Windows CI. I think Johannes has given\nexamples in the past of how to actually connect to a running CI instance\nand debug interactively, but I've never managed to remember the correct\nincantation at the moment I needed it.\n\n> > I'm not necessarily proposing to drop the leaks CI job here. I'm mostly\n> > philosophizing about the greater problem. In the early days of Git, the\n> > cross-platform testing philosophy was: somebody who cares will test on\n> > that platform and write a patch. If they don't, how important could it\n> > be? With CI that happens automatically and it becomes everybody's\n> > problem, which is a blessing and a curse.\n> \n> That's a definitely a bit of an irresistible digression :)\n> \n> First, I think we can agree that however frustrating that's been (and\n> sorry!) it would be a lot worse if my average response time to the leak\n> testing breaking something was measured in months :)\n\nYes, to be clear, I have no problem with you in terms of responsiveness\nas maintaining the leaks system. It's mostly that I have to think of\nthem _at all_ when working on something unrelated (that is not itself\nintroducing new leaks).\n\n> I do think that whatever issues we've had with it (and in retrospect I'd\n> do some of it differently) that it's a lot more mature these days than\n> you might remember.\n\nYeah, it definitely has been getting better, and I admit my opinion is\nbased on a longer-term experience of a changing variable. There's\nprobably some clever name for that kind of bias, but I don't know it\noffhand.\n\n-Peff\n"},{"id":"468558","messageId":"Y45/8WnuUnP9gOMo@nand.local","threadId":"58869","inReplyTo":"221203.86pmd1dyqn.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-05T23:34:09Z","receivedAt":"2022-12-05T23:35:09Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> > I have similar feelings to you here. Back when cmake support was\n> > introduced, I explicitly wanted it to be something for people who cared\n> > about it, but that wouldn't bother people who didn't use it:\n> >\n> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n> >\n> > I stand by that sentiment, but it seems to have crept up as a required\n> > thing to deal with, and that is mostly because of CI. Using cmake in CI\n> > is good for telling developers when a change they make has broken cmake.\n> > But it also makes cmake their problem, and not the folks interested in\n> > cmake.\n>\n> That's a bit of a pain, but I don't think the main problem is its\n> integration with CI. It's that there doesn't really seem to be an\n> interest in its active maintenance & review from its supposed main\n> target audience.\n>\n> Case in point this \"ab/cmake-nix-and-ci\" topic: It's been queued for\n> around 2 months now.\n\nI think CI *is* the problem here. The CMake bits are basically a black\nbox to me (and I suspect a large number of other contributors, too). But\nwhen it breaks, the only reason we as a project end up noticing it is\nbecause it has fallout in CI.\n\nI would not be sad to make CI failures that are derived from CMake\n\"soft\" failures in the sense that they don't make the build red. But I\nthink it's masking over a couple of bigger issues:\n\n - Why do we \"support\" two build systems in CI if one is supposed to\n only be here for those that care about it? IOW, even if we say that\n CMake support is nominally an opt-in thing, in reality it isn't\n because of the dependency via CI.\n\n - Why do we only *notice* these failures in CI? I found during my time\n as interim-maintainer the task of tracking down CI failures to quite\n frustrating. It is often quite difficult to reproduce CI failures\n locally (especially with exotic build and test configurations[^1]).\n\nIt would be nice to be able to more easily see these failures locally\nbefore they hit CI. E.g., is it possible that I would work on a feature\nwhich somehow breaks the CMake build, and fail to notice it if I use\n\"make\" locally?\n\nPersonally, I would not be sad to see CMake removed from the tree\nentirely because it has not seen enough maintenance and seems to be\nquite a headache.\n\nThanks,\nTaylor\n\n[^1]: Not to mention non-Linux failures, though I think that is sort of\n par for the course if you're not using one of those platforms\n yourself.\n"},{"id":"468564","messageId":"221206.864ju9e6fa.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"Y45/8WnuUnP9gOMo@nand.local","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-05T23:46:50Z","receivedAt":"2022-12-06T00:12:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 05 2022, Taylor Blau wrote:\n\n> On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> > I have similar feelings to you here. Back when cmake support was\n>> > introduced, I explicitly wanted it to be something for people who cared\n>> > about it, but that wouldn't bother people who didn't use it:\n>> >\n>> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/\n>> >\n>> > I stand by that sentiment, but it seems to have crept up as a required\n>> > thing to deal with, and that is mostly because of CI. Using cmake in CI\n>> > is good for telling developers when a change they make has broken cmake.\n>> > But it also makes cmake their problem, and not the folks interested in\n>> > cmake.\n>>\n>> That's a bit of a pain, but I don't think the main problem is its\n>> integration with CI. It's that there doesn't really seem to be an\n>> interest in its active maintenance & review from its supposed main\n>> target audience.\n>>\n>> Case in point this \"ab/cmake-nix-and-ci\" topic: It's been queued for\n>> around 2 months now.\n>\n> I think CI *is* the problem here. The CMake bits are basically a black\n> box to me (and I suspect a large number of other contributors, too). But\n> when it breaks, the only reason we as a project end up noticing it is\n> because it has fallout in CI.\n>\n> I would not be sad to make CI failures that are derived from CMake\n> \"soft\" failures in the sense that they don't make the build red. But I\n> think it's masking over a couple of bigger issues:\n>\n> - Why do we \"support\" two build systems in CI if one is supposed to\n> only be here for those that care about it? IOW, even if we say that\n> CMake support is nominally an opt-in thing, in reality it isn't\n> because of the dependency via CI.\n\nI'm just trying to point out that that wouldn't be such a big deal if\nthe cmake parts were being actively maintained.\n\n> - Why do we only *notice* these failures in CI? I found during my time\n> as interim-maintainer the task of tracking down CI failures to quite\n> frustrating. It is often quite difficult to reproduce CI failures\n> locally (especially with exotic build and test configurations[^1]).\n>\n> It would be nice to be able to more easily see these failures locally\n> before they hit CI. E.g., is it possible that I would work on a feature\n> which somehow breaks the CMake build, and fail to notice it if I use\n> \"make\" locally?\n\nYes, some things will just work. E.g. it regex-parses out the Makefile\nto pick up the list of built-ins, so when we add a new one we'd usually\nnot need to patch the cmake bits. Although that regex parsing is its own\nproblem for some Makefile changes.\n\nTo be fair the cases where we've needed to keep it in lockstep since it\nwas added aren't that many. If you skim this and look for commits that\nalter both (or cmake quickly after the Makefile) you can spot them:\n\n git log --oneline --no-merges --full-diff --stat 1c966423263..origin/master -- contrib/buildsystems/CMakeLists.txt -- Makefile \n\n> Personally, I would not be sad to see CMake removed from the tree\n> entirely because it has not seen enough maintenance and seems to be\n> quite a headache.\n>\n> Thanks,\n> Taylor\n>\n> [^1]: Not to mention non-Linux failures, though I think that is sort of\n> par for the course if you're not using one of those platforms\n> yourself.\n\nI wouldn't mind either to see it gone, but when that was last discussed\nsome people chimed in to say that it really made things easier on\nWindows, and I'm inclined to believe them.\n\nSo I'm not trying to take their toys away, just changing it so that if\nyou run into these cmake issues it'll be trivial to debug them, as\nyou'll be able to test & run it outside of Windows.\n\nNow, I'm about to send out a v6 of it, which should address the last\nremaining Windows/VS issues.\n\nI'm not testing directly there, as I don't have such an installation, so\nit's possible I'm wrong. Maybe it works in CI, but still breaks for some\nreason when driven from VS.\n\nBut I think if we can't get anyone who's running Windows+VS to be\ninterested enough in this to test it the better thing is to just take\nthis series.\n\nMaybe it breaks some subtle aspect of the VS integration, but then\nthat'll be easier to fix on top than recovering from a \"git rm\" of it.\n"},{"id":"468567","messageId":"Y46OTYvVjdKJfQai@nand.local","threadId":"58869","inReplyTo":"221206.864ju9e6fa.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-06T00:35:25Z","receivedAt":"2022-12-06T00:37:04Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Tue, Dec 06, 2022 at 12:46:50AM +0100, Ævar Arnfjörð Bjarmason wrote:\n> I'm just trying to point out that that wouldn't be such a big deal if\n> the cmake parts were being actively maintained.\n>\n> > - Why do we only *notice* these failures in CI? I found during my time\n> > as interim-maintainer the task of tracking down CI failures to quite\n> > frustrating. It is often quite difficult to reproduce CI failures\n> > locally (especially with exotic build and test configurations[^1]).\n> >\n> > It would be nice to be able to more easily see these failures locally\n> > before they hit CI. E.g., is it possible that I would work on a feature\n> > which somehow breaks the CMake build, and fail to notice it if I use\n> > \"make\" locally?\n>\n> Yes, some things will just work. E.g. it regex-parses out the Makefile\n> to pick up the list of built-ins, so when we add a new one we'd usually\n> not need to patch the cmake bits. Although that regex parsing is its own\n> problem for some Makefile changes.\n>\n> To be fair the cases where we've needed to keep it in lockstep since it\n> was added aren't that many. If you skim this and look for commits that\n> alter both (or cmake quickly after the Makefile) you can spot them:\n>\n> git log --oneline --no-merges --full-diff --stat 1c966423263..origin/master -- contrib/buildsystems/CMakeLists.txt -- Makefile\n\nI think that this is exactly the problem I'm trying to chime in about.\nWe know that regular contributors can propose sensible changes to the\nMakefile.\n\nBut integrating CMake into our tree in such a way that forces all Git\ndevelopers to *also* be familiar with CMake is too large of a change to\nhoist onto the Git community, I think.\n\n> > Personally, I would not be sad to see CMake removed from the tree\n> > entirely because it has not seen enough maintenance and seems to be\n> > quite a headache.\n> >\n> > Thanks,\n> > Taylor\n> >\n> > [^1]: Not to mention non-Linux failures, though I think that is sort of\n> > par for the course if you're not using one of those platforms\n> > yourself.\n>\n> I wouldn't mind either to see it gone, but when that was last discussed\n> some people chimed in to say that it really made things easier on\n> Windows, and I'm inclined to believe them.\n>\n> So I'm not trying to take their toys away, just changing it so that if\n> you run into these cmake issues it'll be trivial to debug them, as\n> you'll be able to test & run it outside of Windows.\n\nOK. Personally, I wouldn't be sad to see it gone, either. I wonder: is\nthere a way to keep it in our tree such that CMake breakage isn't\neverybody's problem (like Peff originally suggested when we were\ndiscussing this in the first place)?\n\nIf so, I would vastly prefer that to the state that we have now. If not,\nI think moving it out of the tree is a sensible step, and one that I\nwould advocate for.\n\nThanks,\nTaylor\n"},{"id":"468573","messageId":"Y46clyoKk9KzFiqj@coredump.intra.peff.net","threadId":"58869","inReplyTo":"Y45/8WnuUnP9gOMo@nand.local","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-06T01:36:23Z","receivedAt":"2022-12-06T01:36:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 05, 2022 at 06:34:09PM -0500, Taylor Blau wrote:\n\n> I think CI *is* the problem here. The CMake bits are basically a black\n> box to me (and I suspect a large number of other contributors, too). But\n> when it breaks, the only reason we as a project end up noticing it is\n> because it has fallout in CI.\n> \n> I would not be sad to make CI failures that are derived from CMake\n> \"soft\" failures in the sense that they don't make the build red. But I\n> think it's masking over a couple of bigger issues:\n> \n> - Why do we \"support\" two build systems in CI if one is supposed to\n> only be here for those that care about it? IOW, even if we say that\n> CMake support is nominally an opt-in thing, in reality it isn't\n> because of the dependency via CI.\n\nI think part of the reason cmake rose in importance via CI is that it's\nthe de facto way to build for vscode. Before that CI job switched to\ncmake, there was some other alternate build system (vcxproj).\n\nSo two things I'd consider here:\n\n - how important is it for us to do the vscode build as part of regular\n CI (as opposed to folks who are interested in it running it\n themselves). Dscho gave some real data in the thread I linked to\n earlier (which indicates that yes, it helps, but not that often).\n\n - what's the status of cmake versus vcxproj? My impression (though I\n admit based on my half-paying-attention-to of the topic) is that\n cmake should replace vcxproj, and nobody would ever want to work on\n vcxproj anymore. But if that's not right, then does vcxproj cause\n headaches for non-Windows devs less often? I don't really remember\n dealing with it much, but I may have just been lucky.\n\n> - Why do we only *notice* these failures in CI? I found during my time\n> as interim-maintainer the task of tracking down CI failures to quite\n> frustrating. It is often quite difficult to reproduce CI failures\n> locally (especially with exotic build and test configurations[^1]).\n\nI think that's just the normal platform issues. You don't use it on your\nsystem, so you don't notice until it runs in CI. If people don't run CI\nthemselves, then it falls to the maintainer's CI run, which is a pain\nfor them. But that's as true of other operating systems, exotic test\nflags, etc, as it is of cmake.\n\n> It would be nice to be able to more easily see these failures locally\n> before they hit CI. E.g., is it possible that I would work on a feature\n> which somehow breaks the CMake build, and fail to notice it if I use\n> \"make\" locally?\n\nThat seems like going in the opposite direction from what you're saying\nabove: doubling down that if cmake is broken by a change, it is the\nresponsibility of the dev who made the change to find and fix it.\n\nI do like that Ævar is trying to make it easier to run cmake from Linux\nin order to find that without using CI. But that does seem orthogonal to\nme to the notion of \"who is responsible for finding and fixing cmake\nproblems\". To me, that decision is really rooted in \"is cmake something\nthe Git project supports, or is it a side-thing that some folks\nvolunteer to keep working?\".\n\n> Personally, I would not be sad to see CMake removed from the tree\n> entirely because it has not seen enough maintenance and seems to be\n> quite a headache.\n\nI don't mind having it in-tree if I can ignore it (assuming the project\nattitude is the \"it's a side thing\" from above). It's the CI failures\nthat make it hard to ignore.\n\n-Peff\n"},{"id":"468578","messageId":"221206.86zgc1cnc3.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"Y46clyoKk9KzFiqj@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-06T01:43:17Z","receivedAt":"2022-12-06T01:50:27Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 05 2022, Jeff King wrote:\n\n> On Mon, Dec 05, 2022 at 06:34:09PM -0500, Taylor Blau wrote:\n>\n>> I think CI *is* the problem here. The CMake bits are basically a black\n>> box to me (and I suspect a large number of other contributors, too). But\n>> when it breaks, the only reason we as a project end up noticing it is\n>> because it has fallout in CI.\n>> \n>> I would not be sad to make CI failures that are derived from CMake\n>> \"soft\" failures in the sense that they don't make the build red. But I\n>> think it's masking over a couple of bigger issues:\n>> \n>> - Why do we \"support\" two build systems in CI if one is supposed to\n>> only be here for those that care about it? IOW, even if we say that\n>> CMake support is nominally an opt-in thing, in reality it isn't\n>> because of the dependency via CI.\n>\n> I think part of the reason cmake rose in importance via CI is that it's\n> the de facto way to build for vscode. Before that CI job switched to\n> cmake, there was some other alternate build system (vcxproj).\n>\n> So two things I'd consider here:\n>\n> - how important is it for us to do the vscode build as part of regular\n> CI (as opposed to folks who are interested in it running it\n> themselves). Dscho gave some real data in the thread I linked to\n> earlier (which indicates that yes, it helps, but not that often).\n>\n> - what's the status of cmake versus vcxproj? My impression (though I\n> admit based on my half-paying-attention-to of the topic) is that\n> cmake should replace vcxproj, and nobody would ever want to work on\n\nI think the intent was to deprecate vcxproj, but I'm not sure, and I\nwonder if the \"cmake\" is the proposed path forward why we still have it\nin-tree anymore.\n\n> vcxproj anymore. But if that's not right, then does vcxproj cause\n> headaches for non-Windows devs less often? I don't really remember\n> dealing with it much, but I may have just been lucky.\n\nIt was less painful for non-Windows folks, but I understand the cmake\nintegration was also much nicer for VS. I.e. it's picked up by the IDE\nin a way that the \"make\" shim wasn't.\n\n> [...]\n> That seems like going in the opposite direction from what you're saying\n> above: doubling down that if cmake is broken by a change, it is the\n> responsibility of the dev who made the change to find and fix it.\n>\n> I do like that Ævar is trying to make it easier to run cmake from Linux\n> in order to find that without using CI. But that does seem orthogonal to\n> me to the notion of \"who is responsible for finding and fixing cmake\n> problems\". To me, that decision is really rooted in \"is cmake something\n> the Git project supports, or is it a side-thing that some folks\n> volunteer to keep working?\".\n\nI agree with that...\n\n>> Personally, I would not be sad to see CMake removed from the tree\n>> entirely because it has not seen enough maintenance and seems to be\n>> quite a headache.\n>\n> I don't mind having it in-tree if I can ignore it (assuming the project\n> attitude is the \"it's a side thing\" from above). It's the CI failures\n> that make it hard to ignore.\n\n...but on this thread-at-large, I'd much rather see us focus on just\nreviewing the patches I have here than raising the burden of proof to\nwhether we should get rid of it entirely.\n\nIf we make the CI failures \"soft\" failures or move it out-of-tree\nentirely it would still be useful to be able to run the cmake recipe on\n*nix.\n\n"},{"id":"468581","messageId":"Y46jT0i/7DhxHzfS@coredump.intra.peff.net","threadId":"58869","inReplyTo":"221206.86zgc1cnc3.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2022-12-06T02:05:03Z","receivedAt":"2022-12-06T02:05:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:\n\n> > I don't mind having it in-tree if I can ignore it (assuming the project\n> > attitude is the \"it's a side thing\" from above). It's the CI failures\n> > that make it hard to ignore.\n> \n> ...but on this thread-at-large, I'd much rather see us focus on just\n> reviewing the patches I have here than raising the burden of proof to\n> whether we should get rid of it entirely.\n\nFair. In case it is not obvious, I have no interest in reviewing cmake\npatches. ;) But I will at least stop making noise in the thread.\n\n> If we make the CI failures \"soft\" failures or move it out-of-tree\n> entirely it would still be useful to be able to run the cmake recipe on\n> *nix.\n\nAgreed.\n\n-Peff\n"},{"id":"468599","messageId":"221206.86mt81claa.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"Y46jT0i/7DhxHzfS@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-06T02:19:57Z","receivedAt":"2022-12-06T02:34:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Dec 05 2022, Jeff King wrote:\n\n> On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n>> > I don't mind having it in-tree if I can ignore it (assuming the project\n>> > attitude is the \"it's a side thing\" from above). It's the CI failures\n>> > that make it hard to ignore.\n>> \n>> ...but on this thread-at-large, I'd much rather see us focus on just\n>> reviewing the patches I have here than raising the burden of proof to\n>> whether we should get rid of it entirely.\n>\n> Fair. In case it is not obvious, I have no interest in reviewing cmake\n> patches. ;) But I will at least stop making noise in the thread.\n\nI'm fine with the running commentary on the future direction, I think\nit's also very useful.\n\nI just wanted to also note the need to keep the eyes on the ball a bit\n:)\n\n>> If we make the CI failures \"soft\" failures or move it out-of-tree\n>> entirely it would still be useful to be able to run the cmake recipe on\n>> *nix.\n>\n> Agreed.\n\nJust to add my own digression: I asked in some past thread (which I'm\ntoo lazy to dig up) why it was the cmake file couldn't just dispatch to\n\"make\" for most things.\n\nI.e. it needs to at some level be aware of what it's building for the\nIDE integration, but for say making a \"grep.o\" there's no reason it\ncouldn't be running:\n\n\tmake grep.o\n\nInstead of:\n\n cc -o grep grep.c [...]\n\nwhich requires duplicating much of the Makefile logic (possibly with\nsome Makefile shim to not consider any dependencies in that case).\n\nEven if we couldn't do that for *.c code for some reason it could do it\ne.g. creating the generated *.h files, which is logic we currentnly\nduplicate.\n\nThe \"win+VS build\" job even has a hard dependency on GNU make currently,\nin needing to run \"make artifacts-tar\" to get to the \"win+VS test\"\nstage.\n\nBut apparently the reason for *that* is that another goal of the\nintegration was to avoid having to have GNU make installed at all, which\ncomes in a different package than the one that would ship VS+cmake (or\nsomething?).\n\nWhich might be something to re-visit, i.e. maybe we could eventually say\n\"yes, you can have VS+cmake, but it's not too much to ask that you\ninstall GNU make as a one-off\".\n\nDoing that would then reduce the duplication to the point where the\ncmake recipe would be a thin shim around the Makefile.\n\nI don't use this development setup, but if the CI job is managing to\ndownload and run GNU make it can't be that hard for an end-user to\nsimilarly install it (but what do I know?).\n"},{"id":"468602","messageId":"xmqqilipnq8j.fsf@gitster.g","threadId":"58869","inReplyTo":"221206.86mt81claa.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-06T03:52:12Z","receivedAt":"2022-12-06T03:52:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n> Just to add my own digression: I asked in some past thread (which I'm\n> too lazy to dig up) why it was the cmake file couldn't just dispatch to\n> \"make\" for most things.\n>\n> I.e. it needs to at some level be aware of what it's building for the\n> IDE integration, but for say making a \"grep.o\" there's no reason it\n> couldn't be running:\n>\n> \tmake grep.o\n>\n> Instead of:\n>\n> cc -o grep grep.c [...]\n>\n> which requires duplicating much of the Makefile logic (possibly with\n> some Makefile shim to not consider any dependencies in that case).\n\nThat leads to a question at the other extreme. Why does any logic\nin CMakeLists.txt even have to exist at all? Whenever it is asked\nto make foo, it can be running \"make foo\" instead of having its own\nlogic at all. ;-)\n"},{"id":"468610","messageId":"30360f4c-91a5-177b-133f-eb7036ed676a@dunelm.org.uk","threadId":"58869","inReplyTo":"xmqqilipnq8j.fsf@gitster.g","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2022-12-06T09:54:21Z","receivedAt":"2022-12-06T10:03:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/12/2022 03:52, Junio C Hamano wrote:\n> Ævar Arnfjörð Bjarmason writes:\n> \n>> Just to add my own digression: I asked in some past thread (which I'm\n>> too lazy to dig up) why it was the cmake file couldn't just dispatch to\n>> \"make\" for most things.\n\nBecause make is not installed by default on Windows. Our CI job uses \nmsbuild (whatever that is) and when I was playing with Visual Studio \nlast week it was using ninja.\n\n>> I.e. it needs to at some level be aware of what it's building for the\n>> IDE integration, but for say making a \"grep.o\" there's no reason it\n>> couldn't be running:\n>>\n>> \tmake grep.o\n>>\n>> Instead of:\n>>\n>> cc -o grep grep.c [...]\n>>\n>> which requires duplicating much of the Makefile logic (possibly with\n>> some Makefile shim to not consider any dependencies in that case).\n> \n> That leads to a question at the other extreme. Why does any logic\n> in CMakeLists.txt even have to exist at all? Whenever it is asked\n> to make foo, it can be running \"make foo\" instead of having its own\n> logic at all. ;-)\n\nYes, if make was available then we wouldn't need to use CMake.\n\nBest Wishes\n\nPhillip\n"},{"id":"468616","messageId":"221206.865yeodbtg.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"30360f4c-91a5-177b-133f-eb7036ed676a@dunelm.org.uk","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-06T10:57:37Z","receivedAt":"2022-12-06T11:13:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 06 2022, Phillip Wood wrote:\n\n> On 06/12/2022 03:52, Junio C Hamano wrote:\n>> Ævar Arnfjörð Bjarmason writes:\n>> \n>>> Just to add my own digression: I asked in some past thread (which I'm\n>>> too lazy to dig up) why it was the cmake file couldn't just dispatch to\n>>> \"make\" for most things.\n>\n> Because make is not installed by default on Windows. Our CI job uses\n> msbuild (whatever that is) and when I was playing with Visual Studio \n> last week it was using ninja.\n>\n>>> I.e. it needs to at some level be aware of what it's building for the\n>>> IDE integration, but for say making a \"grep.o\" there's no reason it\n>>> couldn't be running:\n>>>\n>>> \tmake grep.o\n>>>\n>>> Instead of:\n>>>\n>>> cc -o grep grep.c [...]\n>>>\n>>> which requires duplicating much of the Makefile logic (possibly with\n>>> some Makefile shim to not consider any dependencies in that case).\n>> That leads to a question at the other extreme. Why does any logic\n>> in CMakeLists.txt even have to exist at all? Whenever it is asked\n>> to make foo, it can be running \"make foo\" instead of having its own\n>> logic at all. ;-)\n>\n> Yes, if make was available then we wouldn't need to use CMake.\n\nI think Junio and I are talking about something slightly different. Yes\n\"make\" isn't available by default. Getting it requires installing a\nlarger SDK.\n\nBut if you look at the history of contrib/vscode/README.md in our tree\nyou'll see that we used to support this \"Visual Studio Solution\" for\nyears via GNU make, it probably still works.\n\nThe change in 4c2c38e800f (ci: modification of main.yml to use cmake for\nvs-build job, 2020-06-26) shows when the CI was switched over to using\ncmake instead.\n\nThe code to support that is still in-tree as the \"vcxproj\" target in\n\"config.mak.uname\", which calls out to the ~1k lines of Perl code in\ncontrib/buildsystems/Generators/*.\n\nI'm not suggesting we go back to that. The question is whether the\ntrade-off of supporting an entirely separate build system without a GNU\nmake dependency was worth it.\n\nOn the one hand those developing on Windows don't need to install it as\na package, on the other hand we end up spending more developer time in\nwriting duplicate build logic.\n\nThe advantage of cmake is that it knows how to generate all that\nVS-integration XML etc., so as soon as it knows how to build Git you get\nthat for free.\n\nBut I think you can get that while not having a \"real\" cmake recipe, but\njust a thin shim for calling out to the Makefile.\n\nIs such a thing a hack? Yes. Is it silly to e.g. build with \"ninja\" and\nreally have it just shell out to \"make\"? Yes.\n\nBut it might still be worth it if we judge that the goal of getting that\nVS integration is sufficient. And that we're not willing to absorb the\ncost of maintaining two distinct build recipes in perpetuity.\n"},{"id":"468662","messageId":"Y4/lu8Ekt/DP6imJ@nand.local","threadId":"58869","inReplyTo":"Y46jT0i/7DhxHzfS@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2022-12-07T01:00:43Z","receivedAt":"2022-12-07T01:00:48Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Mon, Dec 05, 2022 at 09:05:03PM -0500, Jeff King wrote:\n> On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:\n>\n> > > I don't mind having it in-tree if I can ignore it (assuming the project\n> > > attitude is the \"it's a side thing\" from above). It's the CI failures\n> > > that make it hard to ignore.\n> >\n> > ...but on this thread-at-large, I'd much rather see us focus on just\n> > reviewing the patches I have here than raising the burden of proof to\n> > whether we should get rid of it entirely.\n>\n> Fair. In case it is not obvious, I have no interest in reviewing cmake\n> patches. ;) But I will at least stop making noise in the thread.\n\nSame, and same.\n\nSorry for semi-derailing this thread. I don't feel strongly enough about\nthe CMake stuff to start a new thread about it on the list, but while\nothers are discussing it, I figured that I might as well chime in.\n\nThanks\nTaylor\n"},{"id":"468774","messageId":"oq7p2776-po8p-r9s0-82o2-o77so874n419@tzk.qr","threadId":"58869","inReplyTo":"221206.865yeodbtg.gmgdl@evledraar.gmail.com","subject":"ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2022-12-08T09:29:17Z","receivedAt":"2022-12-08T09:29:49Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 6 Dec 2022, Ævar Arnfjörð Bjarmason wrote:\n\n> On Tue, Dec 06 2022, Phillip Wood wrote:\n>\n> > On 06/12/2022 03:52, Junio C Hamano wrote:\n> >> Ævar Arnfjörð Bjarmason writes:\n> >>\n> >>> Just to add my own digression: I asked in some past thread (which\n> >>> I'm too lazy to dig up) why it was the cmake file couldn't just\n> >>> dispatch to \"make\" for most things.\n> >\n> > Because make is not installed by default on Windows. Our CI job uses\n> > msbuild (whatever that is) and when I was playing with Visual Studio\n> > last week it was using ninja.\n> >\n> >>> I.e. it needs to at some level be aware of what it's building for the\n> >>> IDE integration, but for say making a \"grep.o\" there's no reason it\n> >>> couldn't be running:\n> >>>\n> >>> \tmake grep.o\n> >>>\n> >>> Instead of:\n> >>>\n> >>> cc -o grep grep.c [...]\n> >>>\n> >>> which requires duplicating much of the Makefile logic (possibly with\n> >>> some Makefile shim to not consider any dependencies in that case).\n> >> That leads to a question at the other extreme. Why does any logic\n> >> in CMakeLists.txt even have to exist at all? Whenever it is asked\n> >> to make foo, it can be running \"make foo\" instead of having its own\n> >> logic at all. ;-)\n> >\n> > Yes, if make was available then we wouldn't need to use CMake.\n>\n> I think Junio and I are talking about something slightly different. Yes\n> \"make\" isn't available by default. Getting it requires installing a\n> larger SDK.\n>\n> But if you look at the history of contrib/vscode/README.md in our tree\n> you'll see that we used to support this \"Visual Studio Solution\" for\n> years via GNU make, it probably still works.\n\nIt probably doesn't. Last time I had to use it, during the embargoed\nv2.37.1 release process, it didn't. I had to add plenty of patches to make\nit work again:\nhttps://github.com/git-for-windows/git/compare/323a69709944%5E...323a69709944%5E2\n\n> The change in 4c2c38e800f (ci: modification of main.yml to use cmake for\n> vs-build job, 2020-06-26) shows when the CI was switched over to using\n> cmake instead.\n>\n> The code to support that is still in-tree as the \"vcxproj\" target in\n> \"config.mak.uname\", which calls out to the ~1k lines of Perl code in\n> contrib/buildsystems/Generators/*.\n\nAt some stage we can probably get rid of the `vcxproj` code. Before that,\nwe can even get rid of the `vcproj` code that is bit-rotting in\n`contrib/buildsystems/`. But there seems no harm, and less maintenance\nburden, in keeping the `vcxproj`/`vcproj` parts where they are, as they\nare.\n\nTaking a step back, I see that we got far away from the topic that started\nthis thread.\n\nSo here's my take on `ab/cmake-nix-and-ci`: While that patch series'\nintention is apparently to make it easier to diagnose and fix CI problems,\nI only see that it adds new problems. It won't make it possible to\ndiagnose most win+VS problems because they don't reproduce on Linux. But\nthe patches already did introduce Windows-specific problems merely by\ntrying to get the Linux side of CMake to work. And trying to keep CMake\nworking both on Linux and on Windows would cause many more problems in the\nfuture. And we do not even need CMake support for Linux, `make` works well\nthere already. It would increase the maintenance burden unnecessarily.\n\nI am therefore suggesting to drop `cmake-nix-and-ci` entirely.\n\nTo address the concern about broken CI runs, I hoped that monitoring them\nand helping contributors by suggesting fixups was working well enough. It\nused to be okay for patches to be contributed that caused CI to fail, we\nsimply worked together to fix CI, as a team, and that was that. We didn't\n\"blame\" the contributors, or anything, when CI runs failed because of\ntheir patches. After all, possible causes of CI failures include that\npatches might be applied on top of other commits than intended, or that\npatch series interact in unfortunate ways.\n\nJunio, maybe you could clarify your take on this? As project lead, it is\nyour decision to define how Git uses Continuous Builds, and how the\nproject handles failed CI runs.\n\nCiao,\nJohannes\n"},{"id":"468780","messageId":"221208.86wn726qcv.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"oq7p2776-po8p-r9s0-82o2-o77so874n419@tzk.qr","subject":"Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-08T11:34:49Z","receivedAt":"2022-12-08T12:18:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 08 2022, Johannes Schindelin wrote:\n\n> Hi,\n>\n> On Tue, 6 Dec 2022, Ævar Arnfjörð Bjarmason wrote:\n>\n>> On Tue, Dec 06 2022, Phillip Wood wrote:\n>>\n>> > On 06/12/2022 03:52, Junio C Hamano wrote:\n>> >> Ævar Arnfjörð Bjarmason writes:\n>> >>\n>> >>> Just to add my own digression: I asked in some past thread (which\n>> >>> I'm too lazy to dig up) why it was the cmake file couldn't just\n>> >>> dispatch to \"make\" for most things.\n>> >\n>> > Because make is not installed by default on Windows. Our CI job uses\n>> > msbuild (whatever that is) and when I was playing with Visual Studio\n>> > last week it was using ninja.\n>> >\n>> >>> I.e. it needs to at some level be aware of what it's building for the\n>> >>> IDE integration, but for say making a \"grep.o\" there's no reason it\n>> >>> couldn't be running:\n>> >>>\n>> >>> \tmake grep.o\n>> >>>\n>> >>> Instead of:\n>> >>>\n>> >>> cc -o grep grep.c [...]\n>> >>>\n>> >>> which requires duplicating much of the Makefile logic (possibly with\n>> >>> some Makefile shim to not consider any dependencies in that case).\n>> >> That leads to a question at the other extreme. Why does any logic\n>> >> in CMakeLists.txt even have to exist at all? Whenever it is asked\n>> >> to make foo, it can be running \"make foo\" instead of having its own\n>> >> logic at all. ;-)\n>> >\n>> > Yes, if make was available then we wouldn't need to use CMake.\n>>\n>> I think Junio and I are talking about something slightly different. Yes\n>> \"make\" isn't available by default. Getting it requires installing a\n>> larger SDK.\n>>\n>> But if you look at the history of contrib/vscode/README.md in our tree\n>> you'll see that we used to support this \"Visual Studio Solution\" for\n>> years via GNU make, it probably still works.\n>\n> It probably doesn't. Last time I had to use it, during the embargoed\n> v2.37.1 release process, it didn't. I had to add plenty of patches to make\n> it work again:\n> https://github.com/git-for-windows/git/compare/323a69709944%5E...323a69709944%5E2\n>\n>> The change in 4c2c38e800f (ci: modification of main.yml to use cmake for\n>> vs-build job, 2020-06-26) shows when the CI was switched over to using\n>> cmake instead.\n>>\n>> The code to support that is still in-tree as the \"vcxproj\" target in\n>> \"config.mak.uname\", which calls out to the ~1k lines of Perl code in\n>> contrib/buildsystems/Generators/*.\n>\n> At some stage we can probably get rid of the `vcxproj` code. Before that,\n> we can even get rid of the `vcproj` code that is bit-rotting in\n> `contrib/buildsystems/`. But there seems no harm, and less maintenance\n> burden, in keeping the `vcxproj`/`vcproj` parts where they are, as they\n> are.\n>\n> Taking a step back, I see that we got far away from the topic that started\n> this thread.\n>\n> So here's my take on `ab/cmake-nix-and-ci`: While that patch series'\n> intention is apparently to make it easier to diagnose and fix CI problems,\n> I only see that it adds new problems. It won't make it possible to\n> diagnose most win+VS problems because they don't reproduce on Linux.\n\nThat would also be my take if that was the goal of the series. I agree\nthat would be pretty pointless. Why test win+VS-specific code on Linux?\nThat makes no sense.\n\nBut that's not the goal.\n\nIt's to make it easier to test the majority of the platform-agnostic\ncode in the cmake recipe. E.g. here's some past commits from myself,\nJeff King and Han-Wen (who I'm pretty sure doesn't use Windows) where\nwe've had to patch the cmake recipe in addition to the Makefile:\n\t\n\tef8a6c62687 (reftable: utility functions, 2021-10-07)\n\tcfe853e66be (hook-list.h: add a generated list of hooks, like config-list.h, 2021-09-26)\n\td7a5649c82d (make git-bugreport a builtin, 2020-08-13)\n\tb5dd96b70ac (make credential helpers builtins, 2020-08-13)\n\nDoing that currently requires bouncing things off the Windows CI. With\nab/cmake-nix-and-ci you can not only build (which currently works) but\nrun the full test against the cmake build on *nix in minutes. That's a\nbig improvement.\n\nI think this also misrepresents the nature of the cmake recipe, and how\nmuch of it is truly MSVC or VS-specific. I count less than 20 lines in\na ~1.1k line recipe that are really \"MSVC\"-specific. I.e. guarded by\n\"if\" branches checking \"MSVC\" and 'CMAKE_C_COMPILER_ID STREQUAL \"MSVC\"'.\n\nThe rest are general in nature. E.g. you can run the tests with \"ctest\"\nfrom the VS GUI.\n\nThat's not because there's a VS-specific \"hey Visual Studio, here's our\ntests\" part of the recipe. Rather there's a generic cross-platform\nmethod of declaring how to run the tests, which cmake itself then knows\nto pick up and generate a VS-specific asset with.\n\nWhereas you seem to be suggesting that the recipe is so\nWindows+VS-specific that testing it on other platforms isn't going to\ntell you much. I don't think that's true.\n\t\n> But the patches already did introduce Windows-specific problems merely\n> by trying to get the Linux side of CMake to work.\n\nThis seems like vague commentary on past bugs.\n\nDo you have any specific concerns about things that are broken by the\ncurrent v6 iteration?\n\n> And trying to keep CMake\n> working both on Linux and on Windows would cause many more problems in the\n> future. And we do not even need CMake support for Linux, `make` works well\n> there already. It would increase the maintenance burden unnecessarily.\n\nIf you're going to argue that \"we do not even need CMake support for\nLinux\" you're not making an argument against ab/cmake-nix-and-ci, but\nagainst the status quo on \"master\".\n\nIt's already the case that it mostly \"works\" on Linux, that's been the\ncase since day 1. E.g. Yuyi's a561962479c (cmake: fix CMakeLists.txt on\nLinux, 2022-05-24) earlier this year.\n\n> I am therefore suggesting to drop `cmake-nix-and-ci` entirely.\n\nI wouldn't mind if we declare that it should never work on Linux, and I\nwouldn't mind if we drop ab/cmake-nix-and-ci entirely.\n\nBut only *if* the cmake recipe becomes purely a \"lag-behind\" alternative\nbuild system that the Windows folks are responsible for updating, or to\nsubmit follow-up patches for if it breaks.\n\nThat's not the status quo now, as e.g. Jeff King summarized nicely here:\n\n\thttps://lore.kernel.org/git/Y4qF3iHW2s+I0yNe@coredump.intra.peff.net/\n\nYou don't seem to be suggesting a productive way forward with that.\n\nI don't think dropping it, or even making the Windows cmake CI optional\nwould be a productive way forward, I think it would ultimately waste\nmore of your time, and that of other Windows developers.\n\nBut I don't see how that isn't the logical conclusion to not providing\nspecific feedback on this topic & finding a way forward with it.\n"},{"id":"468815","messageId":"xmqqk0311blt.fsf@gitster.g","threadId":"58869","inReplyTo":"oq7p2776-po8p-r9s0-82o2-o77so874n419@tzk.qr","subject":"Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-12-09T03:48:14Z","receivedAt":"2022-12-09T03:48:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Junio, maybe you could clarify your take on this? As project lead, it is\n> your decision to define how Git uses Continuous Builds, and how the\n> project handles failed CI runs.\n\nI have pretty much been with what Peff and Taylor said in the thread\nalready ever since we added CMake support to help Windows/VS folks.\nI agree with you that we do not need to run it for Linux or macOS,\nand if the promised/hoped-for benefit, i.e. that running them on\nnon-Windows build would uncover issues that are common across the\nplatform and help Windows, is not something that is likely to\nmaterialize, I'd prefer to see our resources (CI time and developer\nattention) not spent on that.\n\nI do not think \"how the project handles filed CI runs\" is a very big\nissue. I often ignore partial failures (e.g. \"winVS(n) test job\ntriggered rate limit\") and the only annoyance I feel is that such a\ntemporary failure contribute one more message to my trash mailbox,\nand I can learn to do the same for a test that marked as failed due\nto linux-cmake-ctest job. I expect that regular contributors are\ndoing the same pretty much.\n\nHow blocking is a CI failure for drive-by contributors who use GGG?\nWhile I do not necessarily value drive-by contributions as much as\nyou do, if such \"an unimportant failure we can ignore\" discourages\nthose coming from GGG route, that would be unfortunate, exactly\nbecause they may not have contributed anything to the failures.\nThis is not just cmake-ctest, but the leak checking job where a new\nuse of a tool that is known to be leaky in a test can turn a test\nthat has been passing to fail. If we can mark failures in selected\njobs as non-blocking, we definitely should do so.\n\nBetween keeping and marking linux-cmake-ctest as non-blocking, and\nremoving it altogether, I am inclined to say that I'd favor the\nlatter for the reasons I explained earlier in this message. But to\nhelp casual contributors coming via GGG, we would anyway need to (1)\nallow submitting even with failing tests, and (2) tell them that it\nis OK to do so. Which means it is not the end of the world, from\nthe point of view of helping casual developers, if we had kept these\nbrittle CI jobs like linux-cmake-ctest and linux-leaks.\n\n"},{"id":"468827","messageId":"221209.867cz07jub.gmgdl@evledraar.gmail.com","threadId":"58869","inReplyTo":"xmqqk0311blt.fsf@gitster.g","subject":"Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover \"git\" in subdirs of \"contrib/buildsystems/out\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2022-12-09T13:55:08Z","receivedAt":"2022-12-09T14:08:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Dec 09 2022, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n>> Junio, maybe you could clarify your take on this? As project lead, it is\n>> your decision to define how Git uses Continuous Builds, and how the\n>> project handles failed CI runs.\n>\n> I have pretty much been with what Peff and Taylor said in the thread\n> already ever since we added CMake support to help Windows/VS folks.\n> I agree with you that we do not need to run it for Linux or macOS,\n> and if the promised/hoped-for benefit, i.e. that running them on\n> non-Windows build would uncover issues that are common across the\n> platform and help Windows, is not something that is likely to\n> materialize, I'd prefer to see our resources (CI time and developer\n> attention) not spent on that.\n\nI think this should be addressed by the \"I count less than 20 lines in a\n~1.1k line recipe that are really \"MSVC\"-specific\" in the sibling\nmail[1]. I.e. the large majority of it is generic recipe code that's run\non all platforms.\n\n> I do not think \"how the project handles filed CI runs\" is a very big\n> issue. I often ignore partial failures (e.g. \"winVS(n) test job\n> triggered rate limit\") and the only annoyance I feel is that such a\n> temporary failure contribute one more message to my trash mailbox,\n> and I can learn to do the same for a test that marked as failed due\n> to linux-cmake-ctest job. I expect that regular contributors are\n> doing the same pretty much.\n>\n> How blocking is a CI failure for drive-by contributors who use GGG?\n> While I do not necessarily value drive-by contributions as much as\n> you do, if such \"an unimportant failure we can ignore\" discourages\n> those coming from GGG route, that would be unfortunate, exactly\n> because they may not have contributed anything to the failures.\n> This is not just cmake-ctest, but the leak checking job where a new\n> use of a tool that is known to be leaky in a test can turn a test\n> that has been passing to fail. If we can mark failures in selected\n> jobs as non-blocking, we definitely should do so.\n\nI realize that we've been digressing to the larger topic of what to do\nwith CI in general, but I don't think the question of whether say\n\"win+VS build\" should \"soft fail\" is something we should conflate with\nthis \"ab/cmake-nix-and-ci\" topic.\n\nIt doesn't change the status quo there, and I think is a net\nimprovement.\n\nEven if we make the CI for anything cmake-related soft-fail, this topic\nwill still help to get it back up to speed, as you'll be able to run the\nfull cmake+ctest chain on non-Windows.\n\n> Between keeping and marking linux-cmake-ctest as non-blocking, and\n> removing it altogether, I am inclined to say that I'd favor the\n> latter for the reasons I explained earlier in this message. But to\n> help casual contributors coming via GGG, we would anyway need to (1)\n> allow submitting even with failing tests, and (2) tell them that it\n> is OK to do so. Which means it is not the end of the world, from\n> the point of view of helping casual developers, if we had kept these\n> brittle CI jobs like linux-cmake-ctest and linux-leaks.\n\nI can peel off the commit that adds the \"linux-cmake-ctest\" CI job from\nthis series, or even just make it do the equivalent of:\n\n\tcmake && make || echo oops, we're broken\n\nSo it'll \"soft-fail\" (AFAIK GitHub CI, unlike GitLab[2] doesn't support\na native way to \"soft-fail\").\n\nBut I don't think that doing that would help without *also* making\n\"win+VS {build,test}\" soft-fail. I.e. if \"linux-cmake-ctest\" *and*\n\"win+VS\" (with all else passing) you can be pretty sure it's a generic\ncmake problem.\n\nIf only one or the other is failing somewhere in cmake having the\n\"linux-cmake-ctest\" job now will help narrow down whether it's a\nplatform-specific cmake issue.\n\nSo, just let me know what you'd prefer, but I think per the above even\nif you're impatient with cmake failures the \"linux-cmake-ctest\" job\nshould help spend less time on them.\n\n1. https://lore.kernel.org/git/221208.86wn726qcv.gmgdl@evledraar.gmail.com/\n2. https://docs.gitlab.com/ee/ci/yaml/#allow_failure -- In the UX:\n green=passing, red=failing, yellow=soft-fail\n"}]}