summaryrefslogtreecommitdiffstats
path: root/builtin (follow)
Commit message (Collapse)AuthorAgeFilesLines
* Merge branch 'js/shortlog-sort-stably'Junio C Hamano2022-07-231-1/+1
|\ | | | | | | | | | | | | | | "git shortlog -n" relied on the underlying qsort() to be stable, which shouldn't have. Fixed. * js/shortlog-sort-stably: shortlog: use a stable sort
| * shortlog: use a stable sortJohannes Schindelin2022-07-141-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When sorting the output of `git shortlog` by count, a list of authors in alphabetical order is then sorted by contribution count. Obviously, the idea is to maintain the alphabetical order for items with identical contribution count. At the moment, this job is performed by `qsort()`. As that function is not guaranteed to implement a stable sort algorithm, this can lead to inconsistent and/or surprising behavior: items with identical contribution count could lose their alphabetical sub-order. The `qsort()` in MS Visual C's runtime does _not_ implement a stable sort algorithm, and under certain circumstances this even causes a test failure in t4201.21 "shortlog can match multiple groups", where two authors both are listed with 2 contributions, and are listed in inverse alphabetical order. Let's instead use the stable sort provided by `git_stable_qsort()` to avoid this inconsistency. This is a companion to 2049b8dc65 (diffcore_rename(): use a stable sort, 2019-09-30). Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | Merge branch 'jk/diff-files-cleanup-fix'Junio C Hamano2022-07-201-1/+1
|\ \ | | | | | | | | | | | | | | | | | | | | | An earlier attempt to plug leaks placed a clean-up label to jump to at a bogus place, which as been corrected. * jk/diff-files-cleanup-fix: diff-files: move misplaced cleanup label
| * | diff-files: move misplaced cleanup labelJeff King2022-07-121-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Commit 0139c58ab9 (revisions API users: add "goto cleanup" for release_revisions(), 2022-04-13) converted an early return in cmd_diff_files() into a goto. But it put the cleanup label too early: if read_cache_preload() returns an error, we'll set result to "-1", but then jump to calling run_diff_files(), overwriting our result. We should jump past the call to run_diff_files(). Likewise, we should go past diff_result_code(), which is expecting to see a code from an actual diff, not a negative error code. In practice, I suspect this bug cannot actually be triggered, because read_cache_preload() does not seem to ever return an error. Its return value (eventually) comes from do_read_index(), which gives the number of cache entries found, and calls die() on error. Still, it makes sense to fix the inadvertent change from 0139c58ab9 first, and we can look into the overall error handling of read_cache() separately (which is present in many other callsites). Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | Merge branch 'jk/clone-unborn-confusion'Junio C Hamano2022-07-201-36/+45
|\ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git clone" from a repository with some ref whose HEAD is unborn did not set the HEAD in the resulting repository correctly, which has been corrected. * jk/clone-unborn-confusion: clone: move unborn head creation to update_head() clone: use remote branch if it matches default HEAD clone: propagate empty remote HEAD even with other branches clone: drop extra newline from warning message
| * | | clone: move unborn head creation to update_head()Jeff King2022-07-111-12/+15
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Prior to 4f37d45706 (clone: respect remote unborn HEAD, 2021-02-05), creation of the local HEAD was always done in update_head(). That commit added code to handle an unborn head in an empty repository, and just did all symref creation and config setup there. This makes the code flow a little bit confusing, especially as new corner cases have been covered (like the previous commit to match our default branch name to a non-HEAD remote branch). Let's move the creation of the unborn symref into update_head(). This matches the other HEAD-creation cases, and now the logic is consistently separated: the main cmd_clone() function only examines the situation and sets variables based on what it finds, and update_head() actually performs the update. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | clone: use remote branch if it matches default HEADJeff King2022-07-081-3/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | clone: propagate empty remote HEAD even with other branchesJeff King2022-07-081-22/+17
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | clone: drop extra newline from warning messageJeff King2022-07-081-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>
* | | | Merge branch 'jc/resolve-undo'Junio C Hamano2022-07-201-0/+39
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The resolve-undo information in the index was not protected against GC, which has been corrected. * jc/resolve-undo: fsck: do not dereference NULL while checking resolve-undo data revision: mark blobs needed for resolve-undo as reachable
| * | | | fsck: do not dereference NULL while checking resolve-undo dataJunio C Hamano2022-07-121-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When we found an invalid object recorded in the resolve-undo data, we would have ended up dereferencing NULL while fsck. Reporting the problem and going on to the next object is the right thing to do here. Noticed by SZEDER Gábor. Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | revision: mark blobs needed for resolve-undo as reachableJunio C Hamano2022-06-101-0/+38
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The resolve-undo extension was added to the index in cfc5789a (resolve-undo: record resolved conflicts in a new index extension section, 2009-12-25). This extension records the blob object names and their modes of conflicted paths when the path gets resolved (e.g. with "git add"), to allow "undoing" the resolution with "checkout -m path". These blob objects should be guarded from garbage-collection while we have the resolve-undo information in the index (otherwise unresolve operation may try to use a blob object that has already been pruned away). But the code called from mark_reachable_objects() for the index forgets to do so. Teach add_index_objects_to_pending() helper to also add objects referred to by the resolve-undo extension. Also make matching changes to "fsck", which has code that is fairly similar to the reachability stuff, but have parallel implementations for all these stuff, which may (or may not) someday want to be unified. Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | Merge branch 'sg/multi-pack-index-parse-options-fix'Junio C Hamano2022-07-181-4/+4
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The way "git multi-pack" uses parse-options API has been improved. * sg/multi-pack-index-parse-options-fix: multi-pack-index: simplify handling of unknown --options
| * | | | multi-pack-index: simplify handling of unknown --optionsSZEDER Gábor2022-07-101-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>
* | | | Merge branch 'ab/cocci-unused'Junio C Hamano2022-07-183-8/+1
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add Coccinelle rules to detect the pattern of initializing and then finalizing a structure without using it in between at all, which happens after code restructuring and the compilers fail to recognize as an unused variable. * ab/cocci-unused: cocci: generalize "unused" rule to cover more than "strbuf" cocci: add and apply a rule to find "unused" strbufs cocci: have "coccicheck{,-pending}" depend on "coccicheck-test" cocci: add a "coccicheck-test" target and test *.cocci rules Makefile & .gitignore: ignore & clean "git.res", not "*.res" Makefile: remove mandatory "spatch" arguments from SPATCH_FLAGS
| * | | | cocci: generalize "unused" rule to cover more than "strbuf"Ævar Arnfjörð Bjarmason2022-07-061-2/+0
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | cocci: add and apply a rule to find "unused" strbufsÆvar Arnfjörð Bjarmason2022-07-062-6/+1
| |/ / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
* | | | Merge branch 'gc/submodule-use-super-prefix'Junio C Hamano2022-07-181-64/+22
|\ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Another step to rewrite more parts of "git submodule" in C. * gc/submodule-use-super-prefix: submodule--helper: remove display path helper submodule--helper update: use --super-prefix submodule--helper: remove unused SUPPORT_SUPER_PREFIX flags submodule--helper: use correct display path helper submodule--helper: don't recreate recursive prefix submodule--helper update: use display path helper submodule--helper tests: add missing "display path" coverage
| * | | | submodule--helper: remove display path helperGlen Choo2022-07-011-16/+8
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | All invocations of do_get_submodule_displaypath() pass get_super_prefix() as the super_prefix arg, which is exactly the same as get_submodule_displaypath(). Replace all calls to do_get_submodule_displaypath() with get_submodule_displaypath(), and since it has no more callers, remove it. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper update: use --super-prefixGlen Choo2022-07-011-20/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unlike the other subcommands, "git submodule--helper update" uses the "--recursive-prefix" flag instead of "--super-prefix". The two flags are otherwise identical (they only serve to compute the 'display path' of a submodule), except that there is a dedicated helper function to get the value of "--super-prefix". This inconsistency exists because "git submodule update" used to pass "--recursive-prefix" between shell and C (introduced in [1]) before "--super-prefix" was introduced (in [2]), and for simplicity, we kept this name when "git submodule--helper update" was created. Remove "--recursive-prefix" and its associated code from "git submodule--helper update", replacing it with "--super-prefix". To use "--super-prefix", module_update is marked with SUPPORT_SUPER_PREFIX. Note that module_clone must also be marked with SUPPORT_SUPER_PREFIX, otherwise the "git submodule--helper clone" subprocess will fail check because "--super-prefix" is propagated via the environment. [1] 48308681b0 (git submodule update: have a dedicated helper for cloning, 2016-02-29) [2] 74866d7579 (git: make super-prefix option, 2016-10-07) Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: remove unused SUPPORT_SUPER_PREFIX flagsÆvar Arnfjörð Bjarmason2022-07-011-3/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove the SUPPORT_SUPER_PREFIX flag from "add", "init" and "summary". For the "add" command it hasn't been used since [1], likewise for "init" and "summary" since [2] and [3], respectively. As implemented in 74866d75793 (git: make super-prefix option, 2016-10-07) the SUPPORT_SUPER_PREFIX flag in git.c applies for the entire command, but as implemented in 89c86265576 (submodule helper: support super prefix, 2016-12-08) we assert here in cmd_submodule__helper() that we're not getting the flag unexpectedly. 1. 8c8195e9c3e (submodule--helper: introduce add-clone subcommand, 2021-07-10) 2. 6e7c14e65c8 (submodule update --init: display correct path from submodule, 2017-01-06) 3. 1cf823d8f00 (submodule: remove unnecessary `prefix` based option logic, 2021-06-22) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: use correct display path helperGlen Choo2022-07-011-11/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace a chunk of code in update_submodule() with an equivalent do_get_submodule_displaypath() invocation. This is already tested by t/t7406-submodule-update.sh:'submodule update --init --recursive from subdirectory', so no tests are added. The two are equivalent because: - Exactly one of recursive_prefix|prefix is non-NULL at a time; prefix is set at the superproject level, and recursive_prefix is set when recursing into submodules. There is also a BUG() statement in get_submodule_displaypath() that asserts that both cannot be non-NULL. - In get_submodule_displaypath(), get_super_prefix() always returns NULL because "--super-prefix" is never passed. Thus calling it is equivalent to calling do_get_submodule_displaypath() with super_prefix = NULL. Therefore: - When recursive_prefix is non-NULL, prefix is NULL, and thus get_submodule_displaypath() just returns prefixed_path. This is identical to calling do_get_submodule_displaypath() with super_prefix = recursive_prefix because the return value is still the concatenation of recursive_prefix + update_data->sm_path. - When prefix is non-NULL, prefixed_path = update_data->sm_path. Thus calling get_submodule_displaypath() with prefixed_path is equivalent to calling do_get_submodule_displaypath() with update_data->sm_path Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: don't recreate recursive prefixGlen Choo2022-07-011-11/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | update_submodule() uses duplicated code to compute update_data->displaypath and next.recursive_prefix. The latter is just the former with "/" appended to it, and since update_data->displaypath not changed outside of this statement, we can just reuse the already computed result. We can go one step further and remove the reference to next.recursive_prefix altogether. Since it is only used in update_data_to_args() (to compute the "--recursive-prefix" flag for the recursive update child process) we can just use the already computed .displaypath value of there. Delete the duplicated code, and remove the unnecessary reference to next.recursive_prefix. As a bonus, this fixes a memory leak where prefixed_path was never freed (this leak was first reported in [1]). [1] https://lore.kernel.org/git/877a45867ae368bf9e053caedcb6cf421e02344d.1655336146.git.gitgitgadget@gmail.com Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper update: use display path helperGlen Choo2022-07-011-14/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | There are two locations in prepare_to_clone_next_submodule() that manually calculate the submodule display path, but should just use do_get_submodule_displaypath() for consistency. Do this replacement and reorder the code slightly to avoid computing the display path twice. Until the preceding commit this code had never been tested, with our newly added tests we can see that both these sites have been computing the display path incorrectly ever since they were introduced in 48308681b0 (git submodule update: have a dedicated helper for cloning, 2016-02-29) [1]: - The first hunk puts a "/" between recursive_prefix and ce->name, but recursive_prefix already ends with "/". - The second hunk calls relative_path() on recursive_prefix and ce->name, but relative_path() only makes sense when both paths share the same base directory. This is never the case here: - recursive_prefix is the path from the topmost superproject to the current submodule - ce->name is the path from the root of the current submodule to its submodule. so, e.g. recursive_prefix="super" and ce->name="submodule" produces displayname="../super" instead of "super/submodule". [1] I verified this by applying the tests to 48308681b0. Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | Merge branch 'ab/submodule-cleanup' into gc/submodule-use-super-prefixJunio C Hamano2022-07-011-24/+50
| |\ \ \ \ | | |/ / / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | * ab/submodule-cleanup: git-sh-setup.sh: remove "say" function, change last users git-submodule.sh: use "$quiet", not "$GIT_QUIET" submodule--helper: eliminate internal "--update" option submodule--helper: understand --checkout, --merge and --rebase synonyms submodule--helper: report "submodule" as our name in some "-h" output submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs" submodule update: remove "-v" option submodule--helper: have --require-init imply --init git-submodule.sh: remove unused top-level "--branch" argument git-submodule.sh: make the "$cached" variable a boolean git-submodule.sh: remove unused $prefix variable git-submodule.sh: remove unused sanitize_submodule_env()
* | | | | Merge branch 'ab/leakfix'Junio C Hamano2022-07-188-48/+92
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Plug various memory leaks. * ab/leakfix: pull: fix a "struct oid_array" memory leak cat-file: fix a common "struct object_context" memory leak gc: fix a memory leak checkout: avoid "struct unpack_trees_options" leak merge-file: fix memory leaks on error path merge-file: refactor for subsequent memory leak fix cat-file: fix a memory leak in --batch-command mode revert: free "struct replay_opts" members submodule.c: free() memory from xgetcwd() clone: fix memory leak in wanted_peer_refs() check-ref-format: fix trivial memory leak
| * | | | | pull: fix a "struct oid_array" memory leakÆvar Arnfjörð Bjarmason2022-07-011-6/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | cat-file: fix a common "struct object_context" memory leakÆvar Arnfjörð Bjarmason2022-07-011-10/+22
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | gc: fix a memory leakÆvar Arnfjörð Bjarmason2022-07-011-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>
| * | | | | checkout: avoid "struct unpack_trees_options" leakÆvar Arnfjörð Bjarmason2022-07-011-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>
| * | | | | merge-file: fix memory leaks on error pathÆvar Arnfjörð Bjarmason2022-07-011-4/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak in "merge-file", we need to loop over the "mmfs" array and free() what we've got so far when we error out. 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>
| * | | | | merge-file: refactor for subsequent memory leak fixÆvar Arnfjörð Bjarmason2022-07-011-12/+14
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Refactor the code in builtin/merge-file.c to: * Use the initializer to zero out "mmfs", and use modern C syntax for the rest. * Refactor the the inner loop to use a variable and "if/else if" pattern followed by "return". This will make a change to change it to a "goto cleanup" pattern smaller. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | cat-file: fix a memory leak in --batch-command modeÆvar Arnfjörð Bjarmason2022-07-011-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak introduced in 440c705ea63 (cat-file: add --batch-command mode, 2022-02-18). The free_cmds() function was only called on "queued_nr" if we had a "flush" command. As the "without flush for blob info" test added in the same commit shows we can't rely on that, so let's call free_cmds() again at the end. Since "nr" follows the usual pattern of being set to 0 if we've free()'d the memory already it's OK to call it twice, even in cases where we are doing a "flush". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | revert: free "struct replay_opts" membersÆvar Arnfjörð Bjarmason2022-07-011-0/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Call the release_revisions() function added in 1878b5edc03 (revision.[ch]: provide and start using a release_revisions(), 2022-04-13) in cmd_revert(), as well as freeing the xmalloc()'d "revs" member itself. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | clone: fix memory leak in wanted_peer_refs()Ævar Arnfjörð Bjarmason2022-07-011-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak added in 0ec4b1650cc (clone: fix ref selection in --single-branch --branch=xxx, 2012-06-22). Whether we get our "remote_head" from copy_ref() directly, or with a call to guess_remote_head() it'll be the result of a copy_ref() in either case, as guess_remote_head() is a wrapper for copy_ref() (or it returns NULL). We can't mark any tests passing passing with SANITIZE=leak using "TEST_PASSES_SANITIZE_LEAK=true" as a result of this change, but e.g. "t/t1500-rev-parse.sh" now gets closer to passing. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | | check-ref-format: fix trivial memory leakÆvar Arnfjörð Bjarmason2022-07-011-3/+8
| |/ / / / | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Fix a memory leak in "git check-ref-format" that's been present in the code in one form or another since 38eedc634bc (git check-ref-format --print, 2009-10-12), the code got substantially refactored in cfbe22f03f9 (check-ref-format: handle subcommands in separate functions, 2010-08-05). 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>
* | | | | Merge branch 'jc/builtin-mv-move-array'Junio C Hamano2022-07-181-9/+7
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Apply Coccinelle rule to turn raw memmove() into MOVE_ARRAY() cpp macro, which would improve maintainability and readability. * jc/builtin-mv-move-array: builtin/mv.c: use the MOVE_ARRAY() macro instead of memmove()
| * | | | | builtin/mv.c: use the MOVE_ARRAY() macro instead of memmove()Junio C Hamano2022-07-101-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>
* | | | | Merge branch 'ab/submodule-cleanup'Junio C Hamano2022-07-151-24/+50
|\ \ \ \ \ | | |_|/ / | |/| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Further preparation to turn git-submodule.sh into a builtin. * ab/submodule-cleanup: git-sh-setup.sh: remove "say" function, change last users git-submodule.sh: use "$quiet", not "$GIT_QUIET" submodule--helper: eliminate internal "--update" option submodule--helper: understand --checkout, --merge and --rebase synonyms submodule--helper: report "submodule" as our name in some "-h" output submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs" submodule update: remove "-v" option submodule--helper: have --require-init imply --init git-submodule.sh: remove unused top-level "--branch" argument git-submodule.sh: make the "$cached" variable a boolean git-submodule.sh: remove unused $prefix variable git-submodule.sh: remove unused sanitize_submodule_env()
| * | | | submodule--helper: eliminate internal "--update" optionGlen Choo2022-06-281-20/+13
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Follow-up on the preceding commit which taught "git submodule--helper update" to understand "--merge", "--checkout" and "--rebase" and use those options instead of "--update=(rebase|merge|checkout|none)" when the command invokes itself. Unlike the preceding change this isn't strictly necessary to eventually change "git-submodule.sh" so that it invokes "git submodule--helper update" directly, but let's remove this inconsistency in the command-line interface. We shouldn't need to carry special synonyms for existing options in "git submodule--helper" when that command can use the primary documented names instead. But, as seen in the post-image this makes the control flow within "builtin/submodule--helper.c" simpler, we can now write directly to the "update_default" member of "struct update_data" when parsing the options in "module_update()". Signed-off-by: Glen Choo <chooglen@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: understand --checkout, --merge and --rebase synonymsÆvar Arnfjörð Bjarmason2022-06-281-0/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Understand --checkout, --merge and --rebase synonyms for --update={checkout,merge,rebase}, as well as the short options that 'git submodule' itself understands. This removes a difference between the CLI API of "git submodule" and "git submodule--helper", making it easier to make the latter an alias for the former. See 48308681b07 (git submodule update: have a dedicated helper for cloning, 2016-02-29) for the initial addition of --update. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: report "submodule" as our name in some "-h" outputÆvar Arnfjörð Bjarmason2022-06-281-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Change the user-facing "git submodule--helper" commands so that they'll report their name as being "git submodule". To a user these commands are internal implementation details, and it doesn't make sense to emit usage about an internal helper when "git submodule" is invoked with invalid options. Before this we'd emit e.g.: $ git submodule absorbgitdirs --blah error: unknown option `blah' usage: git submodule--helper absorbgitdirs [<options>] [<path>...] [...] And: $ git submodule set-url -- -- usage: git submodule--helper set-url [--quiet] <path> <newurl> [...] Now we'll start with "usage: git submodule [...]" in both of those cases. This change does not alter the "list", "name", "clone", "config" and "create-branch" commands, those are internal-only (as an aside; their usage info should probably invoke BUG(...)). This only changes the user-facing commands. The "status", "deinit" and "update" commands are not included in this change, because their usage information already used "submodule" rather than "submodule--helper". I don't think it's currently possible to emit some of this usage information in practice, as git-submodule.sh will catch unknown options, and e.g. it doesn't seem to be possible to get "add" to emit its usage information from "submodule--helper". Though that change may be superfluous now, it's also harmless, and will allow us to eventually dispatch further into "git submodule--helper" from git-submodule.sh, while emitting the correct usage output. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: rename "absorb-git-dirs" to "absorbgitdirs"Ævar Arnfjörð Bjarmason2022-06-281-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Rename the "absorb-git-dirs" subcommand to "absorbgitdirs", which is what the "git submodule" command itself has called it since the subcommand was implemented in f6f85861400 (submodule: add absorb-git-dir function, 2016-12-12). Having these two be different will make it more tedious to dispatch to eventually dispatch "git submodule--helper" directly, as we'd need to retain this name mapping. So let's get rid of this needless inconsistency. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
| * | | | submodule--helper: have --require-init imply --initÆvar Arnfjörð Bjarmason2022-06-281-1/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Adjust code added in 0060fd1511b (clone --recurse-submodules: prevent name squatting on Windows, 2019-09-12) to have the internal --require-init option imply --init, rather than having "git-submodule.sh" add it implicitly. This change doesn't make any difference now, but eliminates another special-case where "git submodule--helper update"'s behavior was different from "git submodule update". This will make it easier to eventually replace the cmd_update() function in git-submodule.sh. We'll still need to keep the distinction between "--init" and "--require-init" in git-submodule.sh. Once cmd_update() gets re-implemented in C we'll be able to change variables and other code related to that, but not yet. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
* | | | | Merge branch 'sy/mv-out-of-cone'Junio C Hamano2022-07-151-64/+175
|\ \ \ \ \ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | "git mv A B" in a sparsely populated working tree can be asked to move a path between directories that are "in cone" (i.e. expected to be materialized in the working tree) and "out of cone" (i.e. expected to be hidden). The handling of such cases has been improved. * sy/mv-out-of-cone: mv: add check_dir_in_index() and solve general dir check issue mv: use flags mode for update_mode mv: check if <destination> exists in index to handle overwriting mv: check if out-of-cone file exists in index with SKIP_WORKTREE bit mv: decouple if/else-if checks using goto mv: update sparsity after moving from out-of-cone to in-cone t1092: mv directory from out-of-cone to in-cone t7002: add tests for moving out-of-cone file/directory
| * | | | | mv: add check_dir_in_index() and solve general dir check issueShaoxuan Yuan2022-07-011-6/+44
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | mv: use flags mode for update_modeShaoxuan Yuan2022-07-011-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>
| * | | | | mv: check if <destination> exists in index to handle overwritingShaoxuan Yuan2022-07-011-3/+12
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | mv: check if out-of-cone file exists in index with SKIP_WORKTREE bitShaoxuan Yuan2022-07-011-2/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>
| * | | | | mv: decouple if/else-if checks using gotoShaoxuan Yuan2022-07-011-59/+80
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | 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>