From: Linus Torvalds Date: Sat, 01 Oct 2005 20:24:27 GMT Subject: Re: Destructive side-effect of "cg-status" Message-ID: In-Reply-To: <20051001194216.EE3E5353D8E@atlas.denx.de> On Sat, 1 Oct 2005, Wolfgang Denk wrote: > > The "error: open failed" should at leats include the file name and > the errno/strerror message. Same for the "read_cache: Permission > denied" - of course, if you knot the git internals you will know what > this means, but the average user has no idea that he should check the > permissions of .git/index. Agreed. Something like this would seem sane. Linus --- Subject: Better error reporting for "git status" Instead of "git status" ignoring (and hiding) potential errors from the "git-update-index" call, make it exit if it fails, and show the error. In order to do this, use the "-q" flag (to ignore not-up-to-date files) and add a new "--unmerged" flag that allows unmerged entries in the index without any errors. This also avoids marking the index "changed" if an entry isn't actually modified, and makes sure that we exit with an understandable error message if the index is corrupt or unreadable. "read_cache()" no longer returns an error for the caller to check. Finally, make die() and usage() exit with recognizable error codes, if we ever want to check the failure reason in scripts. Signed-off-by: Linus Torvalds --- diff --git a/git-status.sh b/git-status.sh --- a/git-status.sh +++ b/git-status.sh @@ -37,7 +37,7 @@ refs/heads/master) ;; *) echo "# On branch $branch" ;; esac -git-update-index --refresh >/dev/null 2>&1 +git-update-index -q --unmerged --refresh || exit if test -f "$GIT_DIR/HEAD" then diff --git a/read-cache.c b/read-cache.c --- a/read-cache.c +++ b/read-cache.c @@ -464,11 +464,15 @@ int read_cache(void) errno = EBUSY; if (active_cache) - return error("more than one cachefile"); + return active_nr; + errno = ENOENT; fd = open(get_index_file(), O_RDONLY); - if (fd < 0) - return (errno == ENOENT) ? 0 : error("open failed"); + if (fd < 0) { + if (errno == ENOENT) + return 0; + die("index file open failed (%s)", strerror(errno)); + } size = 0; // avoid gcc warning map = MAP_FAILED; @@ -480,7 +484,7 @@ int read_cache(void) } close(fd); if (map == MAP_FAILED) - return error("mmap failed"); + die("index file mmap failed (%s)", strerror(errno)); hdr = map; if (verify_hdr(hdr, size) < 0) @@ -501,7 +505,7 @@ int read_cache(void) unmap: munmap(map, size); errno = EINVAL; - return error("verify header failed"); + die("index file corrupt"); } #define WRITE_BUFFER_SIZE 8192 diff --git a/update-index.c b/update-index.c --- a/update-index.c +++ b/update-index.c @@ -13,7 +13,7 @@ * like "git-update-index *" and suddenly having all the object * files be revision controlled. */ -static int allow_add = 0, allow_remove = 0, allow_replace = 0, not_new = 0, quiet = 0, info_only = 0; +static int allow_add = 0, allow_remove = 0, allow_replace = 0, allow_unmerged = 0, not_new = 0, quiet = 0, info_only = 0; static int force_remove; /* Three functions to allow overloaded pointer return; see linux/err.h */ @@ -135,7 +135,7 @@ static struct cache_entry *refresh_entry changed = ce_match_stat(ce, &st); if (!changed) - return ce; + return NULL; if (ce_modified(ce, &st)) return ERR_PTR(-EINVAL); @@ -156,16 +156,20 @@ static int refresh_cache(void) struct cache_entry *ce, *new; ce = active_cache[i]; if (ce_stage(ce)) { - printf("%s: needs merge\n", ce->name); - has_errors = 1; while ((i < active_nr) && ! strcmp(active_cache[i]->name, ce->name)) i++; i--; + if (allow_unmerged) + continue; + printf("%s: needs merge\n", ce->name); + has_errors = 1; continue; } new = refresh_entry(ce); + if (!new) + continue; if (IS_ERR(new)) { if (not_new && PTR_ERR(new) == -ENOENT) continue; @@ -335,6 +339,10 @@ int main(int argc, const char **argv) allow_remove = 1; continue; } + if (!strcmp(path, "--unmerged")) { + allow_unmerged = 1; + continue; + } if (!strcmp(path, "--refresh")) { has_errors |= refresh_cache(); continue; diff --git a/usage.c b/usage.c --- a/usage.c +++ b/usage.c @@ -15,7 +15,7 @@ static void report(const char *prefix, c void usage(const char *err) { fprintf(stderr, "usage: %s\n", err); - exit(1); + exit(129); } void die(const char *err, ...) @@ -25,7 +25,7 @@ void die(const char *err, ...) va_start(params, err); report("fatal: ", err, params); va_end(params); - exit(1); + exit(128); } int error(const char *err, ...)