From 45525bd022dda75ec935c9c579e452577dcfd31f Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Thu, 10 Jan 2008 22:49:35 -0800 Subject: [PATCH] Make error messages from cherry-pick/revert more sensible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Junio C Hamano --- builtin-revert.c | 5 ++-- builtin-write-tree.c | 71 ++++++++++---------------------------------- builtin.h | 1 - cache-tree.c | 55 ++++++++++++++++++++++++++++++++++ cache-tree.h | 5 ++++ 5 files changed, 79 insertions(+), 58 deletions(-) diff --git a/builtin-revert.c b/builtin-revert.c index 358af53747..e219859f9b 100644 --- a/builtin-revert.c +++ b/builtin-revert.c @@ -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;) { diff --git a/builtin-write-tree.c b/builtin-write-tree.c index d16b9ed009..e838d01233 100644 --- a/builtin-write-tree.c +++ b/builtin-write-tree.c @@ -11,63 +11,12 @@ static const char write_tree_usage[] = "git-write-tree [--missing-ok] [--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; } diff --git a/builtin.h b/builtin.h index cb675c4d7a..3d1628c597 100644 --- 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); diff --git a/cache-tree.c b/cache-tree.c index 50b35264fd..4e1e7b25cf 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -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; +} diff --git a/cache-tree.h b/cache-tree.h index 8243228e49..44aad426d3 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -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