Make error messages from cherry-pick/revert more sensible
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>
Junio C Hamano committed
Jan 10, 2008 at 22:49 UTC
45525bd022dda75ec935c9c579e452577dcfd31f
5 files changed
+79
-58
builtin-revert.c
+3
-2
@@ -8,6 +8,7 @@
8
#include "exec_cmd.h"
9
#include "utf8.h"
10
#include "parse-options.h"
11
+#include "cache-tree.h"
12
13
/*
14
* This implements the builtins revert and cherry-pick.
@@ -270,7 +271,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
271
* that represents the "current" state for merge-recursive
272
* to work on.
273
*/
273
- if (write_tree(head, 0, NULL))
274
+ if (write_cache_as_tree(head, 0, NULL))
275
die ("Your index file is unmerged.");
276
} else {
277
struct wt_status s;
@@ -357,7 +358,7 @@ static int revert_or_cherry_pick(int argc, const char **argv)
358
if (merge_recursive(sha1_to_hex(base->object.sha1),
359
sha1_to_hex(head), "HEAD",
360
sha1_to_hex(next->object.sha1), oneline) ||
360
- write_tree(head, 0, NULL)) {
361
+ write_cache_as_tree(head, 0, NULL)) {
362
add_to_msg("\nConflicts:\n\n");
363
read_cache();
364
for (i = 0; i < active_nr;) {
builtin-write-tree.c
+16
-55
@@ -11,63 +11,12 @@
11
static const char write_tree_usage[] =
12
"git-write-tree [--missing-ok] [--prefix=<prefix>/]";
13
14
-int write_tree(unsigned char *sha1, int missing_ok, const char *prefix)
15
-{
16
- int entries, was_valid, newfd;
17
-
18
- /* We can't free this memory, it becomes part of a linked list parsed atexit() */
19
- struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
20
-
21
- newfd = hold_locked_index(lock_file, 1);
22
-
23
- entries = read_cache();
24
- if (entries < 0)
25
- die("git-write-tree: error reading cache");
26
-
27
- if (!active_cache_tree)
28
- active_cache_tree = cache_tree();
29
-
30
- was_valid = cache_tree_fully_valid(active_cache_tree);
31
-
32
- if (!was_valid) {
33
- if (cache_tree_update(active_cache_tree,
34
- active_cache, active_nr,
35
- missing_ok, 0) < 0)
36
- die("git-write-tree: error building trees");
37
- if (0 <= newfd) {
38
- if (!write_cache(newfd, active_cache, active_nr) &&
39
- !commit_lock_file(lock_file))
40
- newfd = -1;
41
- }
42
- /* Not being able to write is fine -- we are only interested
43
- * in updating the cache-tree part, and if the next caller
44
- * ends up using the old index with unupdated cache-tree part
45
- * it misses the work we did here, but that is just a
46
- * performance penalty and not a big deal.
47
- */
48
- }
49
-
50
- if (prefix) {
51
- struct cache_tree *subtree =
52
- cache_tree_find(active_cache_tree, prefix);
53
- if (!subtree)
54
- die("git-write-tree: prefix %s not found", prefix);
55
- hashcpy(sha1, subtree->sha1);
56
- }
57
- else
58
- hashcpy(sha1, active_cache_tree->sha1);
59
-
60
- if (0 <= newfd)
61
- rollback_lock_file(lock_file);
62
-
63
- return 0;
64
-}
65
-
14
int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
15
{
16
int missing_ok = 0, ret;
17
const char *prefix = NULL;
18
unsigned char sha1[20];
19
+ const char *me = "git-write-tree";
20
21
git_config(git_default_config);
22
while (1 < argc) {
@@ -84,8 +33,20 @@ int cmd_write_tree(int argc, const char **argv, const char *unused_prefix)
33
if (argc > 2)
34
die("too many options");
35
87
- ret = write_tree(sha1, missing_ok, prefix);
88
- printf("%s\n", sha1_to_hex(sha1));
89
-
36
+ ret = write_cache_as_tree(sha1, missing_ok, prefix);
37
+ switch (ret) {
38
+ case 0:
39
+ printf("%s\n", sha1_to_hex(sha1));
40
+ break;
41
+ case WRITE_TREE_UNREADABLE_INDEX:
42
+ die("%s: error reading the index", me);
43
+ break;
44
+ case WRITE_TREE_UNMERGED_INDEX:
45
+ die("%s: error building trees; the index is unmerged?", me);
46
+ break;
47
+ case WRITE_TREE_PREFIX_ERROR:
48
+ die("%s: prefix %s not found", me, prefix);
49
+ break;
50
+ }
51
return ret;
52
}
builtin.h
-1
@@ -8,7 +8,6 @@ extern const char git_usage_string[];
8
9
extern void list_common_cmds_help(void);
10
extern void help_unknown_cmd(const char *cmd);
11
-extern int write_tree(unsigned char *sha1, int missing_ok, const char *prefix);
11
extern void prune_packed_objects(int);
12
13
extern int cmd_add(int argc, const char **argv, const char *prefix);
cache-tree.c
+55
@@ -529,3 +529,58 @@ struct cache_tree *cache_tree_find(struct cache_tree *it, const char *path)
529
}
530
return it;
531
}
532
+
533
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix)
534
+{
535
+ int entries, was_valid, newfd;
536
+
537
+ /*
538
+ * We can't free this memory, it becomes part of a linked list
539
+ * parsed atexit()
540
+ */
541
+ struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
542
+
543
+ newfd = hold_locked_index(lock_file, 1);
544
+
545
+ entries = read_cache();
546
+ if (entries < 0)
547
+ return WRITE_TREE_UNREADABLE_INDEX;
548
+
549
+ if (!active_cache_tree)
550
+ active_cache_tree = cache_tree();
551
+
552
+ was_valid = cache_tree_fully_valid(active_cache_tree);
553
+
554
+ if (!was_valid) {
555
+ if (cache_tree_update(active_cache_tree,
556
+ active_cache, active_nr,
557
+ missing_ok, 0) < 0)
558
+ return WRITE_TREE_UNMERGED_INDEX;
559
+ if (0 <= newfd) {
560
+ if (!write_cache(newfd, active_cache, active_nr) &&
561
+ !commit_lock_file(lock_file))
562
+ newfd = -1;
563
+ }
564
+ /* Not being able to write is fine -- we are only interested
565
+ * in updating the cache-tree part, and if the next caller
566
+ * ends up using the old index with unupdated cache-tree part
567
+ * it misses the work we did here, but that is just a
568
+ * performance penalty and not a big deal.
569
+ */
570
+ }
571
+
572
+ if (prefix) {
573
+ struct cache_tree *subtree =
574
+ cache_tree_find(active_cache_tree, prefix);
575
+ if (!subtree)
576
+ return WRITE_TREE_PREFIX_ERROR;
577
+ hashcpy(sha1, subtree->sha1);
578
+ }
579
+ else
580
+ hashcpy(sha1, active_cache_tree->sha1);
581
+
582
+ if (0 <= newfd)
583
+ rollback_lock_file(lock_file);
584
+
585
+ return 0;
586
+}
cache-tree.h
+5
@@ -30,4 +30,9 @@ int cache_tree_update(struct cache_tree *, struct cache_entry **, int, int, int)
30
31
struct cache_tree *cache_tree_find(struct cache_tree *, const char *);
32
33
+#define WRITE_TREE_UNREADABLE_INDEX (-1)
34
+#define WRITE_TREE_UNMERGED_INDEX (-2)
35
+#define WRITE_TREE_PREFIX_ERROR (-3)
36
+
37
+int write_cache_as_tree(unsigned char *sha1, int missing_ok, const char *prefix);
38
#endif