On Tue, Apr 10, 2012 at 10:04:32AM -0700, Junio C Hamano wrote:
> 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):
>
Honestly, I wasn't aware there was a faster way. In my last version you
suggested using git diff-index and git rev-parse (allbeit the latter was for use
in the git-rebase script, and I just reused it). Regardless, I was going with
your previous suggestions. I can re-do these to use these new suggestions.
Neil
--
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