summaryrefslogtreecommitdiff
AgeCommit message (Collapse)AuthorFilesLines
2022-07-10multi-pack-index: simplify handling of unknown --optionsSZEDER Gábor1-4/+4
Although parse_options() can handle unknown --options just fine, none of 'git multi-pack-index's subcommands rely on it, but do it on their own: they invoke parse_options() with the PARSE_OPT_KEEP_UNKNOWN flag, then check whether there are any unparsed arguments left, and print usage and quit if necessary. Drop that PARSE_OPT_KEEP_UNKNOWN flag to let parse_options() handle unknown options instead, which has the additional benefit that it prints not only the usage but an "error: unknown option `foo'" message as well. Do leave the unparsed arguments check to catch any unexpected non-option arguments, though, e.g. 'git multi-pack-index write foo'. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-10cocci: avoid normalization rules for memcpyRené Scharfe1-42/+40
Some of the rules for using COPY_ARRAY instead of memcpy with sizeof are intended to reduce the number of sizeof variants to deal with. They can have unintended side effects if only they match, but not the one for the COPY_ARRAY conversion at the end. Avoid these side effects by instead using a self-contained rule for each combination of array and pointer for source and destination which lists all sizeof variants inline. This lets "make contrib/coccinelle/array.cocci.patch" take 15% longer on my machine, but gives peace of mind that no incomplete transformation will be generated. Suggested-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-10sha256: add support for Nettlebrian m. carlson3-1/+44
For SHA-256, we currently have support for OpenSSL and libgcrypt because these two libraries contain optimized implementations that can take advantage of native processor instructions. However, OpenSSL is not suitable for linking against for Linux distros due to licensing incompatibilities with the GPLv2, and libgcrypt has been less favored by cryptographers due to some security-related implementation issues, which, while not affecting our use of hash algorithms, has affected its reputation. Let's add another option that's compatible with the GPLv2, which is Nettle. This is an option which is generally better than libgcrypt because on many distros GnuTLS (which uses Nettle) is used for HTTPS and therefore as a practical matter it will be available on most systems. As a result, prefer it over libgcrypt and our built-in implementation. Nettle also has recently gained support for Intel's SHA-NI instructions, which compare very favorably to other implementations, as well as assembly implementations for when SHA-NI is not available. A git gc on git.git sees a 12% performance improvement with Nettle over our block SHA-256 implementation due to general assembly improvements. With SHA-NI, the performance of raw SHA-256 on a 2 GiB file goes from 7.296 seconds with block SHA-256 to 1.523 seconds with Nettle. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-09builtin/mv.c: use the MOVE_ARRAY() macro instead of memmove()Junio C Hamano1-9/+7
The variables 'source', 'destination', and 'submodule_gitfile' are all of type "const char **", and an element of such an array is of "type const char *", but these memmove() calls were written as if these variables are of type "char **". Once these memmove() calls are fixed to use the correct type to compute the number of bytes to be moved, e.g. - memmove(source + i, source + i + 1, n * sizeof(char *)); + memmove(source + i, source + i + 1, n * sizeof(const char *)); existing contrib/coccinelle/array.cocci rules can recognize them as candidates for turning into MOVE_ARRAY(). While at it, use CALLOC_ARRAY() instead of xcalloc() to allocate the modes[] array that is involved in the change. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08vimdiff: make layout engine more robust against user vim settingsFernando Ramos1-18/+18
'vim' has two configuration options ('splitbelow' and 'splitright') that change the way the 'split' command behaves. When they are set, the commands that the layout engine generates no longer work as expected. In order to fix this we can append special keyword 'leftabove' to each 'split' and 'vertical split' subcommand found inside the command string generated by the layout engine. This works because whatever comes after 'leftabove' will temporally ignore settings 'splitbelow' and 'splitright'. Reported-by: Matthew Klein <mklein994@gmail.com> Signed-off-by: Fernando Ramos <greenfoo@u92.eu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08xdiff: introduce XDL_ALLOC_GROW()Phillip Wood4-16/+33
Add a helper to grow an array. This is analogous to ALLOC_GROW() in the rest of the codebase but returns −1 on allocation failure to accommodate other users of libxdiff such as libgit2. It will also return a error if the multiplication overflows while calculating the new allocation size. Note that this keeps doubling on reallocation like the code it is replacing rather than increasing the existing size by half like ALLOC_GROW(). It does however copy ALLOC_GROW()'s trick of adding a small amount to the new allocation to avoid a lot of reallocations at small sizes. Note that xdl_alloc_grow_helper() uses long rather than size_t for `nr` and `alloc` to match the existing code. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08xdiff: introduce XDL_CALLOC_ARRAY()Phillip Wood4-14/+11
Add a helper for allocating an array and initialize the elements to zero. This is analogous to CALLOC_ARRAY() in the rest of the codebase but it returns NULL on allocation failures rather than dying to accommodate other users of libxdiff such as libgit2. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08xdiff: introduce xdl_callocPhillip Wood4-24/+15
In preparation for introducing XDL_CALLOC_ARRAY() use calloc() to obtain zeroed out memory rather than malloc() followed by memset(). To try and keep the lines a reasonable length this commit also stops casting the pointer returned by calloc() as this is unnecessary. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08xdiff: introduce XDL_ALLOC_ARRAY()Phillip Wood4-7/+12
Add a helper to allocate an array that automatically calculates the allocation size. This is analogous to ALLOC_ARRAY() in the rest of the codebase but returns NULL if the allocation fails to accommodate other users of libxdiff such as libgit2. The helper will also return NULL if the multiplication in the allocation calculation overflows. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-08git-p4: fix bug with encoding of p4 client nameKilian Kilger1-2/+4
The Perforce client name can contain arbitrary characters which do not decode to UTF-8. Use the fallback strategy implemented in metadata_stream_to_writable_bytes() also for the client name. Signed-off-by: Kilian Kilger <kkilger@gmail.com> Reviewed-by: Tao Klerks <tao@klerks.biz> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-07clone: use remote branch if it matches default HEADJeff King3-6/+48
Usually clone tries to use the same local HEAD as the remote (unless the user has given --branch explicitly). Even if the remote HEAD is detached or unborn, we can detect those situations with modern versions of Git. If the remote is too old to support the "unborn" extension (or it has been disabled via config), then we can't know the name of the remote's unborn HEAD, and we fall back whatever the local default branch name is configured to be. But that leads to one weird corner case. It's rare because it needs a number of factors: - the remote has an unborn HEAD - the remote is too old to support "unborn", or has disabled it - the remote has another branch "foo" - the local default branch name is "foo" In that case you end up with a local clone on an unborn "foo" branch, disconnected completely from the remote's "foo". This is rare in practice, but the result is quite confusing. When choosing "foo", we can double check whether the remote has such a name, and if so, start our local "foo" at the same spot, rather than making it unborn. Note that this causes a test failure in t5605, which is cloning from a bundle that doesn't contain HEAD (so it behaves like a remote that doesn't support "unborn"), but has a single "main" branch. That test expects that we end up in the weird "unborn main" case, where we don't actually check out the remote branch of the same name. Even though we have to update the test, this seems like an argument in favor of this patch: checking out main is what I'd expect from such a bundle. So this patch updates the test for the new behavior and adds an adjacent one that checks what the original was going for: if there's no HEAD and the bundle _doesn't_ have a branch that matches our local default name, then we end up with nothing checked out. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-07clone: propagate empty remote HEAD even with other branchesJeff King2-22/+55
Unless "--branch" was given, clone generally tries to match the local HEAD to the remote one. For most repositories, this is easy: the remote tells us which branch HEAD was pointing to, and we call our local checkout() function on that branch. When cloning an empty repository, it's a little more tricky: we have special code that checks the transport's "unborn" extension, or falls back to our local idea of what the default branch should be. In either case, we point the new HEAD to that, and set up the branch.* config. But that leaves one case unhandled: when the remote repository _isn't_ empty, but its HEAD is unborn. The checkout() function is smart enough to realize we didn't fetch the remote HEAD and it bails with a warning. But we'll have ignored any information the remote gave us via the unborn extension. This leads to nonsense outcomes: - If the remote has its HEAD pointing to an unborn "foo" and contains another branch "bar", cloning will get branch "bar" but leave the local HEAD pointing at "master" (or whatever our local default is), which is useless. The project does not use "master" as a branch. - Worse, if the other branch "bar" is instead called "master" (but again, the remote HEAD is not pointing to it), then we end up with a local unborn branch "master", which is not connected to the remote "master" (it shares no history, and there's no branch.* config). Instead, we should try to use the remote's HEAD, even if its unborn, to be consistent with the other cases. The reason this case was missed is that cmd_clone() handles empty and non-empty repositories on two different sides of a conditional: if (we have any refs) { fetch refs; check for --branch; otherwise, try to point our head at remote head; otherwise, our head is NULL; } else { check for --branch; otherwise, try to use "unborn" extension; otherwise, fall back to our default name name; } So the smallest change would be to repeat the "unborn" logic at the end of the first block. But we can note some other overlaps and inconsistencies: - both sides have to handle --branch (though note that it's always an error for the empty repo case, since an empty repo by definition does not have a matching branch) - the fall back to the default name is much more explicit in the empty-repo case. The non-empty case eventually ends up bailing from checkout() with a warning, which produces a similar result, but fails to set up the branch config we do in the empty case. So let's pull the HEAD setup out of this conditional entirely. This de-duplicates some of the code and the result is easy to follow, because helper functions like find_ref_by_name() do the right thing even in the empty-repo case (i.e., by returning NULL). There are two subtleties: - for a remote with a detached HEAD, it will advertise an oid for HEAD (which we store in our "remote_head" variable), but we won't find a matching refname (so our "remote_head_points_at" is NULL). In this case we make a local detached HEAD to match. Right now this happens implicitly by reaching update_head() with a non-NULL remote_head (since we skip all of the unborn-fallback). We'll now need to account for it explicitly before doing the fallback. - for an empty repo, we issue a warning to the user that they've cloned an empty repo. The text of that warning doesn't make sense for a non-empty repo with an unborn HEAD, so we'll have to differentiate the two cases there. We could just use different text, but instead let's allow the code to continue down to checkout(), which will issue an appropriate warning, like: remote HEAD refers to nonexistent ref, unable to checkout Continuing down to checkout() will make it easier to do more fixes on top (see below). Note that this patch fixes the case where the other side reports an unborn head to us using the protocol extension. It _doesn't_ fix the case where the other side doesn't tell us, we locally guess "master", and the other side happens to have a "master" which its HEAD doesn't point. But it doesn't make anything worse there, and it should actually make it easier to fix that problem on top. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-07clone: drop extra newline from warning messageJeff King1-1/+1
We don't need to put a "\n" in calls to warning(), since it adds one itself (and the user sees an extra blank line). Drop it, and while we're here, drop the full-stop from the message, which goes against our guidelines. This bug dates all the way back to 8434c2f1af (Build in clone, 2008-04-27), but presumably nobody noticed because it's hard to trigger: you have to clone a repository whose HEAD is unborn, but which is not otherwise empty. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason4-6/+57
Generalize the newly added "unused.cocci" rule to find more than just "struct strbuf", let's have it find the same unused patterns for "struct string_list", as well as other code that uses similar-looking *_{release,clear,free}() and {release,clear,free}_*() functions. We're intentionally loose in accepting e.g. a "strbuf_init(&sb)" followed by a "string_list_clear(&sb, 0)". It's assumed that the compiler will catch any such invalid code, i.e. that our constructors/destructors don't take a "void *". See [1] for example of code that would be covered by the "get_worktrees()" part of this rule. We'd still need work that the series is based on (we were passing "worktrees" to a function), but could now do the change in [1] automatically. 1. https://lore.kernel.org/git/Yq6eJFUPPTv%2Fzc0o@coredump.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason7-10/+119
Add a coccinelle rule to remove "struct strbuf" initialization followed by calling "strbuf_release()" function, without any uses of the strbuf in the same function. See the tests in contrib/coccinelle/tests/unused.{c,res} for what it's intended to find and replace. The inclusion of "contrib/scalar/scalar.c" is because "spatch" was manually run on it (we don't usually run spatch on contrib). Per the "buggy code" comment we also match a strbuf_init() before the xmalloc(), but we're not seeking to be so strict as to make checks that the compiler will catch for us redundant. Saying we'll match either "init" or "xmalloc" lines makes the rule simpler. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06cocci: have "coccicheck{,-pending}" depend on "coccicheck-test"Ævar Arnfjörð Bjarmason1-0/+2
Have the newly introduced "coccicheck-test" target run implicitly when "coccicheck" itself is run. As with e.g. the "check-chainlint" target (see [1]) it makes sense to run this unconditionally before we run other "spatch" rules as a basic sanity check. See 1. 803394459d4 (t/Makefile: add machinery to check correctness of chainlint.sed, 2018-07-11) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06cocci: add a "coccicheck-test" target and test *.cocci rulesÆvar Arnfjörð Bjarmason4-0/+40
Add a "coccicheck-test" target to test our *.cocci rules, and as a demonstration add tests for the rules added in 39ea59a2570 (remove unnecessary NULL check before free(3), 2016-10-08) and 1b83d1251ed (coccinelle: add a rule to make "expression" code use FREE_AND_NULL(), 2017-06-15). I considered making use of the "spatch --test" option, and the choice of a "tests" over a "t" directory is to make these tests compatible with such a future change. Unfortunately "spatch --test" doesn't return meaningful exit codes, AFAICT you need to "grep" its output to see if the *.res is what you expect. There's "--test-okfailed", but I didn't find a way to sensibly integrate those (it relies on some in-between status files, but doesn't help with the status codes). Instead let's use a "--sp-file" pattern similar to the main "coccicheck" rule, with the difference that we use and compare the two *.res files with cmp(1). The --very-quiet and --no-show-diff options ensure that we don't need to pipe stdout and stderr somewhere. Unlike the "%.cocci.patch" rule we're not using the diff. The "cmp || git diff" is optimistically giving us better output on failure, but even if we only have POSIX cmp and no system git installed we'll still fail with the "cmp", just with an error message that isn't as friendly. The "2>/dev/null" is in case we don't have a "git" installed. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06Makefile & .gitignore: ignore & clean "git.res", not "*.res"Ævar Arnfjörð Bjarmason2-2/+2
Adjust the overly broad .gitignore and "make clean" rule added in ce39c2e04ce (Provide a Windows version resource for the git executables., 2012-05-24). For now this is merely a correctness fix, but needed because a subsequent commit will want to check in *.res files elsewhere in the tree, which we shouldn't have to "git add -f". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGSÆvar Arnfjörð Bjarmason1-2/+3
The "--patch ." part of SPATCH_FLAGS added in f57d11728d1 (coccinelle: put sane filenames into output patches, 2018-07-23) should have been added unconditionally to the "spatch" invocation instead, using it isn't optional. Let's also move the other mandatory flag to come after $(SPATCH_FLAGS), to ensure that our "--sp-file" overrides any provided in the environment, both --sp-file <arg> and --patch <arg> are last-option-wins as far as spatch(1) option parsing is concerned. The environment variable override was initially added in a9a884aea57 (coccicheck: use --all-includes by default, 2016-09-30). In practice there's probably nobody that's using SPATCH_FLAGS to try to intentionally break our invocations, but since we're changing this let's make it clear what (if anything) we expect to be overridden by user-supplied flags. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06ls-files: update test styleLi Linchao5-71/+132
Update test style in t/t30[*].sh for uniformity, that's to keep test title the same line with helper function itself, and fix some indentions. Add a new section "recommended style" in t/README to encourage people to use more modern style in test. Signed-off-by: Li Linchao <lilinchao@oschina.cn> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06merge-ort: fix issue with dual rename and add/add conflictElijah Newren2-3/+9
There is code in both merge-recursive and merge-ort for avoiding doubly transitive renames (i.e. one side renames directory A/ -> B/, and the other side renames directory B/ -> C/), because this combination would otherwise make a mess for new files added to A/ on the first side and wondering which directory they end up in -- especially if there were even more renames such as the first side renaming C/ -> D/. In such cases, it just turns "off" directory rename detection for the higher order transitive cases. The testcases added in t6423 a couple commits ago are slightly different but similar in principle. They involve a similar case of paired renaming but instead of A/ -> B/ and B/ -> C/, the second side renames a leading directory of B/ to C/. And both sides add a new file somewhere under the directory that the other side will rename. While the new files added start within different directories and thus could logically end up within different directories, it is weird for a file on one side to end up where the other one started and not move along with it. So, let's just turn off directory rename detection in this case as well. Another way to look at this is that if the source name involved in a directory rename on one side is the target name of a directory rename operation for a file from the other side, then we avoid the doubly transitive rename. (More concretely, if a directory rename on side D wants to rename a file on side E from OLD_NAME -> NEW_NAME, and side D already had a file named NEW_NAME, and a directory rename on side E wants to rename side D's NEW_NAME -> NEWER_NAME, then we turn off the directory rename detection for NEW_NAME to prevent the NEW_NAME -> NEWER_NAME rename, and instead end up with an add/add conflict on NEW_NAME.) Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06merge-ort: shuffle the computation and cleanup of potential collisionsElijah Newren1-5/+15
Run compute_collisions() for renames on both sides of history before any calls to collect_renames(), and do not free the computed collisions until after both calls to collect_renames(). This is just a code reorganization at this point that doesn't make sense on its own, but will permit us to use the computed collision info from both sides within each call to collect_renames() in a subsequent commit. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06merge-ort: make a separate function for freeing struct collisionsElijah Newren1-16/+22
This commit makes no functional changes, it's just some code movement in preparation for later changes. Signed-off-by: Elijah Newren <newren@palantir.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06merge-ort: small cleanups of check_for_directory_renameElijah Newren1-6/+6
No functional changes, just some preparatory cleanups. Suggested-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Elijah Newren <newren@palantir.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-06t6423: add tests of dual directory rename plus add/add conflictElijah Newren1-0/+105
This is an attempt at minimalizing a testcase reported by Glen Choo with tensorflow where merge-ort would report an assertion failure: Assertion failed: (ci->filemask == 2 || ci->filemask == 4), function apply_directory_rename_modifications, file merge-ort.c, line 2410 reversing the direction of the merge provides a different error: error: cache entry has null sha1: ... fatal: unable to write .git/index so we add testcases for both. With these new testcases, the recursive strategy differs in that it returns the latter error for both merge directions. These testcases are somehow a little different than Glen's original tensorflow testcase in that these ones trigger a bug with the recursive algorithm whereas his testcase didn't. I figure that means these testcases somehow manage to be more comprehensive. Reported-by: Glen Choo <chooglen@google.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-04Git 2.37.1v2.37.1Junio C Hamano3-2/+19
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-04Merge 'js/add-i-delete' into maint-2.37Junio C Hamano2-2/+19
Rewrite of "git add -i" in C that appeared in Git 2.25 didn't correctly record a removed file to the index, which is an old regression but has become widely known because the C version has become the default in the latest release. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-02A regression fix for 2.37Junio C Hamano3-2/+11
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-02Merge branch 'js/add-i-delete'Junio C Hamano2-2/+19
Rewrite of "git add -i" in C that appeared in Git 2.25 didn't correctly record a removed file to the index, which was fixed. * js/add-i-delete: add --interactive: allow `update` to stage deleted files
2022-07-01mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan3-9/+47
Originally, moving a <source> directory which is not on-disk due to its existence outside of sparse-checkout cone, "giv mv" command errors out with "bad source". Add a helper check_dir_in_index() function to see if a directory name exists in the index. Also add a SKIP_WORKTREE_DIR bit to mark such directories. Change the checking logic, so that such <source> directory makes "giv mv" command warns with "advise_on_updating_sparse_paths()" instead of "bad source"; also user now can supply a "--sparse" flag so this operation can be carried out successfully. Helped-by: Victoria Dye <vdye@github.com> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01mv: use flags mode for update_modeShaoxuan Yuan1-8/+17
As suggested by Derrick [1], move the in-line definition of "enum update_mode" to the top of the file and make it use "flags" mode (each state is a different bit in the word). Change the flag assignments from '=' (single assignment) to '|=' (additive). Also change flag evaluation from '==' to '&', etc. [1] https://lore.kernel.org/git/22aadea2-9330-aa9e-7b6a-834585189144@github.com/ Helped-by: Victoria Dye <vdye@github.com> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan2-5/+14
Originally, moving a sparse file into cone can result in unwarned overwrite of existing entry. The expected behavior is that if the <destination> exists in the entry, user should be prompted to supply a [-f|--force] to carry out the operation, or the operation should fail. Add a check mechanism to do that. Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan2-4/+21
Originally, moving a <source> file which is not on-disk but exists in index as a SKIP_WORKTREE enabled cache entry, "giv mv" command errors out with "bad source". Change the checking logic, so that such <source> file makes "giv mv" command warns with "advise_on_updating_sparse_paths()" instead of "bad source"; also user now can supply a "--sparse" flag so this operation can be carried out successfully. Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01mv: decouple if/else-if checks using gotoShaoxuan Yuan1-56/+77
Previous if/else-if chain are highly nested and hard to develop/extend. Refactor to decouple this if/else-if chain by using goto to jump ahead. Suggested-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01mv: update sparsity after moving from out-of-cone to in-coneShaoxuan Yuan1-0/+17
Originally, "git mv" a sparse file from out-of-cone to in-cone does not update the moved file's sparsity (remove its SKIP_WORKTREE bit). And the corresponding cache entry is, unexpectedly, not checked out in the working tree. Update the behavior so that: 1. Moving from out-of-cone to in-cone removes the SKIP_WORKTREE bit from corresponding cache entry. 2. The moved cache entry is checked out in the working tree to reflect the updated sparsity. Helped-by: Victoria Dye <vdye@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01t1092: mv directory from out-of-cone to in-coneShaoxuan Yuan1-0/+25
Add test for "mv: add check_dir_in_index() and solve general dir check issue" in this series. This change tests the following: 1. mv <source> as a directory on the sparse index boundary (where it would be a sparse directory in a sparse index). 2. mv <source> as a directory which is deeper than the boundary (so the sparse index would expand in the cache_name_pos() method). These tests can be written now for correctness, but later the first case can be updated to use the 'ensure_not_expanded' helper in t1092. Suggested-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01t7002: add tests for moving out-of-cone file/directoryShaoxuan Yuan1-0/+84
Add corresponding tests to test following situations: We do not have sufficient coverage of moving files outside of a sparse-checkout cone. Create new tests covering this behavior, keeping in mind that the user can include --sparse (or not), move a file or directory, and the destination can already exist in the index (in this case user can use --force to overwrite existing entry). Helped-by: Victoria Dye <vdye@github.com> Helped-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Shaoxuan Yuan <shaoxuan.yuan02@gmail.com> Acked-by: Derrick Stolee <derrickstolee@github.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool delta: fix a memory leakÆvar Arnfjörð Bjarmason2-7/+16
Fix a memory leak introduced in a310d434946 ([PATCH] Deltification library work by Nicolas Pitre., 2005-05-19), as a result we can mark another test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool ref-store: fix a memory leakÆvar Arnfjörð Bjarmason1-0/+1
Fix a memory leak introduced in fa099d23227 (worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe(), 2017-04-24), as a result we can mark another test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool bloom: fix memory leaksÆvar Arnfjörð Bjarmason2-1/+3
Fix memory leaks introduced with these tests in f1294eaf7fb (bloom.c: introduce core Bloom filter constructs, 2020-03-30), as a result we can mark almost the entirety of t0095-bloom.sh as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true", there's still an unrelated memory leak in "git commit" in one of the tests, let's skip that one under SANITIZE_LEAK for now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool json-writer: fix memory leaksÆvar Arnfjörð Bjarmason2-4/+14
Fix memory leaks introduced with these tests in 75459410edd (json_writer: new routines to create JSON data, 2018-07-13), as a result we can mark a test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool regex: call regfree(), fix memory leaksÆvar Arnfjörð Bjarmason2-3/+7
Fix memory leaks in "test-tool regex" which have been there since c91841594c2 (test-regex: Add a test to check for a bug in the regex routines, 2012-09-01), as a result we can mark a test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". We could regfree() on the die() paths here, which would make some invocations of valgrind(1) happy, but let's just target SANITIZE=leak for now. Variables that are still reachable when we die() are not reported as leaks. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool urlmatch-normalization: fix a memory leakÆvar Arnfjörð Bjarmason2-3/+10
Fix a memory leak in "test-tool urlmatch-normalization", as a result we can mark the corresponding test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool {dump,scrap}-cache-tree: fix memory leaksÆvar Arnfjörð Bjarmason3-1/+9
Fix memory leaks in two test-tools used by t0090-cache-tree.sh. As a result we can mark the test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool path-utils: fix a memory leakÆvar Arnfjörð Bjarmason2-4/+8
Fix a memory leak in "test-tool path-utils", as a result we can mark the corresponding test as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01test-tool test-hash: fix a memory leakÆvar Arnfjörð Bjarmason5-1/+9
Fix a memory leak in "test-tool test-hash" which has been there since b57cbbf8a86 (test-sha1: test hashing large buffer, 2006-06-24), as a result we can mark more tests as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01pull: fix a "struct oid_array" memory leakÆvar Arnfjörð Bjarmason4-7/+12
Fix a memory leak introduced in 44c175c7a46 (pull: error on no merge candidates, 2015-06-18). As a result we can mark several tests as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". Removing the "int ret = 0" assignment added here in a6d7eb2c7a6 (pull: optionally rebase submodules (remote submodule changes only), 2017-06-23) is not a logic error, it could always have been left uninitialized (as "int ret"), now that we'll use the "ret" from the upper scope we can drop the assignment in the "opt_rebase" branch. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01cat-file: fix a common "struct object_context" memory leakÆvar Arnfjörð Bjarmason13-14/+37
Fix a memory leak where "cat-file" will leak the "path" member. See e5fba602e59 (textconv: support for cat_file, 2010-06-15) for the code that introduced the offending get_oid_with_context() call (called get_sha1_with_context() at the time). As a result we can mark several tests as passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true". As noted in dc944b65f1d (get_sha1_with_context: dynamically allocate oc->path, 2017-05-19) callers must free the "path" member. That same commit added the relevant free() to this function, but we weren't catching cases where we'd return early. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01gc: fix a memory leakÆvar Arnfjörð Bjarmason1-1/+7
Fix a memory leak in code added in 41abfe15d95 (maintenance: add pack-refs task, 2021-02-09), we need to call strvec_clear() on the "struct strvec" that we initialized. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-01checkout: avoid "struct unpack_trees_options" leakÆvar Arnfjörð Bjarmason1-14/+22
In 1c41d2805e4 (unpack_trees_options: free messages when done, 2018-05-21) we started calling clear_unpack_trees_porcelain() on this codepath, but missed this error path. We could call clear_unpack_trees_porcelain() just before we error() and return when unmerged_cache() fails, but the more correct fix is to not have the unmerged_cache() check happen in the middle of our "topts" setup. Before 23cbf11b5c0 (merge-recursive: porcelain messages for checkout, 2010-08-11) we would not malloc() to setup our "topts", which is when this started to leak on the error path. Before that this code wasn't conflating the setup of "topts" and the unmerged_cache() call in any meaningful way. The initial version in 782c2d65c24 (Build in checkout, 2008-02-07) just does a "memset" of it, and initializes a single struct member. Then in 8ccba008ee3 (unpack-trees: allow Porcelain to give different error messages, 2008-05-17) we added the initialization of the error message, which as noted above finally started leaking in 23cbf11b5c0. Let's fix the memory leak, and avoid future issues by initializing the "topts" with a helper function. There are no functional changes here. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>