[PATCH] Transitively read alternatives

[PATCH] Transitively read alternatives

[Martin Waitz]

When adding an alternate object store then add entries from its
info/alternates files, too.
Relative entries are only allowed in the current repository.
Loops and duplicate alternates through multiple repositories are ignored.
Just to be sure that nothing breaks it is not allow to build deep
nesting levels using info/alternates.

Signed-off-by: Martin Waitz <tali@admingilde.org>

---

 sha1_file.c |  178 +++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 106 insertions(+), 72 deletions(-)

34981f5467a86bb09120cc3c9637b98048fada04
diff --git a/sha1_file.c b/sha1_file.c
index 5464828..b62d0e3 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -217,6 +217,8 @@ char *sha1_pack_index_name(const unsigne
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 
+static void read_info_alternates(const char * alternates, int depth);
+
 /*
  * Prepare alternate object database registry.
  *
@@ -232,14 +234,85 @@ static struct alternate_object_database 
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
-static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
-				 const char *relative_base)
+static int link_alt_odb_entry(const char * entry, int len, const char * relative_base, int depth)
 {
-	const char *cp, *last;
-	struct alternate_object_database *ent;
+	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;
+		entlen += base_len;
+		pfxlen += base_len;
+	}
+	ent = xmalloc(sizeof(*ent) + entlen);
+
+	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);
+
+	ent->name = ent->base + pfxlen + 1;
+	ent->base[pfxlen + 3] = '/';
+	ent->base[pfxlen] = ent->base[entlen-1] = 0;
+
+	/* Detect cases where alternate disappeared */
+	if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
+		error("object directory %s does not exist; "
+		      "check .git/objects/info/alternates.",
+		      ent->base);
+		free(ent);
+		return -1;
+	}
+
+	/* Prevent the common mistake of listing the same
+	 * thing twice, or object directory itself.
+	 */
+	for (alt = alt_odb_list; alt; alt = alt->next) {
+		if (!memcmp(ent->base, alt->base, pfxlen)) {
+			free(ent);
+			return -1;
+		}
+	}
+	if (!memcmp(ent->base, objdir, pfxlen)) {
+		free(ent);
+		return -1;
+	}
+
+	/* add the alternate entry */
+	*alt_odb_tail = ent;
+	alt_odb_tail = &(ent->next);
+	ent->next = NULL;
+
+	/* recursively add alternates */
+	read_info_alternates(ent->base, depth + 1);
+
+	ent->base[pfxlen] = '/';
+
+	return 0;
+}
+
+static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
+				 const char *relative_base, int depth)
+{
+	const char *cp, *last;
+
+	if (depth > 5) {
+		error("%s: ignoring alternate object stores, nesting too deep.",
+				relative_base);
+		return;
+	}
+
 	last = alt;
 	while (last < ep) {
 		cp = last;
@@ -249,60 +322,15 @@ static void link_alt_odb_entries(const c
 			last = cp + 1;
 			continue;
 		}
-		for ( ; cp < ep && *cp != sep; cp++)
-			;
+		while (cp < ep && *cp != sep)
+			cp++;
 		if (last != cp) {
-			struct stat st;
-			struct alternate_object_database *alt;
-			/* 43 = 40-byte + 2 '/' + terminating NUL */
-			int pfxlen = cp - last;
-			int entlen = pfxlen + 43;
-
-			if (*last != '/' && relative_base) {
-				/* Relative alt-odb */
-				if (base_len < 0)
-					base_len = strlen(relative_base) + 1;
-				entlen += base_len;
-				pfxlen += base_len;
-			}
-			ent = xmalloc(sizeof(*ent) + entlen);
-
-			if (*last != '/' && relative_base) {
-				memcpy(ent->base, relative_base, base_len - 1);
-				ent->base[base_len - 1] = '/';
-				memcpy(ent->base + base_len,
-				       last, cp - last);
-			}
-			else
-				memcpy(ent->base, last, pfxlen);
-
-			ent->name = ent->base + pfxlen + 1;
-			ent->base[pfxlen + 3] = '/';
-			ent->base[pfxlen] = ent->base[entlen-1] = 0;
-
-			/* Detect cases where alternate disappeared */
-			if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
-				error("object directory %s does not exist; "
-				      "check .git/objects/info/alternates.",
-				      ent->base);
-				goto bad;
-			}
-			ent->base[pfxlen] = '/';
-
-			/* Prevent the common mistake of listing the same
-			 * thing twice, or object directory itself.
-			 */
-			for (alt = alt_odb_list; alt; alt = alt->next)
-				if (!memcmp(ent->base, alt->base, pfxlen))
-					goto bad;
-			if (!memcmp(ent->base, objdir, pfxlen)) {
-			bad:
-				free(ent);
-			}
-			else {
-				*alt_odb_tail = ent;
-				alt_odb_tail = &(ent->next);
-				ent->next = NULL;
+			if ((*last != '/') && depth) {
+				error("%s: ignoring relative alternate object store %s",
+						relative_base, last);
+			} else {
+				link_alt_odb_entry(last, cp - last,
+						relative_base, depth);
 			}
 		}
 		while (cp < ep && *cp == sep)
@@ -311,23 +339,14 @@ static void link_alt_odb_entries(const c
 	}
 }
 
-void prepare_alt_odb(void)
+static void read_info_alternates(const char * relative_base, int depth)
 {
-	char path[PATH_MAX];
 	char *map;
-	int fd;
 	struct stat st;
-	char *alt;
-
-	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
-	if (!alt) alt = "";
-
-	if (alt_odb_tail)
-		return;
-	alt_odb_tail = &alt_odb_list;
-	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL);
+	char path[PATH_MAX];
+	int fd;
 
-	sprintf(path, "%s/info/alternates", get_object_directory());
+	sprintf(path, "%s/info/alternates", relative_base);
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		return;
@@ -340,11 +359,26 @@ void prepare_alt_odb(void)
 	if (map == MAP_FAILED)
 		return;
 
-	link_alt_odb_entries(map, map + st.st_size, '\n',
-			     get_object_directory());
+	link_alt_odb_entries(map, map + st.st_size, '\n', relative_base, depth);
+
 	munmap(map, st.st_size);
 }
 
