Skip to content

Commit

Permalink
Merge branch 'jt/avoid-lazy-fetch-commits' into jch
Browse files Browse the repository at this point in the history 
Even in a repository with promissor remote, it is useless to
attempt to lazily attempt fetching an object that is expected to be
commit, because no "filter" mode omits commit objects.  Take
advantage of this assumption to fail fast on errors.

* jt/avoid-lazy-fetch-commits:
  commit: don't lazy-fetch commits
  object-file: emit corruption errors when detected
  object-file: refactor map_loose_object_1()
  object-file: remove OBJECT_INFO_IGNORE_LOOSE
  • Loading branch information
@gitster
gitster committed Dec 26, 2022
2 parents 9f441ac + 7e2ad1c commit 9e20dcc
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 61 deletions.
15 changes: 13 additions & 2 deletions commit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,17 @@ int repo_parse_commit_internal(struct repository *r,
enum object_type type;
void *buffer;
unsigned long size;
struct object_info oi = {
.typep = &type,
.sizep = &size,
.contentp = &buffer,
};
/*
* Git does not support partial clones that exclude commits, so set
* OBJECT_INFO_SKIP_FETCH_OBJECT to fail fast when an object is missing.
*/
int flags = OBJECT_INFO_LOOKUP_REPLACE | OBJECT_INFO_SKIP_FETCH_OBJECT |
OBJECT_INFO_DIE_IF_CORRUPT;
int ret;

if (!item)
Expand All @@ -516,8 +527,8 @@ int repo_parse_commit_internal(struct repository *r,
return 0;
if (use_commit_graph && parse_commit_in_graph(r, item))
return 0;
buffer = repo_read_object_file(r, &item->object.oid, &type, &size);
if (!buffer)

if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
return quiet_on_missing ? -1 :
error("Could not read %s",
oid_to_hex(&item->object.oid));
Expand Down
108 changes: 52 additions & 56 deletions object-file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1211,43 +1211,38 @@ static int quick_has_loose(struct repository *r,
}

/*
* Map the loose object at "path" if it is not NULL, or the path found by
* searching for a loose object named "oid".
* Map and close the given loose object fd. The path argument is used for
* error reporting.
*/
static void *map_loose_object_1(struct repository *r, const char *path,
const struct object_id *oid, unsigned long *size)
static void *map_fd(int fd, const char *path, unsigned long *size)
{
void *map;
int fd;

if (path)
fd = git_open(path);
else
fd = open_loose_object(r, oid, &path);
map = NULL;
if (fd >= 0) {
struct stat st;
void *map = NULL;
struct stat st;

if (!fstat(fd, &st)) {
*size = xsize_t(st.st_size);
if (!*size) {
/* mmap() is forbidden on empty files */
error(_("object file %s is empty"), path);
close(fd);
return NULL;
}
map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
if (!fstat(fd, &st)) {
*size = xsize_t(st.st_size);
if (!*size) {
/* mmap() is forbidden on empty files */
error(_("object file %s is empty"), path);
close(fd);
return NULL;
}
close(fd);
map = xmmap(NULL, *size, PROT_READ, MAP_PRIVATE, fd, 0);
}
close(fd);
return map;
}

void *map_loose_object(struct repository *r,
const struct object_id *oid,
unsigned long *size)
{
return map_loose_object_1(r, NULL, oid, size);
const char *p;
int fd = open_loose_object(r, oid, &p);

if (fd < 0)
return NULL;
return map_fd(fd, p, size);
}

enum unpack_loose_header_result unpack_loose_header(git_zstream *stream,
Expand Down Expand Up @@ -1427,7 +1422,9 @@ static int loose_object_info(struct repository *r,
struct object_info *oi, int flags)
{
int status = 0;
int fd;
unsigned long mapsize;
const char *path;
void *map;
git_zstream stream;
char hdr[MAX_HEADER_LEN];
Expand All @@ -1448,7 +1445,6 @@ static int loose_object_info(struct repository *r,
* object even exists.
*/
if (!oi->typep && !oi->type_name && !oi->sizep && !oi->contentp) {
const char *path;
struct stat st;
if (!oi->disk_sizep && (flags & OBJECT_INFO_QUICK))
return quick_has_loose(r, oid) ? 0 : -1;
Expand All @@ -1459,7 +1455,13 @@ static int loose_object_info(struct repository *r,
return 0;
}

map = map_loose_object(r, oid, &mapsize);
fd = open_loose_object(r, oid, &path);
if (fd < 0) {
if (errno != ENOENT)
error_errno(_("unable to open loose object %s"), oid_to_hex(oid));
return -1;
}
map = map_fd(fd, path, &mapsize);
if (!map)
return -1;

Expand Down Expand Up @@ -1497,6 +1499,10 @@ static int loose_object_info(struct repository *r,
break;
}

if (status && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
die(_("loose object %s (stored in %s) is corrupt"),
oid_to_hex(oid), path);

git_inflate_end(&stream);
cleanup:
munmap(map, mapsize);
Expand Down Expand Up @@ -1575,9 +1581,6 @@ static int do_oid_object_info_extended(struct repository *r,
if (find_pack_entry(r, real, &e))
break;

if (flags & OBJECT_INFO_IGNORE_LOOSE)
return -1;

/* Most likely it's a loose object. */
if (!loose_object_info(r, real, oi, flags))
return 0;
Expand Down Expand Up @@ -1609,6 +1612,15 @@ static int do_oid_object_info_extended(struct repository *r,
continue;
}

if (flags & OBJECT_INFO_DIE_IF_CORRUPT) {
const struct packed_git *p;
if ((flags & OBJECT_INFO_LOOKUP_REPLACE) && !oideq(real, oid))
die(_("replacement %s not found for %s"),
oid_to_hex(real), oid_to_hex(oid));
if ((p = has_packed_and_bad(r, real)))
die(_("packed object %s (stored in %s) is corrupt"),
oid_to_hex(real), p->pack_name);
}
return -1;
}

Expand Down Expand Up @@ -1661,15 +1673,17 @@ int oid_object_info(struct repository *r,

static void *read_object(struct repository *r,
const struct object_id *oid, enum object_type *type,
unsigned long *size)
unsigned long *size,
int die_if_corrupt)
{
struct object_info oi = OBJECT_INFO_INIT;
void *content;
oi.typep = type;
oi.sizep = size;
oi.contentp = &content;

if (oid_object_info_extended(r, oid, &oi, 0) < 0)
if (oid_object_info_extended(r, oid, &oi, die_if_corrupt
? OBJECT_INFO_DIE_IF_CORRUPT : 0) < 0)
return NULL;
return content;
}
Expand Down Expand Up @@ -1705,35 +1719,14 @@ void *read_object_file_extended(struct repository *r,
int lookup_replace)
{
void *data;
const struct packed_git *p;
const char *path;
struct stat st;
const struct object_id *repl = lookup_replace ?
lookup_replace_object(r, oid) : oid;

errno = 0;
data = read_object(r, repl, type, size);
data = read_object(r, repl, type, size, 1);
if (data)
return data;

obj_read_lock();
if (errno && errno != ENOENT)
die_errno(_("failed to read object %s"), oid_to_hex(oid));

/* die if we replaced an object with one that does not exist */
if (repl != oid)
die(_("replacement %s not found for %s"),
oid_to_hex(repl), oid_to_hex(oid));

if (!stat_loose_object(r, repl, &st, &path))
die(_("loose object %s (stored in %s) is corrupt"),
oid_to_hex(repl), path);

if ((p = has_packed_and_bad(r, repl)))
die(_("packed object %s (stored in %s) is corrupt"),
oid_to_hex(repl), p->pack_name);
obj_read_unlock();

return NULL;
}

Expand Down Expand Up @@ -2269,7 +2262,7 @@ int force_object_loose(const struct object_id *oid, time_t mtime)

if (has_loose_object(oid))
return 0;
buf = read_object(the_repository, oid, &type, &len);
buf = read_object(the_repository, oid, &type, &len, 0);
if (!buf)
return error(_("cannot read object for %s"), oid_to_hex(oid));
hdrlen = format_object_header(hdr, sizeof(hdr), type, len);
Expand Down Expand Up @@ -2785,13 +2778,16 @@ int read_loose_object(const char *path,
struct object_info *oi)
{
int ret = -1;
int fd;
void *map = NULL;
unsigned long mapsize;
git_zstream stream;
char hdr[MAX_HEADER_LEN];
unsigned long *size = oi->sizep;

map = map_loose_object_1(the_repository, path, NULL, &mapsize);
fd = git_open(path);
if (fd >= 0)
map = map_fd(fd, path, &mapsize);
if (!map) {
error_errno(_("unable to mmap %s"), path);
goto out;
Expand Down
7 changes: 4 additions & 3 deletions object-store.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,19 +434,20 @@ struct object_info {
#define OBJECT_INFO_ALLOW_UNKNOWN_TYPE 2
/* Do not retry packed storage after checking packed and loose storage */
#define OBJECT_INFO_QUICK 8
/* Do not check loose object */
#define OBJECT_INFO_IGNORE_LOOSE 16
/*
* Do not attempt to fetch the object if missing (even if fetch_is_missing is
* nonzero).
*/
#define OBJECT_INFO_SKIP_FETCH_OBJECT 32
#define OBJECT_INFO_SKIP_FETCH_OBJECT 16
/*
* This is meant for bulk prefetching of missing blobs in a partial
* clone. Implies OBJECT_INFO_SKIP_FETCH_OBJECT and OBJECT_INFO_QUICK
*/
#define OBJECT_INFO_FOR_PREFETCH (OBJECT_INFO_SKIP_FETCH_OBJECT | OBJECT_INFO_QUICK)

/* Die if object corruption (not just an object being missing) was detected. */
#define OBJECT_INFO_DIE_IF_CORRUPT 32

int oid_object_info_extended(struct repository *r,
const struct object_id *,
struct object_info *, unsigned flags);
Expand Down

0 comments on commit 9e20dcc

Please sign in to comment.