commit: Let the callback of for_each_mergetag return on error

This is yet another patch to be filed under the keyword "libification".

There is one subtle change in behavior here, where a `git log` that has
been asked to show the mergetags would now stop reporting the mergetags
upon the first failure, whereas previously, it would have continued to the
next mergetag, if any.

In practice, that change should not matter, as it is 1) uncommon to
perform octopus merges using multiple tags as merge heads, and 2) when the
user asks to be shown those tags, they really should be there.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/replace.c b/builtin/replace.c
index 935647be6bdf2ffc2b460038a10e63c2eb387a5c..245d3f4164e067abef37ae947431e862190ba14b 100644 (file)
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -345,7 +345,7 @@ struct check_mergetag_data {
        const char **argv;
 };
 
-static void check_one_mergetag(struct commit *commit,
+static int check_one_mergetag(struct commit *commit,
                               struct commit_extra_header *extra,
                               void *data)
 {
@@ -368,20 +368,20 @@ static void check_one_mergetag(struct commit *commit,
                if (get_oid(mergetag_data->argv[i], &oid) < 0)
                        die(_("Not a valid object name: '%s'"), mergetag_data->argv[i]);
                if (!oidcmp(&tag->tagged->oid, &oid))
-                       return; /* found */
+                       return 0; /* found */
        }
 
        die(_("original commit '%s' contains mergetag '%s' that is discarded; "
              "use --edit instead of --graft"), ref, oid_to_hex(&tag_oid));
 }
 
-static void check_mergetags(struct commit *commit, int argc, const char **argv)
+static int check_mergetags(struct commit *commit, int argc, const char **argv)
 {
        struct check_mergetag_data mergetag_data;
 
        mergetag_data.argc = argc;
        mergetag_data.argv = argv;
-       for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
+       return for_each_mergetag(check_one_mergetag, commit, &mergetag_data);
 }
 
 static int create_graft(int argc, const char **argv, int force)

diff --git a/commit.c b/commit.c
index ca474a7c112855a49b77cce7cdfb83937fa17fae..2952ec987c5ca0335c7ebaf8e20404597d576055 100644 (file)
--- a/commit.c
+++ b/commit.c
@@ -1288,17 +1288,19 @@ struct commit_extra_header *read_commit_extra_headers(struct commit *commit,
        return extra;
 }
 
-void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
+int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data)
 {
        struct commit_extra_header *extra, *to_free;
+       int res = 0;
 
        to_free = read_commit_extra_headers(commit, NULL);
-       for (extra = to_free; extra; extra = extra->next) {
+       for (extra = to_free; !res && extra; extra = extra->next) {
                if (strcmp(extra->key, "mergetag"))
                        continue; /* not a merge tag */
-               fn(commit, extra, data);
+               res = fn(commit, extra, data);
        }
        free_commit_extra_headers(to_free);
+       return res;
 }
 
 static inline int standard_header_field(const char *field, size_t len)

diff --git a/commit.h b/commit.h
index 0fb8271665c6c98ccca803fbe002327bf38fcfb3..9000895ad912e4fac1aee792429c256f792edc8f 100644 (file)
--- a/commit.h
+++ b/commit.h
@@ -291,10 +291,10 @@ extern const char *find_commit_header(const char *msg, const char *key,
 /* Find the end of the log message, the right place for a new trailer. */
 extern int ignore_non_trailer(const char *buf, size_t len);
 
-typedef void (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
+typedef int (*each_mergetag_fn)(struct commit *commit, struct commit_extra_header *extra,
                                 void *cb_data);
 
-extern void for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data);
+extern int for_each_mergetag(each_mergetag_fn fn, struct commit *commit, void *data);
 
 struct merge_remote_desc {
        struct object *obj; /* the named object, could be a tag */

diff --git a/log-tree.c b/log-tree.c
index d1c0bedf244fce0c8894175bf800384c0474b312..f3a51a6e72678add0816ef4604a7d70d7b62328e 100644 (file)
--- a/log-tree.c
+++ b/log-tree.c
@@ -488,9 +488,9 @@ static int is_common_merge(const struct commit *commit)
                && !commit->parents->next->next);
 }
 
-static void show_one_mergetag(struct commit *commit,
-                             struct commit_extra_header *extra,
-                             void *data)
+static int show_one_mergetag(struct commit *commit,
+                            struct commit_extra_header *extra,
+                            void *data)
 {
        struct rev_info *opt = (struct rev_info *)data;
        struct object_id oid;
@@ -502,7 +502,7 @@ static void show_one_mergetag(struct commit *commit,
        hash_object_file(extra->value, extra->len, type_name(OBJ_TAG), &oid);
        tag = lookup_tag(&oid);
        if (!tag)
-               return; /* error message already given */
+               return -1; /* error message already given */
 
        strbuf_init(&verify_message, 256);
        if (parse_tag_buffer(tag, extra->value, extra->len))
@@ -536,11 +536,12 @@ static void show_one_mergetag(struct commit *commit,
 
        show_sig_lines(opt, status, verify_message.buf);
        strbuf_release(&verify_message);
+       return 0;
 }
 
-static void show_mergetag(struct rev_info *opt, struct commit *commit)
+static int show_mergetag(struct rev_info *opt, struct commit *commit)
 {
-       for_each_mergetag(show_one_mergetag, commit, opt);
+       return for_each_mergetag(show_one_mergetag, commit, opt);
 }
 
 void show_log(struct rev_info *opt)