+void prepare_alt_odb(void)
+{
+	char *alt;
+
+	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
+	if (!alt) alt = "";
+
+	if (alt_odb_tail)
+		return;
+	alt_odb_tail = &alt_odb_list;
+	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL, 0);
+
+	read_info_alternates(get_object_directory(), 0);
+}
+
 static char *find_sha1_file(const unsigned char *sha1, struct stat *st)
 {
 	char *name = sha1_file_name(sha1);
-- 
1.3.1.g6ef7

Re: [PATCH] Transitively read alternatives

[Jakub Narebski]

Martin Waitz wrote:

> When adding an alternate object store then add entries from its
> info/alternates files, too.
> Relative entries are only allowed in the current repository.
> Loops and duplicate alternates through multiple repositories are ignored.
> Just to be sure that nothing breaks it is not allow to build deep
> nesting levels using info/alternates.

> +     if (depth > 5) {
> +             error("%s: ignoring alternate object stores, nesting too deep.",
> +                             relative_base);
> +             return;
> +     }

Please, no magic numbers. Use preprocesor "constants" for that.

-- 
Jakub Narebski
Warsaw, Poland

[PATCH] Transitively read alternatives

[Martin Waitz]

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

Signed-off-by: Martin Waitz <tali@admingilde.org>

---

 sha1_file.c |  165 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 93 insertions(+), 72 deletions(-)

7fabfb214a971b8b45ec36dcf47929f4bf0917eb
diff --git a/sha1_file.c b/sha1_file.c
index f2d33af..ff9c989 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -216,6 +216,8 @@ char *sha1_pack_index_name(const unsigne
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 
+static void read_info_alternates(const char * alternates);
+
 /*
  * Prepare alternate object database registry.
  *
@@ -231,13 +233,77 @@ static struct alternate_object_database 
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
+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;
+		entlen += base_len;
+		pfxlen += base_len;
+	}
+	ent = xmalloc(sizeof(*ent) + entlen);
+
+	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);
+
+	ent->name = ent->base + pfxlen + 1;
+	ent->base[pfxlen + 3] = '/';
+	ent->base[pfxlen] = ent->base[entlen-1] = 0;
+
+	/* Detect cases where alternate disappeared */
+	if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
+		error("object directory %s does not exist; "
+		      "check .git/objects/info/alternates.",
+		      ent->base);
+		free(ent);
+		return -1;
+	}
+
+	/* Prevent the common mistake of listing the same
+	 * thing twice, or object directory itself.
+	 */
+	for (alt = alt_odb_list; alt; alt = alt->next)
+		if (!memcmp(ent->base, alt->base, pfxlen)) {
+			free(ent);
+			return -1;
+		}
+	if (!memcmp(ent->base, objdir, pfxlen)) {
+		free(ent);
+		return -1;
+	}
+
+	/* recursively add alternates */
+	read_info_alternates(ent->base);
+
+	ent->base[pfxlen] = '/';
+
+	/* add the alternate entry */
+	*alt_odb_tail = ent;
+	alt_odb_tail = &(ent->next);
+	ent->next = NULL;
+
+	return 0;
+}
+
 static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
 				 const char *relative_base)
 {
 	const char *cp, *last;
-	struct alternate_object_database *ent;
-	const char *objdir = get_object_directory();
-	int base_len = -1;
 
 	last = alt;
 	while (last < ep) {
@@ -248,61 +314,10 @@ static void link_alt_odb_entries(const c
 			last = cp + 1;
 			continue;
 		}
-		for ( ; cp < ep && *cp != sep; cp++)
-			;
+		while (cp < ep && *cp != sep)
+			cp++;
 		if (last != cp) {
-			struct stat st;
-			struct alternate_object_database *alt;
-			/* 43 = 40-byte + 2 '/' + terminating NUL */
-			int pfxlen = cp - last;
-			int entlen = pfxlen + 43;
-
-			if (*last != '/' && relative_base) {
-				/* Relative alt-odb */
-				if (base_len < 0)
-					base_len = strlen(relative_base) + 1;
-				entlen += base_len;
-				pfxlen += base_len;
-			}
-			ent = xmalloc(sizeof(*ent) + entlen);
-
-			if (*last != '/' && relative_base) {
-				memcpy(ent->base, relative_base, base_len - 1);
-				ent->base[base_len - 1] = '/';
-				memcpy(ent->base + base_len,
-				       last, cp - last);
-			}
-			else
-				memcpy(ent->base, last, pfxlen);
-
-			ent->name = ent->base + pfxlen + 1;
-			ent->base[pfxlen + 3] = '/';
-			ent->base[pfxlen] = ent->base[entlen-1] = 0;
-
-			/* Detect cases where alternate disappeared */
-			if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
-				error("object directory %s does not exist; "
-				      "check .git/objects/info/alternates.",
-				      ent->base);
-				goto bad;
-			}
-			ent->base[pfxlen] = '/';
-
-			/* Prevent the common mistake of listing the same
-			 * thing twice, or object directory itself.
-			 */
-			for (alt = alt_odb_list; alt; alt = alt->next)
-				if (!memcmp(ent->base, alt->base, pfxlen))
-					goto bad;
-			if (!memcmp(ent->base, objdir, pfxlen)) {
-			bad:
-				free(ent);
-			}
-			else {
-				*alt_odb_tail = ent;
-				alt_odb_tail = &(ent->next);
-				ent->next = NULL;
-			}
+			link_alt_odb_entry(last, cp - last, relative_base);
 		}
 		while (cp < ep && *cp == sep)
 			cp++;
@@ -310,23 +325,14 @@ static void link_alt_odb_entries(const c
 	}
 }
 
