swh:1:snp:6df5a50b8107b6bbe1e51d0239d816a7503c536a

sort by:
Revision Author Date Message Commit Date
4fd6c5e Merge branch 'ac/bitmap-lookup-table' Grammofix. * ac/bitmap-lookup-table: pack-bitmap: improve grammar of "xor chain" error message 23 September 2022, 18:07:49 UTC
0d14f80 Merge branch 'ma/scalar-to-main-fix' Fix manpage generation. * ma/scalar-to-main-fix: cmd-list.perl: fix identifying man sections 23 September 2022, 18:07:48 UTC
32c6fff cmd-list.perl: fix identifying man sections We attribute each documentation text file to a man section by finding a line in the file that looks like "gitfoo(<digit>)". Commit cc75e556a9 ("scalar: add to 'git help -a' command list", 2022-09-02) updated this logic to look not only for "gitfoo" but also "scalarfoo". In doing so, it forgot to account for the fact that after the updated regex has found a match, the man section is no longer to be found in `$1` but now lives in `$2`. This makes our git(1) manpage look as follows: Main porcelain commands git-add(git) Add file contents to the index. [...] gitk(git) The Git repository browser. scalar(scalar) A tool for managing large Git repositories. Restore the man sections by not capturing the (git|scalar) part of the match into `$1`. As noted by Ævar [1], we could even match any "foo" rather than just "gitfoo" and "scalarfoo", but that's a larger change. For now, just fix the regression in cc75e556a9. [1] https://lore.kernel.org/git/220923.86wn9u4joo.gmgdl@evledraar.gmail.com/#t Helped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Martin Ågren <martin.agren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 September 2022, 17:01:07 UTC
711340c pack-bitmap: improve grammar of "xor chain" error message Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 23 September 2022, 15:54:05 UTC
4b79ee4 Merge branch 'jk/list-objects-filter-cleanup' Fix uninitialized memory access in a recent fix-up that is already in -rc1. * jk/list-objects-filter-cleanup: list-objects-filter: initialize sub-filter structs 22 September 2022, 22:30:47 UTC
4eaed7c list-objects-filter: initialize sub-filter structs Since commit c54980ab83 (list-objects-filter: convert filter_spec to a strbuf, 2022-09-11), building with SANITIZE=undefined triggers an error in t5616. The problem is that we end up with a strbuf that has been zero-initialized instead of via STRBUF_INIT. Feeding that strbuf to strbuf_addbuf() in list_objects_filter_copy() means we will call memcpy like: memcpy(some_actual_buffer, NULL, 0); This works on most systems because we're copying zero bytes, but it is technically undefined behavior to ever pass NULL to memcpy. Even though c54980ab83 is where the bug manifests, that is only because we switched away from a string_list, which is OK with being zero-initialized (though it may cause other problems by not duplicating the strings, it happened to be OK in this instance). The actual bug is caused by the commit before that, 2a01bdedf8 (list-objects-filter: add and use initializers, 2022-09-11). There we consistently initialize the top-level filter structs, but we forgot the dynamically allocated ones we stick in filter_options->sub when creating combined filters. Note that we need to fix two spots here: where we parse a "combine:" filter, but also where we transform from a single-filter into a combined one after seeing multiple "--filter" options. In the second spot, we'll do some minor refactoring to avoid repeating our very-long array index. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 22 September 2022, 19:43:04 UTC
1b3d6e1 Git 2.38-rc1 Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 22:27:03 UTC
04cc66f Merge branch 'sg/parse-options-subcommand' Fix messages incorrectly marked for translation. * sg/parse-options-subcommand: gc: don't translate literal commands 21 September 2022, 22:27:03 UTC
4140830 Merge branch 'js/typofix' * js/typofix: Documentation: clean up various typos in technical docs Documentation: clean up a few misspelled word typos 21 September 2022, 22:27:02 UTC
17df9d3 Merge branch 'sg/clean-test-results' "make clean" stopped cleaning the test results directory as a side effect of a topic that has nothing to do with "make clean", which has been corrected. * sg/clean-test-results: t/Makefile: remove 'test-results' on 'make clean' 21 September 2022, 22:27:02 UTC
2cf2ae9 Merge branch 'vd/check-docs-fixes' Build fix. * vd/check-docs-fixes: version: fix builtin linking & documentation diagnose: add to command-list.txt 21 September 2022, 22:27:02 UTC
ac45db1 Merge branch 'vd/doc-reviewing-guidelines' Just like we have coding guidelines, we now have guidelines for reviewers. * vd/doc-reviewing-guidelines: Documentation: add ReviewingGuidelines 21 September 2022, 22:27:02 UTC
86c108a Merge branch 'vd/scalar-generalize-diagnose' Portability fix. * vd/scalar-generalize-diagnose: builtin/diagnose.c: don't translate the two mode values diagnose.c: refactor to safely use 'd_type' 21 September 2022, 22:27:01 UTC
370d3a0 Final batch before -rc1 Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 21:23:14 UTC
dd37e56 Merge branch 'fz/help-doublofix' Typofix for topic already in -rc0. * fz/help-doublofix: help: fix doubled words in explanation for developer interfaces 21 September 2022, 21:23:14 UTC
8c88f75 Merge branch 'tz/tech-docs-to-help-fix' Docfix for topic already in -rc0. * tz/tech-docs-to-help-fix: api docs: link to html version of api-trace2 docs: fix a few recently broken links 21 September 2022, 21:23:14 UTC
3239100 Merge branch 'ml/commit-graph-expire-dir-leak-fix' A result from opendir() was leaking in the commit-graph expiration codepath, which has been plugged. * ml/commit-graph-expire-dir-leak-fix: commit-graph: Fix missing closedir in expire_commit_graphs 21 September 2022, 21:23:14 UTC
f73ad8f Merge branch 'ec/reftable-pass-pq-entry-by-reference' Small code clean-up in reftable implementation. * ec/reftable-pass-pq-entry-by-reference: reftable: use a pointer for pq_entry param 21 September 2022, 21:23:13 UTC
d956fa8 builtin/diagnose.c: don't translate the two mode values These strings are not translatable in the diagnose_options array in diagnose.c. Don't translate them in builtin/diagnose.c either. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 18:53:35 UTC
d11b875 t/Makefile: remove 'test-results' on 'make clean' The 't/test-results' directory and its contents are by-products of the test process, so 'make clean' should remove them, but, alas, this has been broken since fee65b194d (t/Makefile: don't remove test-results in "clean-except-prove-cache", 2022-07-28). The 'clean' target in 't/Makefile' was not directly responsible for removing the 'test-results' directory, but relied on its dependency 'clean-except-prove-cache' to do that [1]. ee65b194d broke this, because it only removed the 'rm -r test-results' command from the 'clean-except-prove-cache' target instead of moving it to the 'clean' target, resulting in stray 't/test-results' directories. Add that missing cleanup command to 't/Makefile', and to all sub-Makefiles touched by that commit as well. [1] 60f26f6348 (t/Makefile: retain cache t/.prove across prove runs, 2012-05-02) Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 18:32:13 UTC
8b74492 gc: don't translate literal commands The command you type is still "git maintenance" even in other languages. Signed-off-by: Alex Henrie <alexhenrie24@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 17:43:10 UTC
bbb0c35 Documentation: clean up various typos in technical docs Used GNU "aspell check <filename>" to review various technical documentation files with the default aspell dictionary. Ignored false-positives between american and british english. Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 17:28:36 UTC
72991ff Documentation: clean up a few misspelled word typos Used GNU "aspell check <filename>" to review various documentation files with the default aspell dictionary. Ignored false-positives between american and british english. Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Reviewed-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 21 September 2022, 17:28:35 UTC
9b1dc1c version: fix builtin linking & documentation Like most builtins, 'version' is documented in a corresponding 'Documentation/git-version.txt' and can be invoked with 'git version'. However, the 'check-docs' Makefile target showed that it was "removed but documented: git-version." This was cause by the fact that it is not built as a standalone 'git-version' executable, therefore appearing "removed" to 'check-docs'. Without a precedent for documented builtins that aren't built into an executable *or* any clear reason why a standalone 'git-version' shouldn't exist, the 'check-docs' error appears to correctly identify an issue. To correct that mismatch, add 'git-version' to the 'BUILT_INS' list in the root Makefile (indicating that the 'cmd_version()' function appears in a file that is *not* 'builtin/version.c'). Additionally, to avoid the "no link" message in 'check-docs', list 'git-version' as an "ancilliaryinterrogator" (like 'git help') in 'command-list.txt'. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 20 September 2022, 00:28:25 UTC
89c8048 diagnose: add to command-list.txt Add 'git diagnose' as an "ancilliaryinterrogator" (like 'git bugreport') to 'command-list.txt' in order to have it show up in 'git help -a' and avoid the "no link" warning message from the 'check-docs' Makefile target. Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 20 September 2022, 00:27:26 UTC
e01b851 Documentation: add ReviewingGuidelines Add a reviewing guidelines document including advice and common terminology used in Git mailing list reviews. The document is included in the 'TECH_DOCS' list in order to include it in Git's published documentation. Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Helped-by: Derrick Stolee <derrickstolee@github.com> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Josh Steadmon <steadmon@google.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 19 September 2022, 21:36:08 UTC
dda7228 A bit more of remaining topics before -rc1 Signed-off-by: Junio C Hamano <gitster@pobox.com> 19 September 2022, 21:35:25 UTC
279ebd4 Merge branch 'ad/t1800-cygwin' Test fix. * ad/t1800-cygwin: t1800: correct test to handle Cygwin 19 September 2022, 21:35:25 UTC
42bf77c Merge branch 'vd/scalar-to-main' Hoist the remainder of "scalar" out of contrib/ to the main part of the codebase. * vd/scalar-to-main: Documentation/technical: include Scalar technical doc t/perf: add 'GIT_PERF_USE_SCALAR' run option t/perf: add Scalar performance tests scalar-clone: add test coverage scalar: add to 'git help -a' command list scalar: implement the `help` subcommand git help: special-case `scalar` scalar: include in standard Git build & installation scalar: fix command documentation section header 19 September 2022, 21:35:25 UTC
9d58241 Merge branch 'es/chainlint' Revamp chainlint script for our tests. * es/chainlint: chainlint: colorize problem annotations and test delimiters t: retire unused chainlint.sed t/Makefile: teach `make test` and `make prove` to run chainlint.pl test-lib: replace chainlint.sed with chainlint.pl test-lib: retire "lint harder" optimization hack t/chainlint: add more chainlint.pl self-tests chainlint.pl: allow `|| echo` to signal failure upstream of a pipe chainlint.pl: complain about loops lacking explicit failure handling chainlint.pl: don't flag broken &&-chain if failure indicated explicitly chainlint.pl: don't flag broken &&-chain if `$?` handled explicitly chainlint.pl: don't require `&` background command to end with `&&` t/Makefile: apply chainlint.pl to existing self-tests chainlint.pl: don't require `return|exit|continue` to end with `&&` chainlint.pl: validate test scripts in parallel chainlint.pl: add parser to identify test definitions chainlint.pl: add parser to validate tests chainlint.pl: add POSIX shell parser chainlint.pl: add POSIX shell lexical analyzer t: add skeleton chainlint.pl 19 September 2022, 21:35:24 UTC
298a958 Merge branch 'jk/list-objects-filter-cleanup' A couple of bugfixes with code clean-up. * jk/list-objects-filter-cleanup: list-objects-filter: convert filter_spec to a strbuf list-objects-filter: add and use initializers list-objects-filter: handle null default filter spec list-objects-filter: don't memset after releasing filter struct 19 September 2022, 21:35:24 UTC
f876b5a Merge branch 'zh/ls-files-format' Typofix in the UI of a topic that has graduated to 'master'. * zh/ls-files-format: ls-files: fix black space in error message 19 September 2022, 21:35:24 UTC
339517b Merge branch 'sy/mv-out-of-cone' "git mv A B" in a sparsely populated working tree can be asked to move a path from a directory that is "in cone" to another directory that is "out of cone". Handling of such a case has been improved. * sy/mv-out-of-cone: builtin/mv.c: fix possible segfault in add_slash() mv: check overwrite for in-to-out move advice.h: add advise_on_moving_dirty_path() mv: cleanup empty WORKING_DIRECTORY mv: from in-cone to out-of-cone mv: remove BOTH from enum update_mode mv: check if <destination> is a SKIP_WORKTREE_DIR mv: free the with_slash in check_dir_in_index() mv: rename check_dir_in_index() to empty_dir_has_sparse_contents() t7002: add tests for moving from in-cone to out-of-cone 19 September 2022, 21:35:23 UTC
12f1ae5 commit-graph: Fix missing closedir in expire_commit_graphs The function calls opendir() but missing the corresponding closedir() before exit the function. Add missing closedir() to fix it. Signed-off-by: Miaoqian Lin <linmq006@gmail.com> Reviewed-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 19 September 2022, 17:42:38 UTC
cb98e1d diagnose.c: refactor to safely use 'd_type' Refactor usage of the 'd_type' property of 'struct dirent' in 'diagnose.c' to instead utilize the compatibility macro 'DTYPE()'. On systems where 'd_type' is not present in 'struct dirent', this macro will always return 'DT_UNKNOWN'. In that case, instead fall back on using the 'stat.st_mode' to determine whether the dirent points to a dir, file, or link. Additionally, add a test to 't0092-diagnose.sh' to verify that files (e.g., loose objects) are counted properly. Note that the new function 'get_dtype()' is based on 'resolve_dtype()' in 'dir.c' (which itself was refactored from a prior 'get_dtype()' in ad6f2157f9 (dir: restructure in a way to avoid passing around a struct dirent, 2020-01-16)), but differs in that it is meant for use on arbitrary files, such as those inside the '.git' dir. Because of this, it does not search the index for a matching entry to derive the 'd_type'. Reported-by: Randall S. Becker <rsbecker@nexbridge.com> Signed-off-by: Victoria Dye <vdye@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 19 September 2022, 17:25:01 UTC
225e815 help: fix doubled words in explanation for developer interfaces Signed-off-by: Fangyi Zhou <me@fangyi.io> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 September 2022, 16:20:11 UTC
4945f04 api docs: link to html version of api-trace2 In f6d25d7878 (api docs: document that BUG() emits a trace2 error event, 2021-04-13), a link to the plain text version of api-trace2 was added in `technical/api-error-handling.txt`. All of our other `link:`s point to the html versions. Do the same here. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 September 2022, 15:39:11 UTC
086eaab docs: fix a few recently broken links Some links were broken in the recent move of various technical docs c0f6dd49f1 (Merge branch 'ab/tech-docs-to-help', 2022-08-14). Fix them. Signed-off-by: Todd Zullinger <tmz@pobox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 16 September 2022, 15:38:03 UTC
d3fa443 Git 2.38-rc0 Signed-off-by: Junio C Hamano <gitster@pobox.com> 15 September 2022, 23:09:47 UTC
ca20a44 Merge branch 'jk/proto-v2-ref-prefix-fix' "git fetch" over protocol v2 sent an incorrect ref prefix request to the server and made "git pull" with configured fetch refspec that does not cover the remote branch to merge with fail, which has been corrected. * jk/proto-v2-ref-prefix-fix: fetch: add branch.*.merge to default ref-prefix extension fetch: stop checking for NULL transport->remote in do_fetch() 15 September 2022, 23:09:47 UTC
b7f39a3 Merge branch 'rs/add-p-worktree-mode-prompt-fix' Fix another UI regression in the reimplemented "add -p". * rs/add-p-worktree-mode-prompt-fix: add -p: fix worktree patch mode prompts 15 September 2022, 23:09:46 UTC
5ff02db Merge branch 'js/typofix' Typofix. * js/typofix: Documentation: fix various repeat word typos 15 September 2022, 23:09:46 UTC
d878d83 Merge branch 'en/remerge-diff-fixes' Fix a few "git log --remerge-diff" bugs. * en/remerge-diff-fixes: diff: fix filtering of merge commits under --remerge-diff diff: fix filtering of additional headers under --remerge-diff diff: have submodule_format logic avoid additional diff headers 15 September 2022, 23:09:46 UTC
c18eecb reftable: use a pointer for pq_entry param The speed of the merged_iter_pqueue_add() can be improved by using a pointer to the pq_entry struct, which is 96 bytes. Since the pq_entry param is worked directly on the stack and does not currently have a pointer to it, the merged_iter_pqueue_add() function is slightly slower. References to pq_entry in reftable have typically included pointers, such as both of the params for pq_less(). Since we are working with pointers in the pq_entry param, as keenly pointed out, the pq_entry param has also been made into a const since the contents of the pq_entry param are copied and not manipulated. Signed-off-by: Elijah Conners <business@elijahpepe.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 15 September 2022, 18:32:37 UTC
255a6f9 t1800: correct test to handle Cygwin On Cygwin, when failing to spawn a process using start_command, Git outputs the same error as on Linux systems, rather than using the GIT_WINDOWS_NATIVE-specific error output. The WINDOWS test prerequisite is set in both Cygwin and native Windows environments, which means it's not appropriate to use to anticipate the error output from start_command. Instead, use the MINGW test prerequisite, which is only set for Git in native Windows environments, and not for Cygwin. Signed-off-by: Adam Dinwoodie <adam@dinwoodie.org> Helped-by: Đoàn Trần Công Danh <congdanhqx@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 15 September 2022, 17:29:51 UTC
36f8e7e Prepare for 2.38-rc0 Signed-off-by: Junio C Hamano <gitster@pobox.com> 14 September 2022, 19:56:41 UTC
08d61c7 Merge branch 'jk/plug-list-object-filter-leaks' The code that manages list-object-filter structure, used in partial clones, leaked the instances, which has been plugged. * jk/plug-list-object-filter-leaks: prepare_repo_settings(): plug leak of config values list_objects_filter_options: plug leak of filter_spec strings transport: free filter options in disconnect_git() transport: deep-copy object-filter struct for fetch-pack list_objects_filter_copy(): deep-copy sparse_oid_name field 14 September 2022, 19:56:40 UTC
b563638 Merge branch 'ab/submodule-helper-leakfix' Plugging leaks in submodule--helper. * ab/submodule-helper-leakfix: submodule--helper: fix a configure_added_submodule() leak submodule--helper: free rest of "displaypath" in "struct update_data" submodule--helper: free some "displaypath" in "struct update_data" submodule--helper: fix a memory leak in print_status() submodule--helper: fix a leak in module_add() submodule--helper: fix obscure leak in module_add() submodule--helper: fix "reference" leak submodule--helper: fix a memory leak in get_default_remote_submodule() submodule--helper: fix a leak with repo_clear() submodule--helper: fix "sm_path" and other "module_cb_list" leaks submodule--helper: fix "errmsg_str" memory leak submodule--helper: add and use *_release() functions submodule--helper: don't leak {run,capture}_command() cp.dir argument submodule--helper: "struct pathspec" memory leak in module_update() submodule--helper: fix most "struct pathspec" memory leaks submodule--helper: fix trivial get_default_remote_submodule() leak submodule--helper: fix a leak in "clone_submodule" 14 September 2022, 19:56:40 UTC
7a54d74 Merge branch 'ab/dedup-config-and-command-docs' Share the text used to explain configuration variables used by "git <subcmd>" in "git help <subcmd>" with the text from "git help config". * ab/dedup-config-and-command-docs: docs: add CONFIGURATION sections that fuzzy map to built-ins docs: add CONFIGURATION sections that map to a built-in log docs: de-duplicate configuration sections difftool docs: de-duplicate configuration sections notes docs: de-duplicate and combine configuration sections apply docs: de-duplicate configuration sections send-email docs: de-duplicate configuration sections grep docs: de-duplicate configuration sections docs: add and use include template for config/* includes 14 September 2022, 19:56:40 UTC
dd407f1 Merge branch 'ab/unused-annotation' Undoes 'jk/unused-annotation' topic and redoes it to work around Coccinelle rules misfiring false positives in unrelated codepaths. * ab/unused-annotation: git-compat-util.h: use "deprecated" for UNUSED variables git-compat-util.h: use "UNUSED", not "UNUSED(var)" 14 September 2022, 19:56:39 UTC
a6b42ec Merge branch 'jk/unused-annotation' Annotate function parameters that are not used (but cannot be removed for structural reasons), to prepare us to later compile with -Wunused warning turned on. * jk/unused-annotation: is_path_owned_by_current_uid(): mark "report" parameter as unused run-command: mark unused async callback parameters mark unused read_tree_recursive() callback parameters hashmap: mark unused callback parameters config: mark unused callback parameters streaming: mark unused virtual method parameters transport: mark bundle transport_options as unused refs: mark unused virtual method parameters refs: mark unused reflog callback parameters refs: mark unused each_ref_fn parameters git-compat-util: add UNUSED macro 14 September 2022, 19:56:39 UTC
f6f0ee2 add -p: fix worktree patch mode prompts cee6cb7300 (built-in add -p: implement the "worktree" patch modes, 2019-12-21) added the worktree patch modes to the built-in add -p. Its commit message claims to be a port of 2f0896ec3ad4 (restore: support --patch, 2019-04-25), which did the same for the script git-add--interactive.perl. The script mentioned only the worktree in its prompt messages in worktree mode, while the built-in mentions the worktree and also the index, even though the command doesn't actually affect the index. 2c8bd8471a (checkout -p: handle new files correctly, 2020-05-27) added new prompt messages for addition that also mention the index in worktree mode in the built-in, but not in the script. Correct these prompts to state that only the worktree will be affected. Reported-by: David Plumpton <david.plumpton@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 14 September 2022, 18:14:38 UTC
e188ec3 Sync with 'maint' 13 September 2022, 19:23:48 UTC
a0feb86 Merge a handful of topics from the 'master' front As the 'master' front will soon tag a preview and then release candidates for 2.38, it is unknown if we are going to issue another maintenance release on the 2.37.x track, but as we have accumulated enough material there, let's prepare a draft for it. Even if we end up not tagging 2.37.4, it would help motivated distro packagers to maintain their slightly older and "more stable" versions. Signed-off-by: Junio C Hamano <gitster@pobox.com> 13 September 2022, 19:22:59 UTC
2c75b32 Merge branch 'en/merge-unstash-only-on-clean-merge' into maint The auto-stashed local changes created by "git merge --autostash" was mixed into a conflicted state left in the working tree, which has been corrected. * en/merge-unstash-only-on-clean-merge: merge: only apply autostash when appropriate 13 September 2022, 19:21:11 UTC
4f06dfd Merge branch 'ds/github-actions-use-newer-ubuntu' into maint Update the version of Ubuntu used for GitHub Actions CI from 18.04 to 22.04. * ds/github-actions-use-newer-ubuntu: ci: update 'static-analysis' to Ubuntu 22.04 13 September 2022, 19:21:10 UTC
37317ab Merge branch 'ad/preload-plug-memleak' into maint The preload-index codepath made copies of pathspec to give to multiple threads, which were left leaked. * ad/preload-plug-memleak: preload-index: fix memleak 13 September 2022, 19:21:10 UTC
c61614e Merge branch 'sg/xcalloc-cocci-fix' into maint xcalloc(), imitating calloc(), takes "number of elements of the array", and "size of a single element", in this order. A call that does not follow this ordering has been corrected. * sg/xcalloc-cocci-fix: promisor-remote: fix xcalloc() argument order 13 September 2022, 19:21:09 UTC
aa31cb8 Merge branch 'jk/pipe-command-nonblock' into maint Fix deadlocks between main Git process and subprocess spawned via the pipe_command() API, that can kill "git add -p" that was reimplemented in C recently. * jk/pipe-command-nonblock: pipe_command(): mark stdin descriptor as non-blocking pipe_command(): handle ENOSPC when writing to a pipe pipe_command(): avoid xwrite() for writing to pipe git-compat-util: make MAX_IO_SIZE define globally available nonblock: support Windows compat: add function to enable nonblocking pipes 13 September 2022, 19:21:08 UTC
72869e7 Merge branch 'jk/is-promisor-object-keep-tree-in-use' into maint An earlier optimization discarded a tree-object buffer that is still in use, which has been corrected. * jk/is-promisor-object-keep-tree-in-use: is_promisor_object(): fix use-after-free of tree buffer 13 September 2022, 19:21:07 UTC
21dd13e The twentieth batch Signed-off-by: Junio C Hamano <gitster@pobox.com> 13 September 2022, 18:38:25 UTC
de1fee2 Merge branch 'ow/rev-parse-parseopt-fix' The parser in the script interface to parse-options in "git rev-parse" has been updated to diagnose a bogus input correctly. * ow/rev-parse-parseopt-fix: rev-parse --parseopt: detect missing opt-spec 13 September 2022, 18:38:25 UTC
e4ffba4 Merge branch 'js/builtin-add-p-portability-fix' More fixes to "add -p" * js/builtin-add-p-portability-fix: t6132(NO_PERL): do not run the scripted `add -p` t3701: test the built-in `add -i` regardless of NO_PERL add -p: avoid ambiguous signed/unsigned comparison 13 September 2022, 18:38:24 UTC
76ffa81 Merge branch 'sg/parse-options-subcommand' The codepath for the OPT_SUBCOMMAND facility has been cleaned up. * sg/parse-options-subcommand: notes, remote: show unknown subcommands between `' notes: simplify default operation mode arguments check test-parse-options.c: fix style of comparison with zero test-parse-options.c: don't use for loop initial declaration t0040-parse-options: remove leftover debugging 13 September 2022, 18:38:24 UTC
655e494 Merge branch 'jk/rev-list-verify-objects-fix' "git rev-list --verify-objects" ought to inspect the contents of objects and notice corrupted ones, but it didn't when the commit graph is in use, which has been corrected. * jk/rev-list-verify-objects-fix: rev-list: disable commit graph with --verify-objects lookup_commit_in_graph(): use prepare_commit_graph() to check for graph 13 September 2022, 18:38:24 UTC
8b2f027 Merge branch 'jk/upload-pack-skip-hash-check' The server side that responds to "git fetch" and "git clone" request has been optimized by allowing it to send objects in its object store without recomputing and validating the object names. * jk/upload-pack-skip-hash-check: t1060: check partial clone of misnamed blob parse_object(): check commit-graph when skip_hash set upload-pack: skip parse-object re-hashing of "want" objects parse_object(): allow skipping hash check 13 September 2022, 18:38:23 UTC
e0574c4 Merge branch 'rs/diff-no-index-cleanup' "git diff --no-index A B" managed its the pathnames of its two input files rather haphazardly, sometimes leaking them. The command line argument processing has been straightened out to clean it up. * rs/diff-no-index-cleanup: diff-no-index: simplify argv index calculation diff-no-index: release prefixed filenames diff-no-index: release strbuf on queue error 13 September 2022, 18:38:23 UTC
f322e9f Merge branch 'ab/submodule-helper-prep' Code clean-up of "git submodule--helper". * ab/submodule-helper-prep: (33 commits) submodule--helper: fix bad config API usage submodule--helper: libify even more "die" paths for module_update() submodule--helper: libify more "die" paths for module_update() submodule--helper: check repo{_submodule,}_init() return values submodule--helper: libify "must_die_on_failure" code paths (for die) submodule--helper update: don't override 'checkout' exit code submodule--helper: libify "must_die_on_failure" code paths submodule--helper: libify determine_submodule_update_strategy() submodule--helper: don't exit() on failure, return submodule--helper: use "code" in run_update_command() submodule API: don't handle SM_..{UNSPECIFIED,COMMAND} in to_string() submodule--helper: don't call submodule_strategy_to_string() in BUG() submodule--helper: add missing braces to "else" arm submodule--helper: return "ret", not "1" from update_submodule() submodule--helper: rename "int res" to "int ret" submodule--helper: don't redundantly check "else if (res)" submodule--helper: refactor "errmsg_str" to be a "struct strbuf" submodule--helper: add "const" to passed "struct update_data" submodule--helper: add "const" to copy of "update_data" submodule--helper: add "const" to passed "module_clone_data" ... 13 September 2022, 18:38:23 UTC
0479138 Merge branch 'ed/fsmonitor-on-network-disk' The built-in fsmonitor refuses to work on a network mounted repositories; a configuration knob for users to override this has been introduced. * ed/fsmonitor-on-network-disk: fsmonitor: option to allow fsmonitor to run against network-mounted repos 13 September 2022, 18:38:23 UTC
7c04aa7 chainlint: colorize problem annotations and test delimiters When `chainlint.pl` detects problems in a test definition, it emits the test definition with "?!FOO?!" annotations highlighting the problems it discovered. For instance, given this problematic test: test_expect_success 'discombobulate frobnitz' ' git frob babble && (echo balderdash; echo gnabgib) >expect && for i in three two one do git nitfol $i done >actual test_cmp expect actual ' chainlint.pl will output: # chainlint: t1234-confusing.sh # chainlint: discombobulate frobnitz git frob babble && (echo balderdash ; ?!AMP?! echo gnabgib) >expect && for i in three two one do git nitfol $i ?!LOOP?! done >actual ?!AMP?! test_cmp expect actual in which it may be difficult to spot the "?!FOO?!" annotations. The problem is compounded when multiple tests, possibly in multiple scripts, fail "linting", in which case it may be difficult to spot the "# chainlint:" lines which delimit one problematic test from another. To ameliorate this potential problem, colorize the "?!FOO?!" annotations in order to quickly draw the test author's attention to the problem spots, and colorize the "# chainlint:" lines to help the author identify the name of each script and each problematic test. Colorization is disabled automatically if output is not directed to a terminal or if NO_COLOR environment variable is set. The implementation is specific to Unix (it employs `tput` if available) but works equally well in the Git for Windows development environment which emulates Unix sufficiently. Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 13 September 2022, 04:33:58 UTC
c9dba10 Documentation: fix various repeat word typos Inspired by 24966cd982 ("doc: fix repeated words", 08-09-2019), I ran "egrep -R "\<([a-zA-Z]+)\> \<\1\>" ./Documentation/*" to find current cases of repeated words such as "the the" that were quite clearly typos. There were many false positives reported, such as "really really" or valid uses of "that that" which I left alone. Signed-off-by: Jacob Stopak <jacob@initialcommit.io> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 18:04:55 UTC
746aae3 ls-files: fix black space in error message ce74de9(ls-files: introduce "--format" option) miss a space between two words incorrectly, it leads to wrong i10n messages. So fix it by adding a space at the end of the error message. Signed-off-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 16:25:40 UTC
c54980a list-objects-filter: convert filter_spec to a strbuf Originally, the filter_spec field was just a string pointer. In cf9ceb5a12 (list-objects-filter-options: make filter_spec a string_list, 2019-06-27) it became a string_list, but that commit notes: A strbuf would seem to be a more natural choice for this object, but it unfortunately requires initialization besides just zero'ing out the memory. This results in all container structs, and all containers of those structs, etc., to also require initialization. Initializing them all would be more cumbersome that simply using a string_list, which behaves properly when its contents are zero'd. Now that we've changed the struct to require non-zero initialization anyway (ironically, because string_list also needed non-zero initialization to avoid leaks), we can now convert to that more natural type. This makes the list_objects_filter_spec() function much less awkward, as it had to collapse the string_list to a single-entry list on the fly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 15:38:59 UTC
2a01bde list-objects-filter: add and use initializers In 7e2619d8ff (list_objects_filter_options: plug leak of filter_spec strings, 2022-09-08), we noted that the filter_spec string_list was inconsistent in how it handled memory ownership of strings stored in the list. The fix there was a bit of a band-aid to set the "strdup_strings" variable right before adding anything. That works OK, and it lets the users of the API continue to zero-initialize the struct. But it makes the code a bit hard to follow and accident-prone, as any other spots appending the filter_spec need to think about whether to set the strdup_strings value, too (there's one such spot in partial_clone_get_default_filter_spec(), which is probably a possible memory leak). So let's do that full cleanup now. We'll introduce a LIST_OBJECTS_FILTER_INIT macro and matching function, and use them as appropriate (though it is for the "_options" struct, this matches the corresponding list_objects_filter_release() function). This is harder than it seems! Many other structs, like git_transport_data, embed the filter struct. So they need to initialize it themselves even if the rest of the enclosing struct is OK with zero-initialization. I found all of the relevant spots by grepping manually for declarations of list_objects_filter_options. And then doing so recursively for structs which embed it, and ones which embed those, and so on. I'm pretty sure I got everything, but there's no change that would alert the compiler if any topics in flight added new declarations. To catch this case, we now double-check in the parsing function that things were initialized as expected and BUG() if appropriate. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 15:38:59 UTC
aff4bfc list-objects-filter: handle null default filter spec When we have a remote.*.promisor config variable, we know that we're in a partial clone. Usually there's a matching remote.*.partialclonefilter option, which tells us which filter to use with the remote. If that option is missing, we skip setting up the filter at all. But something funny happens: we stick a NULL entry into the string_list storing the text filter spec. This is a weird state, and could possibly segfault if anybody called called list_objects_filter_spec(), etc. In practice, nobody does, because filter->choice will still be LOFC_DISABLED, so code generally realizes there's no filter to use. And the string_list itself is OK, because it starts in non-dup mode until we actually parse a filter spec. So it blindly stores the NULL without even looking at it. But it's probably worth avoiding this confused state. It's an accident waiting to happen, and it will be a problem if we replace the lazy initialization from 7e2619d8ff (list_objects_filter_options: plug leak of filter_spec strings, 2022-09-08) with a real initialization function. The history is a little interesting here, as the bug was introduced during the merge resolution in 627b826834 (Merge branch 'md/list-objects-filter-combo', 2019-09-18). The original logic comes from cac1137dc4 (list-objects: check if filter is NULL before using, 2018-06-11), where we had a single string via core.partialCloneFilter, and a simple NULL check was sufficient. And it even added a test in t0410 that covers this situation. Later, that was expanded to allow per-remote filters in fa3d1b63e8 (promisor-remote: parse remote.*.partialclonefilter, 2019-06-25). After that commit, we get a promisor struct with a partial_clone_filter string, which could be NULL. The commit checks only that the struct pointer is non-NULL, which is enough. It may pass NULL to gently_parse_list_objects_filter(), but that function is smart enough to consider it a noop. But in parallel, cf9ceb5a12 (list-objects-filter-options: make filter_spec a string_list, 2019-06-27) added a new line of code: before we call gently_parse_list_objets_filter(), we append the filter spec to the string_list. By itself that was OK, since we'd have returned early if the string was NULL. When the two were merged in 627b826834, the result is that we return early only if the struct is NULL, but not the string. And we append to the string_list, meaning we may append NULL. The solution is to return early if either is NULL, as it would mean we don't have a configured filter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 15:38:59 UTC
e40d906 list-objects-filter: don't memset after releasing filter struct If we see an error while parsing a "combine" filter, we call list_objects_filter_release() to free any allocated memory, and then use memset() to return the struct to a known state. But the release function already does that reinitializing. Doing it again is pointless. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 12 September 2022, 15:38:58 UTC
7522bb9 Merge branch 'jk/plug-list-object-filter-leaks' into jk/list-objects-filter-cleanup * jk/plug-list-object-filter-leaks: prepare_repo_settings(): plug leak of config values list_objects_filter_options: plug leak of filter_spec strings transport: free filter options in disconnect_git() transport: deep-copy object-filter struct for fetch-pack list_objects_filter_copy(): deep-copy sparse_oid_name field 12 September 2022, 15:38:47 UTC
7ead468 builtin/mv.c: fix possible segfault in add_slash() A possible segfault was introduced in c08830de41 (mv: check if <destination> is a SKIP_WORKTREE_DIR, 2022-08-09). When running t7001 with SANITIZE=address, problem appears when running: git mv path1/path2/ . or git mv directory ../ or any <destination> that makes dest_path[0] an empty string. The add_slash() call could segfault when path argument to it is an empty string, because it makes an out-of-bounds read to decide if an extra slash '/' needs to be appended to it. As add_slash() is used to make sure that a valid pathname to a file in the given directory can be made by appending a filename after the value returned from it, if path is an empty string, we want to return it as-is. The path to a file "F" in the top-level of the working tree (i.e. path=="") is formed by appending "F" after "" (i.e. path) without any slash in between. So, just like the case where a non-empty path already ends with a slash, return an empty path as-is. Reported-by: Jeff King <peff@peff.net> Helped-by: Jeff King <peff@peff.net> Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 09 September 2022, 22:49:53 UTC
dd3f6c4 The nineteenth batch Signed-off-by: Junio C Hamano <gitster@pobox.com> 09 September 2022, 19:02:26 UTC
fe3939b Merge branch 'vd/sparse-reset-checkout-fixes' Segfault fix-up to an earlier fix to the topic to teach "git reset" and "git checkout" work better in a sparse checkout. * vd/sparse-reset-checkout-fixes: unpack-trees: fix sparse directory recursion check 09 September 2022, 19:02:26 UTC
fd1ec82 Merge branch 'ab/retire-ppc-sha1' Remove the assembly version of SHA-1 implementation for PPC. * ab/retire-ppc-sha1: Makefile: use $(OBJECTS) instead of $(C_OBJ) Makefile + hash.h: remove PPC_SHA1 implementation 09 September 2022, 19:02:25 UTC
00b0199 Merge branch 'cc/doc-trailer-whitespace-rules' Doc update. * cc/doc-trailer-whitespace-rules: Documentation: clarify whitespace rules for trailers 09 September 2022, 19:02:25 UTC
0e2a476 Merge branch 'jc/format-patch-force-in-body-from' "git format-patch --from=<ident>" can be told to add an in-body "From:" line even for commits that are authored by the given <ident> with "--force-in-body-from"option. * jc/format-patch-force-in-body-from: format-patch: learn format.forceInBodyFrom configuration variable format-patch: allow forcing the use of in-body From: header pretty: separate out the logic to decide the use of in-body from 09 September 2022, 19:02:25 UTC
428dce9 Merge branch 'js/range-diff-with-pathspec' Allow passing a pathspec to "git range-diff". * js/range-diff-with-pathspec: range-diff: optionally accept pathspecs range-diff: consistently validate the arguments range-diff: reorder argument handling 09 September 2022, 19:02:25 UTC
526c490 Merge branch 'jk/tempfile-active-flag-cleanup' Code clean-up. * jk/tempfile-active-flag-cleanup: tempfile: update comment describing state transitions tempfile: drop active flag 09 September 2022, 19:02:24 UTC
fb094cb Merge branch 'js/add-p-diff-parsing-fix' Those who use diff-so-fancy as the diff-filter noticed a regression or two in the code that parses the diff output in the built-in version of "add -p", which has been corrected. * js/add-p-diff-parsing-fix: add -p: ignore dirty submodules add -p: gracefully handle unparseable hunk headers in colored diffs add -p: detect more mismatches between plain vs colored diffs 09 September 2022, 19:02:24 UTC
f20b9c3 rev-parse --parseopt: detect missing opt-spec After 2d893dff4c (rev-parse --parseopt: allow [*=?!] in argument hints, 2015-07-14) updated the parser, a line in parseopts's input can start with one of the flag characters and be erroneously parsed as a opt-spec where the short name of the option is the flag character itself and the long name is after the end of the string. This makes Git want to allocate SIZE_MAX bytes of memory at this line: o->long_name = xmemdupz(sb.buf + 2, s - sb.buf - 2); Since s and sb.buf are equal the second argument is -2 (except unsigned) and xmemdupz allocates len + 1 bytes, ie. -1 meaning SIZE_MAX. Avoid this by checking whether a flag character was found in the zeroth position. Reported-by: Ingy dot Net <ingy@ingy.net> Signed-off-by: Øystein Walle <oystwa@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 21:55:07 UTC
49ca2fb fetch: add branch.*.merge to default ref-prefix extension When running "git pull" with no arguments, we'll do a default "git fetch" and then try to merge the branch specified by the branch.*.merge config. There's code in get_ref_map() to treat that "merge" branch as something we want to fetch, even if it is not otherwise covered by the default refspec. This works fine with the v0 protocol, as the server tells us about all of the refs, and get_ref_map() is the ultimate decider of what we fetch. But in the v2 protocol, we send the ref-prefix extension to the server, asking it to limit the ref advertisement. And we only tell it about the default refspec for the remote; we don't mention the branch.*.merge config at all. This usually doesn't matter, because the default refspec matches "refs/heads/*", which covers all branches. But if you explicitly use a narrow refspec, then "git pull" on some branches may fail. The server doesn't advertise the branch, so we don't fetch it, and "git pull" thinks that it went away upstream. We can fix this by including any branch.*.merge entries for the current branch in the list of ref-prefixes we pass to the server. This only needs to happen when using the default configured refspec (since command-line refspecs are already added, and take precedence in deciding what we fetch). We don't otherwise need to replicate any of the "what to fetch" logic in get_ref_map(). These ref-prefixes are an optimization, so it's OK if we tell the server to advertise the branch.*.merge ref, even if we're not going to pull it. We'll just choose not to fetch it. The test here is based on one constructed by Johannes. I modified the branch names to trigger the ref-prefix issue (and be more descriptive), and to confirm that "git pull" actually updated the local ref, which should be more robust than just checking stderr. Reported-by: Lana Deere <lana.deere@gmail.com> Helped-by: Johannes Schindelin <Johannes.Schindelin@gmx.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 20:10:37 UTC
080bc49 fetch: stop checking for NULL transport->remote in do_fetch() This field will never be NULL; if it were, we'd segfault earlier in the function when we unconditionally check transport->remote->fetch_tags. Likewise, many other functions dereference it unconditionally. This is a small simplification, but it will make things easier as we extend this conditional in the next patch. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 20:10:32 UTC
66eede4 prepare_repo_settings(): plug leak of config values We call repo_config_get_string() for fetch.negotiationAlgorithm, which allocates a copy of the string, but we never free it. We could add a call to free(), but there's an even simpler solution: we don't need the string to persist beyond a few strcasecmp() calls, so we can instead use the "_tmp" variant which gives us a const pointer to the cached value. We need to switch the type of "strval" to "const char *" for this to work, which affects a similar call that checks core.untrackedCache. But it's in the same boat! It doesn't actually need the value to persist beyond a maybe_bool() check (though it does remember to correctly free the string afterwards). So we can simplify it at the same time. Note that this core.untrackedCache check arguably should be using repo_config_get_maybe_bool(), but there are some subtle behavior changes. E.g., it doesn't currently allow a value-less "true". Arguably it should, but let's avoid lumping further changes in what should be a simple leak cleanup. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 18:11:28 UTC
7e2619d list_objects_filter_options: plug leak of filter_spec strings The list_objects_filter_options struct contains a string_list to store the filter_spec. Because we allow the options struct to be zero-initialized by callers, the string_list's strdup_strings field is generally not set. Because we don't want to depend on the memory lifetimes of any strings passed in to the list-objects API, everything we add to the string_list is duplicated (either via xstrdup(), or via strbuf_detach()). So far so good, but now we have a problem at cleanup time: when we clear the list, the string_list API doesn't realize that it needs to free all of those strings, and we leak them. One option would be to set strdup_strings right before clearing the list. But this is tricky for two reasons: 1. There's one spot, in partial_clone_get_default_filter_spec(), that fails to duplicate its argument. We could fix that, but... 2. We clear the list in a surprising number of places. As you might expect, we do so in list_objects_filter_release(). But we also clear and rewrite it in expand_list_objects_filter_spec(), list_objects_filter_spec(), and transform_to_combine_type(). We'd have to put the same hack in all of those spots. Instead, let's just set strdup_strings before adding anything. That lets us drop the extra manual xstrdup() calls, fixes the spot mentioned in (1) above that _should_ be duplicating, and future-proofs further calls. We do have to switch the strbuf_detach() calls to use the nodup form, but that's an easy change, and the resulting code more clearly shows the expected ownership transfer. This also resolves a weird inconsistency: when we make a deep copy with list_objects_filter_copy(), it initializes the copy's filter_spec with string_list_init_dup(). So the copy frees its string_list memory correctly, but accidentally leaks the extra manual-xstrdup()'d strings! There is one hiccup, though. In an ideal world, everyone would allocate the list_objects_filter_options struct with an initializer which used STRING_LIST_INIT_DUP under the hood. But there are a bunch of existing callers which think that zero-initializing is good enough. We can leave them as-is by noting that the list is always initially populated via parse_list_objects_filter(). So we can just initialize the strdup_strings flag there. This is arguably a band-aid, but it works reliably. And it doesn't make anything harder if we want to switch all the callers later to a new LIST_OBJECTS_FILTER_INIT. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 18:08:23 UTC
dd49699 transport: free filter options in disconnect_git() If a user of the transport API calls transport_set_option() with TRANS_OPT_LIST_OBJECTS_FILTER, it doesn't pass a struct, but rather a string with the filter-spec, which the transport code then stores in its own list_objects_filter_options struct. When the caller is done and we call transport_disconnect(), the contents of that filter struct are then leaked. We should release it before freeing the transport struct. Another way to solve this would be for transport_set_option() to pass a pointer to the struct. But that's awkward, because there's a generic transport-option interface that always takes a string. Plus it opens up questions of memory lifetimes; by storing its own filter-options struct, the transport code remains self-contained. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 18:07:58 UTC
3f0e86a transport: deep-copy object-filter struct for fetch-pack When the transport code for the git protocol calls into fetch_pack(), it has to fill out a fetch_pack_args struct that is mostly taken from the transport options. We pass along any object-filter data by doing a struct assignment of the list_objects_filter_options struct. But doing so isn't safe; it contains allocated pointers in its filter_spec string_list, which could lead to a double-free if one side mutates or frees the string_list. And indeed, the fetch-pack code does clear and rewrite the list via expand_list_objects_filter_spec(), leaving the transport code with dangling pointers. This hasn't been a problem so far, though, because the transport code doesn't look further at the filter struct. But it should, because in some cases (when fetch-pack doesn't rewrite the list), it ends up leaking the string_list. So let's start by turning this shallow copy into a deep one, which should let us fix the transport leak in a subsequent patch. Likewise, we'll free the deep copy we made here when we're done with it (to avoid leaking). Note that it would also work to pass fetch-pack a pointer to our filter struct, rather than a copy. But it's awkward for fetch-pack to take a pointer in its arg struct; the actual git-fetch-pack command allocates a fetch_pack_args struct on the stack and expects it to contain the filter options. It could be rewritten to avoid this, but a deep copy serves our purposes just as well. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 18:06:14 UTC
3fbfbbb list_objects_filter_copy(): deep-copy sparse_oid_name field The purpose of our copy function is to do a deep copy of each field so that the source and destination structs become independent. We correctly copy the filter_spec string list, but we forgot the sparse_oid_name field. By doing a shallow copy of the pointer, that puts us at risk for a use-after-free if one or both of the structs is cleaned up. I don't think this can be triggered in practice, because we tend to leak the structs rather than actually clean them up. But this should future-proof us for plugging those leaks. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 08 September 2022, 18:05:46 UTC
945ed00 t1060: check partial clone of misnamed blob A recent commit (upload-pack: skip parse-object re-hashing of "want" objects, 2022-09-06) loosened the behavior of upload-pack so that it does not verify the sha1 of objects it receives directly via "want" requests. The existing corruption tests in t1060 aren't affected by this: the corruptions are blobs reachable from commits, and the client requests the commits. The more interesting case here is a partial clone, where the client will directly ask for the corrupted blob when it does an on-demand fetch of the filtered object. And that is not covered at all, so let's add a test. It's important here that we use the "misnamed" corruption and not "bit-error". The latter is sufficiently corrupted that upload-pack cannot even figure out the type of the object, so it bails identically both before and after the recent change. But with "misnamed", with the hash-checks enabled it sees the problem (though the error messages are a bit confusing because of the inability to create a "struct object" to store the flags): error: hash mismatch d95f3ad14dee633a758d2e331151e950dd13e4ed fatal: git upload-pack: not our ref d95f3ad14dee633a758d2e331151e950dd13e4ed fatal: remote error: upload-pack: not our ref d95f3ad14dee633a758d2e331151e950dd13e4ed After the change to skip the hash check, the server side happily sends the bogus object, but the client correctly realizes that it did not get the necessary data: remote: Enumerating objects: 1, done. remote: Counting objects: 100% (1/1), done. remote: Total 1 (delta 0), reused 0 (delta 0), pack-reused 0 Receiving objects: 100% (1/1), 49 bytes | 49.00 KiB/s, done. fatal: bad revision 'd95f3ad14dee633a758d2e331151e950dd13e4ed' error: [...]/misnamed did not send all necessary objects which is exactly what we expect to happen. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 22:08:51 UTC
2b43dd0 diff-no-index: simplify argv index calculation Since 16bb3d714d (diff --no-index: use parse_options() instead of diff_opt_parse(), 2019-03-24) argc must be 2 if we reach the loop, i.e. argc - 2 == 0. Remove that inconsequential term. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 19:36:43 UTC
07a6f94 diff-no-index: release prefixed filenames Callers of prefix_filename() are responsible for freeing its result. Remember the returned strings and release them to appease leak checkers. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 19:34:03 UTC
fffe7d8 diff-no-index: release strbuf on queue error The strbuf is small and we are about to exit, so we could leave its cleanup to the OS. If we release it explicitly at all, however, then we should do it on early exit as well. Move the strbuf_release call to a new cleanup section at the end and make sure all execution paths go through it. Suggested-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 19:33:28 UTC
9a8c3c4 parse_object(): check commit-graph when skip_hash set If the caller told us that they don't care about us checking the object hash, then we're free to implement any optimizations that get us the parsed value more quickly. An obvious one is to check the commit graph before loading an object from disk. And in fact, both of the callers who pass in this flag are already doing so before they call parse_object()! So we can simplify those callers, as well as any possible future ones, by moving the logic into parse_object(). There are two subtle things to note in the diff, but neither has any impact in practice: - it seems least-surprising here to do the graph lookup on the git-replace'd oid, rather than the original. This is in theory a change of behavior from the earlier code, as neither caller did a replace lookup itself. But in practice it doesn't matter, as we disable the commit graph entirely if there are any replace refs. - the caller in get_reference() passes the skip_hash flag only if revs->verify_objects isn't set, whereas it would look in the commit graph unconditionally. In practice this should not matter as we should disable the commit graph entirely when using verify_objects (and that was done recently in another patch). So this should be a pure cleanup with no behavior change. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 19:27:02 UTC
0bc2557 upload-pack: skip parse-object re-hashing of "want" objects Imagine we have a history with commit C pointing to a large blob B. If a client asks us for C, we can generally serve both objects to them without accessing the uncompressed contents of B. In upload-pack, we figure out which commits we have and what the client has, and feed those tips to pack-objects. In pack-objects, we traverse the commits and trees (or use bitmaps!) to find the set of objects needed, but we never open up B. When we serve it to the client, we can often pass the compressed bytes directly from the on-disk packfile over the wire. But if a client asks us directly for B, perhaps because they are doing an on-demand fetch to fill in the missing blob of a partial clone, we end up much slower. Upload-pack calls parse_object() on the oid we receive, which opens up the object and re-checks its hash (even though if it were a commit, we might skip this parse entirely in favor of the commit graph!). And then we feed the oid directly to pack-objects, which again calls parse_object() and opens the object. And then finally, when we write out the result, we may send bytes straight from disk, but only after having unnecessarily uncompressed and computed the sha1 of the object twice! This patch teaches both code paths to use the new SKIP_HASH_CHECK flag for parse_object(). You can see the speed-up in p5600, which does a blob:none clone followed by a checkout. The savings for git.git are modest: Test HEAD^ HEAD ---------------------------------------------------------------------- 5600.3: checkout of result 2.23(4.19+0.24) 1.72(3.79+0.18) -22.9% But the savings scale with the number of bytes. So on a repository like linux.git with more files, we see more improvement (in both absolute and relative numbers): Test HEAD^ HEAD ---------------------------------------------------------------------------- 5600.3: checkout of result 51.62(77.26+2.76) 34.86(61.41+2.63) -32.5% And here's an even more extreme case. This is the android gradle-plugin repository, whose tip checkout has ~3.7GB of files: Test HEAD^ HEAD -------------------------------------------------------------------------- 5600.3: checkout of result 79.51(90.84+5.55) 40.28(51.88+5.67) -49.3% Keep in mind that these timings are of the whole checkout operation. So they count the client indexing the pack and actually writing out the files. If we want to see just the server's view, we can hack up the GIT_TRACE_PACKET output from those operations and replay it via upload-pack. For the gradle example, that gives me: Benchmark 1: GIT_PROTOCOL=version=2 git.old upload-pack ../gradle-plugin <input Time (mean ± σ): 50.884 s ± 0.239 s [User: 51.450 s, System: 1.726 s] Range (min … max): 50.608 s … 51.025 s 3 runs Benchmark 2: GIT_PROTOCOL=version=2 git.new upload-pack ../gradle-plugin <input Time (mean ± σ): 9.728 s ± 0.112 s [User: 10.466 s, System: 1.535 s] Range (min … max): 9.618 s … 9.842 s 3 runs Summary 'GIT_PROTOCOL=version=2 git.new upload-pack ../gradle-plugin <input' ran 5.23 ± 0.07 times faster than 'GIT_PROTOCOL=version=2 git.old upload-pack ../gradle-plugin <input' So a server would see an 80% reduction in CPU serving the initial checkout of a partial clone for this repository. Or possibly even more depending on the packing; most of the time spent in the faster one were objects we had to open during the write phase. In both cases skipping the extra hashing on the server should be pretty safe. The client doesn't trust the server anyway, so it will re-hash all of the objects via index-pack. There is one thing to note, though: the change in get_reference() affects not just pack-objects, but rev-list, git-log, etc. We could use a flag to limit to index-pack here, but we may already skip hash checks in this instance. For commits, we'd skip anything we load via the commit-graph. And while before this commit we would check a blob fed directly to rev-list on the command-line, we'd skip checking that same blob if we found it by traversing a tree. The exception for both is if --verify-objects is used. In that case, we'll skip this optimization, and the new test makes sure we do this correctly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com> 07 September 2022, 19:20:02 UTC
back to top