Neil Horman <nhorman@xxxxxxxxxxxxx> writes:
> + /* keep_if_made_empty implies allow_empty */
> + if (opts->keep_if_made_empty)
> + opts->allow_empty = 1;
> +
OK.
> diff --git a/sequencer.c b/sequencer.c
> index 71929ba..5d033db 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -13,6 +13,7 @@
> #include "rerere.h"
> #include "merge-recursive.h"
> #include "refs.h"
> +#include "argv-array.h"
>
> #define GIT_REFLOG_ACTION "GIT_REFLOG_ACTION"
>
> @@ -258,26 +259,102 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
> * If we are revert, or if our cherry-pick results in a hand merge,
> * we had better say that the current user is responsible for that.
> */
> -static int run_git_commit(const char *defmsg, struct replay_opts *opts)
> +static int run_git_commit(const char *defmsg, struct replay_opts *opts, int empty)
> {
> - /* 7 is max possible length of our args array including NULL */
> - const char *args[7];
> - int i = 0;
> + struct argv_array array;
> + int rc;
> +
> + if (!empty && !opts->keep_if_made_empty) {
> + const char *argv[] = { "diff-index", "--quiet", "--exit-code",
> + "--cached", "HEAD", NULL };
> +
> + /*
> + * If we run git diff-index with the above option and it returns
> + * zero, then there have been no changes made to the index by
> + * this patch, i.e. its empty. Since our previous empty test
> + * indicated that this patch was not created empty, its been made
> + * redundant. Since keep_if_made_empty is not set, we just skip
> + * it
> + */
> + if (run_command_v_opt(argv, RUN_GIT_CMD) == 0)
> + return 0;
Wouldn't it be far simpler to do this without forking diff-index? I
haven't followed the codepath leading to this, but it should be very easy
to find a commit object that corresponds to the current HEAD and peek the
tree object in it to find out the current tree, and because the original
function is going to run an as-is "git commit", the tree you are going to
commit should be available by calling cache_tree_update() like write-tree
does. If they match, you are trying to create an empty commit. E.g. (error
checking elided):
unsigned char head_sha1[20];
struct commit *head_commit;
resolve_ref_unsafe("HEAD", head_sha1, 1, NULL);
head_commit = lookup_commit(head_sha1);
parse_commit(head_commit);
if (!cache_tree_fully_valid(active_cache_tree))
cache_tree_update(active_cache_tree, active_cache,
active_nr, 0);
return hashcmp(active_cache_tree->sha1, head_commit->tree->object.sha1);
> +static int is_original_commit_empty(struct commit *commit)
> +{
> + struct argv_array argv_array;
> + struct child_process cp;
> + char ptree[40], pptree[40];
> + int ret = 0;
> +
> + argv_array_init(&argv_array);
> + memset(&cp, 0, sizeof(struct child_process));
> +
> + argv_array_push(&argv_array, "rev-parse");
> + argv_array_pushf(&argv_array, "%s^{tree}", sha1_to_hex(commit->object.sha1));
Likewise. You have the commit object, so just make sure it was parsed,
and peek its tree object. Also do the same for its parent commit. Now
you have two trees, so you can compare their object names. E.g. (error
checking elided):
const unsigned char *ptree_sha1;
parse_commit(commit);
if (commit->parents) {
struct commit *parent = commit->parents->item;
parse_commit(parent);
ptree_sha1 = parent->tree->object.sha1;
} else {
ptree_sha1 = EMPTY_TREE_SHA1_BIN; /* commit is root */
}
return hashcmp(ptree_sha1, commit->tree->object.sha1);
The higher code structure of this patch looks good, but the lower level
implementation details are done way too inefficiently.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html