Merge branch 'jk/dumb-http-finalize'

The dumb-http code regressed when the result of re-indexing a pack
yielded an *.idx file that differs in content from the *.idx file it
downloaded from the remote. This has been corrected by no longer
relying on the *.idx file we got from the remote.

* jk/dumb-http-finalize:
  packfile: use oidread() instead of hashcpy() to fill object_id
  packfile: use object_id in find_pack_entry_one()
  packfile: convert find_sha1_pack() to use object_id
  http-walker: use object_id instead of bare hash
  packfile: warn people away from parse_packed_git()
  packfile: drop sha1_pack_index_name()
  packfile: drop sha1_pack_name()
  packfile: drop has_pack_index()
  dumb-http: store downloaded pack idx as tempfile
  t5550: count fetches in "previously-fetched .idx" test
  midx: avoid duplicate packed_git entries
This commit is contained in:
Taylor Blau
2024-11-01 12:53:31 -04:00
16 changed files with 153 additions and 101 deletions

43
http.c
View File

@ -19,6 +19,7 @@
#include "string-list.h"
#include "object-file.h"
#include "object-store-ll.h"
#include "tempfile.h"
static struct trace_key trace_curl = TRACE_KEY_INIT(CURL);
static int trace_curl_data = 1;
@ -2390,8 +2391,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.
*
* In theory we could hold on to the tempfile and delete these as soon
* as we download the matching pack, but it would take a bit of
* refactoring. Leaving them until the process ends is probably OK.
*/
tmp = xstrfmt("%s/tmp_pack_%s.idx",
repo_get_object_directory(the_repository),
hash_to_hex(hash));
register_tempfile(tmp);
if (http_get_file(url, tmp, NULL) != HTTP_OK) {
error("Unable to get pack index %s", url);
@ -2405,15 +2422,17 @@ static char *fetch_pack_index(unsigned char *hash, const char *base_url)
static int fetch_and_setup_pack_index(struct packed_git **packs_head,
unsigned char *sha1, const char *base_url)
{
struct packed_git *new_pack;
struct packed_git *new_pack, *p;
char *tmp_idx = NULL;
int ret;
if (has_pack_index(sha1)) {
new_pack = parse_pack_index(sha1, sha1_pack_index_name(sha1));
if (!new_pack)
return -1; /* parse_pack_index() already issued error message */
goto add_pack;
/*
* If we already have the pack locally, no need to fetch its index or
* even add it to list; we already have all of its objects.
*/
for (p = get_all_packs(the_repository); p; p = p->next) {
if (hasheq(p->hash, sha1, the_repository->hash_algo))
return 0;
}
tmp_idx = fetch_pack_index(sha1, base_url);
@ -2429,15 +2448,12 @@ static int fetch_and_setup_pack_index(struct packed_git **packs_head,
}
ret = verify_pack_index(new_pack);
if (!ret) {
if (!ret)
close_pack_index(new_pack);
ret = finalize_object_file(tmp_idx, sha1_pack_index_name(sha1));
}
free(tmp_idx);
if (ret)
return -1;
add_pack:
new_pack->next = *packs_head;
*packs_head = new_pack;
return 0;
@ -2565,7 +2581,8 @@ struct http_pack_request *new_direct_http_pack_request(
preq->url = url;
strbuf_addf(&preq->tmpfile, "%s.temp", sha1_pack_name(packed_git_hash));
odb_pack_name(&preq->tmpfile, packed_git_hash, "pack");
strbuf_addstr(&preq->tmpfile, ".temp");
preq->packfile = fopen(preq->tmpfile.buf, "a");
if (!preq->packfile) {
error("Unable to open local file %s for pack",