Re: Bug report: v2.47.0 cannot fetch version 1 pack indexes

> > @@ -2388,8 +2389,24 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url)
> >  	strbuf_addf(&buf, "objects/pack/pack-%s.idx", hash_to_hex(hash));
> >  	url = strbuf_detach(&buf, NULL);
> >
> > -	strbuf_addf(&buf, "%s.temp", sha1_pack_index_name(hash));
> > -	tmp = strbuf_detach(&buf, NULL);
> > +	/*
> > +	 * Don't put this into packs/, since it's just temporary and we don't
> > +	 * want to confuse it with our local .idx files.  We'll generate our
> > +	 * own index if we choose to download the matching packfile.
> > +	 *
> > +	 * It's tempting to use xmks_tempfile() here, but it's important that
> > +	 * the file not exist, otherwise http_get_file() complains. So we
> > +	 * create a filename that should be unique, and then just register it
> > +	 * as a tempfile so that it will get cleaned up on exit.
> > +	 *
> > +	 * Arguably it would be better to hold on to the tempfile handle so
> > +	 * that we can remove it as soon as we download the pack and generate
> > +	 * the real index, but that might need more surgery.
> > +	 */
> > +	tmp = xstrfmt("%s/tmp_pack_%s.idx",
> > +		      repo_get_object_directory(the_repository),
> > +		      hash_to_hex(hash));
> > +	register_tempfile(tmp);
> 
> Makes perfect sense, and the comment above here is much appreciated.
> 
> I thought about trying to use some intermediate state of the strbuf here
> to avoid an extra xstrfmt() call, but couldn't come up with anything I
> didn't think was awkward.

> > +static char *pack_path_from_idx(const char *idx_path)
> > +{
> > +	size_t len;
> > +	if (!strip_suffix(idx_path, ".idx", &len))
> > +		BUG("idx path does not end in .idx: %s", idx_path);
> > +	return xstrfmt("%.*s.pack", (int)len, idx_path);
> > +}
> > +
> >  struct packed_git *parse_pack_index(unsigned char *sha1, const char *idx_path)
> >  {
> > -	const char *path = sha1_pack_name(sha1);
> > +	char *path = pack_path_from_idx(idx_path);
> 
> Huh. I would have thought we have such a helper function already. I
> guess we probably do, but that it's also defined statically because it's
> so easy to write.

> In any case, this looks like the right thing to do here. It would be
> nice to have a corresponding test here, since unlike the other
> finalize_object_file() changes, this one can be provoked
> deterministically.
> 
> Would you mind submitting this as a bona-fide patch, which I can then
> pick up and start merging down?