Re: [PATCH] Transitively read alternatives

Martin Waitz [off-list ref] writes:

When adding an alternate object store then add entries from its
info/alternates files, too.

Not quite, I'm afraid...

+static int link_alt_odb_entry(const char * entry, int len, const char * relative_base)
+{
+	struct stat st;
+	const char *objdir = get_object_directory();
+	struct alternate_object_database *ent;
+	struct alternate_object_database *alt;
+	/* 43 = 40-byte + 2 '/' + terminating NUL */
+	int pfxlen = len;
+	int entlen = pfxlen + 43;
+	int base_len = -1;
+
+	if (*entry != '/' && relative_base) {
+		/* Relative alt-odb */
+		if (base_len < 0)
+			base_len = strlen(relative_base) + 1;

Wouldn't base_len be always -1 here?

+	if (*entry != '/' && relative_base) {
+		memcpy(ent->base, relative_base, base_len - 1);
+		ent->base[base_len - 1] = '/';
+		memcpy(ent->base + base_len, entry, len);
+	}
+	else
+		memcpy(ent->base, entry, pfxlen);

Handling of full path sounds sensible; with relative_base case,
the referred-to object directory is relative to our object/
directory, so "A/.git/objects/info/alternates" would typically
have "../../../B/.git/objects/" if A borrows from B that lives
in the same subdirectory as A itself.

+	/* recursively add alternates */
+	read_info_alternates(ent->base);

But using that "../../../B/.git/objects/", we would read its
alternates.  If it points at a neighbor we already borrow from,
say C (we would refer to it as "../../../C/.git/objects/"),
prefixing with relative_base would yield

	../../../B/.git/objects/../../../C/.git/objects/

and we would end up getting the same thing twice, which sounds a
bit unfortunate.  Maybe it is easier not to do the recursive
thing unless the alternate is absolute path, and also as a
purely safety measure, limit the maximum recursion depth to
something low like 5, similar to recursive symlink resolution.

Hmm?