# [PATCH] Transitively read alternatives 6 messages from 2006-05-01 to 2006-05-03. Participants: Martin Waitz, Junio C Hamano. Thread: https://gitlist.dev/t/4018 ## Martin Waitz, 2006-05-01 20:36 Subject: [PATCH] Transitively read alternatives Message-ID: <20060501203631.GH20847@admingilde.org> URL: https://gitlist.dev/e/20060501203631.GH20847%40admingilde.org ``` When adding an alternate object store then add entries from its info/alternates files, too. Signed-off-by: Martin Waitz --- 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 ``` ## Junio C Hamano, 2006-05-01 20:56 Subject: Re: [PATCH] Transitively read alternatives Message-ID: <7v1wvdsbuj.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v1wvdsbuj.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060501203631.GH20847@admingilde.org> ``` Martin Waitz writes: > When adding an alternate object store then add entries from its > info/alternates files, too. > > Signed-off-by: Martin Waitz 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. ``` ## Martin Waitz, 2006-05-01 22:53 Subject: Re: [PATCH] Transitively read alternatives Message-ID: <20060501225335.GI20847@admingilde.org> URL: https://gitlist.dev/e/20060501225335.GI20847%40admingilde.org In-Reply-To: <7v1wvdsbuj.fsf@assigned-by-dhcp.cox.net> ``` 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 ``` ## Martin Waitz, 2006-05-02 07:38 Subject: [PATCH] Transitively read alternatives Message-ID: <20060502073847.GK20847@admingilde.org> URL: https://gitlist.dev/e/20060502073847.GK20847%40admingilde.org In-Reply-To: <20060501225335.GI20847@admingilde.org> ``` When adding an alternate object store then add entries from its info/alternates files, too. Signed-off-by: Martin Waitz --- 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 ``` ## Junio C Hamano, 2006-05-03 04:28 Subject: Re: [PATCH] Transitively read alternatives Message-ID: <7v8xpjhguu.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v8xpjhguu.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <20060502073847.GK20847@admingilde.org> ``` Martin Waitz 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? ``` ## Martin Waitz, 2006-05-03 07:51 Subject: Re: [PATCH] Transitively read alternatives Message-ID: <20060503075133.GO20847@admingilde.org> URL: https://gitlist.dev/e/20060503075133.GO20847%40admingilde.org In-Reply-To: <7v8xpjhguu.fsf@assigned-by-dhcp.cox.net> ``` 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 ```