git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: Destructive side-effect of "cg-status"

From
Linus Torvalds <torvalds@osdl.org>
Date
Oct 1, 2005, 20:24 UTC
Message-ID
<Pine.LNX.4.64.0510011303560.3378@g5.osdl.org>
In-Reply-To
<20051001194216.EE3E5353D8E@atlas.denx.de>
On Sat, 1 Oct 2005, Wolfgang Denk wrote:
Show 6 quoted lines
> 
> 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 <torvalds@osdl.org>
---
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, ...)
Previous: Wolfgang Denk
Message 18 of 18 in “Destructive side-effect of "cg-status"”
  1. Wolfgang DenkSep 30, 2005
  2. Martin LanghoffOct 1, 2005
  3. Linus TorvaldsOct 1, 2005
  4. Junio C HamanoOct 1, 2005
  5. Honor extractor's umask in git-tar-tree.Junio C Hamano, Oct 1, 2005
  6. H. Peter AnvinOct 2, 2005
  7. Matthias UrlichsOct 2, 2005
  8. H. Peter AnvinOct 3, 2005
  9. Junio C HamanoOct 3, 2005
  10. H. Peter AnvinOct 3, 2005
  11. Junio C HamanoOct 3, 2005
  12. H. Peter AnvinOct 3, 2005
  13. Linus TorvaldsOct 3, 2005
  14. H. Peter AnvinOct 3, 2005
  15. Linus TorvaldsOct 3, 2005
  16. H. Peter AnvinOct 3, 2005
  17. Wolfgang DenkOct 1, 2005
  18. Linus TorvaldsOct 1, 2005

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.