[TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[Johannes Schindelin]

for_each_ref() used to ignore refs which point nowhere. Making git-fsck-objects
be happy about them.

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

	Of course, it may be by design. But then, it does not feel 
	right to me. If this is intended behaviour, please don't let me 
	die dumb.

 refs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

applies-to: d29f6558d14b1da55642659c9874d4f52dac18bf
047627834b0ecb7ec0e0a98066b4b28688c374bc
diff --git a/refs.c b/refs.c
index a52b038..d74ede9 100644
--- a/refs.c
+++ b/refs.c
@@ -194,9 +194,9 @@ static int do_for_each_ref(const char *b
 				continue;
 			}
 			if (read_ref(git_path("%s", path), sha1) < 0)
-				continue;
+				die("%s points nowhere!", path);
 			if (!has_sha1_file(sha1))
-				continue;
+				die("%s does not point to a valid commit object!", path);
 			retval = fn(path, sha1);
 			if (retval)
 				break;
---
0.99.8.GIT

Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[Junio C Hamano]

The only case that I can think of that this patch may make a
difference in behaviour in a valid repository is the HEAD
pointer that points at nowhere (i.e. refs/heads/master)
immediately after git-init-db, so in that sense this change may
be safe for most purposes.

However, we might want to write a tool to iterate over what we
have under .git/refs/ and warn about them to help the user fix
broken branches/tags, and do_for_each_ref() dying like this
would not help that usage.

Not that the current loop is any better for that purpose.  We
silently ignore not just dangling ref and ref not storing
40-byte hex, but files starting with a period '.',  names longer
than 255 bytes, and unreadable ones, all of which we would
probably want to warn about in such a tool.

-jc

Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[Johannes Schindelin]

Hi,

On Thu, 27 Oct 2005, Junio C Hamano wrote:

> Not that the current loop is any better for that purpose.  We
> silently ignore not just dangling ref and ref not storing
> 40-byte hex, but files starting with a period '.',  names longer
> than 255 bytes, and unreadable ones, all of which we would
> probably want to warn about in such a tool.

Okay, how about 'fprintf(stderr, "Warning: ...\n"); continue;' instead of 
'die("...");' then?

Ciao,
Dscho

Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Thu, 27 Oct 2005, Junio C Hamano wrote:
>
>> Not that the current loop is any better for that purpose.  We
>> silently ignore not just dangling ref and ref not storing
>> 40-byte hex, but files starting with a period '.',  names longer
>> than 255 bytes, and unreadable ones, all of which we would
>> probably want to warn about in such a tool.
>
> Okay, how about 'fprintf(stderr, "Warning: ...\n"); continue;' instead of 
> 'die("...");' then?

Yup.  That sounds sensible.

[PATCH] Warn about invalid refs

[Johannes Schindelin]

Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

---

On Thu, 27 Oct 2005, Junio C Hamano wrote:

	> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
	> 
	> > On Thu, 27 Oct 2005, Junio C Hamano wrote:
	> >
	> >> Not that the current loop is any better for that purpose.  
	> >> We silently ignore not just dangling ref and ref not storing
	> >> 40-byte hex, but files starting with a period '.', names 
	> >> longer than 255 bytes, and unreadable ones, all of which we 
	> >> would probably want to warn about in such a tool.
	> >
	> > Okay, how about 'fprintf(stderr, "Warning: ...\n"); continue;' 
	> > instead of 'die("...");' then?
	> 
	> Yup.  That sounds sensible.

	Sorry for taking so long...

 refs.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/refs.c b/refs.c
index 826ae7a..982ebf8 100644
--- a/refs.c
+++ b/refs.c
@@ -151,10 +151,15 @@ static int do_for_each_ref(const char *b
 					break;
 				continue;
 			}
-			if (read_ref(git_path("%s", path), sha1) < 0)
+			if (read_ref(git_path("%s", path), sha1) < 0) {
+				fprintf(stderr, "%s points nowhere!", path);
 				continue;
-			if (!has_sha1_file(sha1))
+			}
+			if (!has_sha1_file(sha1)) {
+				fprintf(stderr, "%s does not point to a valid "
+						"commit object!", path);
 				continue;
+			}
 			retval = fn(path, sha1);
 			if (retval)
 				break;

Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid

[Horst von Brand]

Junio C Hamano <junkio@cox.net> wrote:

[...]

> Not that the current loop is any better for that purpose.  We
> silently ignore not just dangling ref and ref not storing
> 40-byte hex, but files starting with a period '.',  names longer
> than 255 bytes, and unreadable ones, all of which we would
> probably want to warn about in such a tool.

I have yet to come across a filesystem allowing names of more than 255
characters... 
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                     Fono: +56 32 654431
Universidad Tecnica Federico Santa Maria              +56 32 654239
Casilla 110-V, Valparaiso, Chile                Fax:  +56 32 797513