summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-03-07diff tests: don't ignore "git rev-list" exit codeÆvar Arnfjörð Bjarmason1-5/+2
Change a fragile pattern introduced in 2b459b483cb (diff: make sure work tree side is shown as 0{40} when different, 2008-03-02) to check the exit code of "git rev-list", while we're at it let's get rid of the needless sub-shell for invoking it in favor of the "-C" option. Because of this I'd marked these tests as passing under SANITIZE=leak in 16d4bd4f14e (leak tests: mark some diff tests as passing with SANITIZE=leak, 2021-10-31), let's remove the "TEST_PASSES_SANITIZE_LEAK=true" annotation as they no longer do. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07notes tests: don't ignore "git" exit codeÆvar Arnfjörð Bjarmason3-15/+14
Change a fragile test pattern that's been with us ever since these tests were introduced in [1], [2] and [3] to properly return the exit code of the failing command on failure. Because of this I'd marked this test as passing under SANITIZE=leak in [4] and [5]. We need to remove those annotations as these tests will no longer pass. 1. 9081a421a6d (checkout: fix "branch info" memory leaks, 2021-11-16) 2. 0057c0917d3 (Add selftests verifying that we can parse notes trees with various fanouts, 2009-10-09) 3. 048cdd4665e (t3305: Verify that adding many notes with git-notes triggers increased fanout, 2010-02-13) 4. ca089724952 (leak tests: mark some notes tests as passing with SANITIZE=leak, 2021-10-31) 5. 9081a421a6d (checkout: fix "branch info" memory leaks, 2021-11-16) Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07rev-parse tests: don't ignore "git reflog" exit codeÆvar Arnfjörð Bjarmason1-2/+3
Amend a test added in 9c46c054ae4 (rev-parse: tests git rev-parse --verify master@{n}, for various n, 2010-08-24) so that we'll stop ignoring the exit code of "git reflog" by having it on the left-hand-side of a pipe. Because of this I'd marked this test as passing under SANITIZE=leak in f442c94638d (leak tests: mark some rev-parse tests as passing with SANITIZE=leak, 2021-10-31). As all of it except this specific test will now pass, let's skip it under the !SANITIZE_LEAK prerequisite. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07merge tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason1-16/+6
As in the preceding commit change a similar fragile test pattern introduced in b798671fa93 (merge-recursive: do not rudely die on binary merge, 2007-08-14) to use a "test_must_fail" instead. Before this we wouldn't distinguish normal "git merge" failures from segfaults or abort(). Unlike the preceding commit we didn't end up hiding any SANITIZE=leak failures in this case, but let's correspondingly change these anyway. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07apply tests: use "test_must_fail" instead of ad-hoc patternÆvar Arnfjörð Bjarmason1-16/+2
Change a fragile test pattern introduced in 6b763c424e4 (git-apply: do not read past the end of buffer, 2007-09-05). Before this we wouldn't distinguish normal "git apply" failures from segfaults or abort(). I'd previously marked this test as passing under SANITIZE=leak in f54f48fc074 (leak tests: mark some apply tests as passing with SANITIZE=leak, 2021-10-31). Let's remove that annotation as this test will no longer pass. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07diff tests: don't ignore "git diff" exit code in "read" loopÆvar Arnfjörð Bjarmason1-51/+53
Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm' attribute, 2007-04-22) so that we'll stop using "git diff" on the left-hand-side of a pipe, and thus ignoring its exit code. Rather than use intermediate files let's rewrite these tests to a much simpler but more exhaustive "test_tmp" where we'll ignore certain fields in the output. Note that this is not a faithful conversion of the previous "read/test" in some cases, as we were ignoring more fields there than we strictly needed to. Now we'll "test_cmp" everything we can, and only ignore the likes of paths to $TEMPDIR etc. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07diff tests: don't ignore "git diff" exit codeÆvar Arnfjörð Bjarmason1-27/+22
Fix a test pattern that originated in f1af60bdba4 (Support 'diff=pgm' attribute, 2007-04-22) so that we'll stop using "git diff" on the left-hand-side of a pipe, and thus ignoring its exit code. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07read-tree tests: check "diff-files" exit code on failureÆvar Arnfjörð Bjarmason2-6/+6
Fix an issue with the exit code of "diff-files" being ignored, which has been ignored ever since these tests were originally added in c859600954d ([PATCH] read-tree: save more user hassles during fast-forward., 2005-06-07). Since the exit code was ignored we'd hide errors here under SANITIZE=leak, which resulted in me mistakenly marking these tests as passing under SANITIZE=leak in e5a917fcf42 (unpack-trees: don't leak memory in verify_clean_subdirectory(), 2021-10-07) and 4ea08416b8e (leak tests: mark a read-tree test as passing SANITIZE=leak, 2021-10-31). As it would be non-trivial to fix these tests (the leak is in revision.c) let's un-mark them as passing under SANITIZE=leak in addition to fixing the issue of ignoring the exit code. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07tests: use "test_stdout_line_count", not "test $(git [...] | wc -l)"Ævar Arnfjörð Bjarmason1-22/+21
Use the test_stdout_line_count helper added in cdff1bb5a3d (test-lib-functions: introduce test_stdout_line_count, 2021-07-04) so that we'll spot if git itself dies, segfaults etc in these expressions. Because we didn't distinguish these failure conditions before I'd mistakenly marked these tests as passing under SANITIZE=leak in dd9cede9136 (leak tests: mark some rev-list tests as passing with SANITIZE=leak, 2021-10-31). While we're at it let's re-indent these lines to match our usual style, as we're having to change all of them anyway. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07tests: change some 'test $(git) = "x"' to test_cmpÆvar Arnfjörð Bjarmason4-29/+50
Change some of the patterns in the test suite where we were hiding the exit code from "git" by invoking it in a sub-shell within a "test" expression to use temporary files and test_cmp instead. These are not all the occurrences of this anti-pattern, but these in particular hid issues where LSAN was dying, and I'd thus marked these tests as passing under the linux-leaks CI job in past commits with "TEST_PASSES_SANITIZE_LEAK=true". Let's deal with that by either removing that marking, or skipping specific tests under !SANITIZE_LEAK. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07hooks: fix an obscure TOCTOU "did we just run a hook?" raceÆvar Arnfjörð Bjarmason8-18/+47
Fix a Time-of-check to time-of-use (TOCTOU) race in code added in 680ee550d72 (commit: skip discarding the index if there is no pre-commit hook, 2017-08-14). This obscure race condition can occur if we e.g. ran the "pre-commit" hook and it modified the index, but hook_exists() returns false later on (e.g., because the hook itself went away, the directory became unreadable, etc.). Then we won't call discard_cache() when we should have. The race condition itself probably doesn't matter, and users would have been unlikely to run into it in practice. This problem has been noted on-list when 680ee550d72 was discussed[1], but had not been fixed. This change is mainly intended to improve the readability of the code involved, and to make reasoning about it more straightforward. It wasn't as obvious what we were trying to do here, but by having an "invoked_hook" it's clearer that e.g. our discard_cache() is happening because of the earlier hook execution. Let's also change this for the push-to-checkout hook. Now instead of checking if the hook exists and either doing a push to checkout or a push to deploy we'll always attempt a push to checkout. If the hook doesn't exist we'll fall back on push to deploy. The same behavior as before, without the TOCTOU race. See 0855331941b (receive-pack: support push-to-checkout hook, 2014-12-01) for the introduction of the previous behavior. This leaves uses of hook_exists() in two places that matter. The "reference-transaction" check in refs.c, see 67541597670 (refs: implement reference transaction hook, 2020-06-19), and the "prepare-commit-msg" hook, see 66618a50f9c (sequencer: run 'prepare-commit-msg' hook, 2018-01-24). In both of those cases we're saving ourselves CPU time by not preparing data for the hook that we'll then do nothing with if we don't have the hook. So using this "invoked_hook" pattern doesn't make sense in those cases. The "reference-transaction" and "prepare-commit-msg" hook also aren't racy. In those cases we'll skip the hook runs if we race with a new hook being added, whereas in the TOCTOU races being fixed here we were incorrectly skipping the required post-hook logic. 1. https://lore.kernel.org/git/20170810191613.kpmhzg4seyxy3cpq@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07merge: don't run post-hook logic on --no-verifyÆvar Arnfjörð Bjarmason1-9/+12
Fix a minor bug introduced in bc40ce4de61 (merge: --no-verify to bypass pre-merge-commit hook, 2019-08-07), when that change made the --no-verify option bypass the "pre-merge-commit" hook it didn't update the corresponding find_hook() (later hook_exists()) condition. As can be seen in the preceding commit in 6098817fd7f (git-merge: honor pre-merge-commit hook, 2019-08-07) the two should go hand in hand. There's no point in invoking discard_cache() here if the hook couldn't have possibly updated the index. It's buggy that we use "hook_exist()" here, and as discussed in the subsequent commit it's subject to obscure race conditions that we're about to fix, but for now this change is a strict improvement that retains any caveats to do with the use of "hooks_exist()" as-is. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07string-list API: change "nr" and "alloc" to "size_t"Ævar Arnfjörð Bjarmason9-31/+35
Change the "nr" and "alloc" members of "struct string_list" to use "size_t" instead of "nr". On some platforms the size of an "unsigned int" will be smaller than a "size_t", e.g. a 32 bit unsigned v.s. 64 bit unsigned. As "struct string_list" is a generic API we use in a lot of places this might cause overflows. As one example: code in "refs.c" keeps track of the number of refs with a "size_t", and auxiliary code in builtin/remote.c in get_ref_states() appends those to a "struct string_list". While we're at it split the "nr" and "alloc" in string-list.h across two lines, which is the case for most such struct member declarations (e.g. in "strbuf.h" and "strvec.h"). Changing e.g. "int i" to "size_t i" in run_and_feed_hook() isn't strictly necessary, and there are a lot more cases where we'll use a local "int", "unsigned int" etc. variable derived from the "nr" in the "struct string_list". But in that case as well as add_wrapped_shortlog_msg() in builtin/shortlog.c we need to adjust the printf format referring to "nr" anyway, so let's also change the other variables referring to it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07gettext API users: don't explicitly cast ngettext()'s "n"Ævar Arnfjörð Bjarmason3-4/+4
Change a few stray users of the inline gettext.h Q_() function to stop casting its "n" argument, the vast majority of the users of that wrapper API use the implicit cast to "unsigned long". The ngettext() function (which Q_() resolves to) takes an "unsigned long int", and so does our Q_() wrapper for it, see 0c9ea33b90f (i18n: add stub Q_() wrapper for ngettext, 2011-03-09). The function isn't ours, but provided by e.g. GNU libintl. This amends code added in added in 7171a0b0cf5 (index-pack: correct "len" type in unpack_data(), 2016-07-13). The cast it added for the printf format to die() was needed, but not the cast to Q_(). Likewise the casts in strbuf.c added in 8f354a1faed (l10n: localizable upload progress messages, 2019-07-02) and for builtin/merge-recursive.c in ccf7813139f (i18n: merge-recursive: mark error messages for translation, 2016-09-15) weren't needed. In the latter case the cast was copy/pasted from the argument to warning() itself, added in b74d779bd90 (MinGW: Fix compiler warning in merge-recursive, 2009-05-23). The cast for warning() is needed, but not the one for ngettext()'s "n" argument. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-07commit-graph: declare bankruptcy on GDAT chunksDerrick Stolee2-4/+12
The Generation Data (GDAT) and Generation Data Overflow (GDOV) chunks store corrected commit date offsets, used for generation number v2. Recent changes have demonstrated that previous versions of Git were incorrectly parsing data from these chunks, but might have also been writing them incorrectly. I asserted [1] that the previous fixes were sufficient because the known reasons for incorrectly writing generation number v2 data relied on parsing the information incorrectly out of a commit-graph file, but the previous versions of Git were not reading the generation number v2 data. However, Patrick demonstrated [2] a case where in split commit-graphs across an alternate boundary (and possibly some other special conditions) it was possible to have a commit-graph that was generated by a previous version of Git have incorrect generation number v2 data which results in errors like the following: commit-graph generation for commit <oid> is 1623273624 < 1623273710 [1] https://lore.kernel.org/git/f50e74f0-9ffa-f4f2-4663-269801495ed3@github.com/ [2] https://lore.kernel.org/git/Yh93vOkt2DkrGPh2@ncase/ Clearly, there is something else going on. The situation is not completely understood, but the errors do not reproduce if the commit-graphs are all generated by a Git version including these recent fixes. If we cannot trust the existing data in the GDAT and GDOV chunks, then we can alter the format to change the chunk IDs for these chunks. This causes the new version of Git to silently ignore the older chunks (and disabling generation number v2 in the process) while writing new commit-graph files with correct data in the GDA2 and GDO2 chunks. Update commit-graph-format.txt including a historical note about these deprecated chunks. Reported-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-06The ninth batchJunio C Hamano1-0/+21
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-06Merge branch 'jt/ls-files-stage-recurse'Junio C Hamano3-4/+20
Many output modes of "ls-files" do not work with its "--recurse-submodules" option, but the "-s" mode has been taught to work with it. * jt/ls-files-stage-recurse: ls-files: support --recurse-submodules --stage
2022-03-06Merge branch 'gc/stash-on-branch-with-multi-level-name'Junio C Hamano2-1/+12
"git checkout -b branch/with/multi/level/name && git stash" only recorded the last level component of the branch name, which has been corrected. * gc/stash-on-branch-with-multi-level-name: stash: strip "refs/heads/" with skip_prefix
2022-03-06Merge branch 'ah/advice-switch-requires-detach-to-detach'Junio C Hamano5-11/+35
The error message given by "git switch HEAD~4" has been clarified to suggest the "--detach" option that is required. * ah/advice-switch-requires-detach-to-detach: switch: mention the --detach option when dying due to lack of a branch
2022-03-06Merge branch 'ab/c99-designated-initializers'Junio C Hamano11-177/+187
Use designated initializers we started using in mid 2017 in more parts of the codebase that are relatively quiescent. * ab/c99-designated-initializers: fast-import.c: use designated initializers for "partial" struct assignments refspec.c: use designated initializers for "struct refspec_item" convert.c: use designated initializers for "struct stream_filter*" userdiff.c: use designated initializers for "struct userdiff_driver" archive-*.c: use designated initializers for "struct archiver" object-file: use designated initializers for "struct git_hash_algo" trace2: use designated initializers for "struct tr2_dst" trace2: use designated initializers for "struct tr2_tgt" imap-send.c: use designated initializers for "struct imap_server_conf"
2022-03-06Merge branch 'mc/index-pack-report-max-size'Junio C Hamano2-2/+14
When "index-pack" dies due to incoming data exceeding the maximum allowed input size, include the value of the limit in the error message. * mc/index-pack-report-max-size: index-pack: clarify the breached limit
2022-03-06Merge branch 'ac/usage-string-fixups'Junio C Hamano5-9/+9
Usage-string normalization. * ac/usage-string-fixups: amend remaining usage strings according to style guide
2022-03-06Merge branch 'ab/test-leak-diag'Junio C Hamano1-10/+40
Random test-framework clean-up. * ab/test-leak-diag: test-lib: add "fast_unwind_on_malloc=0" to LSAN_OPTIONS test-lib: make $GIT_BUILD_DIR an absolute path test-lib: correct and assert TEST_DIRECTORY overriding test-lib: add GIT_SAN_OPTIONS, inherit [AL]SAN_OPTIONS
2022-03-06Merge branch 'ab/hook-tests'Junio C Hamano1-41/+53
Test modernization. * ab/hook-tests: hook tests: use a modern style for "pre-push" tests hook tests: test for exact "pre-push" hook input
2022-03-06Merge branch 'en/merge-ort-plug-leaks'Junio C Hamano1-19/+17
Leakfix. * en/merge-ort-plug-leaks: merge-ort: fix small memory leak in unique_path() merge-ort: fix small memory leak in detect_and_process_renames()
2022-03-06Merge branch 'ds/worktree-docs'Junio C Hamano2-195/+207
Tighten the language around "working tree" and "worktree" in the docs. * ds/worktree-docs: worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: use 'worktree' over 'working tree' worktree: extract checkout_worktree() worktree: extract copy_sparse_checkout() worktree: extract copy_filtered_worktree_config() worktree: combine two translatable messages
2022-03-06Merge branch 'jc/rerere-train-modernise'Junio C Hamano1-1/+1
Small modernization of the rerere-train script (in contrib/). * jc/rerere-train-modernise: rerere-train: two fixes to the use of "git show -s"
2022-03-06Merge branch 'rs/bisect-executable-not-found'Junio C Hamano4-14/+125
A not-so-common mistake is to write a script to feed "git bisect run" without making it executable, in which case all tests will exit with 126 or 127 error codes, even on revisions that are marked as good. Try to recognize this situation and stop iteration early. * rs/bisect-executable-not-found: bisect--helper: double-check run command on exit code 126 and 127 bisect: document run behavior with exit codes 126 and 127 bisect--helper: release strbuf and strvec on run error bisect--helper: report actual bisect_state() argument on error
2022-03-06Merge branch 'en/sparse-checkout-fixes'Junio C Hamano2-7/+161
Further polishing of "git sparse-checkout". * en/sparse-checkout-fixes: sparse-checkout: reject arguments in cone-mode that look like patterns sparse-checkout: error or warn when given individual files sparse-checkout: pay attention to prefix for {set, add} sparse-checkout: correctly set non-cone mode when expected sparse-checkout: correct reapply's handling of options
2022-03-06Merge branch 'cg/t3903-modernize'Junio C Hamano2-9/+41
Test modernization. * cg/t3903-modernize: tests: make the code more readable tests: allow testing if a path is truly a file or a directory t/t3903-stash.sh: replace test [-d|-f] with test_path_is_*
2022-03-04submodule--helper update-clone: check for --filter and --initGlen Choo2-6/+19
"git submodule update --filter" also requires the "--init" option. Teach update-clone to do this usage check in C and remove the check from git-submodule.sh. In addition, change update-clone's usage string so that it teaches users about "git submodule update" instead of "git submodule--helper update-clone" (the string is copied from git-submodule.sh). This should be more helpful to users since they don't invoke update-clone directly. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule update: add tests for --filterGlen Choo1-0/+12
Test the "--filter" option to make sure we don't break anything while refactoring "git submodule update". Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: remove ensure-core-worktreeGlen Choo2-12/+2
Move the logic of "git submodule--helper ensure-core-worktree" into run-update-procedure, and since this makes the ensure-core-worktree command obsolete, remove it. As a result, the order of two operations in git-submodule.sh is reversed: 'set the value of core.worktree' now happens after the call to "git submodule--helper relative-path". This is safe - "relative-path" does not depend on the value of core.worktree. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper update-clone: learn --initGlen Choo2-6/+29
Teach "git submodule--helper update-clone" the --init flag and remove the corresponding shell code. When the `--init` flag is passed to the subcommand, we do not spawn a new subprocess and call `submodule--helper init` on the submodule paths, because the Git machinery is not able to pick up the configuration changes introduced by that init call. So we instead run the `init_submodule_cb()` callback over each submodule in the same process. [1] https://lore.kernel.org/git/CAP8UFD0NCQ5w_3GtT_xHr35i7h8BuLX4UcHNY6VHPGREmDVObA@mail.gmail.com/ Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: allow setting superprefix for init_submodule()Atharva Raykar1-3/+7
We allow callers of the `init_submodule()` function to optionally override the superprefix from the environment. We need to enable this option because in our conversion of the update command that will follow, the '--init' option will be handled through this API. We will need to change the superprefix at that time to ensure the display paths show correctly in the output messages. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: refactor get_submodule_displaypath()Atharva Raykar1-4/+10
We create a function called `do_get_submodule_displaypath()` that generates the display path required by several submodule functions, and takes a custom superprefix parameter, instead of reading it from the environment. We then redefine the existing `get_submodule_displaypath()` function as a call to this new function, where the superprefix is obtained from the environment. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper run-update-procedure: learn --remoteGlen Choo2-63/+23
Teach run-update-procedure to handle --remote instead of parsing --remote in git-submodule.sh. As a result, "git submodule--helper [print-default-remote|remote-branch]" have no more callers, so remove them. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: don't use bitfield indirection for parse_options()Ævar Arnfjörð Bjarmason1-13/+10
Do away with the indirection of local variables added in c51f8f94e5b (submodule--helper: run update procedures from C, 2021-08-24). These were only needed because in C you can't get a pointer to a single bit, so we were using intermediate variables instead. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: get remote names from any repositoryAtharva Raykar1-15/+23
`get_default_remote()` retrieves the name of a remote by resolving the refs from of the current repository's ref store. Thus in order to use it for retrieving the remote name of a submodule, we have to start a new subprocess which runs from the submodule directory. Let's instead introduce a function called `repo_get_default_remote()` which takes any repository object and retrieves the remote accordingly. `get_default_remote()` is then defined as a call to `repo_get_default_remote()` with 'the_repository' passed to it. Now that we have `repo_get_default_remote()`, we no longer have to start a subprocess that called `submodule--helper get-default-remote` from within the submodule directory. So let's make a function called `get_default_remote_submodule()` which takes a submodule path, and returns the default remote for that submodule, all within the same process. We can now use this function to save an unnecessary subprocess spawn in `sync_submodule()`, and also in a subsequent patch, which will require this functionality. Mentored-by: Christian Couder <christian.couder@gmail.com> Mentored-by: Shourya Shukla <periperidip@gmail.com> Helped-by: Glen Choo <chooglen@google.com> Signed-off-by: Atharva Raykar <raykar.ath@gmail.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper run-update-procedure: remove --suboidGlen Choo2-10/+7
Teach run-update-procedure to determine the oid of the submodule's HEAD instead of doing it in git-submodule.sh. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: reorganize code for sh to C conversionGlen Choo1-5/+21
Introduce a function, update_submodule2(), that will implement the functionality of run-update-procedure and its surrounding shell code in submodule.sh. This name is temporary; it will replace update_submodule() when the sh to C conversion is complete. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: remove update-module-modeGlen Choo1-24/+0
This is dead code - it has not been used since c51f8f94e5 (submodule--helper: run update procedures from C, 2021-08-24). Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule tests: test for init and update failure outputÆvar Arnfjörð Bjarmason2-3/+25
Amend some submodule tests to test for the failure output of "git submodule [update|init]". The lack of such tests hid a regression in an earlier version of a subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04repository.c: free the "path cache" in repo_clear()Ævar Arnfjörð Bjarmason3-15/+29
The "struct path_cache" added in 102de880d24 (path.c: migrate global git_path_* to take a repository argument, 2018-05-17) is only used directly by code in repository.[ch] (but populated in path.[ch]). Let's move this code to repository.[ch], and stop leaking this memory when we run repo_clear(). To avoid the cast change it from a "const char *" to a "char *". This also removes the "PATH_CACHE_INIT" macro, which has never been used for anything. For the "struct repository" we already make a hard assumption that it (and "the_repository") can be identically initialized by making it a "static" variable, so making use of a "PATH_CACHE_INIT" somewhere would have been confusing. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04range-diff: plug memory leak in read_patches()Ævar Arnfjörð Bjarmason1-17/+12
Amend code added in d9c66f0b5bf (range-diff: first rudimentary implementation, 2018-08-13) to use a "goto cleanup" pattern. This makes for less code, and frees memory that we'd previously leak. The reason for changing free(util) to FREE_AND_NULL(util) is because at the end of the function we append the contents of "util" to a "struct string_list" if it's non-NULL. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04range-diff: plug memory leak in common invocationÆvar Arnfjörð Bjarmason3-1/+9
Create a public release_patch() version of the private free_patch() function added in 13b5af22f39 (apply: move libified code from builtin/apply.c to apply.{c,h}, 2016-04-22). Unlike the existing function this one doesn't free() the "struct patch" itself, so we can use it for variables on the stack. Use it in range-diff.c to fix a memory leak in common range-diff invocations, e.g.: git -P range-diff origin/master origin/next origin/seen Would emit several errors when compiled with SANITIZE=leak, but now runs cleanly. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04lockfile API users: simplify and don't leak "path"Ævar Arnfjörð Bjarmason2-2/+3
Fix a memory leak in code added in 6c622f9f0bb (commit-graph: write commit-graph chains, 2019-06-18). We needed to free the "lock_name" if we encounter errors, and the "graph_name" after we'd run unlink() on it. For the case of write_commit_graph_file() refactoring the code to free the "lock_name" after we were done using the "struct lock_file lk" would have made the control flow more complex. Luckily we can free the "lock_file" right after the hold_lock_file_for_update() call, if it makes use of "path" at all it'll have copied its contents to a "struct strbuf" of its own. While I'm at it let's fix code added in fb10ca5b543 (sparse-checkout: write using lockfile, 2019-11-21) in write_patterns_and_update() to avoid the same complexity that I thought I needed when I wrote the initial fix for write_commit_graph_file(). We can free the "sparse_filename" right after calling hold_lock_file_for_update(), we don't need to wait until we're exiting the function to do so. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04commit-graph: stop fill_oids_from_packs() progress on error and free()Ævar Arnfjörð Bjarmason1-5/+7
Fix a bug in fill_oids_from_packs(), we should always stop_progress(), but did not do so if we returned an error here. This also plugs a memory leak in those cases by releasing the two "struct strbuf" variables the function uses. While I'm at it stop hardcoding "-1" here and just use the return value of error() instead, which happens to be "-1". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04commit-graph: fix memory leak in misused string_list APIÆvar Arnfjörð Bjarmason3-6/+6
When this code was migrated to the string_list API in d88b14b3fd6 (commit-graph: use string-list API for input, 2018-06-27) it was made to use used both STRING_LIST_INIT_NODUP and a strbuf_detach() pattern. Those should not be used together if string_list_clear() is expected to free the memory, instead we need to either use STRING_LIST_INIT_DUP with a string_list_append_nodup(), or a STRING_LIST_INIT_NODUP and manually fiddle with the "strdup_strings" member before calling string_list_clear(). Let's do the former. Since "strdup_strings = 1" is set now other code might be broken by relying on "pack_indexes" not to duplicate it strings, but that doesn't happen. When we pass this down to write_commit_graph() that code uses the "struct string_list" without modifying it. Let's add a "const" to the variable to have the compiler enforce that assumption. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-03-04submodule--helper: fix trivial leak in module_add()Ævar Arnfjörð Bjarmason1-1/+4
Fix a memory leak in code added in a6226fd772b (submodule--helper: convert the bulk of cmd_add() to C, 2021-08-10). If "realrepo" isn't a copy of the "repo" member we should free() it. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>