From: Omar Polo Subject: Re: gotadmin: add a flag to force the usage of ref-delta To: Stefan Sperling Cc: gameoftrees@openbsd.org Date: Fri, 17 Feb 2023 15:46:11 +0100 On 2023/02/17 13:40:05 +0100, Stefan Sperling wrote: > On Fri, Feb 17, 2023 at 10:44:46AM +0100, Omar Polo wrote: > > Diff below adds a -D flag that force gotadmin to generate a packfile > > using ref-delta only. It's meant only for the regress suite, since > > that part of the code is never triggered by the regress. > > > > I'm not sure if documenting this option makes sense. Outside of the > > regress suite it shouldn't be used; if only because it makes packfiles > > bigger. > > There can be various reasons why people might want to use this > option (e.g. compat with very old versions of Git). I would just > leave the purpose documented and not even mention debugging. I forgot about the compat with very old version of Git > If someone doesn't understand what this option does they should > not be using it. And we do in fact document the difference between > the two types of deltas in git-repository(5). and i also forgot that we document the deltas in git-repository! ^^ > > @@ -128,6 +128,9 @@ Unless this option is specified, only loose objects wi > > Add objects to the generated pack file even if they are already packed > > in a different pack file. > > Unless this option is specified, only loose objects will be added. > > +.It Fl D > > +Force the packfile to use the ref-delta representation for objects. > > +It is intended for debug only. > > I would drop "It is intended for debug only." and add: > > If this option is not specified, offset-deltas will be used to > represent deltified objects. Agreed. I've actually tweaked it a bit more and now it reads -D Force the use of ref-delta representation for deltified objects. if this option is not specified, offset-deltas will be used to represent deltified objects. diff /home/op/w/gotd commit - cde544b29d344a5c884ca3948d1826520308a353 path + /home/op/w/gotd blob - e11b3a0b3586e7aac0c6955aac2096e7dc6d7fe6 file + gotadmin/gotadmin.1 --- gotadmin/gotadmin.1 +++ gotadmin/gotadmin.1 @@ -96,7 +96,7 @@ work tree, use the repository path associated with thi .El .It Xo .Cm pack -.Op Fl aq +.Op Fl aDq .Op Fl r Ar repository-path .Op Fl x Ar reference .Op Ar reference ... @@ -128,6 +128,10 @@ Unless this option is specified, only loose objects wi Add objects to the generated pack file even if they are already packed in a different pack file. Unless this option is specified, only loose objects will be added. +.It Fl D +Force the use of ref-delta representation for deltified objects. +If this option is not specified, offset-deltas will be used to represent +deltified objects. .It Fl q Suppress progress reporting output. .It Fl r Ar repository-path blob - 158bbdc93cdc3b2264d3efa8d639edde028f21d8 file + gotadmin/gotadmin.c --- gotadmin/gotadmin.c +++ gotadmin/gotadmin.c @@ -452,7 +452,7 @@ usage_pack(void) __dead static void usage_pack(void) { - fprintf(stderr, "usage: %s pack [-aq] [-r repository-path] " + fprintf(stderr, "usage: %s pack [-aDq] [-r repository-path] " "[-x reference] [reference ...]\n", getprogname()); exit(1); } @@ -692,7 +692,7 @@ cmd_pack(int argc, char *argv[]) const struct got_error *error = NULL; char *repo_path = NULL; struct got_repository *repo = NULL; - int ch, i, loose_obj_only = 1, verbosity = 0; + int ch, i, loose_obj_only = 1, force_refdelta = 0, verbosity = 0; struct got_object_id *pack_hash = NULL; char *id_str = NULL; struct got_pack_progress_arg ppa; @@ -714,11 +714,14 @@ cmd_pack(int argc, char *argv[]) err(1, "pledge"); #endif - while ((ch = getopt(argc, argv, "aqr:x:")) != -1) { + while ((ch = getopt(argc, argv, "aDqr:x:")) != -1) { switch (ch) { case 'a': loose_obj_only = 0; break; + case 'D': + force_refdelta = 1; + break; case 'q': verbosity = -1; break; @@ -802,7 +805,7 @@ cmd_pack(int argc, char *argv[]) error = got_repo_pack_objects(&packfile, &pack_hash, &include_refs, &exclude_refs, repo, loose_obj_only, - pack_progress, &ppa, check_cancelled, NULL); + force_refdelta, pack_progress, &ppa, check_cancelled, NULL); if (error) { if (ppa.printed_something) printf("\n"); blob - 2caf2a94d4ed262a215863f07c9e967a8f052cc3 file + include/got_repository_admin.h --- include/got_repository_admin.h +++ include/got_repository_admin.h @@ -34,7 +34,7 @@ got_repo_pack_objects(FILE **packfile, struct got_obje got_repo_pack_objects(FILE **packfile, struct got_object_id **pack_hash, struct got_reflist_head *include_refs, struct got_reflist_head *exclude_refs, struct got_repository *repo, - int loose_obj_only, + int loose_obj_only, int force_refdelta, got_pack_progress_cb progress_cb, void *progress_arg, got_cancel_cb cancel_cb, void *cancel_arg); blob - 7a3c6579c0c3c5ab813f5c5cdb1093a791ac08b4 file + lib/got_lib_pack_create.h --- lib/got_lib_pack_create.h +++ lib/got_lib_pack_create.h @@ -25,7 +25,7 @@ const struct got_error *got_pack_create(uint8_t *pack_ FILE *delta_cache, struct got_object_id **theirs, int ntheirs, struct got_object_id **ours, int nours, struct got_repository *repo, int loose_obj_only, int allow_empty, - got_pack_progress_cb progress_cb, void *progress_arg, + int force_refdelta, got_pack_progress_cb progress_cb, void *progress_arg, struct got_ratelimit *, got_cancel_cb cancel_cb, void *cancel_arg); const struct got_error * blob - 0dda4f95f2b3f8b16523c768cd55fa04de120f90 file + lib/pack_create.c --- lib/pack_create.c +++ lib/pack_create.c @@ -1550,14 +1550,14 @@ deltahdr(off_t *packfile_size, SHA1_CTX *ctx, int pack } static const struct got_error * -deltahdr(off_t *packfile_size, SHA1_CTX *ctx, int packfd, +deltahdr(off_t *packfile_size, SHA1_CTX *ctx, int packfd, int force_refdelta, struct got_pack_meta *m) { const struct got_error *err; char buf[32]; int nh; - if (m->prev->off != 0) { + if (m->prev->off != 0 && !force_refdelta) { err = packhdr(&nh, buf, sizeof(buf), GOT_OBJ_TYPE_OFFSET_DELTA, m->delta_len); if (err) @@ -1590,7 +1590,7 @@ write_packed_object(off_t *packfile_size, int packfd, write_packed_object(off_t *packfile_size, int packfd, FILE *delta_cache, uint8_t *delta_cache_map, size_t delta_cache_size, struct got_pack_meta *m, int *outfd, SHA1_CTX *ctx, - struct got_repository *repo) + struct got_repository *repo, int force_refdelta) { const struct got_error *err = NULL; struct got_deflate_checksum csum; @@ -1641,7 +1641,7 @@ write_packed_object(off_t *packfile_size, int packfd, got_object_raw_close(raw); raw = NULL; } else if (m->delta_buf) { - err = deltahdr(packfile_size, ctx, packfd, m); + err = deltahdr(packfile_size, ctx, packfd, force_refdelta, m); if (err) goto done; err = hwrite(packfd, m->delta_buf, @@ -1652,7 +1652,7 @@ write_packed_object(off_t *packfile_size, int packfd, free(m->delta_buf); m->delta_buf = NULL; } else if (delta_cache_map) { - err = deltahdr(packfile_size, ctx, packfd, m); + err = deltahdr(packfile_size, ctx, packfd, force_refdelta, m); if (err) goto done; err = hcopy_mmap(delta_cache_map, delta_offset, @@ -1666,7 +1666,7 @@ write_packed_object(off_t *packfile_size, int packfd, err = got_error_from_errno("fseeko"); goto done; } - err = deltahdr(packfile_size, ctx, packfd, m); + err = deltahdr(packfile_size, ctx, packfd, force_refdelta, m); if (err) goto done; err = hcopy(delta_cache, packfd, @@ -1686,7 +1686,7 @@ genpack(uint8_t *pack_sha1, int packfd, struct got_pac FILE *delta_cache, struct got_pack_meta **deltify, int ndeltify, struct got_pack_meta **reuse, int nreuse, int ncolored, int nfound, int ntrees, int nours, - struct got_repository *repo, + struct got_repository *repo, int force_refdelta, got_pack_progress_cb progress_cb, void *progress_arg, struct got_ratelimit *rl, got_cancel_cb cancel_cb, void *cancel_arg) @@ -1750,7 +1750,7 @@ genpack(uint8_t *pack_sha1, int packfd, struct got_pac m = deltify[i]; err = write_packed_object(&packfile_size, packfd, delta_cache, delta_cache_map, delta_cache_size, - m, &outfd, &ctx, repo); + m, &outfd, &ctx, repo, force_refdelta); if (err) goto done; } @@ -1779,7 +1779,7 @@ genpack(uint8_t *pack_sha1, int packfd, struct got_pac m = reuse[i]; err = write_packed_object(&packfile_size, packfd, packfile, reuse_pack->map, reuse_pack->filesize, - m, &outfd, &ctx, repo); + m, &outfd, &ctx, repo, force_refdelta); if (err) goto done; } @@ -1826,7 +1826,7 @@ got_pack_create(uint8_t *packsha1, int packfd, FILE *d struct got_object_id **theirs, int ntheirs, struct got_object_id **ours, int nours, struct got_repository *repo, int loose_obj_only, int allow_empty, - got_pack_progress_cb progress_cb, void *progress_arg, + int force_refdelta, got_pack_progress_cb progress_cb, void *progress_arg, struct got_ratelimit *rl, got_cancel_cb cancel_cb, void *cancel_arg) { const struct got_error *err; @@ -1933,7 +1933,7 @@ got_pack_create(uint8_t *packsha1, int packfd, FILE *d err = genpack(packsha1, packfd, reuse_pack, delta_cache, deltify.meta, deltify.nmeta, reuse.meta, reuse.nmeta, ncolored, nfound, ntrees, - nours, repo, progress_cb, progress_arg, rl, + nours, repo, force_refdelta, progress_cb, progress_arg, rl, cancel_cb, cancel_arg); if (err) goto done; blob - 0be05225c5acde90af06fcbe60d29ec6a8044bbe file + lib/repository_admin.c --- lib/repository_admin.c +++ lib/repository_admin.c @@ -145,7 +145,7 @@ got_repo_pack_objects(FILE **packfile, struct got_obje got_repo_pack_objects(FILE **packfile, struct got_object_id **pack_hash, struct got_reflist_head *include_refs, struct got_reflist_head *exclude_refs, struct got_repository *repo, - int loose_obj_only, + int loose_obj_only, int force_refdelta, got_pack_progress_cb progress_cb, void *progress_arg, got_cancel_cb cancel_cb, void *cancel_arg) { @@ -209,8 +209,9 @@ got_repo_pack_objects(FILE **packfile, struct got_obje } err = got_pack_create((*pack_hash)->sha1, packfd, delta_cache, - theirs, ntheirs, ours, nours, repo, loose_obj_only, 0, - progress_cb, progress_arg, &rl, cancel_cb, cancel_arg); + theirs, ntheirs, ours, nours, repo, loose_obj_only, + 0, force_refdelta, progress_cb, progress_arg, &rl, + cancel_cb, cancel_arg); if (err) goto done; blob - ec59732b5d47bf06ab52a0c1cced9366c7816e51 file + lib/send.c --- lib/send.c +++ lib/send.c @@ -647,7 +647,7 @@ got_send_pack(const char *remote_name, struct got_path ppa.progress_cb = progress_cb; ppa.progress_arg = progress_arg; err = got_pack_create(packsha1, packfd, delta_cache, - their_ids, ntheirs, our_ids, nours, repo, 0, 1, + their_ids, ntheirs, our_ids, nours, repo, 0, 0, 1, pack_progress, &ppa, &rl, cancel_cb, cancel_arg); if (err) goto done;