Make error messages from cherry-pick/revert more sensible
authorJunio C Hamano <gitster@pobox.com>
Fri, 11 Jan 2008 06:49:35 +0000 (22:49 -0800)
committerJunio C Hamano <gitster@pobox.com>
Tue, 5 Feb 2008 08:39:19 +0000 (00:39 -0800)
The original "rewrite in C" did somewhat a sloppy job while
stealing code from git-write-tree.

The caller pretends as if the write_tree() function would return
an error code and being able to issue a sensible error message
itself, but write_tree() function just calls die() and never
returns an error.  Worse yet, the function claims that it was
running git-write-tree (which is no longer true after
cherry-pick stole it).

Tested-by: Björn Steinbrink <B.Steinbrink@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
builtin-revert.c
builtin-write-tree.c
builtin.h
cache-tree.c
cache-tree.h

index 358af53..e219859 100644 (file)
@@ -8,6 +8,7 @@
 #include "exec_cmd.h"
 #include "utf8.h"
 #include "parse-options.h"
+#include "cache-tree.h"
 
 /*
  * This implements the builtins revert and cherry-pick.
@@ -270,7 +271,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
                 * that represents the "current" state for merge-recursive
                 * to work on.
                 */
-               if (write_tree(head, 0, NULL))
+               if (write_cache_as_tree(head, 0, NULL))
                        die ("Your index file is unmerged.");
        } else {
                struct wt_status s;
@@ -357,7 +358,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
        if (merge_recursive(sha1_to_hex(base->object.sha1),
                                sha1_to_hex(head), "HEAD",
                                sha1_to_hex(next->object.sha1), oneline) ||
-                       write_tree(head, 0, NULL)) {
+                       write_cache_as_tree(head, 0, NULL)) {
                add_to_msg("\nConflicts:\n\n");
                read_cache();
                for (i = 0; i < active_nr;) {
index d16b9ed..e838d01 100644 (file)
 static const char write_tree_usage[] =
 "git-write-tree [--missing-ok] [--prefix=<prefix>/]";
 
-int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
-{
-       int entries, was_valid, newfd;
-
-       /* We can't free this memory, it becomes part of a linked list parsed atexit() */
-       struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
-
-       newfd = hold_locked_index(lock_file, 1);
-
-       entries = read_cache();
-       if (entries < 0)
-               die("git-write-tree: error reading cache");
-
-       if (!active_cache_tree)
-               active_cache_tree = cache_tree();
-
-       was_valid = cache_tree_fully_valid(active_cache_tree);
-
-       if (!was_valid) {
-               if (cache_tree_update(active_cache_tree,
-                                     active_cache, active_nr,
-                                     missing_ok, 0) < 0)
-                       die("git-write-tree: error building trees");
-               if (0 <= newfd) {
-                       if (!write_cache(newfd, active_cache, active_nr) &&
-                           !commit_lock_file(lock_file))
-                               newfd = -1;
-               }
-               /* Not being able to write is fine -- we are only interested
-                * in updating the cache-tree part, and if the next caller
-                * ends up using the old index with unupdated cache-tree part
-                * it misses the work we did here, but that is just a
-                * performance penalty and not a big deal.
-                */
-       }
-
-       if (prefix) {
-               struct cache_tree *subtree =
-                       cache_tree_find(active_cache_tree, prefix);
-               if (!subtree)
-                       die("git-write-tree: prefix %s not found", prefix);
-               hashcpy(sha1, subtree->sha1);
-       }
-       else
-               hashcpy(sha1, active_cache_tree->sha1);
-
-       if (0 <= newfd)
-               rollback_lock_file(lock_file);
-
-       return 0;
-}
-
 int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
 {
        int missing_ok = 0, ret;
        const char *prefix = NULL;
        unsigned char sha1[20];
+       const char *me = "git-write-tree";
 
        git_config(git_default_config);
        while (1 < argc) {
@@ -84,8 +33,20 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
        if (argc > 2)
                die("too many options");
 
-       ret = write_tree(sha1, missing_ok, prefix);
-       printf("%s\n", sha1_to_hex(sha1));
-
+       ret = write_cache_as_tree(sha1, missing_ok, prefix);
+       switch (ret) {
+       case 0:
+               printf("%s\n", sha1_to_hex(sha1));
+               break;
+       case WRITE_TREE_UNREADABLE_INDEX:
+               die("%s: error reading the index", me);
+               break;
+       case WRITE_TREE_UNMERGED_INDEX:
+               die("%s: error building trees; the index is unmerged?", me);
+               break;
+       case WRITE_TREE_PREFIX_ERROR:
+               die("%s: prefix %s not found", me, prefix);
+               break;
+       }
        return ret;
 }
index cb675c4..3d1628c 100644 (file)
--- a/builtin.h
+++ b/builtin.h
@@ -8,7 +8,6 @@ extern const char git_usage_string[];
 
 extern void list_common_cmds_help(void);
 extern void help_unknown_cmd(const char *cmd);
-extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 extern void prune_packed_objects(int);
 
 extern int cmd_add(int argc, const char **argv, const char *prefix);
index 50b3526..4e1e7b2 100644 (file)
@@ -529,3 +529,58 @@ struct cache_tree *cache_tree_find(struct cache_tree *it, const char *path)
        }
        return it;
 }
+
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix)
+{
+       int entries, was_valid, newfd;
+
+       /*
+        * We can't free this memory, it becomes part of a linked list
+        * parsed atexit()
+        */
+       struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
+
+       newfd = hold_locked_index(lock_file, 1);
+
+       entries = read_cache();
+       if (entries < 0)
+               return WRITE_TREE_UNREADABLE_INDEX;
+
+       if (!active_cache_tree)
+               active_cache_tree = cache_tree();
+
+       was_valid = cache_tree_fully_valid(active_cache_tree);
+
+       if (!was_valid) {
+               if (cache_tree_update(active_cache_tree,
+                                     active_cache, active_nr,
+                                     missing_ok, 0) < 0)
+                       return WRITE_TREE_UNMERGED_INDEX;
+               if (0 <= newfd) {
+                       if (!write_cache(newfd, active_cache, active_nr) &&
+                           !commit_lock_file(lock_file))
+                               newfd = -1;
+               }
+               /* Not being able to write is fine -- we are only interested
+                * in updating the cache-tree part, and if the next caller
+                * ends up using the old index with unupdated cache-tree part
+                * it misses the work we did here, but that is just a
+                * performance penalty and not a big deal.
+                */
+       }
+
+       if (prefix) {
+               struct cache_tree *subtree =
+                       cache_tree_find(active_cache_tree, prefix);
+               if (!subtree)
+                       return WRITE_TREE_PREFIX_ERROR;
+               hashcpy(sha1, subtree->sha1);
+       }
+       else
+               hashcpy(sha1, active_cache_tree->sha1);
+
+       if (0 <= newfd)
+               rollback_lock_file(lock_file);
+
+       return 0;
+}
index 8243228..44aad42 100644 (file)
@@ -30,4 +30,9 @@ int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int)
 
 struct cache_tree *cache_tree_find(struct cache_tree *, const char *);
 
+#define WRITE_TREE_UNREADABLE_INDEX (-1)
+#define WRITE_TREE_UNMERGED_INDEX (-2)
+#define WRITE_TREE_PREFIX_ERROR (-3)
+
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix);
 #endif