-void prepare_alt_odb(void)
+static void read_info_alternates(const char * relative_base)
 {
-	char path[PATH_MAX];
 	char *map;
-	int fd;
 	struct stat st;
-	char *alt;
-
-	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
-	if (!alt) alt = "";
-
-	if (alt_odb_tail)
-		return;
-	alt_odb_tail = &alt_odb_list;
-	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL);
+	char path[PATH_MAX];
+	int fd;
 
-	sprintf(path, "%s/info/alternates", get_object_directory());
+	sprintf(path, "%s/info/alternates", relative_base);
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		return;
@@ -339,11 +345,26 @@ void prepare_alt_odb(void)
 	if (map == MAP_FAILED)
 		return;
 
-	link_alt_odb_entries(map, map + st.st_size, '\n',
-			     get_object_directory());
+	link_alt_odb_entries(map, map + st.st_size, '\n', relative_base);
+
 	munmap(map, st.st_size);
 }
 
+void prepare_alt_odb(void)
+{
+	char *alt;
+
+	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
+	if (!alt) alt = "";
+
+	if (alt_odb_tail)
+		return;
+	alt_odb_tail = &alt_odb_list;
+	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL);
+
+	read_info_alternates(get_object_directory());
+}
+
 static char *find_sha1_file(const unsigned char *sha1, struct stat *st)
 {
 	char *name = sha1_file_name(sha1);
-- 
1.3.1.gc585-dirty

-- 
Martin Waitz

Re: [PATCH] Transitively read alternatives

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> writes:

> When adding an alternate object store then add entries from its
> info/alternates files, too.
>
> Signed-off-by: Martin Waitz <tali@admingilde.org>

Happy.  Sometimes it pays off to leave things that can be
improved as they are, mention just "we might want to do this
instead", and forget about it myself.  Somebody else comes in
and fixes it for me ;-).

