repack: repack promisor objects if -a or -A is set
Currently, repack does not touch promisor packfiles at all, potentially causing the performance of repositories that have many such packfiles to drop. Therefore, repack all promisor objects if invoked with -a or -A. This is done by an additional invocation of pack-objects on all promisor objects individually given, which takes care of deduplication and allows the resulting packfiles to respect flags such as --max-pack-size. Signed-off-by: Jonathan Tan <jonathantanmy@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jonathan Tan committed
Aug 8, 2018 at 15:34 UTC
5d19e8138d5f24dbbe8d3736b4e57e117fc099b3
3 files changed
+158
-15
Documentation/git-repack.txt
+5
@@ -40,6 +40,11 @@ OPTIONS
40
Note that users fetching over dumb protocols will have to fetch the
41
whole new pack in order to get any contained object, no matter how many
42
other objects in that pack they already have locally.
43
++
44
+Promisor packfiles are repacked separately: if there are packfiles that
45
+have an associated ".promisor" file, these packfiles will be repacked
46
+into another separate pack, and an empty ".promisor" file corresponding
47
+to the new separate pack will be written.
48
49
-A::
50
Same as `-a`, unless `-d` is used. Then any unreachable
builtin/repack.c
+79
-4
@@ -8,6 +8,7 @@
8
#include "strbuf.h"
9
#include "string-list.h"
10
#include "argv-array.h"
11
+#include "packfile.h"
12
13
static int delta_base_offset = 1;
14
static int pack_kept_objects = -1;
@@ -83,7 +84,7 @@ static void remove_pack_on_signal(int signo)
84
85
/*
86
* Adds all packs hex strings to the fname list, which do not
86
- * have a corresponding .keep or .promisor file. These packs are not to
87
+ * have a corresponding .keep file. These packs are not to
88
* be kept if we are going to pack everything into one file.
89
*/
90
static void get_non_kept_pack_filenames(struct string_list *fname_list,
@@ -111,8 +112,7 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,
112
113
fname = xmemdupz(e->d_name, len);
114
114
- if (!file_exists(mkpath("%s/%s.keep", packdir, fname)) &&
115
- !file_exists(mkpath("%s/%s.promisor", packdir, fname)))
115
+ if (!file_exists(mkpath("%s/%s.keep", packdir, fname)))
116
string_list_append_nodup(fname_list, fname);
117
else
118
free(fname);
@@ -122,7 +122,7 @@ static void get_non_kept_pack_filenames(struct string_list *fname_list,
122
123
static void remove_redundant_pack(const char *dir_name, const char *base_name)
124
{
125
- const char *exts[] = {".pack", ".idx", ".keep", ".bitmap"};
125
+ const char *exts[] = {".pack", ".idx", ".keep", ".bitmap", ".promisor"};
126
int i;
127
struct strbuf buf = STRBUF_INIT;
128
size_t plen;
@@ -179,6 +179,76 @@ static void prepare_pack_objects(struct child_process *cmd,
179
cmd->out = -1;
180
}
181
182
+/*
183
+ * Write oid to the given struct child_process's stdin, starting it first if
184
+ * necessary.
185
+ */
186
+static int write_oid(const struct object_id *oid, struct packed_git *pack,
187
+ uint32_t pos, void *data)
188
+{
189
+ struct child_process *cmd = data;
190
+
191
+ if (cmd->in == -1) {
192
+ if (start_command(cmd))
193
+ die("Could not start pack-objects to repack promisor objects");
194
+ }
195
+
196
+ xwrite(cmd->in, oid_to_hex(oid), GIT_SHA1_HEXSZ);
197
+ xwrite(cmd->in, "\n", 1);
198
+ return 0;
199
+}
200
+
201
+static void repack_promisor_objects(const struct pack_objects_args *args,
202
+ struct string_list *names)
203
+{
204
+ struct child_process cmd = CHILD_PROCESS_INIT;
205
+ FILE *out;
206
+ struct strbuf line = STRBUF_INIT;
207
+
208
+ prepare_pack_objects(&cmd, args);
209
+ cmd.in = -1;
210
+
211
+ /*
212
+ * NEEDSWORK: Giving pack-objects only the OIDs without any ordering
213
+ * hints may result in suboptimal deltas in the resulting pack. See if
214
+ * the OIDs can be sent with fake paths such that pack-objects can use a
215
+ * {type -> existing pack order} ordering when computing deltas instead
216
+ * of a {type -> size} ordering, which may produce better deltas.
217
+ */
218
+ for_each_packed_object(write_oid, &cmd,
219
+ FOR_EACH_OBJECT_PROMISOR_ONLY);
220
+
221
+ if (cmd.in == -1)
222
+ /* No packed objects; cmd was never started */
223
+ return;
224
+
225
+ close(cmd.in);
226
+
227
+ out = xfdopen(cmd.out, "r");
228
+ while (strbuf_getline_lf(&line, out) != EOF) {
229
+ char *promisor_name;
230
+ int fd;
231
+ if (line.len != 40)
232
+ die("repack: Expecting 40 character sha1 lines only from pack-objects.");
233
+ string_list_append(names, line.buf);
234
+
235
+ /*
236
+ * pack-objects creates the .pack and .idx files, but not the
237
+ * .promisor file. Create the .promisor file, which is empty.
238
+ */
239
+ promisor_name = mkpathdup("%s-%s.promisor", packtmp,
240
+ line.buf);
241
+ fd = open(promisor_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
242
+ if (fd < 0)
243
+ die_errno("unable to create '%s'", promisor_name);
244
+ close(fd);
245
+ free(promisor_name);
246
+ }
247
+ fclose(out);
248
+ if (finish_command(&cmd))
249
+ die("Could not finish pack-objects to repack promisor objects");
250
+}
251
+
252
#define ALL_INTO_ONE 1
253
#define LOOSEN_UNREACHABLE 2
254
@@ -191,6 +261,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
261
{".pack"},
262
{".idx"},
263
{".bitmap", 1},
264
+ {".promisor", 1},
265
};
266
struct child_process cmd = CHILD_PROCESS_INIT;
267
struct string_list_item *item;
@@ -293,6 +364,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
364
if (pack_everything & ALL_INTO_ONE) {
365
get_non_kept_pack_filenames(&existing_packs, &keep_pack_list);
366
367
+ repack_promisor_objects(&po_args, &names);
368
+
369
if (existing_packs.nr && delete_redundant) {
370
if (unpack_unreachable) {
371
argv_array_pushf(&cmd.args,
@@ -440,6 +513,8 @@ int cmd_repack(int argc, const char **argv, const char *prefix)
513
514
/* End of pack replacement. */
515
516
+ reprepare_packed_git(the_repository);
517
+
518
if (delete_redundant) {
519
int opts = 0;
520
string_list_sort(&names);
t/t0410-partial-clone.sh
+74
-11
@@ -271,28 +271,91 @@ test_expect_success 'rev-list accepts missing and promised objects on command li
271
git -C repo rev-list --exclude-promisor-objects --objects "$COMMIT" "$TREE" "$BLOB"
272
'
273
274
-test_expect_success 'gc does not repack promisor objects' '
274
+test_expect_success 'gc repacks promisor objects separately from non-promisor objects' '
275
rm -rf repo &&
276
test_create_repo repo &&
277
- test_commit -C repo my_commit &&
277
+ test_commit -C repo one &&
278
+ test_commit -C repo two &&
279
279
- TREE_HASH=$(git -C repo rev-parse HEAD^{tree}) &&
280
- HASH=$(printf "$TREE_HASH\n" | pack_as_from_promisor) &&
280
+ TREE_ONE=$(git -C repo rev-parse one^{tree}) &&
281
+ printf "$TREE_ONE\n" | pack_as_from_promisor &&
282
+ TREE_TWO=$(git -C repo rev-parse two^{tree}) &&
283
+ printf "$TREE_TWO\n" | pack_as_from_promisor &&
284
285
git -C repo config core.repositoryformatversion 1 &&
286
git -C repo config extensions.partialclone "arbitrary string" &&
287
git -C repo gc &&
288
286
- # Ensure that the promisor packfile still exists, and remove it
287
- test -e repo/.git/objects/pack/pack-$HASH.pack &&
288
- rm repo/.git/objects/pack/pack-$HASH.* &&
289
-
290
- # Ensure that the single other pack contains the commit, but not the tree
289
+ # Ensure that exactly one promisor packfile exists, and that it
290
+ # contains the trees but not the commits
291
+ ls repo/.git/objects/pack/pack-*.promisor >promisorlist &&
292
+ test_line_count = 1 promisorlist &&
293
+ PROMISOR_PACKFILE=$(sed "s/.promisor/.pack/" <promisorlist) &&
294
+ git verify-pack $PROMISOR_PACKFILE -v >out &&
295
+ grep "$TREE_ONE" out &&
296
+ grep "$TREE_TWO" out &&
297
+ ! grep "$(git -C repo rev-parse one)" out &&
298
+ ! grep "$(git -C repo rev-parse two)" out &&
299
+
300
+ # Remove the promisor packfile and associated files
301
+ rm $(sed "s/.promisor//" <promisorlist).* &&
302
+
303
+ # Ensure that the single other pack contains the commits, but not the
304
+ # trees
305
ls repo/.git/objects/pack/pack-*.pack >packlist &&
306
test_line_count = 1 packlist &&
307
git verify-pack repo/.git/objects/pack/pack-*.pack -v >out &&
294
- grep "$(git -C repo rev-parse HEAD)" out &&
295
- ! grep "$TREE_HASH" out
308
+ grep "$(git -C repo rev-parse one)" out &&
309
+ grep "$(git -C repo rev-parse two)" out &&
310
+ ! grep "$TREE_ONE" out &&
311
+ ! grep "$TREE_TWO" out
312
+'
313
+
314
+test_expect_success 'gc does not repack promisor objects if there are none' '
315
+ rm -rf repo &&
316
+ test_create_repo repo &&
317
+ test_commit -C repo one &&
318
+
319
+ git -C repo config core.repositoryformatversion 1 &&
320
+ git -C repo config extensions.partialclone "arbitrary string" &&
321
+ git -C repo gc &&
322
+
323
+ # Ensure that only one pack exists
324
+ ls repo/.git/objects/pack/pack-*.pack >packlist &&
325
+ test_line_count = 1 packlist
326
+'
327
+
328
+repack_and_check () {
329
+ rm -rf repo2 &&
330
+ cp -r repo repo2 &&
331
+ git -C repo2 repack $1 -d &&
332
+ git -C repo2 fsck &&
333
+
334
+ git -C repo2 cat-file -e $2 &&
335
+ git -C repo2 cat-file -e $3
336
+}
337
+
338
+test_expect_success 'repack -d does not irreversibly delete promisor objects' '
339
+ rm -rf repo &&
340
+ test_create_repo repo &&
341
+ git -C repo config core.repositoryformatversion 1 &&
342
+ git -C repo config extensions.partialclone "arbitrary string" &&
343
+
344
+ git -C repo commit --allow-empty -m one &&
345
+ git -C repo commit --allow-empty -m two &&
346
+ git -C repo commit --allow-empty -m three &&
347
+ git -C repo commit --allow-empty -m four &&
348
+ ONE=$(git -C repo rev-parse HEAD^^^) &&
349
+ TWO=$(git -C repo rev-parse HEAD^^) &&
350
+ THREE=$(git -C repo rev-parse HEAD^) &&
351
+
352
+ printf "$TWO\n" | pack_as_from_promisor &&
353
+ printf "$THREE\n" | pack_as_from_promisor &&
354
+ delete_object repo "$ONE" &&
355
+
356
+ repack_and_check -a "$TWO" "$THREE" &&
357
+ repack_and_check -A "$TWO" "$THREE" &&
358
+ repack_and_check -l "$TWO" "$THREE"
359
'
360
361
test_expect_success 'gc stops traversal when a missing but promised object is reached' '