@matheusbsbs / matheus-bagliano / commits / 199337d6ec

object-file: use real paths when adding alternates

When adding an alternate ODB, we check if the alternate has the same path as the object dir, and if so, we do nothing. However, that comparison does not resolve symlinks. This makes it possible to add the object dir as an alternate, which may result in bad behavior. For example, it can trick "git repack -a -l -d" (possibly run by "git gc") into thinking that all packs come from an alternate and delete all objects. rm -rf test && git clone https://github.com/git/git test && ( cd test && ln -s objects .git/alt-objects && # -c repack.updateserverinfo=false silences a warning about not # being able to update "info/refs", it isn't needed to show the # bad behavior GIT_ALTERNATE_OBJECT_DIRECTORIES=".git/alt-objects" git \ -c repack.updateserverinfo=false repack -a -l -d && # It's broken! git status # Because there are no more objects! ls .git/objects/pack ) Fix this by resolving symlinks and relative paths before comparing the alternate and object dir. This lets us clean up a number of issues noted in 37a95862c6 (alternates: re-allow relative paths from environment, 2016-11-07): - Now that we compare the real paths, duplicate detection is no longer foiled by relative paths. - Using strbuf_realpath() allows us to "normalize" paths that strbuf_normalize_path() can't, so we can stop silently ignoring errors when "normalizing" paths from the environment. - We now store an absolute path based on getcwd() (the "future direction" named in 37a95862c6), so chdir()-ing in the process no longer changes the directory pointed to by the alternate. This is a change in behavior, but a desirable one. Signed-off-by: Glen Choo <chooglen@google.com> Acked-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Glen Choo committed Nov 24, 2022 at 00:55 UTC 199337d6ec5c656e52b914b5dac3820cc5e363f3
2 files changed +29 -13
object-file.c
+13 -13
@@ -508,7 +508,9 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
508 {
509 struct object_directory *ent;
510 struct strbuf pathbuf = STRBUF_INIT;
511 + struct strbuf tmp = STRBUF_INIT;
512 khiter_t pos;
513 + int ret = -1;
514
515 if (!is_absolute_path(entry->buf) && relative_base) {
516 strbuf_realpath(&pathbuf, relative_base, 1);
@@ -516,12 +518,12 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
518 }
519 strbuf_addbuf(&pathbuf, entry);
520
519 - if (strbuf_normalize_path(&pathbuf) < 0 && relative_base) {
521 + if (!strbuf_realpath(&tmp, pathbuf.buf, 0)) {
522 error(_("unable to normalize alternate object path: %s"),
523 pathbuf.buf);
522 - strbuf_release(&pathbuf);
523 - return -1;
524 + goto error;
525 }
526 + strbuf_swap(&pathbuf, &tmp);
527
528 /*
529 * The trailing slash after the directory name is given by
@@ -530,10 +532,8 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
532 while (pathbuf.len && pathbuf.buf[pathbuf.len - 1] == '/')
533 strbuf_setlen(&pathbuf, pathbuf.len - 1);
534
533 - if (!alt_odb_usable(r->objects, &pathbuf, normalized_objdir, &pos)) {
534 - strbuf_release(&pathbuf);
535 - return -1;
536 - }
535 + if (!alt_odb_usable(r->objects, &pathbuf, normalized_objdir, &pos))
536 + goto error;
537
538 CALLOC_ARRAY(ent, 1);
539 /* pathbuf.buf is already in r->objects->odb_by_path */
@@ -548,8 +548,11 @@ static int link_alt_odb_entry(struct repository *r, const struct strbuf *entry,
548
549 /* recursively add alternates */
550 read_info_alternates(r, ent->path, depth + 1);
551 -
552 - return 0;
551 + ret = 0;
552 + error:
553 + strbuf_release(&tmp);
554 + strbuf_release(&pathbuf);
555 + return ret;
556 }
557
558 static const char *parse_alt_odb_entry(const char *string,
@@ -596,10 +599,7 @@ static void link_alt_odb_entries(struct repository *r, const char *alt,
599 return;
600 }
601
599 - strbuf_add_absolute_path(&objdirbuf, r->objects->odb->path);
600 - if (strbuf_normalize_path(&objdirbuf) < 0)
601 - die(_("unable to normalize object directory: %s"),
602 - objdirbuf.buf);
602 + strbuf_realpath(&objdirbuf, r->objects->odb->path, 1);
603
604 while (*alt) {
605 alt = parse_alt_odb_entry(alt, sep, &entry);
t/t7700-repack.sh
+16
@@ -90,6 +90,22 @@ test_expect_success 'loose objects in alternate ODB are not repacked' '
90 test_has_duplicate_object false
91 '
92
93 +test_expect_success SYMLINKS '--local keeps packs when alternate is objectdir ' '
94 + test_when_finished "rm -rf repo" &&
95 + git init repo &&
96 + test_commit -C repo A &&
97 + (
98 + cd repo &&
99 + git repack -a &&
100 + ls .git/objects/pack/*.pack >../expect &&
101 + ln -s objects .git/alt_objects &&
102 + echo "$(pwd)/.git/alt_objects" >.git/objects/info/alternates &&
103 + git repack -a -d -l &&
104 + ls .git/objects/pack/*.pack >../actual
105 + ) &&
106 + test_cmp expect actual
107 +'
108 +
109 test_expect_success 'packed obs in alt ODB are repacked even when local repo is packless' '
110 mkdir alt_objects/pack &&
111 mv .git/objects/pack/* alt_objects/pack &&