# [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid 5 messages from 2005-10-27 to 2006-02-28. Participants: Johannes Schindelin, Junio C Hamano. Thread: https://gitlist.dev/t/2243 ## Johannes Schindelin, 2005-10-27 17:40 Subject: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510271936520.7518%40wbgn013.biozentrum.uni-wuerzburg.de ``` for_each_ref() used to ignore refs which point nowhere. Making git-fsck-objects be happy about them. Signed-off-by: Johannes Schindelin --- 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 ``` ## Junio C Hamano, 2005-10-27 19:01 Subject: Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid Message-ID: <7vacgu3hta.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vacgu3hta.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` 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 ``` ## Johannes Schindelin, 2005-10-27 19:28 Subject: Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0510272125580.28705%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7vacgu3hta.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Junio C Hamano, 2005-10-27 20:45 Subject: Re: [TENTATIVE PATCH] Complain loudly, dying, when a ref is invalid Message-ID: <7voe5a1yft.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7voe5a1yft.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` Johannes Schindelin 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. ``` ## Johannes Schindelin, 2006-02-28 21:16 Subject: [PATCH] Warn about invalid refs Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0602282213180.27405%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <7voe5a1yft.fsf@assigned-by-dhcp.cox.net> ``` Signed-off-by: Johannes Schindelin --- On Thu, 27 Oct 2005, Junio C Hamano wrote: > Johannes Schindelin 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; ```