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:07 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
List information: http://vger.kernel.org/majordomo-info.html
* 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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).