Re: [PATCH 5/8] builtin/clone: move setup of alternates for non-shared local clones
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Aug 31, 2026, 08:13 UTC
- Message-ID
- <apU3xpOZf0b9P6wT@pks.im>
- In-Reply-To
- <874igeuwja.fsf@emacs.iotcl.com>
On Fri, Aug 28, 2026 at 04:52:57PM +0200, Toon Claes wrote:
Show 52 quoted lines
> Patrick Steinhardt <ps@pks.im> writes:
> > diff --git a/builtin/clone.c b/builtin/clone.c
> > index 08c8f5a94f..2e3473fddf 100644
> > --- a/builtin/clone.c
> > +++ b/builtin/clone.c
> > @@ -195,29 +195,41 @@ static void copy_alternates(struct strbuf *src, const char *src_repo)
> > * to turn entries with paths relative to the original
> > * absolute, so that they can be used in the new repository.
> > */
> > - FILE *in = xfopen(src->buf, "r");
> > + FILE *in;
> > + struct strbuf path = STRBUF_INIT;
> > struct strbuf line = STRBUF_INIT;
> >
> > + strbuf_addf(&path, "%s/objects/info/alternates", src_repo);
> > +
> > + in = fopen(path.buf, "r");
> > + if (!in) {
> > + if (errno == ENOENT)
> > + goto out;
> > + die_errno("could not read alternates file '%s'", path.buf);
> > + }
> > +
> > while (strbuf_getline(&line, in) != EOF) {
> > char *abs_path;
> > if (!line.len || line.buf[0] == '#')
> > continue;
> > if (is_absolute_path(line.buf)) {
> > - odb_add_to_alternates_file(the_repository->objects,
> > - line.buf);
> > + strvec_push(alternates, line.buf);
> > continue;
> > }
> > abs_path = mkpathdup("%s/objects/%s", src_repo, line.buf);
> > if (!normalize_path_copy(abs_path, abs_path))
> > - odb_add_to_alternates_file(the_repository->objects,
> > - abs_path);
> > + strvec_push(alternates, abs_path);
> > else
> > warning("skipping invalid relative alternate: %s/%s",
> > src_repo, line.buf);
> > free(abs_path);
> > }
> > +
> > +out:
> > + strbuf_release(&path);
> > strbuf_release(&line);
> > - fclose(in);
> > + if (in)
> > + fclose(in);
>
> Why not put this before the `out` label and remove the if?Mostly because it feels fragile to me. If we were to ever extend this function to have another `goto out` it's easy to miss that we don't close `in` anymore.
Patrick