odb: introduce mtime fields for object info requests

There are some use cases where we need to figure out the mtime for
objects. Most importantly, this is the case when we want to prune
unreachable objects. But getting at that data requires users to manually
derive the info either via the loose object's mtime, the packfiles'
mtime or via the ".mtimes" file.

Introduce a new `struct object_info::mtimep` pointer that allows callers
to request an object's mtime. This new field will be used in a
subsequent commit.

Note that the concept of "mtime" is ambiguous: given an object, it may
be stored multiple times in the object database, and each of these
instances may have a different mtime. Disambiguating these mtimes is
nothing that can happen on the generic ODB layer: the caller may search
for the oldest object, the newest object, or even the relation of object
mtimes depending on the specific source they are located in. As such, it
is the responsibility of the caller to disambiguate mtimes.

A consequence of this is that it's most likely incorrect to look up the
mtime via `odb_read_object_info()`, as this interface does not give us
enough information to disambiguate the mtime. Document this accordingly
and tell users to use `odb_for_each_object()` instead.

Even with this gotcha though it's sensible to have this request as part
of the object info, as the mtime is a property of the object storage
format. If we for example had a "black-box" storage backend, we'd still
need to be able to query it for the mtime info in a generic way.

We could introduce a safety mechanism that for example calls `BUG()` in
case we look up the mtime outside of `odb_for_each_object()`. But that
feels somewhat heavy-handed.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/object-file.c b/object-file.c
index ef2c7618c17a64803d92a5b67aa914f24a768c3a..5537ab2c37099284ef82007ef930bcc73cc5a7de 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -409,6 +409,7 @@ static int read_object_info_from_path(struct odb_source *source,
        char hdr[MAX_HEADER_LEN];
        unsigned long size_scratch;
        enum object_type type_scratch;
+       struct stat st;
 
        /*
         * If we don't care about type or size, then we don't
@@ -421,7 +422,7 @@ static int read_object_info_from_path(struct odb_source *source,
        if (!oi || (!oi->typep && !oi->sizep && !oi->contentp)) {
                struct stat st;
 
-               if ((!oi || !oi->disk_sizep) && (flags & OBJECT_INFO_QUICK)) {
+               if ((!oi || (!oi->disk_sizep && !oi->mtimep)) && (flags & OBJECT_INFO_QUICK)) {
                        ret = quick_has_loose(source->loose, oid) ? 0 : -1;
                        goto out;
                }
@@ -431,8 +432,12 @@ static int read_object_info_from_path(struct odb_source *source,
                        goto out;
                }
 
-               if (oi && oi->disk_sizep)
-                       *oi->disk_sizep = st.st_size;
+               if (oi) {
+                       if (oi->disk_sizep)
+                               *oi->disk_sizep = st.st_size;
+                       if (oi->mtimep)
+                               *oi->mtimep = st.st_mtime;
+               }
 
                ret = 0;
                goto out;
@@ -446,7 +451,21 @@ static int read_object_info_from_path(struct odb_source *source,
                goto out;
        }
 
-       map = map_fd(fd, path, &mapsize);
+       if (fstat(fd, &st)) {
+               close(fd);
+               ret = -1;
+               goto out;
+       }
+
+       mapsize = xsize_t(st.st_size);
+       if (!mapsize) {
+               close(fd);
+               ret = error(_("object file %s is empty"), path);
+               goto out;
+       }
+
+       map = xmmap(NULL, mapsize, PROT_READ, MAP_PRIVATE, fd, 0);
+       close(fd);
        if (!map) {
                ret = -1;
                goto out;
@@ -454,6 +473,8 @@ static int read_object_info_from_path(struct odb_source *source,
 
        if (oi->disk_sizep)
                *oi->disk_sizep = mapsize;
+       if (oi->mtimep)
+               *oi->mtimep = st.st_mtime;
 
        stream_to_end = &stream;
 

diff --git a/odb.c b/odb.c
index 13a415c2c3e415139a8801697f2ee7b40df05e38..9d9a3fad6273697119656addbd730a2f23a03727 100644 (file)
--- a/odb.c
+++ b/odb.c
@@ -702,6 +702,8 @@ static int do_oid_object_info_extended(struct object_database *odb,
                                oidclr(oi->delta_base_oid, odb->repo->hash_algo);
                        if (oi->contentp)
                                *oi->contentp = xmemdupz(co->buf, co->size);
+                       if (oi->mtimep)
+                               *oi->mtimep = 0;
                        oi->whence = OI_CACHED;
                }
                return 0;

diff --git a/odb.h b/odb.h
index b5d28bc188f95704eaa8c13deb66e7a68f22d1f9..8ad0fcc02f148db6ddf1929d32bdc265ff58ea16 100644 (file)
--- a/odb.h
+++ b/odb.h
@@ -318,6 +318,19 @@ struct object_info {
        struct object_id *delta_base_oid;
        void **contentp;
 
+       /*
+        * The time the given looked-up object has been last modified.
+        *
+        * Note: the mtime may be ambiguous in case the object exists multiple
+        * times in the object database. It is thus _not_ recommended to use
+        * this field outside of contexts where you would read every instance
+        * of the object, like for example with `odb_for_each_object()`. As it
+        * is impossible to say at the ODB level what the intent of the caller
+        * is (e.g. whether to find the oldest or newest object), it is the
+        * responsibility of the caller to disambiguate the mtimes.
+        */
+       time_t *mtimep;
+
        /* Response */
        enum {
                OI_CACHED,

diff --git a/packfile.c b/packfile.c
index c54deabd645075edd5a4fef675d982f8983c18b6..845633139f9942b83809944693590f0e18ed355d 100644 (file)
--- a/packfile.c
+++ b/packfile.c
@@ -1578,13 +1578,14 @@ static void add_delta_base_cache(struct packed_git *p, off_t base_offset,
        hashmap_add(&delta_base_cache, &ent->ent);
 }
 
-int packed_object_info(struct packed_git *p,
-                      off_t obj_offset, struct object_info *oi)
+static int packed_object_info_with_index_pos(struct packed_git *p, off_t obj_offset,
+                                            uint32_t *maybe_index_pos, struct object_info *oi)
 {
        struct pack_window *w_curs = NULL;
        unsigned long size;
        off_t curpos = obj_offset;
        enum object_type type = OBJ_NONE;
+       uint32_t pack_pos;
        int ret;
 
        /*
@@ -1619,16 +1620,35 @@ int packed_object_info(struct packed_git *p,
                }
        }
 
-       if (oi->disk_sizep) {
-               uint32_t pos;
-               if (offset_to_pack_pos(p, obj_offset, &pos) < 0) {
+       if (oi->disk_sizep || (oi->mtimep && p->is_cruft)) {
+               if (offset_to_pack_pos(p, obj_offset, &pack_pos) < 0) {
                        error("could not find object at offset %"PRIuMAX" "
                              "in pack %s", (uintmax_t)obj_offset, p->pack_name);
                        ret = -1;
                        goto out;
                }
+       }
+
+       if (oi->disk_sizep)
+               *oi->disk_sizep = pack_pos_to_offset(p, pack_pos + 1) - obj_offset;
+
+       if (oi->mtimep) {
+               if (p->is_cruft) {
+                       uint32_t index_pos;
+
+                       if (load_pack_mtimes(p) < 0)
+                               die(_("could not load .mtimes for cruft pack '%s'"),
+                                   pack_basename(p));
+
+                       if (maybe_index_pos)
+                               index_pos = *maybe_index_pos;
+                       else
+                               index_pos = pack_pos_to_index(p, pack_pos);
 
-               *oi->disk_sizep = pack_pos_to_offset(p, pos + 1) - obj_offset;
+                       *oi->mtimep = nth_packed_mtime(p, index_pos);
+               } else {
+                       *oi->mtimep = p->mtime;
+               }
        }
 
        if (oi->typep) {
@@ -1681,6 +1701,12 @@ out:
        return ret;
 }
 
+int packed_object_info(struct packed_git *p, off_t obj_offset,
+                      struct object_info *oi)
+{
+       return packed_object_info_with_index_pos(p, obj_offset, NULL, oi);
+}
+
 static void *unpack_compressed_entry(struct packed_git *p,
                                    struct pack_window **w_curs,
                                    off_t curpos,
@@ -2378,7 +2404,8 @@ static int packfile_store_for_each_object_wrapper(const struct object_id *oid,
                off_t offset = nth_packed_object_offset(pack, index_pos);
                struct object_info oi = *data->request;
 
-               if (packed_object_info(pack, offset, &oi) < 0) {
+               if (packed_object_info_with_index_pos(pack, offset,
+                                                     &index_pos, &oi) < 0) {
                        mark_bad_packed_object(pack, oid);
                        return -1;
                }