Here's the series I mentioned earlier, to avoid avoid accessing the ref
code if we know we aren't in a repository. For those just joining us,
the end goal is to be able to detect and complain when code tries to
open a ref without having run check_repository_format() or similar. Once
we have pluggable ref backends, doing so would be wrong, since we won't
know which backend we're supposed to be using.
It's already _kind of_ wrong, in the sense that we should not be looking
for file-backend refs if we know we don't have a repository. But nobody
really noticed in practice, because you do not generally have
".git/HEAD" lying around in non-repository directories. So it was a de
facto noop, though you could construct pathological cases where it did
the wrong thing.
The patches are:
[1/6]: setup: make startup_info available everywhere
[2/6]: setup: set startup_info->have_repository more reliably
[3/6]: remote: don't resolve HEAD in non-repository
[4/6]: mailmap: do not resolve blobs in a non-repository
[5/6]: grep: turn off gitlink detection for --no-index
[6/6]: use setup_git_directory() in test-* programs
There's a sort-of "7/6" I used to find these spots:
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 81f68f8..22c1b6d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -947,6 +947,8 @@ static struct ref_cache *lookup_ref_cache(const char
*submodule)
{
struct ref_cache *refs;
+ assert(startup_info->have_repository);
+
if (!submodule || !*submodule)
return &ref_cache;
@@ -1414,6 +1416,8 @@ static const char *resolve_ref_1(const char *refname,
int depth = MAXDEPTH;
int bad_name = 0;
+ assert(startup_info->have_repository);
+
if (flags)
*flags = 0;
Running the test suite with that patch and 1/6 turned up the cases I've
fixed here. But I'm not including it here for a few reasons:
1. I'm not sure that asserting is the best behavior. It's great for
finding problems (and that's why I used assert() -- to get a
coredump for the backtrace), but in practice it might be friendlier
to turn it into a silent noop ("no, I don't even have a ref
backend, so your ref definitely does not exist").
2. In a pluggable ref-backend world, this isn't where we'd put the
asserts, anyway. We'd do it when accessing the ref_backend vtable
(or just starting with a dummy vtable that asserts, or returns
"nope, no refs", or whatever).
So it was useful for debugging/analysis now, and we may want something
like it later, but I think that should come on top the ref-backend
series.
-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