If you have a local copy of Linus, Jeff and David trees, you
could arrange Jeff tree to borrow from Linus and David, and
David tree to borrow from Linus and Jeff.  I wonder what the new
code does when you add a new repository that borrows from Jeff
and David.  Does it borrow from Linus twice?  Four times?  Does
it stop without falling into endless recursion?  I did not do
this myself because I was lazy and did not want to address these
issues.

Re: [PATCH] Transitively read alternatives

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 1206 bytes --]

hoi :)

On Mon, May 01, 2006 at 01:56:36PM -0700, Junio C Hamano wrote:
> If you have a local copy of Linus, Jeff and David trees, you
> could arrange Jeff tree to borrow from Linus and David, and
> David tree to borrow from Linus and Jeff.  I wonder what the new
> code does when you add a new repository that borrows from Jeff
> and David.  Does it borrow from Linus twice?  Four times?  Does
> it stop without falling into endless recursion?  I did not do
> this myself because I was lazy and did not want to address these
> issues.

I haven't tested these corner cases extensively yet, but from reading
the code it should behave like this:

If you have loops or multiple references to the same repository then
only the first occurance is used.  When reading a reference to an alternate
object store for the second time it is simply ignored.

But of course you should be careful when you set up alternate files.
Having two repositories reference each other and then simultanously
running git-prune in both is obviously not a good idea ;-).

... more testing ...

Well of course practice and reality always differ... I'll send an
updated patch shortly ;-)

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

[PATCH] Transitively read alternatives

[Martin Waitz]

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

Signed-off-by: Martin Waitz <tali@admingilde.org>

---

 sha1_file.c |  166 +++++++++++++++++++++++++++++++++--------------------------
 1 files changed, 94 insertions(+), 72 deletions(-)

