| Message ID | e0a7b3b310c69350d8e2c0561e0991bb7045a66d.1638224692.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cruft packs | expand |
On 11/29/2021 5:25 PM, Taylor Blau wrote: > +static int dump_mtimes(struct packed_git *p) nit: you return an int here so you can use it as an error code... > +{ > + uint32_t i; > + if (load_pack_mtimes(p) < 0) > + die("could not load pack .mtimes"); > + > + for (i = 0; i < p->num_objects; i++) { > + struct object_id oid; > + if (nth_packed_object_id(&oid, p, i) < 0) > + die("could not load object id at position %"PRIu32, i); > + > + printf("%s %"PRIu32"\n", > + oid_to_hex(&oid), nth_packed_mtime(p, i)); > + } > + > + return 0; But always return 0 unless you die(). > + return p ? dump_mtimes(p) : 1; It makes this line concise, I suppose. Perhaps just use "return dump_mtimes(p)" and have dump_mtimes() return 1 if the given pack is NULL? Thanks, -Stolee
On Mon, Dec 06, 2021 at 04:16:04PM -0500, Derrick Stolee wrote: > On 11/29/2021 5:25 PM, Taylor Blau wrote: > > +static int dump_mtimes(struct packed_git *p) > > nit: you return an int here so you can use it as an error code... > > > +{ > > + uint32_t i; > > + if (load_pack_mtimes(p) < 0) > > + die("could not load pack .mtimes"); > > + > > + for (i = 0; i < p->num_objects; i++) { > > + struct object_id oid; > > + if (nth_packed_object_id(&oid, p, i) < 0) > > + die("could not load object id at position %"PRIu32, i); > > + > > + printf("%s %"PRIu32"\n", > > + oid_to_hex(&oid), nth_packed_mtime(p, i)); > > + } > > + > > + return 0; > > But always return 0 unless you die(). > > > + return p ? dump_mtimes(p) : 1; > > It makes this line concise, I suppose. > > Perhaps just use "return dump_mtimes(p)" and have dump_mtimes() > return 1 if the given pack is NULL? I think just dying in the case we have a NULL pack is fine, and it should be OK to lump it in the same case as "could not load pack .mtimes". But we may want to catch the case a little earlier while we still have the pack name handy. Perhaps something like this on top: --- 8< --- diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c index b143f62520..f7b79daf4c 100644 --- a/t/helper/test-pack-mtimes.c +++ b/t/helper/test-pack-mtimes.c @@ -5,7 +5,7 @@ #include "packfile.h" #include "pack-mtimes.h" -static int dump_mtimes(struct packed_git *p) +static void dump_mtimes(struct packed_git *p) { uint32_t i; if (load_pack_mtimes(p) < 0) @@ -19,8 +19,6 @@ static int dump_mtimes(struct packed_git *p) printf("%s %"PRIu32"\n", oid_to_hex(&oid), nth_packed_mtime(p, i)); } - - return 0; } static const char *pack_mtimes_usage = "\n" @@ -49,5 +47,10 @@ int cmd__pack_mtimes(int argc, const char **argv) strbuf_release(&buf); - return p ? dump_mtimes(p) : 1; + if (!p) + die("could not find pack '%s'", argv[1]); + + dump_mtimes(p); + + return 0; } --- >8 --- Thanks, Taylor
diff --git a/Makefile b/Makefile index efd5e00717..a7382cbfc1 100644 --- a/Makefile +++ b/Makefile @@ -721,6 +721,7 @@ TEST_BUILTINS_OBJS += test-oid-array.o TEST_BUILTINS_OBJS += test-oidmap.o TEST_BUILTINS_OBJS += test-oidtree.o TEST_BUILTINS_OBJS += test-online-cpus.o +TEST_BUILTINS_OBJS += test-pack-mtimes.o TEST_BUILTINS_OBJS += test-parse-options.o TEST_BUILTINS_OBJS += test-parse-pathspec-file.o TEST_BUILTINS_OBJS += test-partial-clone.o diff --git a/t/helper/test-pack-mtimes.c b/t/helper/test-pack-mtimes.c new file mode 100644 index 0000000000..b143f62520 --- /dev/null +++ b/t/helper/test-pack-mtimes.c @@ -0,0 +1,53 @@ +#include "git-compat-util.h" +#include "test-tool.h" +#include "strbuf.h" +#include "object-store.h" +#include "packfile.h" +#include "pack-mtimes.h" + +static int dump_mtimes(struct packed_git *p) +{ + uint32_t i; + if (load_pack_mtimes(p) < 0) + die("could not load pack .mtimes"); + + for (i = 0; i < p->num_objects; i++) { + struct object_id oid; + if (nth_packed_object_id(&oid, p, i) < 0) + die("could not load object id at position %"PRIu32, i); + + printf("%s %"PRIu32"\n", + oid_to_hex(&oid), nth_packed_mtime(p, i)); + } + + return 0; +} + +static const char *pack_mtimes_usage = "\n" +" test-tool pack-mtimes <pack-name.mtimes>"; + +int cmd__pack_mtimes(int argc, const char **argv) +{ + struct strbuf buf = STRBUF_INIT; + struct packed_git *p; + + setup_git_directory(); + + if (argc != 2) + usage(pack_mtimes_usage); + + for (p = get_all_packs(the_repository); p; p = p->next) { + strbuf_addstr(&buf, basename(p->pack_name)); + strbuf_strip_suffix(&buf, ".pack"); + strbuf_addstr(&buf, ".mtimes"); + + if (!strcmp(buf.buf, argv[1])) + break; + + strbuf_reset(&buf); + } + + strbuf_release(&buf); + + return p ? dump_mtimes(p) : 1; +} diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c index 3ce5585e53..1bb1c4b562 100644 --- a/t/helper/test-tool.c +++ b/t/helper/test-tool.c @@ -46,6 +46,7 @@ static struct test_cmd cmds[] = { { "oidmap", cmd__oidmap }, { "oidtree", cmd__oidtree }, { "online-cpus", cmd__online_cpus }, + { "pack-mtimes", cmd__pack_mtimes }, { "parse-options", cmd__parse_options }, { "parse-pathspec-file", cmd__parse_pathspec_file }, { "partial-clone", cmd__partial_clone }, diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h index 9f0f522850..07a2d3f94e 100644 --- a/t/helper/test-tool.h +++ b/t/helper/test-tool.h @@ -35,6 +35,7 @@ int cmd__mktemp(int argc, const char **argv); int cmd__oidmap(int argc, const char **argv); int cmd__oidtree(int argc, const char **argv); int cmd__online_cpus(int argc, const char **argv); +int cmd__pack_mtimes(int argc, const char **argv); int cmd__parse_options(int argc, const char **argv); int cmd__parse_pathspec_file(int argc, const char** argv); int cmd__partial_clone(int argc, const char **argv);
In the next patch, we will implement and test support for writing a cruft pack via a special mode of `git pack-objects`. To make sure that objects are written with the correct timestamps, and a new test-tool that can dump the object names and corresponding timestamps from a given `.mtimes` file. Signed-off-by: Taylor Blau <me@ttaylorr.com> --- Makefile | 1 + t/helper/test-pack-mtimes.c | 53 +++++++++++++++++++++++++++++++++++++ t/helper/test-tool.c | 1 + t/helper/test-tool.h | 1 + 4 files changed, 56 insertions(+) create mode 100644 t/helper/test-pack-mtimes.c