* What's cooking in git.git (Nov 2022, #07; Tue, 29)
@ 2022-11-29 9:40 Junio C Hamano
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
` (4 more replies)
0 siblings, 5 replies; 43+ messages in thread
From: Junio C Hamano @ 2022-11-29 9:40 UTC (permalink / raw)
To: git
Here are the topics that have been cooking in my tree. Commits
prefixed with '+' are in 'next' (being in 'next' is a sign that a
topic is stable enough to be used and are candidate to be in a future
release). Commits prefixed with '-' are only in 'seen', and aren't
considered "accepted" at all. A topic without enough support may be
discarded after a long period of no activity.
The preview release -rc0 for this cycle has been tagged.
Copies of the source code to Git live in many repositories, and the
following is a list of the ones I push into or their mirrors. Some
repositories have only a subset of branches.
With maint, master, next, seen, todo:
git://git.kernel.org/pub/scm/git/git.git/
git://repo.or.cz/alt-git.git/
https://kernel.googlesource.com/pub/scm/git/git/
https://github.com/git/git/
https://gitlab.com/git-vcs/git/
With all the integration branches and topics broken out:
https://github.com/gitster/git/
Even though the preformatted documentation in HTML and man format
are not sources, they are published in these repositories for
convenience (replace "htmldocs" with "manpages" for the manual
pages):
git://git.kernel.org/pub/scm/git/git-htmldocs.git/
https://github.com/gitster/git-htmldocs.git/
Release tarballs are available at:
https://www.kernel.org/pub/software/scm/git/
--------------------------------------------------
[Graduated to 'master']
* ab/doc-synopsis-and-cmd-usage (2022-11-27) 1 commit
(merged to 'next' on 2022-11-28 at a517ea2f95)
+ i18n: fix command template placeholder format
Doc and message fix.
source: <pull.1435.git.1669483442230.gitgitgadget@gmail.com>
* ab/fewer-the-index-macros (2022-11-21) 11 commits
(merged to 'next' on 2022-11-23 at de20206cac)
+ cocci: apply "pending" index-compatibility to some "builtin/*.c"
+ cache.h & test-tool.h: add & use "USE_THE_INDEX_VARIABLE"
+ {builtin/*,repository}.c: add & use "USE_THE_INDEX_VARIABLE"
+ cocci: apply "pending" index-compatibility to "t/helper/*.c"
+ cocci & cache.h: apply variable section of "pending" index-compatibility
+ cocci & cache.h: apply a selection of "pending" index-compatibility
+ cocci: add a index-compatibility.pending.cocci
+ read-cache API & users: make discard_index() return void
+ cocci & cache.h: remove rarely used "the_index" compat macros
+ builtin/{grep,log}.: don't define "USE_THE_INDEX_COMPATIBILITY_MACROS"
+ cache.h: remove unused "the_index" compat macros
Progress on removing 'the_index' convenience wrappers.
source: <cover-v2-00.11-00000000000-20221119T125550Z-avarab@gmail.com>
* ah/chainlint-cpuinfo-parse-fix (2022-11-23) 1 commit
(merged to 'next' on 2022-11-28 at 1e51eafde5)
+ chainlint.pl: fix /proc/cpuinfo regexp
The format of a line in /proc/cpuinfo that describes a CPU on s390x
looked different from everybody else, and the code in chainlint.pl
failed to parse it.
source: <pull.1385.git.git.1669148861635.gitgitgadget@gmail.com>
* es/locate-httpd-module-location-in-test (2022-11-22) 1 commit
(merged to 'next' on 2022-11-23 at dfa19a744f)
+ lib-httpd: extend module location auto-detection
Add one more candidate directory that may house httpd modules while
running tests.
source: <pull.1426.git.1668999695898.gitgitgadget@gmail.com>
* ew/prune-with-missing-objects-pack (2022-11-21) 1 commit
(merged to 'next' on 2022-11-23 at bd328c5d01)
+ prune: quiet ENOENT on missing directories
"git prune" may try to iterate over .git/objects/pack for trash
files to remove in it, and loudly fail when the directory is
missing, which is not necessary. The command has been taught to
ignore such a failure.
source: <20221119201213.2398081-1-e@80x24.org>
* gc/resolve-alternate-symlinks (2022-11-25) 1 commit
(merged to 'next' on 2022-11-28 at 509d2005aa)
+ object-file: use real paths when adding alternates
Resolve symbolic links when processing the locations of alternate
object stores, since failing to do so can lead to confusing and buggy
behavior.
source: <pull.1382.v3.git.git.1669251331340.gitgitgadget@gmail.com>
* jh/trace2-timers-and-counters (2022-11-25) 1 commit
(merged to 'next' on 2022-11-28 at 2fa64103b7)
+ trace2 tests: guard pthread test with "PTHREAD"
Test fix.
source: <patch-1.1-f7f21c94a6c-20221124T214813Z-avarab@gmail.com>
* jk/parse-object-type-mismatch (2022-11-22) 3 commits
(merged to 'next' on 2022-11-22 at 69dc60536b)
+ parse_object(): simplify blob conditional
(merged to 'next' on 2022-11-18 at 1ee133a089)
+ parse_object(): check on-disk type of suspected blob
+ parse_object(): drop extra "has" check before checking object type
(this branch is used by ab/tag-object-type-errors.)
`parse_object()` hardening when checking for the existence of a
suspected blob object.
source: <Y3vQ/6QcTEFfpjLt@coredump.intra.peff.net>
* jx/ci-ubuntu-fix (2022-11-27) 4 commits
(merged to 'next' on 2022-11-28 at 2eaa5b6c61)
+ ci: install python on ubuntu
+ ci: use the same version of p4 on both Linux and macOS
+ ci: remove the pipe after "p4 -V" to catch errors
+ github-actions: run gcc-8 on ubuntu-20.04 image
Adjust the GitHub CI to newer ubuntu release.
source: <20221124153934.12470-1-worldhello.net@gmail.com>
* km/merge-recursive-typofix (2022-11-27) 1 commit
(merged to 'next' on 2022-11-28 at cb6c488e9c)
+ merge-recursive: fix variable typo in error message
Fix an old typo in an error message.
source: <20221125173745.738643-1-kyle@kyleam.com>
* pw/config-int-parse-fixes (2022-11-09) 3 commits
(merged to 'next' on 2022-11-23 at 06ee2fff7b)
+ git_parse_signed(): avoid integer overflow
+ config: require at least one digit when parsing numbers
+ git_parse_unsigned: reject negative values
Assorted fixes of parsing end-user input as integers.
source: <pull.1389.v2.git.1668003388.gitgitgadget@gmail.com>
* rs/list-objects-filter-leakfix (2022-11-21) 1 commit
(merged to 'next' on 2022-11-23 at 1e148fc7d2)
+ list-objects-filter: plug combine_filter_data leak
Leakfix.
source: <bc25ac8c-ce9a-2385-be0d-0c72798d319d@web.de>
* sg/plug-line-log-leaks (2022-11-02) 3 commits
(merged to 'next' on 2022-11-23 at 5d2e1b065b)
+ diff.c: use diff_free_queue()
+ line-log: free the diff queues' arrays when processing merge commits
+ line-log: free diff queue when processing non-merge commits
A handful of leaks in the line-log machinery have been plugged.
source: <20221102220142.574890-1-szeder.dev@gmail.com>
* zk/push-use-bitmaps (2022-11-22) 1 commit
(merged to 'next' on 2022-11-23 at 2ce20bcaf5)
+ t5516: fail to run in verbose mode
Test fix.
source: <20221121134040.12260-1-worldhello.net@gmail.com>
--------------------------------------------------
[New Topics]
* rr/status-untracked-advice (2022-11-25) 1 commit
- status: modernize git-status "slow untracked files" advice
The advice message given by "git status" when it takes long time to
enumerate untracked paths has been updated.
Will merge to 'next'?
source: <pull.1384.v8.git.1669154823035.gitgitgadget@gmail.com>
* sa/git-var-empty (2022-11-27) 2 commits
- var: allow GIT_EDITOR to return null
- var: do not print usage() with a correct invocation
"git var UNKNOWN_VARIABLE" and "git var VARIABLE" with the variable
given an empty value used to behave identically. Now the latter
just gives an empty output, while the former still gives an error
message.
Will merge to 'next'?
source: <pull.1434.v3.git.1669472277.gitgitgadget@gmail.com>
* jx/t1301-updates (2022-11-29) 3 commits
- t1301: do not change $CWD in "shared=all" test case
- t1301: use test_when_finished for cleanup
- t1301: fix wrong template dir for git-init
Test updates.
Seems to break CI.
source: <20221127145130.16155-1-worldhello.net@gmail.com>
* km/send-email-with-v-reroll-count (2022-11-27) 1 commit
- send-email: relay '-v N' to format-patch
"git send-email -v 3" used to be expanded to "git send-email
--validate 3" when the user meant to pass them down to
"format-patch", which has been corrected.
Seems to break CI.
source: <87edtp5uws.fsf@kyleam.com>
* ps/gnumake-4.4-fix (2022-11-28) 1 commit
(merged to 'next' on 2022-11-29 at 1151bc06fc)
+ Makefile: avoid multiple patterns when recipes generate one file
Adjust Makefile for GNU make 4.4
Will cook in 'next'.
source: <20221127224251.2508200-2-psmith@gnu.org>
* so/diff-merges-more (2022-11-28) 5 commits
- diff-merges: issue warning on lone '-m' option
- diff-merges: support list of values for --diff-merges
- diff-merges: implement log.diffMergesForce config
- diff-merges: implement log.diffMerges-m-imply-p config
- diff-merges: implement [no-]hide option and log.diffMergesHide config
Assorted updates to "--diff-merges=X" option.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3560918726
source: <20221127093721.31012-1-sorganov@gmail.com>
--------------------------------------------------
[Stalled]
* ms/sendemail-validate-headers (2022-11-11) 1 commit
. Expose header information to git-send-email's sendemail-validate hook
Expecting a reroll.
Appears to break t9001 completely?
source: <20221111194223.644845-2-michael.strawbridge@amd.com>
* hl/archive-recursive (2022-10-19) 10 commits
. fixup! archive: add tests for git archive --recurse-submodules
. archive: add tests for git archive --recurse-submodules
. archive: add --recurse-submodules to git-archive command
. archive: remove global repository from archive_args
. archive: pass repo objects to write_archive handlers
. tree: add repository parameter to read_tree_fn_t
. tree: handle submodule case for read_tree_at properly
. tree: increase test coverage for tree.c
. tree: update cases to use repo_ tree methods
. tree: do not use the_repository for tree traversal methods.
"git archive" has been taught "--recurse-submodules" option to
create a tarball that includes contents from submodules.
Expecting a reroll.
Seems to break win+VS test(8).
cf. https://github.com/git/git/actions/runs/3293333066 whose only
difference from https://github.com/git/git/actions/runs/3293553109
is the inclusion of this topic.
source: <pull.1359.v3.git.git.1665973401.gitgitgadget@gmail.com>
* pw/test-todo (2022-10-06) 3 commits
. test_todo: allow [verbose] test as the command
. test_todo: allow [!] grep as the command
. tests: add test_todo() to mark known breakages
RFC for test framework improvement.
Needs review.
source: <pull.1374.git.1665068476.gitgitgadget@gmail.com>
* cw/submodule-status-in-parallel (2022-11-08) 6 commits
- diff-lib: parallelize run_diff_files for submodules
- diff-lib: refactor match_stat_with_submodule
- submodule: move status parsing into function
- submodule: strbuf variable rename
- run-command: add duplicate_output_fn to run_processes_parallel_opts
- Merge branch 'ab/run-hook-api-cleanup' into cw/submodule-status-in-parallel
Allow the internal "diff-files" engine to run "how has this
submodule changed?" in parallel to speed up "git status".
Waiting for review.
source: <20221020232532.1128326-1-calvinwan@google.com>
* js/bisect-in-c (2022-08-30) 17 commits
. bisect: no longer try to clean up left-over `.git/head-name` files
. bisect: remove Cogito-related code
. Turn `git bisect` into a full built-in
. bisect: move even the command-line parsing to `bisect--helper`
. bisect--helper: make `state` optional
. bisect--helper: calling `bisect_state()` without an argument is a bug
. bisect: avoid double-quoting when printing the failed command
. bisect run: fix the error message
. bisect: verify that a bogus option won't try to start a bisection
. bisect--helper: migrate to OPT_SUBCOMMAND()
. bisect--helper: make the order consistently `argc, argv`
. bisect--helper: make `terms` an explicit singleton
. bisect--helper: simplify exit code computation
. bisect--helper: really retire `--bisect-autostart`
. bisect--helper: really retire --bisect-next-check
. bisect--helper: retire the --no-log option
. Merge branch 'sg/parse-options-subcommand' into js/bisect-in-c
Final bits of "git bisect.sh" have been rewritten in C.
Temporarily ejected from 'seen'. Waiting for a response on how this
topic interacts with 'dd/git-bisect-builtin'.
cf. <xmqqv8pr8903.fsf@gitster.g>
source: <pull.1132.v6.git.1661885419.gitgitgadget@gmail.com>
* od/ci-use-checkout-v3-when-applicable (2022-10-10) 2 commits
. ci(main): linux32 uses actions/checkout@v2
. ci(main): upgrade actions/checkout to v3
Attempt to update GitHub CI to use actions/checkout@v3
Expecting a reroll.
Seems to break the CI completely.
source: <pull.1354.git.git.1665388136.gitgitgadget@gmail.com>
* ed/fsmonitor-inotify (2022-11-25) 6 commits
- fsmonitor: update doc for Linux
- fsmonitor: test updates
- fsmonitor: enable fsmonitor for Linux
- fsmonitor: implement filesystem change listener for Linux
- fsmonitor: determine if filesystem is local or remote
- fsmonitor: prepare to share code between Mac OS and Linux
Bundled fsmonitor for Linux using inotify API.
Needs review on the updated round.
source: <pull.1352.v4.git.git.1669230044.gitgitgadget@gmail.com>
* ag/merge-strategies-in-c (2022-08-10) 14 commits
. sequencer: use the "octopus" strategy without forking
. sequencer: use the "resolve" strategy without forking
. merge: use the "octopus" strategy without forking
. merge: use the "resolve" strategy without forking
. merge-octopus: rewrite in C
. merge-recursive: move better_branch_name() to merge.c
. merge-resolve: rewrite in C
. merge-one-file: rewrite in C
. update-index: move add_cacheinfo() to read-cache.c
. merge-index: add a new way to invoke `git-merge-one-file'
. merge-index: drop the index
. merge-index: libify merge_one_path() and merge_all()
. t6060: add tests for removed files
. t6060: modify multiple files to expose a possible issue with merge-index
An attempt to rewrite remaining merge strategies from shell to C.
Needs more work.
At the minimum, we should lose 11/14 and possibly 08/14.
cf. <xmqq7d36vfur.fsf@gitster.g>
source: <20220809185429.20098-1-alban.gruin@gmail.com>
* es/doc-creation-factor-fix (2022-07-28) 2 commits
. range-diff: clarify --creation-factor=<factor>
. format-patch: clarify --creation-factor=<factor>
Expecting a reroll by somebody more familiar with the logic
cf. <xmqqo7wfix7p.fsf@gitster.g>
source: <7229p500-p2r4-on87-6802-8o90s36rr3s4@tzk.qr>
* cw/remote-object-info (2022-08-13) 7 commits
. SQUASH???
. cat-file: add remote-object-info to batch-command
. transport: add client support for object-info
. serve: advertise object-info feature
. protocol-caps: initialization bug fix
. fetch-pack: move fetch initialization
. fetch-pack: refactor packet writing
A client component to talk with the object-info endpoint.
Expecting a reroll.
Under SANITIZE=address, t1006-cat-file.sh finds a breakage.
cf. <20220728230210.2952731-1-calvinwan@google.com>
cf. <CAFySSZDvgwbbHCHfyuaqX3tKsr-GjJ9iihygg6rNNe46Ys7_EA@mail.gmail.com>
source: <20220728230210.2952731-1-calvinwan@google.com>
--------------------------------------------------
[Cooking]
* ab/config-multi-and-nonbool (2022-11-27) 9 commits
- for-each-repo: with bad config, don't conflate <path> and <cmd>
- config API: add "string" version of *_value_multi(), fix segfaults
- config API users: test for *_get_value_multi() segfaults
- for-each-repo: error on bad --config
- config API: have *_multi() return an "int" and take a "dest"
- versioncmp.c: refactor config reading next commit
- config tests: add "NULL" tests for *_get_value_multi()
- config tests: cover blind spots in git_die_config() tests
- for-each-repo tests: test bad --config keys
Assorted config API updates.
Waiting for review.
source: <cover-v3-0.9-00000000000-20221125T093158Z-avarab@gmail.com>
* yn/git-jump-emacs (2022-11-27) 3 commits
(merged to 'next' on 2022-11-29 at d0960938a0)
+ git-jump: invoke emacs/emacsclient
+ git-jump: move valid-mode check earlier
+ git-jump: add an optional argument '--stdout'
"git jump" (in contrib/) learned to present the "quickfix list" to
its standard output (instead of letting it consumed by the editor
it invokes), and learned to also drive emacs/emacsclient.
Will cook in 'next'.
source: <pull.1423.v8.git.1669511933.gitgitgadget@gmail.com>
* sa/cat-file-mailmap--batch-check (2022-11-21) 2 commits
- cat-file: add mailmap support to --batch-check option
- cat-file: add mailmap support to -s option
'cat-file' gains mailmap support for its '--batch-check' and '-s'
options.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3560918726
source: <20221120074852.121346-1-siddharthasthana31@gmail.com>
* ab/tag-object-type-errors (2022-11-22) 5 commits
- tag: don't emit potentially incorrect "object is a X, not a Y"
- tag: don't misreport type of tagged objects in errors
- object tests: add test for unexpected objects in tags
- object-file.c: free the "t.tag" in check_tag()
- Merge branch 'jk/parse-object-type-mismatch' into ab/tag-object-type-errors
Hardening checks around mismatched object types when one of those
objects is a tag.
Needs review.
source: <cover-0.4-00000000000-20221118T113442Z-avarab@gmail.com>
* js/range-diff-mbox (2022-11-23) 1 commit
- range-diff: support reading mbox files
'git range-diff' gained support for reading either side from an .mbox
file instead of a revision range.
Expecting review responses.
source: <pull.1420.v3.git.1669108102092.gitgitgadget@gmail.com>
* rj/branch-copy-and-rename (2022-11-17) 2 commits
- branch: clear target branch configuration before copying or renaming
- branch: force-copy a branch to itself via @{-1} is a no-op
Fix a pair of bugs in 'git branch'.
Waiting for review discussion to settle.
source: <f0b2d46c-2e9c-2630-2870-8ed550dd1606@gmail.com>
* tl/ls-tree--pattern (2022-11-17) 6 commits
- ls-tree: introduce '--pattern' option
- ls-tree: introduce 'match_pattern()' function
- ls-tree: improving cohension in the print code
- ls-tree: optimize params of 'show_tree_common_default_long()'
- t3104: remove shift code in 'test_ls_tree_format'
- ls-tree: cleanup the redundant SPACE
A synonym for "ls-tree | grep <pattern>", "ls-tree
--pattern=<pattern>" was introduced.
Waiting for review response, but leaning negative.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3560918726
source: <20221117113023.65865-1-tenglong.tl@alibaba-inc.com>
* tr/am--no-verify (2022-11-29) 1 commit
- am: Allow passing --no-verify flag
Conditionally skip the pre-applypatch and applypatch-msg hooks when
applying patches with 'git am'.
Expecting a (hopefully final) reroll.
source: <20221128174825.1510407-1-thierry.reding@gmail.com>
* ab/remove--super-prefix (2022-11-21) 12 commits
. fetch: rename "--submodule-prefix" to "--super-prefix"
. read-tree: add "--super-prefix" option, eliminate global
. submodule--helper: convert "{update,clone}" to their own "--super-prefix"
. submodule--helper: convert "status" to its own "--super-prefix"
. submodule--helper: convert "sync" to its own "--super-prefix"
. submodule--helper: convert "foreach" to its own "--super-prefix"
. submodule--helper: don't use global --super-prefix in "absorbgitdirs"
. submodule.c & submodule--helper: pass along "super_prefix" param
. read-tree + fetch tests: test failing "--super-prefix" interaction
. Merge branch 'ab/submodule-no-abspath' into ab/remove--super-prefix
(merged to 'next' on 2022-11-18 at 34d0accc7b)
+ submodule--helper absorbgitdirs: no abspaths in "Migrating git..."
. Merge branch 'ab/submodule-helper-prep-only' into ab/remove--super-prefix
Remove the top-level `--super-prefix` option.
source: <cover-v3-0.9-00000000000-20221119T122853Z-avarab@gmail.com>
* ab/submodule-no-abspath (2022-11-23) 2 commits
(merged to 'next' on 2022-11-23 at 97b6096e7c)
+ submodule absorbgitdirs: use relative <from> and <to> paths
(merged to 'next' on 2022-11-18 at 34d0accc7b)
+ submodule--helper absorbgitdirs: no abspaths in "Migrating git..."
Remove an absolute path in the "Migrating git directory" message.
Will cook in 'next', but leaning negative.
source: <patch-1.1-34b54fdd9bb-20221109T020347Z-avarab@gmail.com>
source: <patch-1.1-065be1da895-20221122T224306Z-avarab@gmail.com>
* ew/format-patch-mboxrd (2022-11-14) 1 commit
- format-patch: add --mboxrd alias for --pretty=mboxrd
Teach `format-patch` a convenient alias for `--pretty=mboxrd`.
Waiting for discussion to settle.
source: <20221114094114.18986-1-e@80x24.org>
* js/drop-mingw-test-cmp (2022-11-14) 2 commits
- tests(mingw): avoid very slow `mingw_test_cmp`
- t0021: use Windows-friendly `pwd`
Use `git diff --no-index` as a test_cmp on Windows.
Waiting for review response.
source: <pull.1309.v4.git.1668434812.gitgitgadget@gmail.com>
* mc/switch-advice (2022-11-09) 1 commit
- po: use `switch` over `checkout` in error message
Use 'switch' instead of 'checkout' in an error message.
Waiting for review response.
source: <pull.1308.git.git.1668018620148.gitgitgadget@gmail.com>
* rs/multi-filter-args (2022-11-21) 3 commits
- Revert "pack-objects: lazily set up "struct rev_info", don't leak"
- t5317: demonstrate failure to handle multiple --filter options
- t5317: stop losing return codes of git ls-files
Fix a bug where `pack-objects` would not respect multiple `--filter`
arguments when invoked directly.
source: <d19c6cb4-611f-afea-8a14-5e58d7509113@web.de>
* ab/various-leak-fixes (2022-11-21) 16 commits
(merged to 'next' on 2022-11-21 at eff484a27c)
+ built-ins: use free() not UNLEAK() if trivial, rm dead code
+ revert: fix parse_options_concat() leak
+ cherry-pick: free "struct replay_opts" members
+ rebase: don't leak on "--abort"
+ connected.c: free the "struct packed_git"
+ sequencer.c: fix "opts->strategy" leak in read_strategy_opts()
+ ls-files: fix a --with-tree memory leak
+ revision API: call graph_clear() in release_revisions()
+ unpack-file: fix ancient leak in create_temp_file()
+ built-ins & libs & helpers: add/move destructors, fix leaks
+ dir.c: free "ident" and "exclude_per_dir" in "struct untracked_cache"
+ read-cache.c: clear and free "sparse_checkout_patterns"
+ commit: discard partial cache before (re-)reading it
+ {reset,merge}: call discard_index() before returning
+ tests: mark tests as passing with SANITIZE=leak
+ Merge branch 'pw/rebase-no-reflog-action' into ab/various-leak-fixes
Various leak fixes.
Will cook in 'next'.
source: <cover-v2-00.15-00000000000-20221108T172650Z-avarab@gmail.com>
* aw/complete-case-insensitive (2022-11-07) 2 commits
- completion: add case-insensitive match of pseudorefs
- completion: add optional ignore-case when matching refs
Introduce a case insensitive mode to the Bash completion helpers.
Waiting for review.
Seems to break CI.
source: <pull.1374.git.git.1667669315.gitgitgadget@gmail.com>
* dd/git-bisect-builtin (2022-11-15) 13 commits
(merged to 'next' on 2022-11-15 at e16e754058)
+ bisect; remove unused "git-bisect.sh" and ".gitignore" entry
(merged to 'next' on 2022-11-14 at fc304fb52f)
+ Turn `git bisect` into a full built-in
+ bisect--helper: log: allow arbitrary number of arguments
+ bisect--helper: handle states directly
+ bisect--helper: emit usage for "git bisect"
+ bisect test: test exit codes on bad usage
+ bisect--helper: identify as bisect when report error
+ bisect-run: verify_good: account for non-negative exit status
+ bisect run: keep some of the post-v2.30.0 output
+ bisect: fix output regressions in v2.30.0
+ bisect: refactor bisect_run() to match CodingGuidelines
+ bisect tests: test for v2.30.0 "bisect run" regressions
+ Merge branch 'dd/bisect-helper-subcommand' into dd/git-bisect-builtin
`git bisect` becomes a builtin.
Will cook in 'next'.
source: <cover.1668097962.git.congdanhqx@gmail.com>
* ds/packed-refs-v2 (2022-11-07) 30 commits
- refs: skip hashing when writing packed-refs v2
- p1401: create performance test for ref operations
- ci: run GIT_TEST_PACKED_REFS_VERSION=2 in some builds
- t*: skip packed-refs v2 over http tests
- t3210: require packed-refs v1 for some tests
- t5502: add PACKED_REFS_V1 prerequisite
- t5312: allow packed-refs v2 format
- t1409: test with packed-refs v2
- packed-backend: create GIT_TEST_PACKED_REFS_VERSION
- packed-refs: write prefix chunks
- packed-refs: read optional prefix chunks
- packed-refs: read file format v2
- packed-refs: write file format version 2
- packed-backend: create shell of v2 writes
- config: add config values for packed-refs v2
- packed-backend: create abstraction for writing refs
- packed-backend: extract iterator/updates merge
- packed-backend: extract add_write_error()
- refs: extract packfile format to new file
- chunk-format: parse trailing table of contents
- chunk-format: allow trailing table of contents
- chunk-format: store chunk offset during write
- chunk-format: document trailing table of contents
- chunk-format: number of chunks is optional
- refs: allow loose files without packed-refs
- repository: wire ref extensions to ref backends
- config: fix multi-level bulleted list
- extensions: add refFormat extension
- read-cache: add index.computeHash config option
- hashfile: allow skipping the hash function
Waiting for review.
source: <pull.1408.git.1667846164.gitgitgadget@gmail.com>
* ja/worktree-orphan (2022-11-10) 2 commits
- worktree add: add --orphan flag
- worktree add: Include -B in usage docs
'git worktree add' learned how to create a worktree based on an
orphaned branch with `--orphan`.
Expecting another round?
source: <20221110233137.10414-1-jacobabel@nullpo.dev>
* tb/ci-concurrency (2022-11-08) 1 commit
- ci: avoid unnecessary builds
Avoid unnecessary builds in CI, with settings configured in
ci-config.
Waiting for review.
source: <ff172f1de982f6f79b598e4ac6d5b2964ca4a098.1667931937.git.me@ttaylorr.com>
* tl/notes--blankline (2022-11-09) 5 commits
- notes.c: introduce "--no-blank-line" option
- notes.c: provide tips when target and append note are both empty
- notes.c: drop unreachable code in 'append_edit()'
- notes.c: cleanup for "designated init" and "char ptr init"
- notes.c: cleanup 'strbuf_grow' call in 'append_edit'
'git notes append' was taught '--[no-]blank-line' to conditionally
add a LF between a new and existing note.
Expecting a reroll.
source: <cover.1667980450.git.dyroneteng@gmail.com>
* ds/bundle-uri-4 (2022-11-16) 9 commits
- clone: unbundle the advertised bundles
- bundle-uri: download bundles from an advertised list
- bundle-uri: allow relative URLs in bundle lists
- strbuf: introduce strbuf_strip_file_from_path()
- bundle-uri client: add boolean transfer.bundleURI setting
- bundle-uri: serve bundle.* keys from config
- bundle-uri client: add helper for testing server
- bundle-uri client: add minimal NOOP client
- protocol v2: add server-side "bundle-uri" skeleton
Bundle URIs part 4.
Waiting for review.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3560918726
source: <pull.1400.v2.git.1668628302.gitgitgadget@gmail.com>
* tl/pack-bitmap-absolute-paths (2022-11-29) 4 commits
- pack-bitmap.c: trace bitmap ignore logs when midx-bitmap is found
- pack-bitmap.c: break out of the bitmap loop early if not tracing
(merged to 'next' on 2022-11-14 at 34eb0ea05a)
+ pack-bitmap.c: avoid exposing absolute paths
+ pack-bitmap.c: remove unnecessary "open_pack_index()" calls
The pack-bitmap machinery is taught to log the paths of redundant
bitmap(s) to trace2 instead of stderr.
Will merge to 'next'.
source: <cover.1669644101.git.dyroneteng@gmail.com>
* ab/cmake-nix-and-ci (2022-11-04) 14 commits
(merged to 'next' on 2022-11-08 at 6ef4e93b36)
+ CI: add a "linux-cmake-test" to run cmake & ctest on linux
+ cmake: copy over git-p4.py for t983[56] perforce test
+ cmake: only look for "sh" in "C:/Program Files" on Windows
+ cmake: increase test timeout on Windows only
+ cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
+ Makefile + cmake: use environment, not GIT-BUILD-DIR
+ test-lib.sh: support a "GIT_TEST_BUILD_DIR"
+ cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
+ cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
+ cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
+ cmake: don't copy chainlint.pl to build directory
+ cmake: update instructions for portable CMakeLists.txt
+ cmake: use "-S" and "-B" to specify source and build directories
+ cmake: don't invoke msgfmt with --statistics
Fix assorted issues with CTest on *nix machines.
Will cook in 'next'.
source: <cover-v4-00.14-00000000000-20221103T160255Z-avarab@gmail.com>
* ab/make-bin-wrappers (2022-10-31) 4 commits
. Makefile: simplify $(test_bindir_programs) rule by splitting it up
. Makefile: rename "test_bindir_programs" variable, pre-declare
. Makefile: define "TEST_{PROGRAM,OBJS}" variables earlier
. Makefile: factor sed-powered '#!/bin/sh' munging into a variable
Resolve issues with the bin-wrappers/% rules where "make
bin-wrappers/git" would generate the script but not "git" itself.
Waiting for review discussion to settle, but leaning negative.
source: <cover-v3-0.4-00000000000-20221031T222249Z-avarab@gmail.com>
* kz/merge-tree-merge-base (2022-11-25) 3 commits
(merged to 'next' on 2022-11-25 at 298ca8e2e8)
+ docs: fix description of the `--merge-base` option
(merged to 'next' on 2022-11-14 at 76d48ae21f)
+ merge-tree.c: allow specifying the merge-base when --stdin is passed
+ merge-tree.c: add --merge-base=<commit> option
"merge-tree" learns a new `--merge-base` option.
Will cook in 'next'.
source: <pull.1397.v7.git.1668210314.gitgitgadget@gmail.com>
source: <c21466d1db0e7f7fcd7308b61aa1e3cd4e3d67c4.1669261026.git.gitgitgadget@gmail.com>
* po/pretty-hard-trunc (2022-11-13) 1 commit
- pretty-formats: add hard truncation, without ellipsis, options
Add a new pretty format which truncates without ellipsis.
Waiting for review.
source: <20221112143616.1429-1-philipoakley@iee.email>
* rr/long-status-advice (2022-11-15) 1 commit
- status: long status advice adapted to recent capabilities
The advice message emitted by a slow "status" run is amended to
mention fsmonitor.
Waiting for reviewer feedback on the updated round.
source: <pull.1384.v6.git.1668547188070.gitgitgadget@gmail.com>
* gc/submodule-clone-update-with-branches (2022-10-30) 8 commits
- clone, submodule update: create and check out branches
- submodule--helper: remove update_data.suboid
- submodule update: refactor update targets
- submodule: return target of submodule symref
- t5617: drop references to remote-tracking branches
- submodule--helper clone: create named branch
- repo-settings: add submodule_propagate_branches
- clone: teach --detach option
"git clone --recurse-submodules" and "git submodule update" learns
to honor the "propagete branches" option.
Waiting for review on the updated round.
source: <pull.1321.v3.git.git.1666988096.gitgitgadget@gmail.com>
* cc/filtered-repack (2022-11-23) 2 commits
- repack: add --filter=<filter-spec> option
- pack-objects: allow --filter without --stdout
"git repack" learns to discard objects that ought to be retrievable
again from the promissor remote.
Needs review.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3560918726
source: <20221122175150.366828-1-christian.couder@gmail.com>
* mc/credential-helper-auth-headers (2022-11-02) 11 commits
- t5556: add HTTP authentication tests
- test-http-server: add simple authentication
- test-http-server: pass Git requests to http-backend
- test-http-server: add HTTP request parsing
- test-http-server: add HTTP error response function
- test-http-server: add stub HTTP server test helper
- http: set specific auth scheme depending on credential
- http: move proactive auth to first slot creation
- http: store all request headers on active_request_slot
- credential: add WWW-Authenticate header to cred requests
- http: read HTTP WWW-Authenticate response headers
Extending credential helper protocol.
Needs review.
Seems to break CI.
cf. https://github.com/git/git/actions/runs/3562942886/jobs/5985179202
source: <pull.1352.v3.git.1667426969.gitgitgadget@gmail.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
@ 2022-11-29 18:59 ` Glen Choo
2022-11-30 3:43 ` Junio C Hamano
2022-11-29 19:08 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Glen Choo
` (3 subsequent siblings)
4 siblings, 1 reply; 43+ messages in thread
From: Glen Choo @ 2022-11-29 18:59 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ævar Arnfjörð Bjarmason
Junio C Hamano <gitster@pobox.com> writes:
> Here are the topics that have been cooking in my tree. Commits
> prefixed with '+' are in 'next' (being in 'next' is a sign that a
> topic is stable enough to be used and are candidate to be in a future
> release). Commits prefixed with '-' are only in 'seen', and aren't
> considered "accepted" at all. A topic without enough support may be
> discarded after a long period of no activity.
>
> The preview release -rc0 for this cycle has been tagged.
[...]
> * ab/remove--super-prefix (2022-11-21) 12 commits
> . fetch: rename "--submodule-prefix" to "--super-prefix"
> . read-tree: add "--super-prefix" option, eliminate global
> . submodule--helper: convert "{update,clone}" to their own "--super-prefix"
> . submodule--helper: convert "status" to its own "--super-prefix"
> . submodule--helper: convert "sync" to its own "--super-prefix"
> . submodule--helper: convert "foreach" to its own "--super-prefix"
> . submodule--helper: don't use global --super-prefix in "absorbgitdirs"
> . submodule.c & submodule--helper: pass along "super_prefix" param
> . read-tree + fetch tests: test failing "--super-prefix" interaction
> . Merge branch 'ab/submodule-no-abspath' into ab/remove--super-prefix
> (merged to 'next' on 2022-11-18 at 34d0accc7b)
> + submodule--helper absorbgitdirs: no abspaths in "Migrating git..."
> . Merge branch 'ab/submodule-helper-prep-only' into ab/remove--super-prefix
>
> Remove the top-level `--super-prefix` option.
> source: <cover-v3-0.9-00000000000-20221119T122853Z-avarab@gmail.com>
Hm, it looks like ab/remove--super-prefix missed the preview release..
Per the discussion ending at [1] I think my one-patch fix to "git
fetch" [2] should have made it into the release (it's pretty low-risk
and doesn't introduce too much churn to ab/remove--super-prefix). Is it
too late for that?
[1] https://lore.kernel.org/git/221117.86y1s9h2q5.gmgdl@evledraar.gmail.com
[2] https://lore.kernel.org/git/pull.1378.git.git.1668210935360.gitgitgadget@gmail.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
@ 2022-11-29 19:08 ` Glen Choo
2022-11-30 3:45 ` Junio C Hamano
2022-11-29 21:16 ` ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)) Derrick Stolee
` (2 subsequent siblings)
4 siblings, 1 reply; 43+ messages in thread
From: Glen Choo @ 2022-11-29 19:08 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Jonathan Tan, Calvin Wan
Junio C Hamano <gitster@pobox.com> writes:
> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits
> - clone, submodule update: create and check out branches
> - submodule--helper: remove update_data.suboid
> - submodule update: refactor update targets
> - submodule: return target of submodule symref
> - t5617: drop references to remote-tracking branches
> - submodule--helper clone: create named branch
> - repo-settings: add submodule_propagate_branches
> - clone: teach --detach option
>
> "git clone --recurse-submodules" and "git submodule update" learns
> to honor the "propagete branches" option.
>
> Waiting for review on the updated round.
> source: <pull.1321.v3.git.git.1666988096.gitgitgadget@gmail.com>
Jonathan left adequate feedback on the updated round, and we had decided
on a direction for the reroll in [1].
However, Calvin is also looking at how we could parallelize worktree
updates to speed up "git clone --recurse-submodules", so the Google
folks are taking an even bigger step back to figure out how worktree
updating should look, which will probably end in a different approach
from [1], but it should answer the questions on that thread about "git
checkout" with branches.
[1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
2022-11-29 19:08 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Glen Choo
@ 2022-11-29 21:16 ` Derrick Stolee
2022-12-01 15:06 ` Derrick Stolee
2022-11-30 9:57 ` ab/cmake-nix-and-ci " Phillip Wood
2022-11-30 10:02 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Phillip Wood
4 siblings, 1 reply; 43+ messages in thread
From: Derrick Stolee @ 2022-11-29 21:16 UTC (permalink / raw)
To: Junio C Hamano, git
On 11/29/2022 4:40 AM, Junio C Hamano wrote:
> * ds/bundle-uri-4 (2022-11-16) 9 commits
> - clone: unbundle the advertised bundles
> - bundle-uri: download bundles from an advertised list
> - bundle-uri: allow relative URLs in bundle lists
> - strbuf: introduce strbuf_strip_file_from_path()
> - bundle-uri client: add boolean transfer.bundleURI setting
> - bundle-uri: serve bundle.* keys from config
> - bundle-uri client: add helper for testing server
> - bundle-uri client: add minimal NOOP client
> - protocol v2: add server-side "bundle-uri" skeleton
>
> Bundle URIs part 4.
>
> Waiting for review.
> Seems to break CI.
> cf. https://github.com/git/git/actions/runs/3560918726
> source: <pull.1400.v2.git.1668628302.gitgitgadget@gmail.com>
Thanks for pointing this out. I'll be sure to fix the relevant
failures in linux-TEST-vars [1] before the next version.
[1] https://github.com/git/git/actions/runs/3560918726/jobs/5981349269#step:4:1854
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
@ 2022-11-30 3:43 ` Junio C Hamano
2022-11-30 18:14 ` Glen Choo
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2022-11-30 3:43 UTC (permalink / raw)
To: Glen Choo; +Cc: git, Ævar Arnfjörð Bjarmason
Glen Choo <chooglen@google.com> writes:
> Hm, it looks like ab/remove--super-prefix missed the preview release..
> Per the discussion ending at [1] I think my one-patch fix to "git
> fetch" [2] should have made it into the release (it's pretty low-risk
> and doesn't introduce too much churn to ab/remove--super-prefix). Is it
> too late for that?
Nobody seemed to have commented on [2]. Is this fixing recent
regressions, or is it more like addressing an "if it hurts, do not
do it then" problem?
The fact alone that these questions need to be asked _now_ is a good
indication that it is way too late for this cycle, I would have to
say.
Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)
2022-11-29 19:08 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Glen Choo
@ 2022-11-30 3:45 ` Junio C Hamano
2022-11-30 18:08 ` Glen Choo
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2022-11-30 3:45 UTC (permalink / raw)
To: Glen Choo; +Cc: git, Jonathan Tan, Calvin Wan
Glen Choo <chooglen@google.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits
>> - clone, submodule update: create and check out branches
>> - submodule--helper: remove update_data.suboid
>> - submodule update: refactor update targets
>> - submodule: return target of submodule symref
>> - t5617: drop references to remote-tracking branches
>> - submodule--helper clone: create named branch
>> - repo-settings: add submodule_propagate_branches
>> - clone: teach --detach option
>>
>> "git clone --recurse-submodules" and "git submodule update" learns
>> to honor the "propagete branches" option.
>>
>> Waiting for review on the updated round.
>> source: <pull.1321.v3.git.git.1666988096.gitgitgadget@gmail.com>
>
> Jonathan left adequate feedback on the updated round, and we had decided
> on a direction for the reroll in [1].
>
> However, Calvin is also looking at how we could parallelize worktree
> updates to speed up "git clone --recurse-submodules", so the Google
> folks are taking an even bigger step back to figure out how worktree
> updating should look, which will probably end in a different approach
> from [1], but it should answer the questions on that thread about "git
> checkout" with branches.
>
> [1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com
Thanks for a status report.
So it sounds more like "discard for now" to expect a fresh restart.
^ permalink raw reply [flat|nested] 43+ messages in thread
* ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
` (2 preceding siblings ...)
2022-11-29 21:16 ` ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)) Derrick Stolee
@ 2022-11-30 9:57 ` Phillip Wood
2022-11-30 10:16 ` Ævar Arnfjörð Bjarmason
2022-11-30 10:02 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Phillip Wood
4 siblings, 1 reply; 43+ messages in thread
From: Phillip Wood @ 2022-11-30 9:57 UTC (permalink / raw)
To: Junio C Hamano, git; +Cc: Ævar Arnfjörð Bjarmason
Hi Junio
On 29/11/2022 09:40, Junio C Hamano wrote:
> * ab/cmake-nix-and-ci (2022-11-04) 14 commits
> (merged to 'next' on 2022-11-08 at 6ef4e93b36)
> + CI: add a "linux-cmake-test" to run cmake & ctest on linux
> + cmake: copy over git-p4.py for t983[56] perforce test
> + cmake: only look for "sh" in "C:/Program Files" on Windows
> + cmake: increase test timeout on Windows only
> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
> + Makefile + cmake: use environment, not GIT-BUILD-DIR
> + test-lib.sh: support a "GIT_TEST_BUILD_DIR"
> + cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
> + cmake: don't copy chainlint.pl to build directory
> + cmake: update instructions for portable CMakeLists.txt
> + cmake: use "-S" and "-B" to specify source and build directories
> + cmake: don't invoke msgfmt with --statistics
>
> Fix assorted issues with CTest on *nix machines.
If that's all this series did then I think it would be fine. However it
also makes changes to test-lib.sh to hard code the build directory in an
attempt to remove GIT-BUILD-DIR. I'm not convinced that is an
improvement on the status quo. As I mentioned previously [1] I think the
non-*nix related patches could do with a review from the windows folks
before this hits master.
Best Wishes
Phillip
[1]
https://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/
> Will cook in 'next'.
> source: <cover-v4-00.14-00000000000-20221103T160255Z-avarab@gmail.com>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
` (3 preceding siblings ...)
2022-11-30 9:57 ` ab/cmake-nix-and-ci " Phillip Wood
@ 2022-11-30 10:02 ` Phillip Wood
4 siblings, 0 replies; 43+ messages in thread
From: Phillip Wood @ 2022-11-30 10:02 UTC (permalink / raw)
To: Junio C Hamano, git
On 29/11/2022 09:40, Junio C Hamano wrote:
> * pw/test-todo (2022-10-06) 3 commits
> . test_todo: allow [verbose] test as the command
> . test_todo: allow [!] grep as the command
> . tests: add test_todo() to mark known breakages
>
> RFC for test framework improvement.
>
> Needs review.
> source: <pull.1374.git.1665068476.gitgitgadget@gmail.com>
I'm planning to re-roll this but I probably wont have time before the
new year. If anyone has time to take a look before that would be really
helpful. Ævar has given some useful feedback but it would be good to
know if others think this is useful.
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 9:57 ` ab/cmake-nix-and-ci " Phillip Wood
@ 2022-11-30 10:16 ` Ævar Arnfjörð Bjarmason
2022-12-01 14:23 ` Phillip Wood
0 siblings, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-30 10:16 UTC (permalink / raw)
To: phillip.wood; +Cc: Junio C Hamano, git
On Wed, Nov 30 2022, Phillip Wood wrote:
> Hi Junio
Hi, and thanks for your review of this series.
> On 29/11/2022 09:40, Junio C Hamano wrote:
>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits
>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)
>> + CI: add a "linux-cmake-test" to run cmake & ctest on linux
>> + cmake: copy over git-p4.py for t983[56] perforce test
>> + cmake: only look for "sh" in "C:/Program Files" on Windows
>> + cmake: increase test timeout on Windows only
>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
>> + Makefile + cmake: use environment, not GIT-BUILD-DIR
>> + test-lib.sh: support a "GIT_TEST_BUILD_DIR"
>> + cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
>> + cmake: don't copy chainlint.pl to build directory
>> + cmake: update instructions for portable CMakeLists.txt
>> + cmake: use "-S" and "-B" to specify source and build directories
>> + cmake: don't invoke msgfmt with --statistics
>> Fix assorted issues with CTest on *nix machines.
>
> If that's all this series did then I think it would be fine. However
> it also makes changes to test-lib.sh to hard code the build directory
> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an
> improvement on the status quo.
I think the series as it stands addresses those concerns. In particular
building outside of contrib/buildsystems/out works, just as before:
cmake -S contrib/buildsystems -B /tmp/git-build -DCMAKE_BUILD_TYPE=Debug &&
make -C /tmp/git-build &&
ctest --test-dir /tmp/git-build -R t0001
Per [1] and [2] which added the "ctest" support that's the use-case for
this part of the build: running the tests with ctest, which works as
before with the default or custom directories.
Perhaps the reason this has been a sticking point for you is that in
summarizing this, Johannes's [3] didn't make that distinction between
running the tests with "ctest" and running them manually by entering the
"t/" directory after the build. I.e.:
(cd t && ./t0001-init.sh)
It's only that part which acts differently in this series. I.e. if you
were to build in /tmp/git-build this would no longer find your built
assets:
$ ./t0001-init.sh
error: GIT-BUILD-OPTIONS missing (has Git been built?).
If you just leave it at the default of "contrib/buildsystems/out" it'll
work:
(cd t && ./t0001-init.sh)
ok 1 [...]
I think my [4] convincingly makes the case that nobody will
care. I.e. as the [5] it links to the use-case for running the test
after the build without ctest was ("[...]" insert is mine):
[To build and test with VS] open the worktree as a folder, and
Visual Studio will find the `CMakeLists.txt` file and
automatically generate the project files.
I.e. we want to support the user who builds with that method, and runs
the tests manually. I think you're worrying about an edge case that
nobody's using in practice.
> As I mentioned previously [1] I think
> the non-*nix related patches could do with a review from the windows
> folks before this hits master.
I'd welcome another review of it, but at this point it's not for lack of
waiting for interest from the CC'd Windows people.
Per the above I don't think any special Windows knowledge is really
needed, just a reading of the above history & use-cases.
All of which I've been careful not to break, and which you can now
simply test on *nix with this series, so no Windows-specific testing is
needed anymore for this concern you're raising.
*If* we have someone that's been using this on Windows and e.g. building
this in /tmp/git-build (or whatever the Windows equivalent) with a
custom recipe all they'll need to have it work as before is:
GIT_TEST_BUILD_DIR=/tmp/git-build ./t0001-init.sh
I think nobody's straying off the golden path to do that, but if they
are doing so and building in some custom directory they're already
tweaking, just setting an environment variable doesn't seem like a big
imposition.
The flip-side of that trade-off is (on both Windows and *nix) that the
existing way to support the use-case has unintended side-effects, which
the series improves:
* When we pick up the not-a-Makefile tree implicitly like this we'll
now emit a message telling you what git we're implicitly using.
* We no longer have edge cases where you can e.g. build with make, then
cmake, then run some "make" target that won't go through the path to
remove GIT-BUILD-DIR (e.g. changing "git.c" and "make git").
Then when you run the tests you'll end up with a different git running
it than what you'd expect, i.e. the old stale cmake one.
Even that hypothetical user who's going to need to set
"GIT_TEST_BUILD_DIR" would benefit from that, as they'd no longer
accidentally flip-flop between the two if they ran "make" and wiped away
the rather fragile link between the source directory & the linked-to
cmake build directory.
> [1]
> https://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/
>
>> Will cook in 'next'.
>> source: <cover-v4-00.14-00000000000-20221103T160255Z-avarab@gmail.com>
1. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26)
2. 7f5397a07c6 (cmake: support for testing git when building out of the
source tree, 2020-06-26)
3. ee9e66e4e76 (cmake: avoid editing t/test-lib.sh, 2022-10-18)
4. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,
2022-11-03)
5. 3eccc7b99d4 (cmake: ignore files generated by CMake as run in Visual
Studio, 2020-09-25)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)
2022-11-30 3:45 ` Junio C Hamano
@ 2022-11-30 18:08 ` Glen Choo
0 siblings, 0 replies; 43+ messages in thread
From: Glen Choo @ 2022-11-30 18:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jonathan Tan, Calvin Wan
Junio C Hamano <gitster@pobox.com> writes:
> Glen Choo <chooglen@google.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> * gc/submodule-clone-update-with-branches (2022-10-30) 8 commits
>>> - clone, submodule update: create and check out branches
>>> - submodule--helper: remove update_data.suboid
>>> - submodule update: refactor update targets
>>> - submodule: return target of submodule symref
>>> - t5617: drop references to remote-tracking branches
>>> - submodule--helper clone: create named branch
>>> - repo-settings: add submodule_propagate_branches
>>> - clone: teach --detach option
>>>
>>> "git clone --recurse-submodules" and "git submodule update" learns
>>> to honor the "propagete branches" option.
>>>
>>> Waiting for review on the updated round.
>>> source: <pull.1321.v3.git.git.1666988096.gitgitgadget@gmail.com>
>>
>> Jonathan left adequate feedback on the updated round, and we had decided
>> on a direction for the reroll in [1].
>>
>> However, Calvin is also looking at how we could parallelize worktree
>> updates to speed up "git clone --recurse-submodules", so the Google
>> folks are taking an even bigger step back to figure out how worktree
>> updating should look, which will probably end in a different approach
>> from [1], but it should answer the questions on that thread about "git
>> checkout" with branches.
>>
>> [1] https://lore.kernel.org/git/20221123013319.1597025-1-jonathantanmy@google.com
>
> Thanks for a status report.
>
> So it sounds more like "discard for now" to expect a fresh restart.
Yes, that is accurate :)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 3:43 ` Junio C Hamano
@ 2022-11-30 18:14 ` Glen Choo
2022-11-30 19:43 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 43+ messages in thread
From: Glen Choo @ 2022-11-30 18:14 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Ævar Arnfjörð Bjarmason
Junio C Hamano <gitster@pobox.com> writes:
> Glen Choo <chooglen@google.com> writes:
>
>> Hm, it looks like ab/remove--super-prefix missed the preview release..
>> Per the discussion ending at [1] I think my one-patch fix to "git
>> fetch" [2] should have made it into the release (it's pretty low-risk
>> and doesn't introduce too much churn to ab/remove--super-prefix). Is it
>> too late for that?
>
> Nobody seemed to have commented on [2]. Is this fixing recent
> regressions, or is it more like addressing an "if it hurts, do not
> do it then" problem?
Ævar did comment on the patch in [2], but unfortunately it happened on
the thread ending at [1] (and others), so it's not easy to follow.
It's solidly in the latter category. I don't think this has ever worked.
c.f. https://lore.kernel.org/git/kl6lsfiivcau.fsf@chooglen-macbookpro.roam.corp.google.com/
> The fact alone that these questions need to be asked _now_ is a good
> indication that it is way too late for this cycle, I would have to
> say.
At any rate, we shouldn't be rushing review, so this is fair (though
unfortunate). Let's continue counting on ab/remove--super-prefix and
ignoring my one patch, then.
>
> Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 18:14 ` Glen Choo
@ 2022-11-30 19:43 ` Ævar Arnfjörð Bjarmason
2022-12-01 5:20 ` Junio C Hamano
2022-12-01 17:44 ` Glen Choo
0 siblings, 2 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-11-30 19:43 UTC (permalink / raw)
To: Glen Choo; +Cc: Junio C Hamano, git
On Wed, Nov 30 2022, Glen Choo wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> Glen Choo <chooglen@google.com> writes:
>>
>>> Hm, it looks like ab/remove--super-prefix missed the preview release..
>>> Per the discussion ending at [1] I think my one-patch fix to "git
>>> fetch" [2] should have made it into the release (it's pretty low-risk
>>> and doesn't introduce too much churn to ab/remove--super-prefix). Is it
>>> too late for that?
>>
>> Nobody seemed to have commented on [2]. Is this fixing recent
>> regressions, or is it more like addressing an "if it hurts, do not
>> do it then" problem?
>
> Ævar did comment on the patch in [2], but unfortunately it happened on
> the thread ending at [1] (and others), so it's not easy to follow.
>
> It's solidly in the latter category. I don't think this has ever worked.
> c.f. https://lore.kernel.org/git/kl6lsfiivcau.fsf@chooglen-macbookpro.roam.corp.google.com/
>
>> The fact alone that these questions need to be asked _now_ is a good
>> indication that it is way too late for this cycle, I would have to
>> say.
>
> At any rate, we shouldn't be rushing review, so this is fair (though
> unfortunate). Let's continue counting on ab/remove--super-prefix and
> ignoring my one patch, then.
For my part I was waiting to see what Junio would do with
"ab/submodule-no-abspath", which is already in "next". Depending on
whether it's ejected or not I'd need to re-roll
"ab/remove--super-prefix" on top of a new "master", as it extends the
tests it added.
You noted in [1] that you strongly preferred seeing
"ab/submodule-no-abspath" ejected. I think you're right that the output
is a bit weird, but:
A. I think it's mainly odd/unintuitive for the recursive cases, I think
outside of our own test suite absorbing repositories recursively
almost never happens.
B. I think it's an improvement in the output compared to the absolute
paths we have now, especially for the common case of non-recursive.
C. Changing it made it easier to test it, which is how it ended up as a
supposedly quick prerequisite for "ab/remove--super-prefix": It's
otherwise changing a test blindspot.
D. As you note in [1] the data we'd need to pass around to make it
sensible (maybe it should always be consistent with "git mv -v"?)
would require passing more state around, some of which is tricky.
I'd prefer to just have it graduate as-is, and build
"ab/remove--super-prefix" on top. We can always further tweak the output
later.
But if you & Junio feel otherwise I think the best way forward would be
to eject both topics, and I'd submit a re-rolled
"ab/remove--super-prefix".
Either would work as a way forward. Just let me know what you both
prefer.
1. https://lore.kernel.org/git/kl6l7czmec10.fsf@chooglen-macbookpro.roam.corp.google.com/
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 19:43 ` Ævar Arnfjörð Bjarmason
@ 2022-12-01 5:20 ` Junio C Hamano
2022-12-01 17:44 ` Glen Choo
1 sibling, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2022-12-01 5:20 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Glen Choo, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> For my part I was waiting to see what Junio would do with
> "ab/submodule-no-abspath", which is already in "next".
I have no strong opinion on it myself, but have been carrying the
"leaning negative" label from Taylor, so the default action would be
to eject when 'next' rewinds.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/cmake-nix-and-ci (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 10:16 ` Ævar Arnfjörð Bjarmason
@ 2022-12-01 14:23 ` Phillip Wood
2022-12-01 16:39 ` [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out" Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 43+ messages in thread
From: Phillip Wood @ 2022-12-01 14:23 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Junio C Hamano, git, Johannes Schindelin
On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:
>
> On Wed, Nov 30 2022, Phillip Wood wrote:
>
>> Hi Junio
>
> Hi, and thanks for your review of this series.
>
>> On 29/11/2022 09:40, Junio C Hamano wrote:
>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits
>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)
>>> + CI: add a "linux-cmake-test" to run cmake & ctest on linux
>>> + cmake: copy over git-p4.py for t983[56] perforce test
>>> + cmake: only look for "sh" in "C:/Program Files" on Windows
>>> + cmake: increase test timeout on Windows only
>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR
>>> + test-lib.sh: support a "GIT_TEST_BUILD_DIR"
>>> + cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
>>> + cmake: don't copy chainlint.pl to build directory
>>> + cmake: update instructions for portable CMakeLists.txt
>>> + cmake: use "-S" and "-B" to specify source and build directories
>>> + cmake: don't invoke msgfmt with --statistics
>>> Fix assorted issues with CTest on *nix machines.
Junio please drop this series when you rebuild next as it breaks
manually running individual test scripts when building with Visual Studio.
>> If that's all this series did then I think it would be fine. However
>> it also makes changes to test-lib.sh to hard code the build directory
>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an
>> improvement on the status quo.
>
> I think the series as it stands addresses those concerns. In particular
> building outside of contrib/buildsystems/out works, just as before:
>
> cmake -S contrib/buildsystems -B /tmp/git-build -DCMAKE_BUILD_TYPE=Debug &&
> make -C /tmp/git-build &&
> ctest --test-dir /tmp/git-build -R t0001
>
> Per [1] and [2] which added the "ctest" support that's the use-case for
> this part of the build: running the tests with ctest, which works as
> before with the default or custom directories.
>
> Perhaps the reason this has been a sticking point for you is that in
> summarizing this, Johannes's [3] didn't make that distinction between
> running the tests with "ctest" and running them manually by entering the
> "t/" directory after the build. I.e.:
In other words Johannes thinks both are equally important. The windows
build has always supported running the tests manually from /t and he
quite reasonable wants that to continue working.
> (cd t && ./t0001-init.sh)
>
> It's only that part which acts differently in this series. I.e. if you
> were to build in /tmp/git-build this would no longer find your built
> assets:
>
> $ ./t0001-init.sh
> error: GIT-BUILD-OPTIONS missing (has Git been built?).
>
> If you just leave it at the default of "contrib/buildsystems/out" it'll
> work:
>
> (cd t && ./t0001-init.sh)
> ok 1 [...]
>
> I think my [4] convincingly makes the case that nobody will
> care. I.e. as the [5] it links to the use-case for running the test
> after the build without ctest was ("[...]" insert is mine):
>
> [To build and test with VS] open the worktree as a folder, and
> Visual Studio will find the `CMakeLists.txt` file and
> automatically generate the project files.
>
> I.e. we want to support the user who builds with that method, and runs
> the tests manually. I think you're worrying about an edge case that
> nobody's using in practice.
You seem to be assuming that Visual Studio creates its build artifacts
in contrib/buildsystems/out based on a gitignore rule. Given the rule
ignores _all_ subdirectories below contrib/buildsystems/out that is a
big assumption. Despite me repeatedly raising concerns about the hard
coded build directory you do not seem to have checked exactly where
Visual Studio creates its build artifacts. This morning I installed
Visual Studio to check this and discovered the build is in a
subdirectory below contrib/buildsystems/out so this series will break
manual test runs for anyone building git using the recommend method. I
find it rather frustrating that you argue below that Windows specific
knowledge and testing are not required when you're altering the Windows
build.
Best Wishes
Phillip
>> As I mentioned previously [1] I think
>> the non-*nix related patches could do with a review from the windows
>> folks before this hits master.
>
> I'd welcome another review of it, but at this point it's not for lack of
> waiting for interest from the CC'd Windows people.
>
> Per the above I don't think any special Windows knowledge is really
> needed, just a reading of the above history & use-cases.
>
> All of which I've been careful not to break, and which you can now
> simply test on *nix with this series, so no Windows-specific testing is
> needed anymore for this concern you're raising.
>
> *If* we have someone that's been using this on Windows and e.g. building
> this in /tmp/git-build (or whatever the Windows equivalent) with a
> custom recipe all they'll need to have it work as before is:
>
> GIT_TEST_BUILD_DIR=/tmp/git-build ./t0001-init.sh
>
> I think nobody's straying off the golden path to do that, but if they
> are doing so and building in some custom directory they're already
> tweaking, just setting an environment variable doesn't seem like a big
> imposition.
>
> The flip-side of that trade-off is (on both Windows and *nix) that the
> existing way to support the use-case has unintended side-effects, which
> the series improves:
>
> * When we pick up the not-a-Makefile tree implicitly like this we'll
> now emit a message telling you what git we're implicitly using.
>
> * We no longer have edge cases where you can e.g. build with make, then
> cmake, then run some "make" target that won't go through the path to
> remove GIT-BUILD-DIR (e.g. changing "git.c" and "make git").
>
> Then when you run the tests you'll end up with a different git running
> it than what you'd expect, i.e. the old stale cmake one.
>
> Even that hypothetical user who's going to need to set
> "GIT_TEST_BUILD_DIR" would benefit from that, as they'd no longer
> accidentally flip-flop between the two if they ran "make" and wiped away
> the rather fragile link between the source directory & the linked-to
> cmake build directory.
>
>> [1]
>> https://lore.kernel.org/git/64b91b29-bbcd-e946-1f20-c0a5be63d9b7@dunelm.org.uk/
>>
>>> Will cook in 'next'.
>>> source: <cover-v4-00.14-00000000000-20221103T160255Z-avarab@gmail.com>
>
> 1. c4b2f41b5f5 (cmake: support for testing git with ctest, 2020-06-26)
> 2. 7f5397a07c6 (cmake: support for testing git when building out of the
> source tree, 2020-06-26)
> 3. ee9e66e4e76 (cmake: avoid editing t/test-lib.sh, 2022-10-18)
> 4. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,
> 2022-11-03)
> 5. 3eccc7b99d4 (cmake: ignore files generated by CMake as run in Visual
> Studio, 2020-09-25)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-29 21:16 ` ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)) Derrick Stolee
@ 2022-12-01 15:06 ` Derrick Stolee
2022-12-02 0:25 ` Junio C Hamano
0 siblings, 1 reply; 43+ messages in thread
From: Derrick Stolee @ 2022-12-01 15:06 UTC (permalink / raw)
To: Junio C Hamano, git
On 11/29/2022 4:16 PM, Derrick Stolee wrote:
> On 11/29/2022 4:40 AM, Junio C Hamano wrote:
>
>> * ds/bundle-uri-4 (2022-11-16) 9 commits
>> - clone: unbundle the advertised bundles
>> - bundle-uri: download bundles from an advertised list
>> - bundle-uri: allow relative URLs in bundle lists
>> - strbuf: introduce strbuf_strip_file_from_path()
>> - bundle-uri client: add boolean transfer.bundleURI setting
>> - bundle-uri: serve bundle.* keys from config
>> - bundle-uri client: add helper for testing server
>> - bundle-uri client: add minimal NOOP client
>> - protocol v2: add server-side "bundle-uri" skeleton
>>
>> Bundle URIs part 4.
>>
>> Waiting for review.
>> Seems to break CI.
>> cf. https://github.com/git/git/actions/runs/3560918726
>> source: <pull.1400.v2.git.1668628302.gitgitgadget@gmail.com>
>
> Thanks for pointing this out. I'll be sure to fix the relevant
> failures in linux-TEST-vars [1] before the next version.
>
> [1] https://github.com/git/git/actions/runs/3560918726/jobs/5981349269#step:4:1854
I have discovered that the problem is due to the test variable
GIT_TEST_PACKED_REFS_VERSION=2, so instead it's probably best
to eject ds/packed-refs-v2 from 'seen' instead of ds/bundle-uri-4.
Thanks,
-Stolee
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-01 14:23 ` Phillip Wood
@ 2022-12-01 16:39 ` Ævar Arnfjörð Bjarmason
2022-12-01 16:48 ` Phillip Wood
2022-12-01 23:00 ` Junio C Hamano
0 siblings, 2 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-01 16:39 UTC (permalink / raw)
To: git
Cc: Phillip Wood, Junio C Hamano, Johannes Schindelin,
Ævar Arnfjörð Bjarmason
Fix a regression in [1] and discover git built by cmake in subdirs of
"contrib/buildsystems/out", in addition to the "out" directory itself.
As noted in [2] the default for Visual Studio is to use
"out/build/<config>", where "<config>" is a default configuration
name. We might be able to make this deterministic in the future with a
"CMakePresets.json", but that facility is newer than our oldest
supported CMake and VS version.
1. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,
2022-11-03)
2. https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
On Thu, Dec 01 2022, Phillip Wood wrote:
> On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:
>> On Wed, Nov 30 2022, Phillip Wood wrote:
>>
>>> Hi Junio
>> Hi, and thanks for your review of this series.
>>
>>> On 29/11/2022 09:40, Junio C Hamano wrote:
>>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits
>>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)
>>>> + CI: add a "linux-cmake-test" to run cmake & ctest on linux
>>>> + cmake: copy over git-p4.py for t983[56] perforce test
>>>> + cmake: only look for "sh" in "C:/Program Files" on Windows
>>>> + cmake: increase test timeout on Windows only
>>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
>>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR
>>>> + test-lib.sh: support a "GIT_TEST_BUILD_DIR"
>>>> + cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
>>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
>>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
>>>> + cmake: don't copy chainlint.pl to build directory
>>>> + cmake: update instructions for portable CMakeLists.txt
>>>> + cmake: use "-S" and "-B" to specify source and build directories
>>>> + cmake: don't invoke msgfmt with --statistics
>>>> Fix assorted issues with CTest on *nix machines.
>
> Junio please drop this series when you rebuild next as it breaks
> manually running individual test scripts when building with Visual
> Studio.
I think the issue you've spotted is easily fixed on top. See below.
>>> If that's all this series did then I think it would be fine. However
>>> it also makes changes to test-lib.sh to hard code the build directory
>>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an
>>> improvement on the status quo.
>> I think the series as it stands addresses those concerns. In
>> particular
>> building outside of contrib/buildsystems/out works, just as before:
>> cmake -S contrib/buildsystems -B /tmp/git-build
>> -DCMAKE_BUILD_TYPE=Debug &&
>> make -C /tmp/git-build &&
>> ctest --test-dir /tmp/git-build -R t0001
>> Per [1] and [2] which added the "ctest" support that's the use-case
>> for
>> this part of the build: running the tests with ctest, which works as
>> before with the default or custom directories.
>> Perhaps the reason this has been a sticking point for you is that in
>> summarizing this, Johannes's [3] didn't make that distinction between
>> running the tests with "ctest" and running them manually by entering the
>> "t/" directory after the build. I.e.:
>
> In other words Johannes thinks both are equally important. The windows
> build has always supported running the tests manually from /t and he
> quite reasonable wants that to continue working.
Yes, that should work. Now, clearly I missed that VS doesn't use "out"
by default, but a subdirectory, which the below patch fixes.
But I think we can still draw a distinction between anything under
"out" and arbitrary user-supplied directories, which can possibly be
located outside of the source tree.
>> (cd t && ./t0001-init.sh)
>> It's only that part which acts differently in this series. I.e. if
>> you
>> were to build in /tmp/git-build this would no longer find your built
>> assets:
>> $ ./t0001-init.sh
>> error: GIT-BUILD-OPTIONS missing (has Git been built?).
>> If you just leave it at the default of "contrib/buildsystems/out"
>> it'll
>> work:
>> (cd t && ./t0001-init.sh)
>> ok 1 [...]
>> I think my [4] convincingly makes the case that nobody will
>> care. I.e. as the [5] it links to the use-case for running the test
>> after the build without ctest was ("[...]" insert is mine):
>> [To build and test with VS] open the worktree as a folder, and
>> Visual Studio will find the `CMakeLists.txt` file and
>> automatically generate the project files.
>> I.e. we want to support the user who builds with that method, and
>> runs
>> the tests manually. I think you're worrying about an edge case that
>> nobody's using in practice.
>
> You seem to be assuming that Visual Studio creates its build artifacts
> in contrib/buildsystems/out based on a gitignore rule. Given the rule
> ignores _all_ subdirectories below contrib/buildsystems/out that is a
> big assumption. Despite me repeatedly raising concerns about the hard
> coded build directory you do not seem to have checked exactly where
> Visual Studio creates its build artifacts.
I did check, but I got it wrong from reading the docs & commit message.
I don't have a local Windows or VS setup. Thanks for testing it.
> This morning I installed
> Visual Studio to check this and discovered the build is in a
> subdirectory below contrib/buildsystems/out so this series will break
> manual test runs for anyone building git using the recommend method. I
> find it rather frustrating that you argue below that Windows specific
> knowledge and testing are not required when you're altering the
> Windows build.
I was saying that you could round-trip test the auto-discovery of the
build directory on *nix.
Now, clearly I missed the "in a subdir of out" case. But that's
separate from whether you can test that case cross-platform. With the
below patch this works:
cmake -S contrib/buildsystems -B contrib/buildsystems/out/a/b/c -DCMAKE_BUILD_TYPE=Debug &&
make -C contrib/buildsystems/out/a/b/c &&
(cd t && ./t0071-sort.sh)
But did this work for you before, and does it work on "master"? I
think this never worked out of the box until my series. I.e. if you do
that on v2.38.0 (before "js/cmake-updates") you'll get:
$ ./t0071-sort.sh
error: GIT-BUILD-OPTIONS missing (has Git been built?).
The reason is that before "js/cmake-updates" the part of the cmake
recipe that established the ling between the test-lib.sh and the
cmake-built tree was contingent on running ctest. E.g.:
ctest --test-dir contrib/buildsystems/out/a/b/c -R t0071
Once you did that it would patch your t/test-lib.sh:
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a65df2fd220..70b0a633e4c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -44,1 +44,1 @@ fi
-GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
+GIT_BUILD_DIR="$TEST_DIRECTORY/../contrib/buildsystems/out/a/b/c"
Likewise, with "js/cmake-updates" it also doesn't work after you
*build*. I. with it it would create a "GIT-BUILD-DIR" file, but only
once ctest is run. I.e.:
(cd t && ./t0071-sort.sh);
file GIT-BUILD-DIR;
ctest --test-dir contrib/buildsystems/out/a/b/c -R t0071
file GIT-BUILD-DIR;
(cd t && ./t0071-sort.sh)
Would emit:
error: GIT-BUILD-OPTIONS missing (has Git been built?).
GIT-BUILD-DIR: cannot open `GIT-BUILD-DIR' (No such file or directory)
<ctest output>
GIT-BUILD-DIR: ASCII text, with no line terminators
<test output>
It's only with my series that it started working wihout having to run
"ctest". With the below the test-lib.sh will optimistically find the
"git" in "contrib/buildsystems/out".
Does the VS integration run the equivalent of "ctest" by default?
t/test-lib.sh | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ce319c9963e..9c63ee428f2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -57,9 +57,11 @@ if test -n "$GIT_TEST_BUILD_DIR"
then
GIT_BUILD_DIR="$GIT_TEST_BUILD_DIR"
elif ! test -x "$GIT_BUILD_DIR/git" &&
- test -x "$GIT_BUILD_DIR/contrib/buildsystems/out/git"
+ test -d "$GIT_BUILD_DIR/contrib/buildsystems/out"
then
- GIT_BUILD_DIR="$GIT_SOURCE_DIR/contrib/buildsystems/out"
+ GIT_BUILD_OPTIONS="$(find "$GIT_BUILD_DIR/contrib/buildsystems/out" \
+ -type f -name 'GIT-BUILD-OPTIONS')"
+ GIT_BUILD_DIR="${GIT_BUILD_OPTIONS%/GIT-BUILD-OPTIONS}"
GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT=t
# On Windows, we must convert Windows paths lest they contain a colon
@@ -1646,7 +1648,7 @@ remove_trash_directory "$TRASH_DIRECTORY" || {
# anything using lib-subtest.sh
if test -n "$GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT" && test -t 1
then
- say "setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git"
+ say "setup: had no ../git, but found & used cmake built git in '$GIT_BUILD_DIR/git'"
fi
remove_trash=t
--
2.39.0.rc1.974.g67e2c53d827
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-01 16:39 ` [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out" Ævar Arnfjörð Bjarmason
@ 2022-12-01 16:48 ` Phillip Wood
2022-12-01 17:13 ` Ævar Arnfjörð Bjarmason
2022-12-01 23:00 ` Junio C Hamano
1 sibling, 1 reply; 43+ messages in thread
From: Phillip Wood @ 2022-12-01 16:48 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason, git
Cc: Phillip Wood, Junio C Hamano, Johannes Schindelin
On 01/12/2022 16:39, Ævar Arnfjörð Bjarmason wrote:
> Fix a regression in [1] and discover git built by cmake in subdirs of
> "contrib/buildsystems/out", in addition to the "out" directory itself.
How do you propose to find the most recent build? There are different
directories for different architectures and different directories for
debug and release builds.
Hard coding the build directory is a bad idea and should be dropped.
Best Wishes
Phillip
> As noted in [2] the default for Visual Studio is to use
> "out/build/<config>", where "<config>" is a default configuration
> name. We might be able to make this deterministic in the future with a
> "CMakePresets.json", but that facility is newer than our oldest
> supported CMake and VS version.
>
> 1. 16a5421a654 (Makefile + cmake: use environment, not GIT-BUILD-DIR,
> 2022-11-03)
> 2. https://learn.microsoft.com/en-us/cpp/build/cmake-projects-in-visual-studio?view=msvc-170
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>
> On Thu, Dec 01 2022, Phillip Wood wrote:
>
>> On 30/11/2022 10:16, Ævar Arnfjörð Bjarmason wrote:
>>> On Wed, Nov 30 2022, Phillip Wood wrote:
>>>
>>>> Hi Junio
>>> Hi, and thanks for your review of this series.
>>>
>>>> On 29/11/2022 09:40, Junio C Hamano wrote:
>>>>> * ab/cmake-nix-and-ci (2022-11-04) 14 commits
>>>>> (merged to 'next' on 2022-11-08 at 6ef4e93b36)
>>>>> + CI: add a "linux-cmake-test" to run cmake & ctest on linux
>>>>> + cmake: copy over git-p4.py for t983[56] perforce test
>>>>> + cmake: only look for "sh" in "C:/Program Files" on Windows
>>>>> + cmake: increase test timeout on Windows only
>>>>> + cmake: support GIT_TEST_OPTS, abstract away WIN32 defaults
>>>>> + Makefile + cmake: use environment, not GIT-BUILD-DIR
>>>>> + test-lib.sh: support a "GIT_TEST_BUILD_DIR"
>>>>> + cmake: set "USE_LIBPCRE2" in "GIT-BUILD-OPTIONS" for test-lib.sh
>>>>> + cmake & test-lib.sh: add a $GIT_SOURCE_DIR variable
>>>>> + cmake: chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4
>>>>> + cmake: don't copy chainlint.pl to build directory
>>>>> + cmake: update instructions for portable CMakeLists.txt
>>>>> + cmake: use "-S" and "-B" to specify source and build directories
>>>>> + cmake: don't invoke msgfmt with --statistics
>>>>> Fix assorted issues with CTest on *nix machines.
>>
>> Junio please drop this series when you rebuild next as it breaks
>> manually running individual test scripts when building with Visual
>> Studio.
>
> I think the issue you've spotted is easily fixed on top. See below.
>
>>>> If that's all this series did then I think it would be fine. However
>>>> it also makes changes to test-lib.sh to hard code the build directory
>>>> in an attempt to remove GIT-BUILD-DIR. I'm not convinced that is an
>>>> improvement on the status quo.
>>> I think the series as it stands addresses those concerns. In
>>> particular
>>> building outside of contrib/buildsystems/out works, just as before:
>>> cmake -S contrib/buildsystems -B /tmp/git-build
>>> -DCMAKE_BUILD_TYPE=Debug &&
>>> make -C /tmp/git-build &&
>>> ctest --test-dir /tmp/git-build -R t0001
>>> Per [1] and [2] which added the "ctest" support that's the use-case
>>> for
>>> this part of the build: running the tests with ctest, which works as
>>> before with the default or custom directories.
>>> Perhaps the reason this has been a sticking point for you is that in
>>> summarizing this, Johannes's [3] didn't make that distinction between
>>> running the tests with "ctest" and running them manually by entering the
>>> "t/" directory after the build. I.e.:
>>
>> In other words Johannes thinks both are equally important. The windows
>> build has always supported running the tests manually from /t and he
>> quite reasonable wants that to continue working.
>
> Yes, that should work. Now, clearly I missed that VS doesn't use "out"
> by default, but a subdirectory, which the below patch fixes.
>
> But I think we can still draw a distinction between anything under
> "out" and arbitrary user-supplied directories, which can possibly be
> located outside of the source tree.
>
>>> (cd t && ./t0001-init.sh)
>>> It's only that part which acts differently in this series. I.e. if
>>> you
>>> were to build in /tmp/git-build this would no longer find your built
>>> assets:
>>> $ ./t0001-init.sh
>>> error: GIT-BUILD-OPTIONS missing (has Git been built?).
>>> If you just leave it at the default of "contrib/buildsystems/out"
>>> it'll
>>> work:
>>> (cd t && ./t0001-init.sh)
>>> ok 1 [...]
>>> I think my [4] convincingly makes the case that nobody will
>>> care. I.e. as the [5] it links to the use-case for running the test
>>> after the build without ctest was ("[...]" insert is mine):
>>> [To build and test with VS] open the worktree as a folder, and
>>> Visual Studio will find the `CMakeLists.txt` file and
>>> automatically generate the project files.
>>> I.e. we want to support the user who builds with that method, and
>>> runs
>>> the tests manually. I think you're worrying about an edge case that
>>> nobody's using in practice.
>>
>> You seem to be assuming that Visual Studio creates its build artifacts
>> in contrib/buildsystems/out based on a gitignore rule. Given the rule
>> ignores _all_ subdirectories below contrib/buildsystems/out that is a
>> big assumption. Despite me repeatedly raising concerns about the hard
>> coded build directory you do not seem to have checked exactly where
>> Visual Studio creates its build artifacts.
>
> I did check, but I got it wrong from reading the docs & commit message.
>
> I don't have a local Windows or VS setup. Thanks for testing it.
>
>> This morning I installed
>> Visual Studio to check this and discovered the build is in a
>> subdirectory below contrib/buildsystems/out so this series will break
>> manual test runs for anyone building git using the recommend method. I
>> find it rather frustrating that you argue below that Windows specific
>> knowledge and testing are not required when you're altering the
>> Windows build.
>
> I was saying that you could round-trip test the auto-discovery of the
> build directory on *nix.
>
> Now, clearly I missed the "in a subdir of out" case. But that's
> separate from whether you can test that case cross-platform. With the
> below patch this works:
>
> cmake -S contrib/buildsystems -B contrib/buildsystems/out/a/b/c -DCMAKE_BUILD_TYPE=Debug &&
> make -C contrib/buildsystems/out/a/b/c &&
> (cd t && ./t0071-sort.sh)
>
> But did this work for you before, and does it work on "master"? I
> think this never worked out of the box until my series. I.e. if you do
> that on v2.38.0 (before "js/cmake-updates") you'll get:
>
> $ ./t0071-sort.sh
> error: GIT-BUILD-OPTIONS missing (has Git been built?).
>
> The reason is that before "js/cmake-updates" the part of the cmake
> recipe that established the ling between the test-lib.sh and the
> cmake-built tree was contingent on running ctest. E.g.:
>
> ctest --test-dir contrib/buildsystems/out/a/b/c -R t0071
>
> Once you did that it would patch your t/test-lib.sh:
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index a65df2fd220..70b0a633e4c 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -44,1 +44,1 @@ fi
> -GIT_BUILD_DIR="${TEST_DIRECTORY%/t}"
> +GIT_BUILD_DIR="$TEST_DIRECTORY/../contrib/buildsystems/out/a/b/c"
>
> Likewise, with "js/cmake-updates" it also doesn't work after you
> *build*. I. with it it would create a "GIT-BUILD-DIR" file, but only
> once ctest is run. I.e.:
>
> (cd t && ./t0071-sort.sh);
> file GIT-BUILD-DIR;
> ctest --test-dir contrib/buildsystems/out/a/b/c -R t0071
> file GIT-BUILD-DIR;
> (cd t && ./t0071-sort.sh)
>
> Would emit:
>
> error: GIT-BUILD-OPTIONS missing (has Git been built?).
> GIT-BUILD-DIR: cannot open `GIT-BUILD-DIR' (No such file or directory)
> <ctest output>
> GIT-BUILD-DIR: ASCII text, with no line terminators
> <test output>
>
> It's only with my series that it started working wihout having to run
> "ctest". With the below the test-lib.sh will optimistically find the
> "git" in "contrib/buildsystems/out".
>
> Does the VS integration run the equivalent of "ctest" by default?
>
> t/test-lib.sh | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index ce319c9963e..9c63ee428f2 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -57,9 +57,11 @@ if test -n "$GIT_TEST_BUILD_DIR"
> then
> GIT_BUILD_DIR="$GIT_TEST_BUILD_DIR"
> elif ! test -x "$GIT_BUILD_DIR/git" &&
> - test -x "$GIT_BUILD_DIR/contrib/buildsystems/out/git"
> + test -d "$GIT_BUILD_DIR/contrib/buildsystems/out"
> then
> - GIT_BUILD_DIR="$GIT_SOURCE_DIR/contrib/buildsystems/out"
> + GIT_BUILD_OPTIONS="$(find "$GIT_BUILD_DIR/contrib/buildsystems/out" \
> + -type f -name 'GIT-BUILD-OPTIONS')"
> + GIT_BUILD_DIR="${GIT_BUILD_OPTIONS%/GIT-BUILD-OPTIONS}"
> GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT=t
>
> # On Windows, we must convert Windows paths lest they contain a colon
> @@ -1646,7 +1648,7 @@ remove_trash_directory "$TRASH_DIRECTORY" || {
> # anything using lib-subtest.sh
> if test -n "$GIT_AUTO_CONTRIB_BUILDSYSTEMS_OUT" && test -t 1
> then
> - say "setup: had no ../git, but found & used cmake built git in ../contrib/buildsystems/out/git"
> + say "setup: had no ../git, but found & used cmake built git in '$GIT_BUILD_DIR/git'"
> fi
>
> remove_trash=t
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-01 16:48 ` Phillip Wood
@ 2022-12-01 17:13 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-01 17:13 UTC (permalink / raw)
To: phillip.wood; +Cc: git, Phillip Wood, Junio C Hamano, Johannes Schindelin
On Thu, Dec 01 2022, Phillip Wood wrote:
> On 01/12/2022 16:39, Ævar Arnfjörð Bjarmason wrote:
>> Fix a regression in [1] and discover git built by cmake in subdirs of
>> "contrib/buildsystems/out", in addition to the "out" directory itself.
>
> How do you propose to find the most recent build? There are different
> directories for different architectures and different directories for
> debug and release builds.
I'm not proposing to do that, besides, if you built locally for
different architectures running the tests would just fail, wouldn't it?
> Hard coding the build directory is a bad idea and should be dropped.
I think it's a better idea to target the common case of a user building
& being able to run the tests form t/, rather than requiring
bootstrapping that process by running "ctest".
Did you test that case on v2.38.0 and/or master? I.e. is VS running that
ctest portion automatically?
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-11-30 19:43 ` Ævar Arnfjörð Bjarmason
2022-12-01 5:20 ` Junio C Hamano
@ 2022-12-01 17:44 ` Glen Choo
2022-12-01 21:26 ` Junio C Hamano
1 sibling, 1 reply; 43+ messages in thread
From: Glen Choo @ 2022-12-01 17:44 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason; +Cc: Junio C Hamano, git
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> For my part I was waiting to see what Junio would do with
> "ab/submodule-no-abspath", which is already in "next". Depending on
> whether it's ejected or not I'd need to re-roll
> "ab/remove--super-prefix" on top of a new "master", as it extends the
> tests it added.
Thanks for the status update!
> You noted in [1] that you strongly preferred seeing
> "ab/submodule-no-abspath" ejected. I think you're right that the output
> is a bit weird, but:
>
> A. I think it's mainly odd/unintuitive for the recursive cases, I think
> outside of our own test suite absorbing repositories recursively
> almost never happens.
Frankly, I find it odd in the non-recursive case too e.g. in one of the
non-recursive tests, we have:
Migrating git directory of '\''sub1'\'' from '\''sub1/.git'\'' to '\''../../.git/modules/sub1'\''
where all 3 paths are relative, the first two share the same base but
not the last one. I don't think a casual reader can easily tell that the
last one should be relative to the second one.
> B. I think it's an improvement in the output compared to the absolute
> paths we have now, especially for the common case of non-recursive.
For the reason above, this doesn't feel like an improvement to me :/
> C. Changing it made it easier to test it, which is how it ended up as a
> supposedly quick prerequisite for "ab/remove--super-prefix": It's
> otherwise changing a test blindspot.
With abspaths, couldn't we test this with $PWD?
> D. As you note in [1] the data we'd need to pass around to make it
> sensible (maybe it should always be consistent with "git mv -v"?)
> would require passing more state around, some of which is tricky.
Yeah I think this a good to have in the long run, but let's punt on it
for now.
> I'd prefer to just have it graduate as-is, and build
> "ab/remove--super-prefix" on top. We can always further tweak the output
> later.
>
> But if you & Junio feel otherwise I think the best way forward would be
> to eject both topics, and I'd submit a re-rolled
> "ab/remove--super-prefix".
A re-rolled "ab/remove--super-prefix" makes sense to me. Sorry again for
not voicing my thoughts sooner and saving us from this churn :(
>
> Either would work as a way forward. Just let me know what you both
> prefer.
>
> 1. https://lore.kernel.org/git/kl6l7czmec10.fsf@chooglen-macbookpro.roam.corp.google.com/
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-12-01 17:44 ` Glen Choo
@ 2022-12-01 21:26 ` Junio C Hamano
0 siblings, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2022-12-01 21:26 UTC (permalink / raw)
To: Glen Choo; +Cc: Ævar Arnfjörð Bjarmason, git
Glen Choo <chooglen@google.com> writes:
> Frankly, I find it odd in the non-recursive case too e.g. in one of the
> non-recursive tests, we have:
>
> Migrating git directory of '\''sub1'\'' from '\''sub1/.git'\'' to '\''../../.git/modules/sub1'\''
>
> where all 3 paths are relative, the first two share the same base but
> not the last one. I don't think a casual reader can easily tell that the
> last one should be relative to the second one.
That's a good example, although I can understand why a developer
(not a user) might choose to show the last one relative to the one
that comes before it.
Thanks, both.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-01 16:39 ` [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out" Ævar Arnfjörð Bjarmason
2022-12-01 16:48 ` Phillip Wood
@ 2022-12-01 23:00 ` Junio C Hamano
2022-12-02 15:14 ` Phillip Wood
1 sibling, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2022-12-01 23:00 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Johannes Schindelin
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> Junio please drop this series when you rebuild next as it breaks
>> manually running individual test scripts when building with Visual
>> Studio.
>
> I think the issue you've spotted is easily fixed on top. See below.
Smells more like papering over than fixed, but let's see how folks
who need cmake/ctest feel about it.
Let's mark the series never to graduate to 'master' for now,
optionally revert it out of 'next'.
Phillip, you asked about rebuilding 'next', which would not
happen until 2.39.0 final---did you mean reverting the topic out
of 'next'? Do you need 'next' without this topic, not just
'master'?
I'll then wait for something both camps (you and folks on Visual
Studio?) can agree on to requeue.
Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29))
2022-12-01 15:06 ` Derrick Stolee
@ 2022-12-02 0:25 ` Junio C Hamano
0 siblings, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2022-12-02 0:25 UTC (permalink / raw)
To: Derrick Stolee; +Cc: git
Derrick Stolee <derrickstolee@github.com> writes:
> I have discovered that the problem is due to the test variable
> GIT_TEST_PACKED_REFS_VERSION=2, so instead it's probably best
> to eject ds/packed-refs-v2 from 'seen' instead of ds/bundle-uri-4.
Thanks for digging. I've shuffled the order of topics in 'seen'.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-01 23:00 ` Junio C Hamano
@ 2022-12-02 15:14 ` Phillip Wood
2022-12-02 16:40 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 43+ messages in thread
From: Phillip Wood @ 2022-12-02 15:14 UTC (permalink / raw)
To: Junio C Hamano, Ævar Arnfjörð Bjarmason
Cc: git, Phillip Wood, Johannes Schindelin
Hi Junio
On 01/12/2022 23:00, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>>> Junio please drop this series when you rebuild next as it breaks
>>> manually running individual test scripts when building with Visual
>>> Studio.
>>
>> I think the issue you've spotted is easily fixed on top. See below.
>
> Smells more like papering over than fixed, but let's see how folks
> who need cmake/ctest feel about it.
As MSVC uses different directories for debug and release builds there
can be more than one build directory. I don't think selecting one of
them at random using 'find' is a good idea.
> Let's mark the series never to graduate to 'master' for now,
> optionally revert it out of 'next'.
>
> Phillip, you asked about rebuilding 'next', which would not
> happen until 2.39.0 final---did you mean reverting the topic out
> of 'next'? Do you need 'next' without this topic, not just
> 'master'?
I don't mind waiting but I'm not a Windows user. I only tested this
topic under Windows because I knew Ævar had not and a quick web search
for "MSVC CMake" made me worry it was broken.
I'm afraid I wont be spending anymore time on this topic. I had hoped
that having the CMake build work under Linux would help developers avoid
breaking it. However I'm concerned that if developers do not appreciate
that there are differences between the Linux and Windows builds it will
actually create a false sense of security and be used as an excuse not
to properly test under Windows[1]. Recent events have confirmed my view
that changes like this need the attention of someone with experience of
Windows development and given that yesterday was the first time I'd used
MSVC since about 1994 I do not fit that description.
In addition to the breakage I reported yesterday 623fde1438 (cmake:
chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4, 2022-11-03)
causes CMake older that 3.19 to error out when run from MSVC because
chmod does not exist on Windows. Also when running 'ctest' on "next" I
see tests failing because they cannot find 'test-tool' (I haven't tried
running the failing tests manually)
Best Wishes
Phillip
[1] While our CI helps the MSVC job runs CMake manually, performs an
in-tree build and does not use ctest. In contrast a user running the
MSVC GUI does not run CMake themselves, ends up with an out-of-tree
build and runs the tests with ctest.
> I'll then wait for something both camps (you and folks on Visual
> Studio?) can agree on to requeue.
>
> Thanks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-02 15:14 ` Phillip Wood
@ 2022-12-02 16:40 ` Ævar Arnfjörð Bjarmason
2022-12-02 23:10 ` Jeff King
0 siblings, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-02 16:40 UTC (permalink / raw)
To: phillip.wood; +Cc: Junio C Hamano, git, Phillip Wood, Johannes Schindelin
On Fri, Dec 02 2022, Phillip Wood wrote:
> On 01/12/2022 23:00, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>>> Junio please drop this series when you rebuild next as it breaks
>>>> manually running individual test scripts when building with Visual
>>>> Studio.
>>>
>>> I think the issue you've spotted is easily fixed on top. See below.
>> Smells more like papering over than fixed, but let's see how folks
>> who need cmake/ctest feel about it.
>
> As MSVC uses different directories for debug and release builds there
> can be more than one build directory. I don't think selecting one of
> them at random using 'find' is a good idea.
I think the latest iteration submitted today should fix that issue:
https://lore.kernel.org/git/cover-v5-00.15-00000000000-20221202T110947Z-avarab@gmail.com/
>> Let's mark the series never to graduate to 'master' for now,
>> optionally revert it out of 'next'.
>> Phillip, you asked about rebuilding 'next', which would not
>> happen until 2.39.0 final---did you mean reverting the topic out
>> of 'next'? Do you need 'next' without this topic, not just
>> 'master'?
>
> I don't mind waiting but I'm not a Windows user. I only tested this
> topic under Windows because I knew Ævar had not and a quick web search
> for "MSVC CMake" made me worry it was broken.
>
> I'm afraid I wont be spending anymore time on this topic. I had hoped
> that having the CMake build work under Linux would help developers
> avoid breaking it. However I'm concerned that if developers do not
> appreciate that there are differences between the Linux and Windows
> builds it will actually create a false sense of security and be used
> as an excuse not to properly test under Windows[1]. Recent events have
> confirmed my view that changes like this need the attention of someone
> with experience of Windows development and given that yesterday was
> the first time I'd used MSVC since about 1994 I do not fit that
> description.
> [...]
> [1] While our CI helps the MSVC job runs CMake manually, performs an
> in-tree build and does not use ctest. In contrast a user running the
> MSVC GUI does not run CMake themselves, ends up with an out-of-tree
> build and runs the tests with ctest.
I don't run Windows by choice, and I'm not interested in running a
propriterary IDE (VS) either.
The main reason I'm working on this series is that while we as a project
are happy to support proprietary OS's, it hasn't been a requirement for
participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or
whatever to submit patches.
Of course we have platform-specific code. but this CMake component is
unique in how invasive it is.
It's easy to e.g. stay away from the OSX-specific code in
compat/fsmonitor/*darwin*.[ch], or generally speaking the
Windows-specific C code.
But for CMake it's become a hard requirenment for many changes, even
though it's a contrib/ component.
Now, I'm not looking to get rid of it, it's clearly useful, particularly
with MSVC (or so I've gathered).
But I'd also like some future where any time I and others who don't use
Windows need to patch certain parts of the Makefile that we don't need
to spend a long time bouncing thigs against the Windows CI, but can run
this component other platforms..
E.g. I think for cfe853e66be (hook-list.h: add a generated list of
hooks, like config-list.h, 2021-09-26) I spent at least 3-4 hours on
what should have been a 5-10 minute task of making the relatively minor
change to generate hook-list.h. It was a push, wait 30-60 minutes, find
some minor (e.g. syntax) issue, rinse & repeat.
Now, clearly the outstanding issues with (if any) need to be fixed, and
thanks for sticking with testing it for so long. But hopefully the above
gives you some background.
> In addition to the breakage I reported yesterday 623fde1438 (cmake:
> chmod +x the bin-wrappers/* & SCRIPT_{SH,PERL} & git-p4, 2022-11-03)
> causes CMake older that 3.19 to error out when run from MSVC because
> chmod does not exist on Windows. Also when running 'ctest' on "next" I
> see tests failing because they cannot find 'test-tool' (I haven't
> tried running the failing tests manually)
That's odd, do you happen to have some output from "ctest" for that?
Our "cmake" build already invokes shell scripts which rely on "grep",
"sed" etc, and the test suite itself invokes "chmod", I just understand
that it's a noop wrapper on Windows.
So I can also re-roll and just ifdef that part awa on win32, but I
wonder what's going on there. I wonder if it's because we'd need to
spawn it via "/bin/sh" (but just skippin it seems better).
One of the things I didn't change with this series is how "test-tool" is
handled. I.e. it's there before in the generated "t/helper" directory,
and in the generated "bin-wrappers/".
We also find it when running the tests with Windows in GitHub CI.
Did you try on v2.38.0 and/or "master" as well?
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-02 16:40 ` Ævar Arnfjörð Bjarmason
@ 2022-12-02 23:10 ` Jeff King
2022-12-03 1:12 ` Junio C Hamano
2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
0 siblings, 2 replies; 43+ messages in thread
From: Jeff King @ 2022-12-02 23:10 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Fri, Dec 02, 2022 at 05:40:34PM +0100, Ævar Arnfjörð Bjarmason wrote:
> > [1] While our CI helps the MSVC job runs CMake manually, performs an
> > in-tree build and does not use ctest. In contrast a user running the
> > MSVC GUI does not run CMake themselves, ends up with an out-of-tree
> > build and runs the tests with ctest.
>
> I don't run Windows by choice, and I'm not interested in running a
> propriterary IDE (VS) either.
>
> The main reason I'm working on this series is that while we as a project
> are happy to support proprietary OS's, it hasn't been a requirement for
> participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or
> whatever to submit patches.
>
> Of course we have platform-specific code. but this CMake component is
> unique in how invasive it is.
>
> It's easy to e.g. stay away from the OSX-specific code in
> compat/fsmonitor/*darwin*.[ch], or generally speaking the
> Windows-specific C code.
>
> But for CMake it's become a hard requirenment for many changes, even
> though it's a contrib/ component.
I have similar feelings to you here. Back when cmake support was
introduced, I explicitly wanted it to be something for people who cared
about it, but that wouldn't bother people who didn't use it:
https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
I stand by that sentiment, but it seems to have crept up as a required
thing to deal with, and that is mostly because of CI. Using cmake in CI
is good for telling developers when a change they make has broken cmake.
But it also makes cmake their problem, and not the folks interested in
cmake.
Now maybe attitudes have changed, and I am out of date, and cmake
support is considered mature and really important (or maybe nobody even
agreed with me back then ;) ). But if not, should we consider softening
the CI output so that cmake failures aren't "real" failures? That seems
drastic and mean, and I don't like it. But it's the root of the issue,
IMHO.
As a side note, this isn't the only such instance of this problem. Two
other things to think about:
- You mentioned darwin fsmonitor code. And it's true that you can
largely ignore it if you don't touch it. But every once in a while
you get bit by it (e.g., enabling a new compiler warning which
triggers in code you don't compile on your platform, and now you
have to guess-and-check the fix with CI). This sucks, but is kind of
inevitable on a cross-platform system. I think the issue with cmake
is that because it's basically duplicating/wrapping the Makefile, it
_feels_ unnecessary to people on platforms with working make, and
triggers more frequently (because changes to the rest of the build
system may break cmake in subtle ways).
- I'd actually put the leak-checking CI in the same boat. It's a good
goal, and one I hope we work towards. But it feels like the current
state is not very mature, and people often end up wrestling with CI
to deal with failures that they didn't even introduce (e.g., adding
a new test that happens to run a Git program that has an existing
leak, and now you are on the hook for figuring out why the existing
"passes leaks" annotation is wrong).
My original hope is that we would introduce leak-checking tooling
that people interested in leaks could use, and other people could
ignore until we reached a leak-free state. Because it's in CI it
means that people get notified of new leaks in code they write
(which is good, and helps people interested in leaks), but it also
means they have to deal with the immature state.
I'm not necessarily proposing to drop the leaks CI job here. I'm mostly
philosophizing about the greater problem. In the early days of Git, the
cross-platform testing philosophy was: somebody who cares will test on
that platform and write a patch. If they don't, how important could it
be? With CI that happens automatically and it becomes everybody's
problem, which is a blessing and a curse.
-Peff
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-02 23:10 ` Jeff King
@ 2022-12-03 1:12 ` Junio C Hamano
2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
1 sibling, 0 replies; 43+ messages in thread
From: Junio C Hamano @ 2022-12-03 1:12 UTC (permalink / raw)
To: Jeff King
Cc: Ævar Arnfjörð Bjarmason, phillip.wood, git,
Phillip Wood, Johannes Schindelin
Jeff King <peff@peff.net> writes:
> I have similar feelings to you here. Back when cmake support was
> introduced, I explicitly wanted it to be something for people who cared
> about it, but that wouldn't bother people who didn't use it:
>
> https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
>
> I stand by that sentiment, but it seems to have crept up as a required
> thing to deal with, and that is mostly because of CI. Using cmake in CI
> is good for telling developers when a change they make has broken cmake.
> But it also makes cmake their problem, and not the folks interested in
> cmake.
>
> Now maybe attitudes have changed, and I am out of date, and cmake
> support is considered mature and really important (or maybe nobody even
> agreed with me back then ;) ). But if not, should we consider softening
> the CI output so that cmake failures aren't "real" failures? That seems
> drastic and mean, and I don't like it. But it's the root of the issue,
> IMHO.
It makes the two of us (or three couning Ævar?).
> - I'd actually put the leak-checking CI in the same boat. It's a good
> goal, and one I hope we work towards. But it feels like the current
> state is not very mature, and people often end up wrestling with CI
> to deal with failures that they didn't even introduce (e.g., adding
> a new test that happens to run a Git program that has an existing
> leak, and now you are on the hook for figuring out why the existing
> "passes leaks" annotation is wrong).
Hear, hear.
> I'm not necessarily proposing to drop the leaks CI job here. I'm mostly
> philosophizing about the greater problem. In the early days of Git, the
> cross-platform testing philosophy was: somebody who cares will test on
> that platform and write a patch. If they don't, how important could it
> be? With CI that happens automatically and it becomes everybody's
> problem, which is a blessing and a curse.
True.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-02 23:10 ` Jeff King
2022-12-03 1:12 ` Junio C Hamano
@ 2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
2022-12-05 9:15 ` Jeff King
2022-12-05 23:34 ` Taylor Blau
1 sibling, 2 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-03 1:41 UTC (permalink / raw)
To: Jeff King
Cc: phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Fri, Dec 02 2022, Jeff King wrote:
> On Fri, Dec 02, 2022 at 05:40:34PM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > [1] While our CI helps the MSVC job runs CMake manually, performs an
>> > in-tree build and does not use ctest. In contrast a user running the
>> > MSVC GUI does not run CMake themselves, ends up with an out-of-tree
>> > build and runs the tests with ctest.
>>
>> I don't run Windows by choice, and I'm not interested in running a
>> propriterary IDE (VS) either.
>>
>> The main reason I'm working on this series is that while we as a project
>> are happy to support proprietary OS's, it hasn't been a requirement for
>> participation that you need to buy a copy of Windows, OSX, AIX, HP/UX or
>> whatever to submit patches.
>>
>> Of course we have platform-specific code. but this CMake component is
>> unique in how invasive it is.
>>
>> It's easy to e.g. stay away from the OSX-specific code in
>> compat/fsmonitor/*darwin*.[ch], or generally speaking the
>> Windows-specific C code.
>>
>> But for CMake it's become a hard requirenment for many changes, even
>> though it's a contrib/ component.
>
> I have similar feelings to you here. Back when cmake support was
> introduced, I explicitly wanted it to be something for people who cared
> about it, but that wouldn't bother people who didn't use it:
>
> https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
>
> I stand by that sentiment, but it seems to have crept up as a required
> thing to deal with, and that is mostly because of CI. Using cmake in CI
> is good for telling developers when a change they make has broken cmake.
> But it also makes cmake their problem, and not the folks interested in
> cmake.
That's a bit of a pain, but I don't think the main problem is its
integration with CI. It's that there doesn't really seem to be an
interest in its active maintenance & review from its supposed main
target audience.
Case in point this "ab/cmake-nix-and-ci" topic: It's been queued for
around 2 months now.
I don't use Windows or VS. I'd just like it not to take half may day to
throw things at Windows CI whenever some obscure aspect of cmake breaks.
The only person who's been showing it any notable review interest
(Phillip) apparently hasn't used Windows actively since the mid-90's :)
Which is a very different situation than if it were still something that
broke CI, but there was someone (or more than one person) active on list
willing to test patches to get it working, was familiar with the cmake
language & could help write the cmake version of Makefile changes when
those were needed etc.
> Now maybe attitudes have changed, and I am out of date, and cmake
> support is considered mature and really important (or maybe nobody even
> agreed with me back then ;) ). But if not, should we consider softening
> the CI output so that cmake failures aren't "real" failures? That seems
> drastic and mean, and I don't like it. But it's the root of the issue,
> IMHO.
Yeah, maybe. Maybe if we broke it we'd get people showing up to maintain
it again :)
I do think the series I've got here is the most practical way forward at
this point (any outstanding issues aside). I.e. it's usually fairly
easy-ish to amend the cmake recipe.
It's just been taking *ages* because it's been a dumpste fire outside
of Windows, so if you don't have that OS available to you you need to
wait for the CI. Being able to run it in seconds on *nix really helps..
> As a side note, this isn't the only such instance of this problem. Two
> other things to think about:
>
> - You mentioned darwin fsmonitor code. And it's true that you can
> largely ignore it if you don't touch it. But every once in a while
> you get bit by it (e.g., enabling a new compiler warning which
> triggers in code you don't compile on your platform, and now you
> have to guess-and-check the fix with CI). This sucks, but is kind of
> inevitable on a cross-platform system. I think the issue with cmake
> is that because it's basically duplicating/wrapping the Makefile, it
> _feels_ unnecessary to people on platforms with working make, and
> triggers more frequently (because changes to the rest of the build
> system may break cmake in subtle ways).
Yeah, we'll always have some cross-platform pain.
But e.g. "chmod +x" just works in the Makefile, including when we run it
on Windows. And I've run it on Windows CI. But just upthread of here
Phillip is reporting that it doesn't work from the context of the CMake
recipe.
I've been throwing some things at Windows CI, but I'm pretty stumped as
to what that might be.
Some warning on Mac OS X is trivial by comparison :)
> - I'd actually put the leak-checking CI in the same boat. It's a good
> goal, and one I hope we work towards. But it feels like the current
> state is not very mature, and people often end up wrestling with CI
> to deal with failures that they didn't even introduce (e.g., adding
> a new test that happens to run a Git program that has an existing
> leak, and now you are on the hook for figuring out why the existing
> "passes leaks" annotation is wrong).
>
> My original hope is that we would introduce leak-checking tooling
> that people interested in leaks could use, and other people could
> ignore until we reached a leak-free state. Because it's in CI it
> means that people get notified of new leaks in code they write
> (which is good, and helps people interested in leaks), but it also
> means they have to deal with the immature state.
>
> I'm not necessarily proposing to drop the leaks CI job here. I'm mostly
> philosophizing about the greater problem. In the early days of Git, the
> cross-platform testing philosophy was: somebody who cares will test on
> that platform and write a patch. If they don't, how important could it
> be? With CI that happens automatically and it becomes everybody's
> problem, which is a blessing and a curse.
That's a definitely a bit of an irresistible digression :)
First, I think we can agree that however frustrating that's been (and
sorry!) it would be a lot worse if my average response time to the leak
testing breaking something was measured in months :)
I do think that whatever issues we've had with it (and in retrospect I'd
do some of it differently) that it's a lot more mature these days than
you might remember.
I've been making an effort to specifically address the sorts of leaks
that would cause that sort of pain, e.g. adding an unsespecting "git
log" to an existing test file and the like.
I have a report I generate [1] of the outstanding leaks, sorted by
de-duping stack traces, and an estimate of how many tests would start
passing if a combination of leaks were solved[1].
On the one hand it's ~60k lines of scary stack traces, but on the other
hand it used to be easily 3-4x that (I can't recall the exact size
offhand).
But more importantly while we do have some widespread leaks still, it
used to be the case that e.g. some leaks would show up in 50-100 test
files.
Now the #1 leak by number of times it's seen in test files happens in
just 12 filess, and there's a very long tail of leaks that only happen
in one test somewhere. I.e. are specific to their own area.
Just anecdotally I think it's had sort of the opposite problem recently,
that it's getting good enough to start flagging about new leaks that
really are specific to newly queued topics. E.g. [2] and [3] are two
recent examples.
But in both cases there seems to have been an understandable assumption
that it was probably just "linux-leaks" being noisy, due to the job
crying wolf in the past.
1. https://vm.nix.is/~avar/noindex/2022-12-03-git-leak-report.txt
2. https://lore.kernel.org/git/221108.864jv9sc9r.gmgdl@evledraar.gmail.com/
3. https://lore.kernel.org/git/2488058d-dc59-e8c1-0611-fbcaeb083d73@web.de/
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
@ 2022-12-05 9:15 ` Jeff King
2022-12-05 23:34 ` Taylor Blau
1 sibling, 0 replies; 43+ messages in thread
From: Jeff King @ 2022-12-05 9:15 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
> > I have similar feelings to you here. Back when cmake support was
> > introduced, I explicitly wanted it to be something for people who cared
> > about it, but that wouldn't bother people who didn't use it:
> >
> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
> >
> > I stand by that sentiment, but it seems to have crept up as a required
> > thing to deal with, and that is mostly because of CI. Using cmake in CI
> > is good for telling developers when a change they make has broken cmake.
> > But it also makes cmake their problem, and not the folks interested in
> > cmake.
>
> That's a bit of a pain, but I don't think the main problem is its
> integration with CI. It's that there doesn't really seem to be an
> interest in its active maintenance & review from its supposed main
> target audience.
Yeah, there are two issues:
1. People who don't care about cmake having to think about cmake at
all.
2. People trying to fix cmake and not getting traction.
My pain has usually been (1), but you were nice enough here to make it
to (2). :)
> > Now maybe attitudes have changed, and I am out of date, and cmake
> > support is considered mature and really important (or maybe nobody even
> > agreed with me back then ;) ). But if not, should we consider softening
> > the CI output so that cmake failures aren't "real" failures? That seems
> > drastic and mean, and I don't like it. But it's the root of the issue,
> > IMHO.
>
> Yeah, maybe. Maybe if we broke it we'd get people showing up to maintain
> it again :)
By the way, I looked in the archive, and me complaining about cmake has
come up once or twice. Johannes offered some helpful guidance on the
value of running the vsbuild tests and how we might work around them:
https://lore.kernel.org/git/nycvar.QRO.7.76.6.2008141352430.54@tvgsbejvaqbjf.bet/
> > As a side note, this isn't the only such instance of this problem. Two
> > other things to think about:
> >
> > - You mentioned darwin fsmonitor code. And it's true that you can
> > largely ignore it if you don't touch it. But every once in a while
> > you get bit by it (e.g., enabling a new compiler warning which
> > triggers in code you don't compile on your platform, and now you
> > have to guess-and-check the fix with CI). This sucks, but is kind of
> > inevitable on a cross-platform system. I think the issue with cmake
> > is that because it's basically duplicating/wrapping the Makefile, it
> > _feels_ unnecessary to people on platforms with working make, and
> > triggers more frequently (because changes to the rest of the build
> > system may break cmake in subtle ways).
>
> Yeah, we'll always have some cross-platform pain.
>
> But e.g. "chmod +x" just works in the Makefile, including when we run it
> on Windows. And I've run it on Windows CI. But just upthread of here
> Phillip is reporting that it doesn't work from the context of the CMake
> recipe.
>
> I've been throwing some things at Windows CI, but I'm pretty stumped as
> to what that might be.
>
> Some warning on Mac OS X is trivial by comparison :)
That was just an example, of course. :) I have also done this kind of
"guess and check" with the Windows CI. I think Johannes has given
examples in the past of how to actually connect to a running CI instance
and debug interactively, but I've never managed to remember the correct
incantation at the moment I needed it.
> > I'm not necessarily proposing to drop the leaks CI job here. I'm mostly
> > philosophizing about the greater problem. In the early days of Git, the
> > cross-platform testing philosophy was: somebody who cares will test on
> > that platform and write a patch. If they don't, how important could it
> > be? With CI that happens automatically and it becomes everybody's
> > problem, which is a blessing and a curse.
>
> That's a definitely a bit of an irresistible digression :)
>
> First, I think we can agree that however frustrating that's been (and
> sorry!) it would be a lot worse if my average response time to the leak
> testing breaking something was measured in months :)
Yes, to be clear, I have no problem with you in terms of responsiveness
as maintaining the leaks system. It's mostly that I have to think of
them _at all_ when working on something unrelated (that is not itself
introducing new leaks).
> I do think that whatever issues we've had with it (and in retrospect I'd
> do some of it differently) that it's a lot more mature these days than
> you might remember.
Yeah, it definitely has been getting better, and I admit my opinion is
based on a longer-term experience of a changing variable. There's
probably some clever name for that kind of bias, but I don't know it
offhand.
-Peff
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
2022-12-05 9:15 ` Jeff King
@ 2022-12-05 23:34 ` Taylor Blau
2022-12-05 23:46 ` Ævar Arnfjörð Bjarmason
2022-12-06 1:36 ` Jeff King
1 sibling, 2 replies; 43+ messages in thread
From: Taylor Blau @ 2022-12-05 23:34 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jeff King, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
> > I have similar feelings to you here. Back when cmake support was
> > introduced, I explicitly wanted it to be something for people who cared
> > about it, but that wouldn't bother people who didn't use it:
> >
> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
> >
> > I stand by that sentiment, but it seems to have crept up as a required
> > thing to deal with, and that is mostly because of CI. Using cmake in CI
> > is good for telling developers when a change they make has broken cmake.
> > But it also makes cmake their problem, and not the folks interested in
> > cmake.
>
> That's a bit of a pain, but I don't think the main problem is its
> integration with CI. It's that there doesn't really seem to be an
> interest in its active maintenance & review from its supposed main
> target audience.
>
> Case in point this "ab/cmake-nix-and-ci" topic: It's been queued for
> around 2 months now.
I think CI *is* the problem here. The CMake bits are basically a black
box to me (and I suspect a large number of other contributors, too). But
when it breaks, the only reason we as a project end up noticing it is
because it has fallout in CI.
I would not be sad to make CI failures that are derived from CMake
"soft" failures in the sense that they don't make the build red. But I
think it's masking over a couple of bigger issues:
- Why do we "support" two build systems in CI if one is supposed to
only be here for those that care about it? IOW, even if we say that
CMake support is nominally an opt-in thing, in reality it isn't
because of the dependency via CI.
- Why do we only *notice* these failures in CI? I found during my time
as interim-maintainer the task of tracking down CI failures to quite
frustrating. It is often quite difficult to reproduce CI failures
locally (especially with exotic build and test configurations[^1]).
It would be nice to be able to more easily see these failures locally
before they hit CI. E.g., is it possible that I would work on a feature
which somehow breaks the CMake build, and fail to notice it if I use
"make" locally?
Personally, I would not be sad to see CMake removed from the tree
entirely because it has not seen enough maintenance and seems to be
quite a headache.
Thanks,
Taylor
[^1]: Not to mention non-Linux failures, though I think that is sort of
par for the course if you're not using one of those platforms
yourself.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-05 23:34 ` Taylor Blau
@ 2022-12-05 23:46 ` Ævar Arnfjörð Bjarmason
2022-12-06 0:35 ` Taylor Blau
2022-12-06 1:36 ` Jeff King
1 sibling, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-05 23:46 UTC (permalink / raw)
To: Taylor Blau
Cc: Jeff King, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Mon, Dec 05 2022, Taylor Blau wrote:
> On Sat, Dec 03, 2022 at 02:41:04AM +0100, Ævar Arnfjörð Bjarmason wrote:
>> > I have similar feelings to you here. Back when cmake support was
>> > introduced, I explicitly wanted it to be something for people who cared
>> > about it, but that wouldn't bother people who didn't use it:
>> >
>> > https://lore.kernel.org/git/20200427200852.GC1728884@coredump.intra.peff.net/
>> >
>> > I stand by that sentiment, but it seems to have crept up as a required
>> > thing to deal with, and that is mostly because of CI. Using cmake in CI
>> > is good for telling developers when a change they make has broken cmake.
>> > But it also makes cmake their problem, and not the folks interested in
>> > cmake.
>>
>> That's a bit of a pain, but I don't think the main problem is its
>> integration with CI. It's that there doesn't really seem to be an
>> interest in its active maintenance & review from its supposed main
>> target audience.
>>
>> Case in point this "ab/cmake-nix-and-ci" topic: It's been queued for
>> around 2 months now.
>
> I think CI *is* the problem here. The CMake bits are basically a black
> box to me (and I suspect a large number of other contributors, too). But
> when it breaks, the only reason we as a project end up noticing it is
> because it has fallout in CI.
>
> I would not be sad to make CI failures that are derived from CMake
> "soft" failures in the sense that they don't make the build red. But I
> think it's masking over a couple of bigger issues:
>
> - Why do we "support" two build systems in CI if one is supposed to
> only be here for those that care about it? IOW, even if we say that
> CMake support is nominally an opt-in thing, in reality it isn't
> because of the dependency via CI.
I'm just trying to point out that that wouldn't be such a big deal if
the cmake parts were being actively maintained.
> - Why do we only *notice* these failures in CI? I found during my time
> as interim-maintainer the task of tracking down CI failures to quite
> frustrating. It is often quite difficult to reproduce CI failures
> locally (especially with exotic build and test configurations[^1]).
>
> It would be nice to be able to more easily see these failures locally
> before they hit CI. E.g., is it possible that I would work on a feature
> which somehow breaks the CMake build, and fail to notice it if I use
> "make" locally?
Yes, some things will just work. E.g. it regex-parses out the Makefile
to pick up the list of built-ins, so when we add a new one we'd usually
not need to patch the cmake bits. Although that regex parsing is its own
problem for some Makefile changes.
To be fair the cases where we've needed to keep it in lockstep since it
was added aren't that many. If you skim this and look for commits that
alter both (or cmake quickly after the Makefile) you can spot them:
git log --oneline --no-merges --full-diff --stat 1c966423263..origin/master -- contrib/buildsystems/CMakeLists.txt -- Makefile
> Personally, I would not be sad to see CMake removed from the tree
> entirely because it has not seen enough maintenance and seems to be
> quite a headache.
>
> Thanks,
> Taylor
>
> [^1]: Not to mention non-Linux failures, though I think that is sort of
> par for the course if you're not using one of those platforms
> yourself.
I wouldn't mind either to see it gone, but when that was last discussed
some people chimed in to say that it really made things easier on
Windows, and I'm inclined to believe them.
So I'm not trying to take their toys away, just changing it so that if
you run into these cmake issues it'll be trivial to debug them, as
you'll be able to test & run it outside of Windows.
Now, I'm about to send out a v6 of it, which should address the last
remaining Windows/VS issues.
I'm not testing directly there, as I don't have such an installation, so
it's possible I'm wrong. Maybe it works in CI, but still breaks for some
reason when driven from VS.
But I think if we can't get anyone who's running Windows+VS to be
interested enough in this to test it the better thing is to just take
this series.
Maybe it breaks some subtle aspect of the VS integration, but then
that'll be easier to fix on top than recovering from a "git rm" of it.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-05 23:46 ` Ævar Arnfjörð Bjarmason
@ 2022-12-06 0:35 ` Taylor Blau
0 siblings, 0 replies; 43+ messages in thread
From: Taylor Blau @ 2022-12-06 0:35 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jeff King, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Tue, Dec 06, 2022 at 12:46:50AM +0100, Ævar Arnfjörð Bjarmason wrote:
> I'm just trying to point out that that wouldn't be such a big deal if
> the cmake parts were being actively maintained.
>
> > - Why do we only *notice* these failures in CI? I found during my time
> > as interim-maintainer the task of tracking down CI failures to quite
> > frustrating. It is often quite difficult to reproduce CI failures
> > locally (especially with exotic build and test configurations[^1]).
> >
> > It would be nice to be able to more easily see these failures locally
> > before they hit CI. E.g., is it possible that I would work on a feature
> > which somehow breaks the CMake build, and fail to notice it if I use
> > "make" locally?
>
> Yes, some things will just work. E.g. it regex-parses out the Makefile
> to pick up the list of built-ins, so when we add a new one we'd usually
> not need to patch the cmake bits. Although that regex parsing is its own
> problem for some Makefile changes.
>
> To be fair the cases where we've needed to keep it in lockstep since it
> was added aren't that many. If you skim this and look for commits that
> alter both (or cmake quickly after the Makefile) you can spot them:
>
> git log --oneline --no-merges --full-diff --stat 1c966423263..origin/master -- contrib/buildsystems/CMakeLists.txt -- Makefile
I think that this is exactly the problem I'm trying to chime in about.
We know that regular contributors can propose sensible changes to the
Makefile.
But integrating CMake into our tree in such a way that forces all Git
developers to *also* be familiar with CMake is too large of a change to
hoist onto the Git community, I think.
> > Personally, I would not be sad to see CMake removed from the tree
> > entirely because it has not seen enough maintenance and seems to be
> > quite a headache.
> >
> > Thanks,
> > Taylor
> >
> > [^1]: Not to mention non-Linux failures, though I think that is sort of
> > par for the course if you're not using one of those platforms
> > yourself.
>
> I wouldn't mind either to see it gone, but when that was last discussed
> some people chimed in to say that it really made things easier on
> Windows, and I'm inclined to believe them.
>
> So I'm not trying to take their toys away, just changing it so that if
> you run into these cmake issues it'll be trivial to debug them, as
> you'll be able to test & run it outside of Windows.
OK. Personally, I wouldn't be sad to see it gone, either. I wonder: is
there a way to keep it in our tree such that CMake breakage isn't
everybody's problem (like Peff originally suggested when we were
discussing this in the first place)?
If so, I would vastly prefer that to the state that we have now. If not,
I think moving it out of the tree is a sensible step, and one that I
would advocate for.
Thanks,
Taylor
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-05 23:34 ` Taylor Blau
2022-12-05 23:46 ` Ævar Arnfjörð Bjarmason
@ 2022-12-06 1:36 ` Jeff King
2022-12-06 1:43 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 43+ messages in thread
From: Jeff King @ 2022-12-06 1:36 UTC (permalink / raw)
To: Taylor Blau
Cc: Ævar Arnfjörð Bjarmason, phillip.wood,
Junio C Hamano, git, Phillip Wood, Johannes Schindelin
On Mon, Dec 05, 2022 at 06:34:09PM -0500, Taylor Blau wrote:
> I think CI *is* the problem here. The CMake bits are basically a black
> box to me (and I suspect a large number of other contributors, too). But
> when it breaks, the only reason we as a project end up noticing it is
> because it has fallout in CI.
>
> I would not be sad to make CI failures that are derived from CMake
> "soft" failures in the sense that they don't make the build red. But I
> think it's masking over a couple of bigger issues:
>
> - Why do we "support" two build systems in CI if one is supposed to
> only be here for those that care about it? IOW, even if we say that
> CMake support is nominally an opt-in thing, in reality it isn't
> because of the dependency via CI.
I think part of the reason cmake rose in importance via CI is that it's
the de facto way to build for vscode. Before that CI job switched to
cmake, there was some other alternate build system (vcxproj).
So two things I'd consider here:
- how important is it for us to do the vscode build as part of regular
CI (as opposed to folks who are interested in it running it
themselves). Dscho gave some real data in the thread I linked to
earlier (which indicates that yes, it helps, but not that often).
- what's the status of cmake versus vcxproj? My impression (though I
admit based on my half-paying-attention-to of the topic) is that
cmake should replace vcxproj, and nobody would ever want to work on
vcxproj anymore. But if that's not right, then does vcxproj cause
headaches for non-Windows devs less often? I don't really remember
dealing with it much, but I may have just been lucky.
> - Why do we only *notice* these failures in CI? I found during my time
> as interim-maintainer the task of tracking down CI failures to quite
> frustrating. It is often quite difficult to reproduce CI failures
> locally (especially with exotic build and test configurations[^1]).
I think that's just the normal platform issues. You don't use it on your
system, so you don't notice until it runs in CI. If people don't run CI
themselves, then it falls to the maintainer's CI run, which is a pain
for them. But that's as true of other operating systems, exotic test
flags, etc, as it is of cmake.
> It would be nice to be able to more easily see these failures locally
> before they hit CI. E.g., is it possible that I would work on a feature
> which somehow breaks the CMake build, and fail to notice it if I use
> "make" locally?
That seems like going in the opposite direction from what you're saying
above: doubling down that if cmake is broken by a change, it is the
responsibility of the dev who made the change to find and fix it.
I do like that Ævar is trying to make it easier to run cmake from Linux
in order to find that without using CI. But that does seem orthogonal to
me to the notion of "who is responsible for finding and fixing cmake
problems". To me, that decision is really rooted in "is cmake something
the Git project supports, or is it a side-thing that some folks
volunteer to keep working?".
> Personally, I would not be sad to see CMake removed from the tree
> entirely because it has not seen enough maintenance and seems to be
> quite a headache.
I don't mind having it in-tree if I can ignore it (assuming the project
attitude is the "it's a side thing" from above). It's the CI failures
that make it hard to ignore.
-Peff
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 1:36 ` Jeff King
@ 2022-12-06 1:43 ` Ævar Arnfjörð Bjarmason
2022-12-06 2:05 ` Jeff King
0 siblings, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-06 1:43 UTC (permalink / raw)
To: Jeff King
Cc: Taylor Blau, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Mon, Dec 05 2022, Jeff King wrote:
> On Mon, Dec 05, 2022 at 06:34:09PM -0500, Taylor Blau wrote:
>
>> I think CI *is* the problem here. The CMake bits are basically a black
>> box to me (and I suspect a large number of other contributors, too). But
>> when it breaks, the only reason we as a project end up noticing it is
>> because it has fallout in CI.
>>
>> I would not be sad to make CI failures that are derived from CMake
>> "soft" failures in the sense that they don't make the build red. But I
>> think it's masking over a couple of bigger issues:
>>
>> - Why do we "support" two build systems in CI if one is supposed to
>> only be here for those that care about it? IOW, even if we say that
>> CMake support is nominally an opt-in thing, in reality it isn't
>> because of the dependency via CI.
>
> I think part of the reason cmake rose in importance via CI is that it's
> the de facto way to build for vscode. Before that CI job switched to
> cmake, there was some other alternate build system (vcxproj).
>
> So two things I'd consider here:
>
> - how important is it for us to do the vscode build as part of regular
> CI (as opposed to folks who are interested in it running it
> themselves). Dscho gave some real data in the thread I linked to
> earlier (which indicates that yes, it helps, but not that often).
>
> - what's the status of cmake versus vcxproj? My impression (though I
> admit based on my half-paying-attention-to of the topic) is that
> cmake should replace vcxproj, and nobody would ever want to work on
I think the intent was to deprecate vcxproj, but I'm not sure, and I
wonder if the "cmake" is the proposed path forward why we still have it
in-tree anymore.
> vcxproj anymore. But if that's not right, then does vcxproj cause
> headaches for non-Windows devs less often? I don't really remember
> dealing with it much, but I may have just been lucky.
It was less painful for non-Windows folks, but I understand the cmake
integration was also much nicer for VS. I.e. it's picked up by the IDE
in a way that the "make" shim wasn't.
> [...]
> That seems like going in the opposite direction from what you're saying
> above: doubling down that if cmake is broken by a change, it is the
> responsibility of the dev who made the change to find and fix it.
>
> I do like that Ævar is trying to make it easier to run cmake from Linux
> in order to find that without using CI. But that does seem orthogonal to
> me to the notion of "who is responsible for finding and fixing cmake
> problems". To me, that decision is really rooted in "is cmake something
> the Git project supports, or is it a side-thing that some folks
> volunteer to keep working?".
I agree with that...
>> Personally, I would not be sad to see CMake removed from the tree
>> entirely because it has not seen enough maintenance and seems to be
>> quite a headache.
>
> I don't mind having it in-tree if I can ignore it (assuming the project
> attitude is the "it's a side thing" from above). It's the CI failures
> that make it hard to ignore.
...but on this thread-at-large, I'd much rather see us focus on just
reviewing the patches I have here than raising the burden of proof to
whether we should get rid of it entirely.
If we make the CI failures "soft" failures or move it out-of-tree
entirely it would still be useful to be able to run the cmake recipe on
*nix.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 1:43 ` Ævar Arnfjörð Bjarmason
@ 2022-12-06 2:05 ` Jeff King
2022-12-06 2:19 ` Ævar Arnfjörð Bjarmason
2022-12-07 1:00 ` Taylor Blau
0 siblings, 2 replies; 43+ messages in thread
From: Jeff King @ 2022-12-06 2:05 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Taylor Blau, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:
> > I don't mind having it in-tree if I can ignore it (assuming the project
> > attitude is the "it's a side thing" from above). It's the CI failures
> > that make it hard to ignore.
>
> ...but on this thread-at-large, I'd much rather see us focus on just
> reviewing the patches I have here than raising the burden of proof to
> whether we should get rid of it entirely.
Fair. In case it is not obvious, I have no interest in reviewing cmake
patches. ;) But I will at least stop making noise in the thread.
> If we make the CI failures "soft" failures or move it out-of-tree
> entirely it would still be useful to be able to run the cmake recipe on
> *nix.
Agreed.
-Peff
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 2:05 ` Jeff King
@ 2022-12-06 2:19 ` Ævar Arnfjörð Bjarmason
2022-12-06 3:52 ` Junio C Hamano
2022-12-07 1:00 ` Taylor Blau
1 sibling, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-06 2:19 UTC (permalink / raw)
To: Jeff King
Cc: Taylor Blau, phillip.wood, Junio C Hamano, git, Phillip Wood,
Johannes Schindelin
On Mon, Dec 05 2022, Jeff King wrote:
> On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
>> > I don't mind having it in-tree if I can ignore it (assuming the project
>> > attitude is the "it's a side thing" from above). It's the CI failures
>> > that make it hard to ignore.
>>
>> ...but on this thread-at-large, I'd much rather see us focus on just
>> reviewing the patches I have here than raising the burden of proof to
>> whether we should get rid of it entirely.
>
> Fair. In case it is not obvious, I have no interest in reviewing cmake
> patches. ;) But I will at least stop making noise in the thread.
I'm fine with the running commentary on the future direction, I think
it's also very useful.
I just wanted to also note the need to keep the eyes on the ball a bit
:)
>> If we make the CI failures "soft" failures or move it out-of-tree
>> entirely it would still be useful to be able to run the cmake recipe on
>> *nix.
>
> Agreed.
Just to add my own digression: I asked in some past thread (which I'm
too lazy to dig up) why it was the cmake file couldn't just dispatch to
"make" for most things.
I.e. it needs to at some level be aware of what it's building for the
IDE integration, but for say making a "grep.o" there's no reason it
couldn't be running:
make grep.o
Instead of:
cc <args> -o grep grep.c [...]
which requires duplicating much of the Makefile logic (possibly with
some Makefile shim to not consider any dependencies in that case).
Even if we couldn't do that for *.c code for some reason it could do it
e.g. creating the generated *.h files, which is logic we currentnly
duplicate.
The "win+VS build" job even has a hard dependency on GNU make currently,
in needing to run "make artifacts-tar" to get to the "win+VS test"
stage.
But apparently the reason for *that* is that another goal of the
integration was to avoid having to have GNU make installed at all, which
comes in a different package than the one that would ship VS+cmake (or
something?).
Which might be something to re-visit, i.e. maybe we could eventually say
"yes, you can have VS+cmake, but it's not too much to ask that you
install GNU make as a one-off".
Doing that would then reduce the duplication to the point where the
cmake recipe would be a thin shim around the Makefile.
I don't use this development setup, but if the CI job is managing to
download and run GNU make it can't be that hard for an end-user to
similarly install it (but what do I know?).
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 2:19 ` Ævar Arnfjörð Bjarmason
@ 2022-12-06 3:52 ` Junio C Hamano
2022-12-06 9:54 ` Phillip Wood
0 siblings, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2022-12-06 3:52 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: Jeff King, Taylor Blau, phillip.wood, git, Phillip Wood,
Johannes Schindelin
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Just to add my own digression: I asked in some past thread (which I'm
> too lazy to dig up) why it was the cmake file couldn't just dispatch to
> "make" for most things.
>
> I.e. it needs to at some level be aware of what it's building for the
> IDE integration, but for say making a "grep.o" there's no reason it
> couldn't be running:
>
> make grep.o
>
> Instead of:
>
> cc <args> -o grep grep.c [...]
>
> which requires duplicating much of the Makefile logic (possibly with
> some Makefile shim to not consider any dependencies in that case).
That leads to a question at the other extreme. Why does any logic
in CMakeLists.txt even have to exist at all? Whenever it is asked
to make foo, it can be running "make foo" instead of having its own
logic at all. ;-)
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 3:52 ` Junio C Hamano
@ 2022-12-06 9:54 ` Phillip Wood
2022-12-06 10:57 ` Ævar Arnfjörð Bjarmason
0 siblings, 1 reply; 43+ messages in thread
From: Phillip Wood @ 2022-12-06 9:54 UTC (permalink / raw)
To: Junio C Hamano, Ævar Arnfjörð Bjarmason
Cc: Jeff King, Taylor Blau, git, Phillip Wood, Johannes Schindelin
On 06/12/2022 03:52, Junio C Hamano wrote:
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> Just to add my own digression: I asked in some past thread (which I'm
>> too lazy to dig up) why it was the cmake file couldn't just dispatch to
>> "make" for most things.
Because make is not installed by default on Windows. Our CI job uses
msbuild (whatever that is) and when I was playing with Visual Studio
last week it was using ninja.
>> I.e. it needs to at some level be aware of what it's building for the
>> IDE integration, but for say making a "grep.o" there's no reason it
>> couldn't be running:
>>
>> make grep.o
>>
>> Instead of:
>>
>> cc <args> -o grep grep.c [...]
>>
>> which requires duplicating much of the Makefile logic (possibly with
>> some Makefile shim to not consider any dependencies in that case).
>
> That leads to a question at the other extreme. Why does any logic
> in CMakeLists.txt even have to exist at all? Whenever it is asked
> to make foo, it can be running "make foo" instead of having its own
> logic at all. ;-)
Yes, if make was available then we wouldn't need to use CMake.
Best Wishes
Phillip
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 9:54 ` Phillip Wood
@ 2022-12-06 10:57 ` Ævar Arnfjörð Bjarmason
2022-12-08 9:29 ` ab/cmake-nix-and-ci, was " Johannes Schindelin
0 siblings, 1 reply; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-06 10:57 UTC (permalink / raw)
To: phillip.wood
Cc: Junio C Hamano, Jeff King, Taylor Blau, git, Phillip Wood,
Johannes Schindelin
On Tue, Dec 06 2022, Phillip Wood wrote:
> On 06/12/2022 03:52, Junio C Hamano wrote:
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>
>>> Just to add my own digression: I asked in some past thread (which I'm
>>> too lazy to dig up) why it was the cmake file couldn't just dispatch to
>>> "make" for most things.
>
> Because make is not installed by default on Windows. Our CI job uses
> msbuild (whatever that is) and when I was playing with Visual Studio
> last week it was using ninja.
>
>>> I.e. it needs to at some level be aware of what it's building for the
>>> IDE integration, but for say making a "grep.o" there's no reason it
>>> couldn't be running:
>>>
>>> make grep.o
>>>
>>> Instead of:
>>>
>>> cc <args> -o grep grep.c [...]
>>>
>>> which requires duplicating much of the Makefile logic (possibly with
>>> some Makefile shim to not consider any dependencies in that case).
>> That leads to a question at the other extreme. Why does any logic
>> in CMakeLists.txt even have to exist at all? Whenever it is asked
>> to make foo, it can be running "make foo" instead of having its own
>> logic at all. ;-)
>
> Yes, if make was available then we wouldn't need to use CMake.
I think Junio and I are talking about something slightly different. Yes
"make" isn't available by default. Getting it requires installing a
larger SDK.
But if you look at the history of contrib/vscode/README.md in our tree
you'll see that we used to support this "Visual Studio Solution" for
years via GNU make, it probably still works.
The change in 4c2c38e800f (ci: modification of main.yml to use cmake for
vs-build job, 2020-06-26) shows when the CI was switched over to using
cmake instead.
The code to support that is still in-tree as the "vcxproj" target in
"config.mak.uname", which calls out to the ~1k lines of Perl code in
contrib/buildsystems/Generators/*.
I'm not suggesting we go back to that. The question is whether the
trade-off of supporting an entirely separate build system without a GNU
make dependency was worth it.
On the one hand those developing on Windows don't need to install it as
a package, on the other hand we end up spending more developer time in
writing duplicate build logic.
The advantage of cmake is that it knows how to generate all that
VS-integration XML etc., so as soon as it knows how to build Git you get
that for free.
But I think you can get that while not having a "real" cmake recipe, but
just a thin shim for calling out to the Makefile.
Is such a thing a hack? Yes. Is it silly to e.g. build with "ninja" and
really have it just shell out to "make"? Yes.
But it might still be worth it if we judge that the goal of getting that
VS integration is sufficient. And that we're not willing to absorb the
cost of maintaining two distinct build recipes in perpetuity.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 2:05 ` Jeff King
2022-12-06 2:19 ` Ævar Arnfjörð Bjarmason
@ 2022-12-07 1:00 ` Taylor Blau
1 sibling, 0 replies; 43+ messages in thread
From: Taylor Blau @ 2022-12-07 1:00 UTC (permalink / raw)
To: Jeff King
Cc: Ævar Arnfjörð Bjarmason, phillip.wood,
Junio C Hamano, git, Phillip Wood, Johannes Schindelin
On Mon, Dec 05, 2022 at 09:05:03PM -0500, Jeff King wrote:
> On Tue, Dec 06, 2022 at 02:43:17AM +0100, Ævar Arnfjörð Bjarmason wrote:
>
> > > I don't mind having it in-tree if I can ignore it (assuming the project
> > > attitude is the "it's a side thing" from above). It's the CI failures
> > > that make it hard to ignore.
> >
> > ...but on this thread-at-large, I'd much rather see us focus on just
> > reviewing the patches I have here than raising the burden of proof to
> > whether we should get rid of it entirely.
>
> Fair. In case it is not obvious, I have no interest in reviewing cmake
> patches. ;) But I will at least stop making noise in the thread.
Same, and same.
Sorry for semi-derailing this thread. I don't feel strongly enough about
the CMake stuff to start a new thread about it on the list, but while
others are discussing it, I figured that I might as well chime in.
Thanks
Taylor
^ permalink raw reply [flat|nested] 43+ messages in thread
* ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-06 10:57 ` Ævar Arnfjörð Bjarmason
@ 2022-12-08 9:29 ` Johannes Schindelin
2022-12-08 11:34 ` Ævar Arnfjörð Bjarmason
2022-12-09 3:48 ` Junio C Hamano
0 siblings, 2 replies; 43+ messages in thread
From: Johannes Schindelin @ 2022-12-08 9:29 UTC (permalink / raw)
To: Ævar Arnfjörð Bjarmason
Cc: phillip.wood, Junio C Hamano, Jeff King, Taylor Blau, git,
Phillip Wood
[-- Attachment #1: Type: text/plain, Size: 4196 bytes --]
Hi,
On Tue, 6 Dec 2022, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Dec 06 2022, Phillip Wood wrote:
>
> > On 06/12/2022 03:52, Junio C Hamano wrote:
> >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> >>
> >>> Just to add my own digression: I asked in some past thread (which
> >>> I'm too lazy to dig up) why it was the cmake file couldn't just
> >>> dispatch to "make" for most things.
> >
> > Because make is not installed by default on Windows. Our CI job uses
> > msbuild (whatever that is) and when I was playing with Visual Studio
> > last week it was using ninja.
> >
> >>> I.e. it needs to at some level be aware of what it's building for the
> >>> IDE integration, but for say making a "grep.o" there's no reason it
> >>> couldn't be running:
> >>>
> >>> make grep.o
> >>>
> >>> Instead of:
> >>>
> >>> cc <args> -o grep grep.c [...]
> >>>
> >>> which requires duplicating much of the Makefile logic (possibly with
> >>> some Makefile shim to not consider any dependencies in that case).
> >> That leads to a question at the other extreme. Why does any logic
> >> in CMakeLists.txt even have to exist at all? Whenever it is asked
> >> to make foo, it can be running "make foo" instead of having its own
> >> logic at all. ;-)
> >
> > Yes, if make was available then we wouldn't need to use CMake.
>
> I think Junio and I are talking about something slightly different. Yes
> "make" isn't available by default. Getting it requires installing a
> larger SDK.
>
> But if you look at the history of contrib/vscode/README.md in our tree
> you'll see that we used to support this "Visual Studio Solution" for
> years via GNU make, it probably still works.
It probably doesn't. Last time I had to use it, during the embargoed
v2.37.1 release process, it didn't. I had to add plenty of patches to make
it work again:
https://github.com/git-for-windows/git/compare/323a69709944%5E...323a69709944%5E2
> The change in 4c2c38e800f (ci: modification of main.yml to use cmake for
> vs-build job, 2020-06-26) shows when the CI was switched over to using
> cmake instead.
>
> The code to support that is still in-tree as the "vcxproj" target in
> "config.mak.uname", which calls out to the ~1k lines of Perl code in
> contrib/buildsystems/Generators/*.
At some stage we can probably get rid of the `vcxproj` code. Before that,
we can even get rid of the `vcproj` code that is bit-rotting in
`contrib/buildsystems/`. But there seems no harm, and less maintenance
burden, in keeping the `vcxproj`/`vcproj` parts where they are, as they
are.
Taking a step back, I see that we got far away from the topic that started
this thread.
So here's my take on `ab/cmake-nix-and-ci`: While that patch series'
intention is apparently to make it easier to diagnose and fix CI problems,
I only see that it adds new problems. It won't make it possible to
diagnose most win+VS problems because they don't reproduce on Linux. But
the patches already did introduce Windows-specific problems merely by
trying to get the Linux side of CMake to work. And trying to keep CMake
working both on Linux and on Windows would cause many more problems in the
future. And we do not even need CMake support for Linux, `make` works well
there already. It would increase the maintenance burden unnecessarily.
I am therefore suggesting to drop `cmake-nix-and-ci` entirely.
To address the concern about broken CI runs, I hoped that monitoring them
and helping contributors by suggesting fixups was working well enough. It
used to be okay for patches to be contributed that caused CI to fail, we
simply worked together to fix CI, as a team, and that was that. We didn't
"blame" the contributors, or anything, when CI runs failed because of
their patches. After all, possible causes of CI failures include that
patches might be applied on top of other commits than intended, or that
patch series interact in unfortunate ways.
Junio, maybe you could clarify your take on this? As project lead, it is
your decision to define how Git uses Continuous Builds, and how the
project handles failed CI runs.
Ciao,
Johannes
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-08 9:29 ` ab/cmake-nix-and-ci, was " Johannes Schindelin
@ 2022-12-08 11:34 ` Ævar Arnfjörð Bjarmason
2022-12-09 3:48 ` Junio C Hamano
1 sibling, 0 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-08 11:34 UTC (permalink / raw)
To: Johannes Schindelin
Cc: phillip.wood, Junio C Hamano, Jeff King, Taylor Blau, git,
Phillip Wood, Han-Wen Nienhuys, Yuyi Wang
On Thu, Dec 08 2022, Johannes Schindelin wrote:
> Hi,
>
> On Tue, 6 Dec 2022, Ævar Arnfjörð Bjarmason wrote:
>
>> On Tue, Dec 06 2022, Phillip Wood wrote:
>>
>> > On 06/12/2022 03:52, Junio C Hamano wrote:
>> >> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>> >>
>> >>> Just to add my own digression: I asked in some past thread (which
>> >>> I'm too lazy to dig up) why it was the cmake file couldn't just
>> >>> dispatch to "make" for most things.
>> >
>> > Because make is not installed by default on Windows. Our CI job uses
>> > msbuild (whatever that is) and when I was playing with Visual Studio
>> > last week it was using ninja.
>> >
>> >>> I.e. it needs to at some level be aware of what it's building for the
>> >>> IDE integration, but for say making a "grep.o" there's no reason it
>> >>> couldn't be running:
>> >>>
>> >>> make grep.o
>> >>>
>> >>> Instead of:
>> >>>
>> >>> cc <args> -o grep grep.c [...]
>> >>>
>> >>> which requires duplicating much of the Makefile logic (possibly with
>> >>> some Makefile shim to not consider any dependencies in that case).
>> >> That leads to a question at the other extreme. Why does any logic
>> >> in CMakeLists.txt even have to exist at all? Whenever it is asked
>> >> to make foo, it can be running "make foo" instead of having its own
>> >> logic at all. ;-)
>> >
>> > Yes, if make was available then we wouldn't need to use CMake.
>>
>> I think Junio and I are talking about something slightly different. Yes
>> "make" isn't available by default. Getting it requires installing a
>> larger SDK.
>>
>> But if you look at the history of contrib/vscode/README.md in our tree
>> you'll see that we used to support this "Visual Studio Solution" for
>> years via GNU make, it probably still works.
>
> It probably doesn't. Last time I had to use it, during the embargoed
> v2.37.1 release process, it didn't. I had to add plenty of patches to make
> it work again:
> https://github.com/git-for-windows/git/compare/323a69709944%5E...323a69709944%5E2
>
>> The change in 4c2c38e800f (ci: modification of main.yml to use cmake for
>> vs-build job, 2020-06-26) shows when the CI was switched over to using
>> cmake instead.
>>
>> The code to support that is still in-tree as the "vcxproj" target in
>> "config.mak.uname", which calls out to the ~1k lines of Perl code in
>> contrib/buildsystems/Generators/*.
>
> At some stage we can probably get rid of the `vcxproj` code. Before that,
> we can even get rid of the `vcproj` code that is bit-rotting in
> `contrib/buildsystems/`. But there seems no harm, and less maintenance
> burden, in keeping the `vcxproj`/`vcproj` parts where they are, as they
> are.
>
> Taking a step back, I see that we got far away from the topic that started
> this thread.
>
> So here's my take on `ab/cmake-nix-and-ci`: While that patch series'
> intention is apparently to make it easier to diagnose and fix CI problems,
> I only see that it adds new problems. It won't make it possible to
> diagnose most win+VS problems because they don't reproduce on Linux.
That would also be my take if that was the goal of the series. I agree
that would be pretty pointless. Why test win+VS-specific code on Linux?
That makes no sense.
But that's not the goal.
It's to make it easier to test the majority of the platform-agnostic
code in the cmake recipe. E.g. here's some past commits from myself,
Jeff King and Han-Wen (who I'm pretty sure doesn't use Windows) where
we've had to patch the cmake recipe in addition to the Makefile:
ef8a6c62687 (reftable: utility functions, 2021-10-07)
cfe853e66be (hook-list.h: add a generated list of hooks, like config-list.h, 2021-09-26)
d7a5649c82d (make git-bugreport a builtin, 2020-08-13)
b5dd96b70ac (make credential helpers builtins, 2020-08-13)
Doing that currently requires bouncing things off the Windows CI. With
ab/cmake-nix-and-ci you can not only build (which currently works) but
run the full test against the cmake build on *nix in minutes. That's a
big improvement.
I think this also misrepresents the nature of the cmake recipe, and how
much of it is truly MSVC or VS-specific. I count less than 20 lines in
a ~1.1k line recipe that are really "MSVC"-specific. I.e. guarded by
"if" branches checking "MSVC" and 'CMAKE_C_COMPILER_ID STREQUAL "MSVC"'.
The rest are general in nature. E.g. you can run the tests with "ctest"
from the VS GUI.
That's not because there's a VS-specific "hey Visual Studio, here's our
tests" part of the recipe. Rather there's a generic cross-platform
method of declaring how to run the tests, which cmake itself then knows
to pick up and generate a VS-specific asset with.
Whereas you seem to be suggesting that the recipe is so
Windows+VS-specific that testing it on other platforms isn't going to
tell you much. I don't think that's true.
> But the patches already did introduce Windows-specific problems merely
> by trying to get the Linux side of CMake to work.
This seems like vague commentary on past bugs.
Do you have any specific concerns about things that are broken by the
current v6 iteration?
> And trying to keep CMake
> working both on Linux and on Windows would cause many more problems in the
> future. And we do not even need CMake support for Linux, `make` works well
> there already. It would increase the maintenance burden unnecessarily.
If you're going to argue that "we do not even need CMake support for
Linux" you're not making an argument against ab/cmake-nix-and-ci, but
against the status quo on "master".
It's already the case that it mostly "works" on Linux, that's been the
case since day 1. E.g. Yuyi's a561962479c (cmake: fix CMakeLists.txt on
Linux, 2022-05-24) earlier this year.
> I am therefore suggesting to drop `cmake-nix-and-ci` entirely.
I wouldn't mind if we declare that it should never work on Linux, and I
wouldn't mind if we drop ab/cmake-nix-and-ci entirely.
But only *if* the cmake recipe becomes purely a "lag-behind" alternative
build system that the Windows folks are responsible for updating, or to
submit follow-up patches for if it breaks.
That's not the status quo now, as e.g. Jeff King summarized nicely here:
https://lore.kernel.org/git/Y4qF3iHW2s+I0yNe@coredump.intra.peff.net/
You don't seem to be suggesting a productive way forward with that.
I don't think dropping it, or even making the Windows cmake CI optional
would be a productive way forward, I think it would ultimately waste
more of your time, and that of other Windows developers.
But I don't see how that isn't the logical conclusion to not providing
specific feedback on this topic & finding a way forward with it.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-08 9:29 ` ab/cmake-nix-and-ci, was " Johannes Schindelin
2022-12-08 11:34 ` Ævar Arnfjörð Bjarmason
@ 2022-12-09 3:48 ` Junio C Hamano
2022-12-09 13:55 ` Ævar Arnfjörð Bjarmason
1 sibling, 1 reply; 43+ messages in thread
From: Junio C Hamano @ 2022-12-09 3:48 UTC (permalink / raw)
To: Johannes Schindelin
Cc: Ævar Arnfjörð Bjarmason, phillip.wood, Jeff King,
Taylor Blau, git, Phillip Wood
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Junio, maybe you could clarify your take on this? As project lead, it is
> your decision to define how Git uses Continuous Builds, and how the
> project handles failed CI runs.
I have pretty much been with what Peff and Taylor said in the thread
already ever since we added CMake support to help Windows/VS folks.
I agree with you that we do not need to run it for Linux or macOS,
and if the promised/hoped-for benefit, i.e. that running them on
non-Windows build would uncover issues that are common across the
platform and help Windows, is not something that is likely to
materialize, I'd prefer to see our resources (CI time and developer
attention) not spent on that.
I do not think "how the project handles filed CI runs" is a very big
issue. I often ignore partial failures (e.g. "winVS(n) test job
triggered rate limit") and the only annoyance I feel is that such a
temporary failure contribute one more message to my trash mailbox,
and I can learn to do the same for a test that marked as failed due
to linux-cmake-ctest job. I expect that regular contributors are
doing the same pretty much.
How blocking is a CI failure for drive-by contributors who use GGG?
While I do not necessarily value drive-by contributions as much as
you do, if such "an unimportant failure we can ignore" discourages
those coming from GGG route, that would be unfortunate, exactly
because they may not have contributed anything to the failures.
This is not just cmake-ctest, but the leak checking job where a new
use of a tool that is known to be leaky in a test can turn a test
that has been passing to fail. If we can mark failures in selected
jobs as non-blocking, we definitely should do so.
Between keeping and marking linux-cmake-ctest as non-blocking, and
removing it altogether, I am inclined to say that I'd favor the
latter for the reasons I explained earlier in this message. But to
help casual contributors coming via GGG, we would anyway need to (1)
allow submitting even with failing tests, and (2) tell them that it
is OK to do so. Which means it is not the end of the world, from
the point of view of helping casual developers, if we had kept these
brittle CI jobs like linux-cmake-ctest and linux-leaks.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: ab/cmake-nix-and-ci, was Re: [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out"
2022-12-09 3:48 ` Junio C Hamano
@ 2022-12-09 13:55 ` Ævar Arnfjörð Bjarmason
0 siblings, 0 replies; 43+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2022-12-09 13:55 UTC (permalink / raw)
To: Junio C Hamano
Cc: Johannes Schindelin, phillip.wood, Jeff King, Taylor Blau, git,
Phillip Wood
On Fri, Dec 09 2022, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>> Junio, maybe you could clarify your take on this? As project lead, it is
>> your decision to define how Git uses Continuous Builds, and how the
>> project handles failed CI runs.
>
> I have pretty much been with what Peff and Taylor said in the thread
> already ever since we added CMake support to help Windows/VS folks.
> I agree with you that we do not need to run it for Linux or macOS,
> and if the promised/hoped-for benefit, i.e. that running them on
> non-Windows build would uncover issues that are common across the
> platform and help Windows, is not something that is likely to
> materialize, I'd prefer to see our resources (CI time and developer
> attention) not spent on that.
I think this should be addressed by the "I count less than 20 lines in a
~1.1k line recipe that are really "MSVC"-specific" in the sibling
mail[1]. I.e. the large majority of it is generic recipe code that's run
on all platforms.
> I do not think "how the project handles filed CI runs" is a very big
> issue. I often ignore partial failures (e.g. "winVS(n) test job
> triggered rate limit") and the only annoyance I feel is that such a
> temporary failure contribute one more message to my trash mailbox,
> and I can learn to do the same for a test that marked as failed due
> to linux-cmake-ctest job. I expect that regular contributors are
> doing the same pretty much.
>
> How blocking is a CI failure for drive-by contributors who use GGG?
> While I do not necessarily value drive-by contributions as much as
> you do, if such "an unimportant failure we can ignore" discourages
> those coming from GGG route, that would be unfortunate, exactly
> because they may not have contributed anything to the failures.
> This is not just cmake-ctest, but the leak checking job where a new
> use of a tool that is known to be leaky in a test can turn a test
> that has been passing to fail. If we can mark failures in selected
> jobs as non-blocking, we definitely should do so.
I realize that we've been digressing to the larger topic of what to do
with CI in general, but I don't think the question of whether say
"win+VS build" should "soft fail" is something we should conflate with
this "ab/cmake-nix-and-ci" topic.
It doesn't change the status quo there, and I think is a net
improvement.
Even if we make the CI for anything cmake-related soft-fail, this topic
will still help to get it back up to speed, as you'll be able to run the
full cmake+ctest chain on non-Windows.
> Between keeping and marking linux-cmake-ctest as non-blocking, and
> removing it altogether, I am inclined to say that I'd favor the
> latter for the reasons I explained earlier in this message. But to
> help casual contributors coming via GGG, we would anyway need to (1)
> allow submitting even with failing tests, and (2) tell them that it
> is OK to do so. Which means it is not the end of the world, from
> the point of view of helping casual developers, if we had kept these
> brittle CI jobs like linux-cmake-ctest and linux-leaks.
I can peel off the commit that adds the "linux-cmake-ctest" CI job from
this series, or even just make it do the equivalent of:
cmake && make || echo oops, we're broken
So it'll "soft-fail" (AFAIK GitHub CI, unlike GitLab[2] doesn't support
a native way to "soft-fail").
But I don't think that doing that would help without *also* making
"win+VS {build,test}" soft-fail. I.e. if "linux-cmake-ctest" *and*
"win+VS" (with all else passing) you can be pretty sure it's a generic
cmake problem.
If only one or the other is failing somewhere in cmake having the
"linux-cmake-ctest" job now will help narrow down whether it's a
platform-specific cmake issue.
So, just let me know what you'd prefer, but I think per the above even
if you're impatient with cmake failures the "linux-cmake-ctest" job
should help spend less time on them.
1. https://lore.kernel.org/git/221208.86wn726qcv.gmgdl@evledraar.gmail.com/
2. https://docs.gitlab.com/ee/ci/yaml/#allow_failure -- In the UX:
green=passing, red=failing, yellow=soft-fail
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2022-12-09 14:09 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29 9:40 What's cooking in git.git (Nov 2022, #07; Tue, 29) Junio C Hamano
2022-11-29 18:59 ` ab/remove--super-prefix and -rc0 (was What's cooking in git.git (Nov 2022, #07; Tue, 29)) Glen Choo
2022-11-30 3:43 ` Junio C Hamano
2022-11-30 18:14 ` Glen Choo
2022-11-30 19:43 ` Ævar Arnfjörð Bjarmason
2022-12-01 5:20 ` Junio C Hamano
2022-12-01 17:44 ` Glen Choo
2022-12-01 21:26 ` Junio C Hamano
2022-11-29 19:08 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Glen Choo
2022-11-30 3:45 ` Junio C Hamano
2022-11-30 18:08 ` Glen Choo
2022-11-29 21:16 ` ds/bundle-uri-4 (was Re: What's cooking in git.git (Nov 2022, #07; Tue, 29)) Derrick Stolee
2022-12-01 15:06 ` Derrick Stolee
2022-12-02 0:25 ` Junio C Hamano
2022-11-30 9:57 ` ab/cmake-nix-and-ci " Phillip Wood
2022-11-30 10:16 ` Ævar Arnfjörð Bjarmason
2022-12-01 14:23 ` Phillip Wood
2022-12-01 16:39 ` [PATCH] test-lib.sh: discover "git" in subdirs of "contrib/buildsystems/out" Ævar Arnfjörð Bjarmason
2022-12-01 16:48 ` Phillip Wood
2022-12-01 17:13 ` Ævar Arnfjörð Bjarmason
2022-12-01 23:00 ` Junio C Hamano
2022-12-02 15:14 ` Phillip Wood
2022-12-02 16:40 ` Ævar Arnfjörð Bjarmason
2022-12-02 23:10 ` Jeff King
2022-12-03 1:12 ` Junio C Hamano
2022-12-03 1:41 ` Ævar Arnfjörð Bjarmason
2022-12-05 9:15 ` Jeff King
2022-12-05 23:34 ` Taylor Blau
2022-12-05 23:46 ` Ævar Arnfjörð Bjarmason
2022-12-06 0:35 ` Taylor Blau
2022-12-06 1:36 ` Jeff King
2022-12-06 1:43 ` Ævar Arnfjörð Bjarmason
2022-12-06 2:05 ` Jeff King
2022-12-06 2:19 ` Ævar Arnfjörð Bjarmason
2022-12-06 3:52 ` Junio C Hamano
2022-12-06 9:54 ` Phillip Wood
2022-12-06 10:57 ` Ævar Arnfjörð Bjarmason
2022-12-08 9:29 ` ab/cmake-nix-and-ci, was " Johannes Schindelin
2022-12-08 11:34 ` Ævar Arnfjörð Bjarmason
2022-12-09 3:48 ` Junio C Hamano
2022-12-09 13:55 ` Ævar Arnfjörð Bjarmason
2022-12-07 1:00 ` Taylor Blau
2022-11-30 10:02 ` What's cooking in git.git (Nov 2022, #07; Tue, 29) Phillip Wood
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).