6ef777610f4e314fd93ecf672b9898fb95a7d5af
diff --git a/sha1_file.c b/sha1_file.c
index f2d33af..fb5778c 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -216,6 +216,8 @@ char *sha1_pack_index_name(const unsigne
 struct alternate_object_database *alt_odb_list;
 static struct alternate_object_database **alt_odb_tail;
 
+static void read_info_alternates(const char * alternates);
+
 /*
  * Prepare alternate object database registry.
  *
@@ -231,13 +233,78 @@ static struct alternate_object_database 
  * SHA1, an extra slash for the first level indirection, and the
  * terminating NUL.
  */
+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;
+		entlen += base_len;
+		pfxlen += base_len;
+	}
+	ent = xmalloc(sizeof(*ent) + entlen);
+
+	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);
+
+	ent->name = ent->base + pfxlen + 1;
+	ent->base[pfxlen + 3] = '/';
+	ent->base[pfxlen] = ent->base[entlen-1] = 0;
+
+	/* Detect cases where alternate disappeared */
+	if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
+		error("object directory %s does not exist; "
+		      "check .git/objects/info/alternates.",
+		      ent->base);
+		free(ent);
+		return -1;
+	}
+
+	/* Prevent the common mistake of listing the same
+	 * thing twice, or object directory itself.
+	 */
+	for (alt = alt_odb_list; alt; alt = alt->next) {
+		if (!memcmp(ent->base, alt->base, pfxlen)) {
+			free(ent);
+			return -1;
+		}
+	}
+	if (!memcmp(ent->base, objdir, pfxlen)) {
+		free(ent);
+		return -1;
+	}
+
+	/* add the alternate entry */
+	*alt_odb_tail = ent;
+	alt_odb_tail = &(ent->next);
+	ent->next = NULL;
+
+	/* recursively add alternates */
+	read_info_alternates(ent->base);
+
+	ent->base[pfxlen] = '/';
+
+	return 0;
+}
+
 static void link_alt_odb_entries(const char *alt, const char *ep, int sep,
 				 const char *relative_base)
 {
 	const char *cp, *last;
-	struct alternate_object_database *ent;
-	const char *objdir = get_object_directory();
-	int base_len = -1;
 
 	last = alt;
 	while (last < ep) {
@@ -248,61 +315,10 @@ static void link_alt_odb_entries(const c
 			last = cp + 1;
 			continue;
 		}
-		for ( ; cp < ep && *cp != sep; cp++)
-			;
+		while (cp < ep && *cp != sep)
+			cp++;
 		if (last != cp) {
-			struct stat st;
-			struct alternate_object_database *alt;
-			/* 43 = 40-byte + 2 '/' + terminating NUL */
-			int pfxlen = cp - last;
-			int entlen = pfxlen + 43;
-
-			if (*last != '/' && relative_base) {
-				/* Relative alt-odb */
-				if (base_len < 0)
-					base_len = strlen(relative_base) + 1;
-				entlen += base_len;
-				pfxlen += base_len;
-			}
-			ent = xmalloc(sizeof(*ent) + entlen);
-
-			if (*last != '/' && relative_base) {
-				memcpy(ent->base, relative_base, base_len - 1);
-				ent->base[base_len - 1] = '/';
-				memcpy(ent->base + base_len,
-				       last, cp - last);
-			}
-			else
-				memcpy(ent->base, last, pfxlen);
-
-			ent->name = ent->base + pfxlen + 1;
-			ent->base[pfxlen + 3] = '/';
-			ent->base[pfxlen] = ent->base[entlen-1] = 0;
-
-			/* Detect cases where alternate disappeared */
-			if (stat(ent->base, &st) || !S_ISDIR(st.st_mode)) {
-				error("object directory %s does not exist; "
-				      "check .git/objects/info/alternates.",
-				      ent->base);
-				goto bad;
-			}
-			ent->base[pfxlen] = '/';
-
-			/* Prevent the common mistake of listing the same
-			 * thing twice, or object directory itself.
-			 */
-			for (alt = alt_odb_list; alt; alt = alt->next)
-				if (!memcmp(ent->base, alt->base, pfxlen))
-					goto bad;
-			if (!memcmp(ent->base, objdir, pfxlen)) {
-			bad:
-				free(ent);
-			}
-			else {
-				*alt_odb_tail = ent;
-				alt_odb_tail = &(ent->next);
-				ent->next = NULL;
-			}
+			link_alt_odb_entry(last, cp - last, relative_base);
 		}
 		while (cp < ep && *cp == sep)
 			cp++;
@@ -310,23 +326,14 @@ static void link_alt_odb_entries(const c
 	}
 }
 
-void prepare_alt_odb(void)
+static void read_info_alternates(const char * relative_base)
 {
-	char path[PATH_MAX];
 	char *map;
-	int fd;
 	struct stat st;
-	char *alt;
-
-	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
-	if (!alt) alt = "";
-
-	if (alt_odb_tail)
-		return;
-	alt_odb_tail = &alt_odb_list;
-	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL);
+	char path[PATH_MAX];
+	int fd;
 
-	sprintf(path, "%s/info/alternates", get_object_directory());
+	sprintf(path, "%s/info/alternates", relative_base);
 	fd = open(path, O_RDONLY);
 	if (fd < 0)
 		return;
@@ -339,11 +346,26 @@ void prepare_alt_odb(void)
 	if (map == MAP_FAILED)
 		return;
 
-	link_alt_odb_entries(map, map + st.st_size, '\n',
-			     get_object_directory());
+	link_alt_odb_entries(map, map + st.st_size, '\n', relative_base);
+
 	munmap(map, st.st_size);
 }
 
+void prepare_alt_odb(void)
+{
+	char *alt;
+
+	alt = getenv(ALTERNATE_DB_ENVIRONMENT);
+	if (!alt) alt = "";
+
+	if (alt_odb_tail)
+		return;
+	alt_odb_tail = &alt_odb_list;
+	link_alt_odb_entries(alt, alt + strlen(alt), ':', NULL);
+
+	read_info_alternates(get_object_directory());
+}
+
 static char *find_sha1_file(const unsigned char *sha1, struct stat *st)
 {
 	char *name = sha1_file_name(sha1);
-- 
1.3.1.g6ef7

-- 
Martin Waitz

Re: [PATCH] Transitively read alternatives

[Junio C Hamano]

Martin Waitz <tali@admingilde.org> 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?

Re: [PATCH] Transitively read alternatives

[Martin Waitz]

[-- Attachment #1: Type: text/plain, Size: 632 bytes --]

hoi :)

On Tue, May 02, 2006 at 09:28:25PM -0700, Junio C Hamano wrote:
> > +	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?

hmm, I just moved that part around.

> 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.

Agreed.
Lets see if I have some free time in the evening ;-)

-- 
Martin Waitz

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]