From: "Seija K. via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Seija K <pi1024e@github.com>
Subject: [PATCH v3] Simplify merge logic
Date: Tue, 17 Nov 2020 22:06:01 +0000 [thread overview]
Message-ID: <pull.911.v3.git.git.1605650762205.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.911.v2.git.git.1605650072908.gitgitgadget@gmail.com>
From: Seija K <pi1024e@github.com>
commit: Avoid extraneous boolean checking by simplifying the if statements.
This is so that the code can be clearer and we can avoid duplicate checks.
Changes since v1: Undid if statement combos as suggested by Junio C Hamano.
Currently, the logic for merging is somewhat confusing.
So I simplified said logic to be equivalent, but simpler.
I tested all my changes to ensure in practice they work as well.
First, I tested out which branch would occur for every possible value below:
remoteheads->next | common->next | option_commit | Branch
-- | -- | -- | --
T | T | T | A
T | T | F | A
T | F | T | A
T | F | F | A
F | T | T | C
F | T | F | C
F | F | T | B
F | F | F | A
Then, using this truth table, I was able to deduce that if the second branch ran,
the if statement for the first branch would be false.
Then, taking the inverse, I found that many of the checks were redundant,
so I rewrote the if statements to have each branch run under the same exact conditions,
except each value is evaluated as little as possible.
I hope you find value in these changes.
Thank you!
Signed-off-by: Seija K. <pi1024e@github.com>
---
Simplify merge logic
commit: Avoid extraneous boolean checking by simplifying the if
statements. This is so that the code can be clearer and we can avoid
duplicate checks.
Changes since v1: Undid if statement combos as suggested by Junio C
Hamano.
Currently, the logic for merging is somewhat confusing. So I simplified
said logic to be equivalent, but simpler. I tested all my changes to
ensure in practice they work as well.
First, I tested out which branch would occur for every possible value
below:
remoteheads->nextcommon->nextoption_commitBranchTTTATTFATFTATFFAFTTCFTFC
FFTBFFFAThen, using this truth table, I was able to deduce that if the
second branch ran, the if statement for the first branch would be false.
Then, taking the inverse, I found that many of the checks were
redundant, so I rewrote the if statements to have each branch run under
the same exact conditions, except each value is evaluated as little as
possible.
I hope you find value in these changes.
Thank you!
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-911%2Fpi1024e%2Fmerge-cleanup-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-911/pi1024e/merge-cleanup-v3
Pull-Request: https://github.com/git/git/pull/911
Range-diff vs v2:
1: dd1c4dd678 ! 1: 92e0b7ec32 Simplify merge logic
@@ Commit message
Simplify merge logic
commit: Avoid extraneous boolean checking by simplifying the if statements.
- This is so that the code can be clearer and avoid duplicate boolean checks.
+ This is so that the code can be clearer and we can avoid duplicate checks.
Changes since v1: Undid if statement combos as suggested by Junio C Hamano.
- The current logic for the merging is somewhat confusing.
+ Currently, the logic for merging is somewhat confusing.
So I simplified said logic to be equivalent, but simpler.
I tested all my changes to ensure in practice they work as well.
builtin/merge.c | 82 +++++++++++++++++++++++--------------------------
1 file changed, 38 insertions(+), 44 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 4c133402a6..148d08f8db 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -1213,7 +1213,7 @@ static int merging_a_throwaway_tag(struct commit *commit)
if (!merge_remote_util(commit) ||
!merge_remote_util(commit)->obj ||
merge_remote_util(commit)->obj->type != OBJ_TAG)
- return is_throwaway_tag;
+ return 0;
/*
* Now we know we are merging a tag object. Are we downstream
@@ -1460,12 +1460,13 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
}
if (!use_strategies) {
- if (!remoteheads)
- ; /* already up-to-date */
- else if (!remoteheads->next)
- add_strategies(pull_twohead, DEFAULT_TWOHEAD);
- else
- add_strategies(pull_octopus, DEFAULT_OCTOPUS);
+ if (remoteheads) {
+ /* not up-to-date */
+ if (remoteheads->next)
+ add_strategies(pull_octopus, DEFAULT_OCTOPUS);
+ else
+ add_strategies(pull_twohead, DEFAULT_TWOHEAD);
+ }
}
for (i = 0; i < use_strategies_nr; i++) {
@@ -1475,15 +1476,15 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
allow_trivial = 0;
}
- if (!remoteheads)
- ; /* already up-to-date */
- else if (!remoteheads->next)
- common = get_merge_bases(head_commit, remoteheads->item);
- else {
- struct commit_list *list = remoteheads;
- commit_list_insert(head_commit, &list);
- common = get_octopus_merge_bases(list);
- free(list);
+ if (remoteheads) {
+ /* not up-to-date */
+ if (remoteheads->next) {
+ struct commit_list *list = remoteheads;
+ commit_list_insert(head_commit, &list);
+ common = get_octopus_merge_bases(list);
+ free(list);
+ } else
+ common = get_merge_bases(head_commit, remoteheads->item);
}
update_ref("updating ORIG_HEAD", "ORIG_HEAD",
@@ -1542,31 +1543,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
finish(head_commit, remoteheads, &commit->object.oid, msg.buf);
remove_merge_branch_state(the_repository);
goto done;
- } else if (!remoteheads->next && common->next)
- ;
- /*
- * We are not doing octopus and not fast-forward. Need
- * a real merge.
- */
- else if (!remoteheads->next && !common->next && option_commit) {
- /*
- * We are not doing octopus, not fast-forward, and have
- * only one common.
- */
- refresh_cache(REFRESH_QUIET);
- if (allow_trivial && fast_forward != FF_ONLY) {
- /* See if it is really trivial. */
- git_committer_info(IDENT_STRICT);
- printf(_("Trying really trivial in-index merge...\n"));
- if (!read_tree_trivial(&common->item->object.oid,
- &head_commit->object.oid,
- &remoteheads->item->object.oid)) {
- ret = merge_trivial(head_commit, remoteheads);
- goto done;
- }
- printf(_("Nope.\n"));
- }
- } else {
+ } else if (remoteheads->next || (!common->next && !option_commit)) {
/*
* An octopus. If we can reach all the remote we are up
* to date.
@@ -1592,6 +1569,24 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
finish_up_to_date(_("Already up to date. Yeeah!"));
goto done;
}
+ } else if (!common->next) {
+ /*
+ * We are not doing octopus, not fast-forward, and have
+ * only one common.
+ */
+ refresh_cache(REFRESH_QUIET);
+ if (allow_trivial && fast_forward != FF_ONLY) {
+ /* See if it is really trivial. */
+ git_committer_info(IDENT_STRICT);
+ printf(_("Trying really trivial in-index merge...\n"));
+ if (!read_tree_trivial(&common->item->object.oid,
+ &head_commit->object.oid,
+ &remoteheads->item->object.oid)) {
+ ret = merge_trivial(head_commit, remoteheads);
+ goto done;
+ }
+ printf(_("Nope.\n"));
+ }
}
if (fast_forward == FF_ONLY)
@@ -1685,9 +1680,8 @@ int cmd_merge(int argc, const char **argv, const char *prefix)
use_strategies[0]->name);
ret = 2;
goto done;
- } else if (best_strategy == wt_strategy)
- ; /* We already have its result in the working tree. */
- else {
+ } else if (best_strategy != wt_strategy) {
+ /* We do not have its result in the working tree. */
printf(_("Rewinding the tree to pristine...\n"));
restore_state(&head_commit->object.oid, &stash);
printf(_("Using the %s to prepare resolving by hand.\n"),
base-commit: e31aba42fb12bdeb0f850829e008e1e3f43af500
--
gitgitgadget
prev parent reply other threads:[~2020-11-17 22:06 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-08 21:37 [PATCH] Simplified merge logic Seija K. via GitGitGadget
2020-11-09 23:17 ` Junio C Hamano
2020-11-17 21:54 ` [PATCH v2] Simplify " Seija K. via GitGitGadget
2020-11-17 22:06 ` Seija K. via GitGitGadget [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.911.v3.git.git.1605650762205.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=pi1024e@github.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.