summaryrefslogtreecommitdiff
path: root/builtin/merge.c
AgeCommit message (Collapse)Author
2023-01-13merge: break out of all_strategy loop when strategy is foundSeija Kijin
Once we find a match, there is no point to try finding the second match in the inner loop. Break out of the loop once we find the first match. Signed-off-by: Seija Kijin <doremylover123@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-12-26Merge branch 'jk/unused-post-2.39'Junio C Hamano
Code clean-up around unused function parameters. * jk/unused-post-2.39: userdiff: mark unused parameter in internal callback list-objects-filter: mark unused parameters in virtual functions diff: mark unused parameters in callbacks xdiff: mark unused parameter in xdl_call_hunk_func() xdiff: drop unused parameter in def_ff() ws: drop unused parameter from ws_blank_line() list-objects: drop process_gitlink() function blob: drop unused parts of parse_blob_buffer() ls-refs: use repository parameter to iterate refs
2022-12-14Merge branch 'ab/various-leak-fixes'Junio C Hamano
Various leak fixes. * ab/various-leak-fixes: built-ins: use free() not UNLEAK() if trivial, rm dead code revert: fix parse_options_concat() leak cherry-pick: free "struct replay_opts" members rebase: don't leak on "--abort" connected.c: free the "struct packed_git" sequencer.c: fix "opts->strategy" leak in read_strategy_opts() ls-files: fix a --with-tree memory leak revision API: call graph_clear() in release_revisions() unpack-file: fix ancient leak in create_temp_file() built-ins & libs & helpers: add/move destructors, fix leaks dir.c: free "ident" and "exclude_per_dir" in "struct untracked_cache" read-cache.c: clear and free "sparse_checkout_patterns" commit: discard partial cache before (re-)reading it {reset,merge}: call discard_index() before returning tests: mark tests as passing with SANITIZE=leak
2022-12-13diff: mark unused parameters in callbacksJeff King
The diff code provides a format_callback interface, but not every callback needs each parameter (e.g., the "opt" and "data" parameters are frequently left unused). Likewise for the output_prefix callback, the low-level change/add_remove interfaces, the callbacks used by xdi_diff(), etc. Mark unused arguments in the callback implementations to quiet -Wunused-parameter. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-21{reset,merge}: call discard_index() before returningÆvar Arnfjörð Bjarmason
These two built-ins both deal with the index, but weren't discarding it. In subsequent commits we'll add more free()-ing to discard_index() that we've missed, but let's first call the existing function. We can doubtless add discard_index() (or its alias discard_cache()) to a lot more places, but let's just add it here for now. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-11-21cocci: apply "pending" index-compatibility to some "builtin/*.c"Ævar Arnfjörð Bjarmason
Apply "index-compatibility.pending.cocci" rule to "builtin/*", but exclude those where we conflict with in-flight changes. As a result some of them end up using only "the_index", so let's have them use the more narrow "USE_THE_INDEX_VARIABLE" rather than "USE_THE_INDEX_COMPATIBILITY_MACROS". Manual changes not made by coccinelle, that were squashed in: * Whitespace-wrap argument lists for repo_hold_locked_index(), repo_read_index_preload() and repo_refresh_and_write_index(), in cases where the line became too long after the transformation. * Change "refresh_cache()" to "refresh_index()" in a comment in "builtin/update-index.c". * For those whose call was followed by perror("<macro-name>"), change it to perror("<function-name>"), referring to the new function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-21cocci & cache.h: apply variable section of "pending" index-compatibilityÆvar Arnfjörð Bjarmason
Mostly apply the part of "index-compatibility.pending.cocci" that renames the global variables like "active_nr", which are a shorthand to referencing (in that case) a struct member as "the_index.cache_nr". In doing so move more of "index-compatibility.pending.cocci" to "index-compatibility.cocci". In the case of "active_nr" we'd have a textual conflict with "ab/various-leak-fixes" in "next"[1]. Let's exclude that specific case while moving the rule over from "pending". 1. 407b94280f8 (commit: discard partial cache before (re-)reading it, 2022-11-08) Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-21cocci & cache.h: apply a selection of "pending" index-compatibilityÆvar Arnfjörð Bjarmason
Apply a selection of rules in "index-compatibility.pending.cocci" tree-wide, and in doing so migrate them to "index-compatibility.cocci". As in preceding commits the only manual changes here are the macro removals in "cache.h", and the update to the '*.cocci" rules. The rest of the C code changes are the result of applying those updated rules. Move rules for some rarely used cache compatibility macros from "index-compatibility.pending.cocci" to "index-compatibility.cocci" and apply them. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-21read-cache API & users: make discard_index() return voidÆvar Arnfjörð Bjarmason
The discard_index() function has not returned non-zero since 7a51ed66f65 (Make on-disk index representation separate from in-core one, 2008-01-14), but we've had various code in-tree still acting as though that might be the case. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-11-21cocci & cache.h: remove rarely used "the_index" compat macrosÆvar Arnfjörð Bjarmason
Since 4aab5b46f44 (Make read-cache.c "the_index" free., 2007-04-01) we've been undergoing a slow migration away from these macros, but haven't made much progress since f8adbec9fea (cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch, 2019-01-24). Let's move forward a bit by changing the users of those macros that are rare enough that we can convert them in one go, and then remove the compatibility shim. The only manual change to the C code here is to "cache.h", the rest is all the result of applying the new "index-compatibility.cocci". Even though it's a one-off, let's keep the coccinelle rules for now. We'll extend them in subsequent commits, and this will help anything that's in-flight or out-of-tree to migrate. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-10-30use child_process members "args" and "env" directlyRené Scharfe
Build argument list and environment of child processes by using struct child_process and populating its members "args" and "env" directly instead of maintaining separate strvecs and letting run_command_v_opt() and friends populate these members. This is simpler, shorter and slightly more efficient. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30use child_process member "args" instead of string array variableRené Scharfe
Use run_command() with a struct child_process variable and populate its "args" member directly instead of building a string array and passing it to run_command_v_opt(). This avoids the use of magic index numbers and makes simplifies the possible addition of more arguments in the future. Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-10-30merge: remove always-the-same "verbose" argumentsÆvar Arnfjörð Bjarmason
Simplify the code that builds the arguments for the "read-tree" invocation in reset_hard() and read_empty() to remove the "verbose" parameter. Before 172b6428d06 (do not overwrite untracked during merge from unborn branch, 2010-11-14) there was a "reset_hard()" function that would be called in two places, one of those passed a "verbose=1", the other a "verbose=0". After 172b6428d06 when read_empty() was split off from reset_hard() both of these functions only had one caller. The "verbose" in read_empty() would always be false, and the one in reset_hard() would always be true. There was never a good reason for the code to act this way, it happened because the read_empty() function was a copy/pasted and adjusted version of reset_hard(). Since we're no longer conditionally adding the "-v" parameter here (and we'd only add it for "reset_hard()" we'll be able to move to a simpler and safer run-command API in the subsequent commit. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Taylor Blau <me@ttaylorr.com>
2022-09-01Merge branch 'en/merge-multi-strategies'Junio C Hamano
The code that implements multi-strategy support in "git merge" has been clean-up a bit. * en/merge-multi-strategies: merge: small code readability improvement merge: cleanup confusing logic for handling successful merges
2022-09-01Merge branch 'en/merge-unstash-only-on-clean-merge'Junio C Hamano
The auto-stashed local changes created by "git merge --autostash" was mixed into a conflicted state left in the working tree, which has been corrected. * en/merge-unstash-only-on-clean-merge: merge: only apply autostash when appropriate
2022-08-24merge: small code readability improvementElijah Newren
After our loop through the selected strategies, we compare best_strategy to wt_strategy. This is fine, but the fact that the code setting best_strategy sets it to use_strategies[i]->name requires a little bit of extra checking to determine that at the time of setting, that's the same as wt_strategy. Just setting best_strategy to wt_strategy makes it a little easier to verify what the loop is doing, at least for this reader. Further, use_strategies[i]->name is used in a number of places, where we could just use wt_strategy. The latter takes less time for this reader to parse (one variable name instead of three), so just use wt_strategy to make the code slightly faster for human readers to parse. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-24merge: cleanup confusing logic for handling successful mergesElijah Newren
builtin/merge.c has a loop over the specified strategies, where if they all fail with conflicts, it picks the one with the least number of conflicts. In the codepath that finds a successful merge, if an automatic commit was wanted, the code breaks out of the above loop, which makes sense. However, if the user requested there be no automatic commit, the loop would continue. That seems weird; --no-commit should not affect the choice of merge strategy, but the code as written makes one think it does. However, since the loop itself embeds "!merge_was_ok" as a condition on continuing to loop, it actually would also exit early if --no-commit was specified, it just exited from a different location. Restructure the code slightly to make it clear that the loop will immediately exit whenever we find a merge strategy that is successful. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-08-24merge: only apply autostash when appropriateElijah Newren
If a merge failed and we are leaving conflicts in the working directory for the user to resolve, we should not attempt to apply any autostash. Further, if we fail to apply the autostash (because either the merge failed, or the user requested --no-commit), then we should instruct the user how to apply it later. Add a testcase verifying we have corrected this behavior. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: do not exit restore_state() prematurelyElijah Newren
Previously, if the user: * Had no local changes before starting the merge * A merge strategy makes changes to the working tree/index but returns with exit status 2 Then we'd call restore_state() to clean up the changes and either let the next merge strategy run (if there is one), or exit telling the user that no merge strategy could handle the merge. Unfortunately, restore_state() did not clean up the changes as expected; that function was a no-op if the stash was a null, and the stash would be null if there were no local changes before starting the merge. So, instead of "Rewinding the tree to pristine..." as the code claimed, restore_state() would leave garbage around in the index and working tree (possibly including conflicts) for either the next merge strategy or for the user after aborting the merge. And in the case of aborting the merge, the user would be unable to run "git merge --abort" to get rid of the unintended leftover conflicts, because the merge control files were not written as it was presumed that we had restored to a clean state already. Fix the main problem by making sure that restore_state() only skips the stash application if the stash is null rather than skipping the whole function. However, there is a secondary problem -- since merge.c forks subprocesses to do the cleanup, the in-memory index is left out-of-sync. While there was a refresh_cache(REFRESH_QUIET) call that attempted to correct that, that function would not handle cases where the previous merge strategy added conflicted entries. We need to drop the index and re-read it to handle such cases. (Alternatively, we could stop forking subprocesses and instead call some appropriate function to do the work which would update the in-memory index automatically. For now, just do the simple fix.) Also, add a testcase checking this, one for which the octopus strategy fails on the first commit it attempts to merge, and thus which it cannot handle at all and must completely bail on (as per the "exit 2" code path of commit 98efc8f3d8 ("octopus: allow manual resolve on the last round.", 2006-01-13)). Reported-by: ZheNing Hu <adlternative@gmail.com> Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: ensure we can actually restore pre-merge stateElijah Newren
Merge strategies can: * succeed with a clean merge * succeed with a conflicted merge * fail to handle the given type of merge If one is thinking in terms of automatic mergeability, they would use the word "fail" instead of "succeed" for the second bullet, but I am focusing here on ability of the merge strategy to handle the given inputs, not on whether the given inputs are mergeable. The third category is about the merge strategy failing to know how to handle the given data; examples include: * Passing more than 2 branches to 'recursive' or 'ort' * Passing 2 or fewer branches to 'octopus' * Trying to do more complicated merges with 'resolve' (I believe directory/file conflicts will cause it to bail.) * Octopus running into a merge conflict for any branch OTHER than the final one (see the "exit 2" codepath of commit 98efc8f3d8 ("octopus: allow manual resolve on the last round.", 2006-01-13)) That final one is particularly interesting, because it shows that the merge strategy can muck with the index and working tree, and THEN bail and say "sorry, this strategy cannot handle this type of merge; use something else". Further, we do not currently expect the individual strategies to clean up after themselves, but instead expect builtin/merge.c to do so. For it to be able to, it needs to save the state before trying the merge strategy so it can have something to restore to. Therefore, remove the shortcut bypassing the save_state() call. There is another bug on the restore_state() side of things, so no testcase will be added until the next commit when we have addressed that issue as well. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: make restore_state() restore staged state tooElijah Newren
There are multiple issues at play here: 1) If `git merge` is invoked with staged changes, it should abort without doing any merging, and the user's working tree and index should be the same as before merge was invoked. 2) Merge strategies are responsible for enforcing the index == HEAD requirement. (See 9822175d2b ("Ensure index matches head before invoking merge machinery, round N", 2019-08-17) for some history around this.) 3) Merge strategies can bail saying they are not an appropriate handler for the merge in question (possibly allowing other strategies to be used instead). 4) Merge strategies can make changes to the index and working tree, and have no expectation to clean up after themselves, *even* if they bail out and say they are not an appropriate handler for the merge in question. (The `octopus` merge strategy does this, for example.) 5) Because of (3) and (4), builtin/merge.c stashes state before trying merge strategies and restores it afterward. Unfortunately, if users had staged changes before calling `git merge`, builtin/merge.c could do the following: * stash the changes, in order to clean up after the strategies * try all the merge strategies in turn, each of which report they cannot function due to the index not matching HEAD * restore the changes via "git stash apply" But that last step would have the net effect of unstaging the user's changes. Fix this by adding the "--index" option to "git stash apply". While at it, also squelch the stash apply output; we already report "Rewinding the tree to pristine..." and don't need a detailed `git status` report afterwards. Also while at it, switch to using strvec so folks don't have to count the arguments to ensure we avoided an off-by-one error, and so it's easier to add additional arguments to the command. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: fix save_state() to work when there are stat-dirty filesElijah Newren
When there are stat-dirty files, but no files are modified, `git stash create` exits with unsuccessful status. This causes merge to fail. Copy some code from sequencer.c's create_autostash to refresh the index first to avoid this problem. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: do not abort early if one strategy fails to handle the mergeElijah Newren
builtin/merge is setup to allow multiple strategies to be specified, and it will find the "best" result and use it. This is defeated if some of the merge strategies abort early when they cannot handle the merge. Fix the logic that calls recursive and ort to not do such an early abort, but instead return "2" or "unhandled" so that the next strategy can try to handle the merge. Coming up with a testcase for this is somewhat difficult, since recursive and ort both handle nearly any two-headed merge (there is a separate code path that checks for non-two-headed merges and already returns "2" for them). So use a somewhat synthetic testcase of having the index not match HEAD before the merge starts, since all merge strategies will abort for that. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-07-22merge: abort if index does not match HEAD for trivial mergesElijah Newren
As noted in the last commit and the links therein (especially commit 9822175d2b ("Ensure index matches head before invoking merge machinery, round N", 2019-08-17), we have had a very long history of problems with failing to enforce the requirement that index matches HEAD when starting a merge. The "trivial merge" logic in builtin/merge.c is yet another such case we previously missed. Add a check for it to ensure it aborts if the index does not match HEAD, and add a testcase where this fix is needed. Note that the fix here would also incidentally be an alternative fix for the testcase added in the last patch, but the fix in the last patch is still needed when multiple merge strategies are in use, so tweak the testcase from the previous commit so that it continues to exercise the codepath added in the last commit. Signed-off-by: Elijah Newren <newren@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ð Bjarmason
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-04-13revisions API users: add straightforward release_revisions()Ævar Arnfjörð Bjarmason
Add a release_revisions() to various users of "struct rev_list" in those straightforward cases where we only need to add the release_revisions() call to the end of a block, and don't need to e.g. refactor anything to use a "goto cleanup" pattern. 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ð Bjarmason
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ð Bjarmason
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-02-18Merge branch 'pw/use-in-process-checkout-in-rebase'Junio C Hamano
Use an internal call to reset_head() helper function instead of spawning "git checkout" in "rebase", and update code paths that are involved in the change. * pw/use-in-process-checkout-in-rebase: rebase -m: don't fork git checkout rebase --apply: set ORIG_HEAD correctly rebase --apply: fix reflog reset_head(): take struct rebase_head_opts rebase: cleanup reset_head() calls create_autostash(): remove unneeded parameter reset_head(): make default_reflog_action optional reset_head(): factor out ref updates reset_head(): remove action parameter rebase --apply: don't run post-checkout hook if there is an error rebase: do not remove untracked files on checkout rebase: pass correct arguments to post-checkout hook t5403: refactor rebase post-checkout hook tests rebase: factor out checkout for up to date branch
2022-02-09Merge branch 'en/plug-leaks-in-merge'Junio C Hamano
Leakfix. * en/plug-leaks-in-merge: merge: fix memory leaks in cmd_merge() merge-ort: fix memory leak in merge_ort_internal()
2022-02-09Merge branch 'ab/config-based-hooks-2'Junio C Hamano
More "config-based hooks". * ab/config-based-hooks-2: run-command: remove old run_hook_{le,ve}() hook API receive-pack: convert push-to-checkout hook to hook.h read-cache: convert post-index-change to use hook.h commit: convert {pre-commit,prepare-commit-msg} hook to hook.h git-p4: use 'git hook' to run hooks send-email: use 'git hook run' for 'sendemail-validate' git hook run: add an --ignore-missing flag hooks: convert worktree 'post-checkout' hook to hook library hooks: convert non-worktree 'post-checkout' hook to hook library merge: convert post-merge to use hook.h am: convert applypatch-msg to use hook.h rebase: convert pre-rebase to use hook.h hook API: add a run_hooks_l() wrapper am: convert {pre,post}-applypatch to use hook.h gc: use hook library for pre-auto-gc hook hook API: add a run_hooks() wrapper hook: add 'run' subcommand
2022-01-26create_autostash(): remove unneeded parameterPhillip Wood
The default_reflog parameter of create_autostash() is passed to reset_head(). However as creating a stash does not involve updating any refs the parameter is not used by reset_head(). Removing the parameter from create_autostash() simplifies the callers. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-21merge: fix memory leaks in cmd_merge()Elijah Newren
There were two commit_lists created in cmd_merge() that were only conditionally free()'d. Add a quick conditional call to free_commit_list() for each of them at the end of the function. Testing this commit against t6404 under valgrind shows that this patch fixes the following two leaks: 16 bytes in 1 blocks are definitely lost in loss record 16 of 126 at 0x484086F: malloc (vg_replace_malloc.c:380) by 0x69FFEB: do_xmalloc (wrapper.c:41) by 0x6A0073: xmalloc (wrapper.c:62) by 0x52A72D: commit_list_insert (commit.c:556) by 0x47FC93: reduce_parents (merge.c:1114) by 0x4801EE: collect_parents (merge.c:1214) by 0x480B56: cmd_merge (merge.c:1465) by 0x40686E: run_builtin (git.c:464) by 0x406C51: handle_builtin (git.c:716) by 0x406E96: run_argv (git.c:783) by 0x40730A: cmd_main (git.c:914) by 0x4E7DFA: main (common-main.c:56) 8 (16 direct, 32 indirect) bytes in 1 blocks are definitely lost in \ loss record 61 of 126 at 0x484086F: malloc (vg_replace_malloc.c:380) by 0x69FFEB: do_xmalloc (wrapper.c:41) by 0x6A0073: xmalloc (wrapper.c:62) by 0x52A72D: commit_list_insert (commit.c:556) by 0x52A8F2: commit_list_insert_by_date (commit.c:620) by 0x5270AC: get_merge_bases_many_0 (commit-reach.c:413) by 0x52716C: repo_get_merge_bases (commit-reach.c:438) by 0x480E5A: cmd_merge (merge.c:1520) by 0x40686E: run_builtin (git.c:464) by 0x406C51: handle_builtin (git.c:716) by 0x406E96: run_argv (git.c:783) by 0x40730A: cmd_main (git.c:914) There are still 3 leaks in chdir_notify_register() after this, but chdir_notify_register() has been brought up on the list before and folks were not a fan of fixing those, so I'm not touching them. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-10Merge branch 'ja/i18n-similar-messages'Junio C Hamano
Similar message templates have been consolidated so that translators need to work on fewer number of messages. * ja/i18n-similar-messages: i18n: turn even more messages into "cannot be used together" ones i18n: ref-filter: factorize "%(foo) atom used without %(bar) atom" i18n: factorize "--foo outside a repository" i18n: refactor "unrecognized %(foo) argument" strings i18n: factorize "no directory given for --foo" i18n: factorize "--foo requires --bar" and the like i18n: tag.c factorize i18n strings i18n: standardize "cannot open" and "cannot read" i18n: turn "options are incompatible" into "cannot be used together" i18n: refactor "%s, %s and %s are mutually exclusive" i18n: refactor "foo and bar are mutually exclusive"
2022-01-07merge: convert post-merge to use hook.hEmily Shaffer
Teach post-merge to use the hook.h library instead of the run-command.h library to run hooks. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Acked-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-05Merge branch 'jc/merge-detached-head-name'Junio C Hamano
The default merge message prepared by "git merge" records the name of the current branch; the name can be overridden with a new option to allow users to pretend a merge is made on a different branch. * jc/merge-detached-head-name: merge: allow to pretend a merge is made into a different branch
2022-01-05i18n: turn "options are incompatible" into "cannot be used together"Jean-Noël Avila
Signed-off-by: Jean-Noël Avila <jn.avila@free.fr> Reviewed-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-12-20merge: allow to pretend a merge is made into a different branchJunio C Hamano
When a series of patches for a topic-B depends on having topic-A, the workflow to prepare the topic-B branch would look like this: $ git checkout -b topic-B main $ git merge --no-ff --no-edit topic-A $ git am <mbox-for-topic-B When topic-A gets updated, recreating the first merge and rebasing the rest of the topic-B, all on detached HEAD, is a useful technique. After updating topic-A with its new round of patches: $ git checkout topic-B $ prev=$(git rev-parse 'HEAD^{/^Merge branch .topic-A. into}') $ git checkout --detach $prev^1 $ git merge --no-ff --no-edit topic-A $ git rebase --onto HEAD $prev @{-1}^0 $ git checkout -B @{-1} This will (0) check out the current topic-B. (1) find the previous merge of topic-A into topic-B. (2) detach the HEAD to the parent of the previous merge. (3) merge the updated topic-A to it. (4) reapply the patches to rebuild the rest of topic-B. (5) update topic-B with the result. without contaminating the reflog of topic-B too much. topic-B@{1} is the "logically previous" state before topic-A got updated, for example. At (4), comparison (e.g. range-diff) between HEAD and @{-1} is a meaningful way to sanity check the result, and the same can be done at (5) by comparing topic-B and topic-B@{1}. But there is one glitch. The merge into the detached HEAD done in the step (3) above gives us "Merge branch 'topic-A' into HEAD", and does not say "into topic-B". Teach the "--into-name=<branch>" option to "git merge" and its underlying "git fmt-merge-message", to pretend as if we were merging into <branch>, no matter what branch we are actually merging into, when they prepare the merge message. The pretend name honors the usual "into <target>" suppression mechanism, which can be seen in the tests added here. Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-11-25run-command API users: use strvec_pushl(), not argv constructionÆvar Arnfjörð Bjarmason
Change a pattern of hardcoding an "argv" array size, populating it and assigning to the "argv" member of "struct child_process" to instead use "strvec_pushl()" to add data to the "args" member. This implements the same behavior as before in fewer lines of code, and moves us further towards being able to remove the "argv" member in a subsequent commit. Since we've entirely removed the "argv" variable(s) we can be sure that no potential logic errors of the type discussed in a preceding commit are being introduced here, i.e. ones where the local "argv" was being modified after the assignment to "struct child_process"'s "argv". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-10-25Merge branch 'ab/mark-leak-free-tests-more'Junio C Hamano
Bunch of tests are marked as "passing leak check". * ab/mark-leak-free-tests-more: merge: add missing strbuf_release() ls-files: add missing string_list_clear() ls-files: fix a trivial dir_clear() leak tests: fix test-oid-array leak, test in SANITIZE=leak tests: fix a memory leak in test-oidtree.c tests: fix a memory leak in test-parse-options.c tests: fix a memory leak in test-prio-queue.c
2021-10-13Merge branch 'ab/config-based-hooks-1'Junio C Hamano
Mostly preliminary clean-up in the hook API. * ab/config-based-hooks-1: hook-list.h: add a generated list of hooks, like config-list.h hook.c users: use "hook_exists()" instead of "find_hook()" hook.c: add a hook_exists() wrapper and use it in bugreport.c hook.[ch]: move find_hook() from run-command.c to hook.c Makefile: remove an out-of-date comment Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H) Makefile: stop hardcoding {command,config}-list.h Makefile: mark "check" target as .PHONY
2021-10-13Merge branch 'en/removing-untracked-fixes'Junio C Hamano
Various fixes in code paths that move untracked files away to make room. * en/removing-untracked-fixes: Documentation: call out commands that nuke untracked files/directories Comment important codepaths regarding nuking untracked files/dirs unpack-trees: avoid nuking untracked dir in way of locally deleted file unpack-trees: avoid nuking untracked dir in way of unmerged file Change unpack_trees' 'reset' flag into an enum Remove ignored files by default when they are in the way unpack-trees: make dir an internal-only struct unpack-trees: introduce preserve_ignored to unpack_trees_options read-tree, merge-recursive: overwrite ignored files by default checkout, read-tree: fix leak of unpack_trees_options.dir t2500: add various tests for nuking untracked files
2021-10-07merge: add missing strbuf_release()Ævar Arnfjörð Bjarmason
We strbuf_reset() this "struct strbuf" in a loop earlier, but never freed it. Plugs a memory leak that's been here ever since this code got introduced in 1c7b76be7d6 (Build in merge, 2008-07-07). This takes us from 68 failed tests in "t7600-merge.sh" to 59 under SANITIZE=leak, and makes "t7604-merge-custom-message.sh" pass! Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27Remove ignored files by default when they are in the wayElijah Newren
Change several commands to remove ignored files by default when they are in the way. Since some commands (checkout, merge) take a --no-overwrite-ignore option to allow the user to configure this, and it may make sense to add that option to more commands (and in the case of merge, actually plumb that configuration option through to more of the backends than just the fast-forwarding special case), add little comments about where such flags would be used. Incidentally, this fixes a test failure in t7112. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27unpack-trees: introduce preserve_ignored to unpack_trees_optionsElijah Newren
Currently, every caller of unpack_trees() that wants to ensure ignored files are overwritten by default needs to: * allocate unpack_trees_options.dir * flip the DIR_SHOW_IGNORED flag in unpack_trees_options.dir->flags * call setup_standard_excludes AND then after the call to unpack_trees() needs to * call dir_clear() * deallocate unpack_trees_options.dir That's a fair amount of boilerplate, and every caller uses identical code. Make this easier by instead introducing a new boolean value where the default value (0) does what we want so that new callers of unpack_trees() automatically get the appropriate behavior. And move all the handling of unpack_trees_options.dir into unpack_trees() itself. While preserve_ignored = 0 is the behavior we feel is the appropriate default, we defer fixing commands to use the appropriate default until a later commit. So, this commit introduces several locations where we manually set preserve_ignored=1. This makes it clear where code paths were previously preserving ignored files when they should not have been; a future commit will flip these to instead use a value of 0 to get the behavior we want. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27hook.c users: use "hook_exists()" instead of "find_hook()"Ævar Arnfjörð Bjarmason
Use the new hook_exists() function instead of find_hook() where the latter was called in boolean contexts. This make subsequent changes in a series where we further refactor the hook API clearer, as we won't conflate wanting to get the path of the hook with checking for its existence. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-27hook.[ch]: move find_hook() from run-command.c to hook.cÆvar Arnfjörð Bjarmason
Move the find_hook() function from run-command.c to a new hook.c library. This change establishes a stub library that's pretty pointless right now, but will see much wider use with Emily Shaffer's upcoming "configuration-based hooks" series. Eventually all the hook related code will live in hook.[ch]. Let's start that process by moving the simple find_hook() function over as-is. Signed-off-by: Emily Shaffer <emilyshaffer@google.com> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-09-20Merge branch 'js/run-command-close-packs'Junio C Hamano
The run-command API has been updated so that the callers can easily ask the file descriptors open for packfiles to be closed immediately before spawning commands that may trigger auto-gc. * js/run-command-close-packs: Close object store closer to spawning child processes run_auto_maintenance(): implicitly close the object store run-command: offer to close the object store before running run-command: prettify the `RUN_COMMAND_*` flags pull: release packs before fetching commit-graph: when closing the graph, also release the slab
2021-09-20Merge branch 'ds/mergies-with-sparse-index'Junio C Hamano
Various mergy operations have been prepared to work efficiently with the sparse index. * ds/mergies-with-sparse-index: sparse-index: integrate with cherry-pick and rebase sequencer: ensure full index if not ORT strategy t1092: add cherry-pick, rebase tests merge-ort: expand only for out-of-cone conflicts merge: make sparse-aware with ORT diff: ignore sparse paths in diffstat
2021-09-10Merge branch 'ab/retire-advice-config'Junio C Hamano
Code clean up to migrate callers from older advice_config[] based API to newer advice_if_enabled() and advice_enabled() API. * ab/retire-advice-config: advice: move advice.graftFileDeprecated squashing to commit.[ch] advice: remove use of global advice_add_embedded_repo advice: remove read uses of most global `advice_` variables advice: add enum variants for missing advice variables