bundle: arguments can be read from stdin
In order to create an incremental bundle, we need to pass many arguments to let git-bundle ignore some already packed commits. It will be more convenient to pass args via stdin. But the current implementation does not allow us to do this. This is because args are parsed twice when creating bundle. The first time for parsing args is in `compute_and_write_prerequisites()` by running `git-rev-list` command to write prerequisites in bundle file, and stdin is consumed in this step if "--stdin" option is provided for `git-bundle`. Later nothing can be read from stdin when running `setup_revisions()` in `create_bundle()`. The solution is to parse args once by removing the entire function `compute_and_write_prerequisites()` and then calling function `setup_revisions()`. In order to write prerequisites for bundle, will call `prepare_revision_walk()` and `traverse_commit_list()`. But after calling `prepare_revision_walk()`, the object array `revs.pending` is left empty, and the following steps could not work properly with the empty object array (`revs.pending`). Therefore, make a copy of `revs` to `revs_copy` for later use right after calling `setup_revisions()`. The copy of `revs_copy` is not a deep copy, it shares the same objects with `revs`. The object array of `revs` has been cleared, but objects themselves are still kept. Flags of objects may change after calling `prepare_revision_walk()`, we can use these changed flags without calling the `git rev-list` command and parsing its output like the former implementation. Also add testcases for git bundle in t6020, which read args from stdin. Signed-off-by: Jiang Xin <zhiyou.jx@alibaba-inc.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jiang Xin committed
Jan 11, 2021 at 21:27 UTC
5bb0fd2cab5a18bc188c8154097857f51a0e9feb
3 files changed
+134
-56
bundle.c
+59
-50
@@ -338,48 +338,6 @@ static int write_pack_data(int bundle_fd, struct rev_info *revs, struct strvec *
338
return 0;
339
}
340
341
-static int compute_and_write_prerequisites(int bundle_fd,
342
- struct rev_info *revs,
343
- int argc, const char **argv)
344
-{
345
- struct child_process rls = CHILD_PROCESS_INIT;
346
- struct strbuf buf = STRBUF_INIT;
347
- FILE *rls_fout;
348
- int i;
349
-
350
- strvec_pushl(&rls.args,
351
- "rev-list", "--boundary", "--pretty=oneline",
352
- NULL);
353
- for (i = 1; i < argc; i++)
354
- strvec_push(&rls.args, argv[i]);
355
- rls.out = -1;
356
- rls.git_cmd = 1;
357
- if (start_command(&rls))
358
- return -1;
359
- rls_fout = xfdopen(rls.out, "r");
360
- while (strbuf_getwholeline(&buf, rls_fout, '\n') != EOF) {
361
- struct object_id oid;
362
- if (buf.len > 0 && buf.buf[0] == '-') {
363
- write_or_die(bundle_fd, buf.buf, buf.len);
364
- if (!get_oid_hex(buf.buf + 1, &oid)) {
365
- struct object *object = parse_object_or_die(&oid,
366
- buf.buf);
367
- object->flags |= UNINTERESTING;
368
- add_pending_object(revs, object, buf.buf);
369
- }
370
- } else if (!get_oid_hex(buf.buf, &oid)) {
371
- struct object *object = parse_object_or_die(&oid,
372
- buf.buf);
373
- object->flags |= SHOWN;
374
- }
375
- }
376
- strbuf_release(&buf);
377
- fclose(rls_fout);
378
- if (finish_command(&rls))
379
- return error(_("rev-list died"));
380
- return 0;
381
-}
382
-
341
/*
342
* Write out bundle refs based on the tips already
343
* parsed into revs.pending. As a side effect, may
@@ -474,6 +432,38 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)
432
return ref_count;
433
}
434
435
+struct bundle_prerequisites_info {
436
+ struct object_array *pending;
437
+ int fd;
438
+};
439
+
440
+static void write_bundle_prerequisites(struct commit *commit, void *data)
441
+{
442
+ struct bundle_prerequisites_info *bpi = data;
443
+ struct object *object;
444
+ struct pretty_print_context ctx = { 0 };
445
+ struct strbuf buf = STRBUF_INIT;
446
+
447
+ if (!(commit->object.flags & BOUNDARY))
448
+ return;
449
+ strbuf_addf(&buf, "-%s ", oid_to_hex(&commit->object.oid));
450
+ write_or_die(bpi->fd, buf.buf, buf.len);
451
+
452
+ ctx.fmt = CMIT_FMT_ONELINE;
453
+ ctx.output_encoding = get_log_output_encoding();
454
+ strbuf_reset(&buf);
455
+ pretty_print_commit(&ctx, commit, &buf);
456
+ strbuf_trim(&buf);
457
+
458
+ object = (struct object *)commit;
459
+ object->flags |= UNINTERESTING;
460
+ add_object_array_with_path(object, buf.buf, bpi->pending, S_IFINVALID,
461
+ NULL);
462
+ strbuf_addch(&buf, '\n');
463
+ write_or_die(bpi->fd, buf.buf, buf.len);
464
+ strbuf_release(&buf);
465
+}
466
+
467
int create_bundle(struct repository *r, const char *path,
468
int argc, const char **argv, struct strvec *pack_options, int version)
469
{
@@ -481,8 +471,10 @@ int create_bundle(struct repository *r, const char *path,
471
int bundle_fd = -1;
472
int bundle_to_stdout;
473
int ref_count = 0;
484
- struct rev_info revs;
474
+ struct rev_info revs, revs_copy;
475
int min_version = the_hash_algo == &hash_algos[GIT_HASH_SHA1] ? 2 : 3;
476
+ struct bundle_prerequisites_info bpi;
477
+ int i;
478
479
bundle_to_stdout = !strcmp(path, "-");
480
if (bundle_to_stdout)
@@ -512,10 +504,6 @@ int create_bundle(struct repository *r, const char *path,
504
save_commit_buffer = 0;
505
repo_init_revisions(r, &revs, NULL);
506
515
- /* write prerequisites */
516
- if (compute_and_write_prerequisites(bundle_fd, &revs, argc, argv))
517
- goto err;
518
-
507
argc = setup_revisions(argc, argv, &revs, NULL);
508
509
if (argc > 1) {
@@ -523,16 +511,37 @@ int create_bundle(struct repository *r, const char *path,
511
goto err;
512
}
513
526
- object_array_remove_duplicates(&revs.pending);
514
+ /* save revs.pending in revs_copy for later use */
515
+ memcpy(&revs_copy, &revs, sizeof(revs));
516
+ revs_copy.pending.nr = 0;
517
+ revs_copy.pending.alloc = 0;
518
+ revs_copy.pending.objects = NULL;
519
+ for (i = 0; i < revs.pending.nr; i++) {
520
+ struct object_array_entry *e = revs.pending.objects + i;
521
+ if (e)
522
+ add_object_array_with_path(e->item, e->name,
523
+ &revs_copy.pending,
524
+ e->mode, e->path);
525
+ }
526
528
- ref_count = write_bundle_refs(bundle_fd, &revs);
527
+ /* write prerequisites */
528
+ revs.boundary = 1;
529
+ if (prepare_revision_walk(&revs))
530
+ die("revision walk setup failed");
531
+ bpi.fd = bundle_fd;
532
+ bpi.pending = &revs_copy.pending;
533
+ traverse_commit_list(&revs, write_bundle_prerequisites, NULL, &bpi);
534
+ object_array_remove_duplicates(&revs_copy.pending);
535
+
536
+ /* write bundle refs */
537
+ ref_count = write_bundle_refs(bundle_fd, &revs_copy);
538
if (!ref_count)
539
die(_("Refusing to create empty bundle."));
540
else if (ref_count < 0)
541
goto err;
542
543
/* write pack */
535
- if (write_pack_data(bundle_fd, &revs, pack_options))
544
+ if (write_pack_data(bundle_fd, &revs_copy, pack_options))
545
goto err;
546
547
if (!bundle_to_stdout) {
t/t5607-clone-bundle.sh
+2
-2
@@ -38,13 +38,13 @@ test_expect_success 'die if bundle file cannot be created' '
38
test_must_fail git bundle create adir --all
39
'
40
41
-test_expect_failure 'bundle --stdin' '
41
+test_expect_success 'bundle --stdin' '
42
echo master | git bundle create stdin-bundle.bdl --stdin &&
43
git ls-remote stdin-bundle.bdl >output &&
44
grep master output
45
'
46
47
-test_expect_failure 'bundle --stdin <rev-list options>' '
47
+test_expect_success 'bundle --stdin <rev-list options>' '
48
echo master | git bundle create hybrid-bundle.bdl --stdin tag &&
49
git ls-remote hybrid-bundle.bdl >output &&
50
grep master output
t/t6020-bundle-misc.sh
+73
-4
@@ -242,8 +242,16 @@ test_expect_success 'create bundle with --since option' '
242
'
243
244
test_expect_success 'create bundle 1 - no prerequisites' '
245
+ # create bundle from args
246
git bundle create 1.bdl topic/1 topic/2 &&
247
248
+ # create bundle from stdin
249
+ cat >input <<-\EOF &&
250
+ topic/1
251
+ topic/2
252
+ EOF
253
+ git bundle create stdin-1.bdl --stdin <input &&
254
+
255
cat >expect <<-\EOF &&
256
The bundle contains these 2 refs:
257
<COMMIT-D> refs/heads/topic/1
@@ -256,10 +264,16 @@ test_expect_success 'create bundle 1 - no prerequisites' '
264
make_user_friendly_and_stable_output >actual &&
265
test_i18ncmp expect actual &&
266
259
- test_bundle_object_count 1.bdl 24
267
+ git bundle verify stdin-1.bdl |
268
+ make_user_friendly_and_stable_output >actual &&
269
+ test_i18ncmp expect actual &&
270
+
271
+ test_bundle_object_count 1.bdl 24 &&
272
+ test_bundle_object_count stdin-1.bdl 24
273
'
274
275
test_expect_success 'create bundle 2 - has prerequisites' '
276
+ # create bundle from args
277
git bundle create 2.bdl \
278
--ignore-missing \
279
^topic/deleted \
@@ -267,6 +281,18 @@ test_expect_success 'create bundle 2 - has prerequisites' '
281
^topic/2 \
282
release &&
283
284
+ # create bundle from stdin
285
+ # input has a non-exist reference: "topic/deleted"
286
+ cat >input <<-EOF &&
287
+ ^topic/deleted
288
+ ^$D
289
+ ^topic/2
290
+ EOF
291
+ git bundle create stdin-2.bdl \
292
+ --ignore-missing \
293
+ --stdin \
294
+ release <input &&
295
+
296
cat >expect <<-\EOF &&
297
The bundle contains this ref:
298
<COMMIT-N> refs/heads/release
@@ -280,7 +306,12 @@ test_expect_success 'create bundle 2 - has prerequisites' '
306
make_user_friendly_and_stable_output >actual &&
307
test_i18ncmp expect actual &&
308
283
- test_bundle_object_count 2.bdl 16
309
+ git bundle verify stdin-2.bdl |
310
+ make_user_friendly_and_stable_output >actual &&
311
+ test_i18ncmp expect actual &&
312
+
313
+ test_bundle_object_count 2.bdl 16 &&
314
+ test_bundle_object_count stdin-2.bdl 16
315
'
316
317
test_expect_success 'fail to verify bundle without prerequisites' '
@@ -295,10 +326,15 @@ test_expect_success 'fail to verify bundle without prerequisites' '
326
327
test_must_fail git -C test1.git bundle verify ../2.bdl 2>&1 |
328
make_user_friendly_and_stable_output >actual &&
329
+ test_i18ncmp expect actual &&
330
+
331
+ test_must_fail git -C test1.git bundle verify ../stdin-2.bdl 2>&1 |
332
+ make_user_friendly_and_stable_output >actual &&
333
test_i18ncmp expect actual
334
'
335
336
test_expect_success 'create bundle 3 - two refs, same object' '
337
+ # create bundle from args
338
git bundle create --version=3 3.bdl \
339
^release \
340
^topic/1 \
@@ -306,6 +342,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
342
main \
343
HEAD &&
344
345
+ # create bundle from stdin
346
+ cat >input <<-\EOF &&
347
+ ^release
348
+ ^topic/1
349
+ ^topic/2
350
+ EOF
351
+ git bundle create --version=3 stdin-3.bdl \
352
+ --stdin \
353
+ main HEAD <input &&
354
+
355
cat >expect <<-\EOF &&
356
The bundle contains these 2 refs:
357
<COMMIT-P> refs/heads/main
@@ -319,10 +365,16 @@ test_expect_success 'create bundle 3 - two refs, same object' '
365
make_user_friendly_and_stable_output >actual &&
366
test_i18ncmp expect actual &&
367
322
- test_bundle_object_count 3.bdl 4
368
+ git bundle verify stdin-3.bdl |
369
+ make_user_friendly_and_stable_output >actual &&
370
+ test_i18ncmp expect actual &&
371
+
372
+ test_bundle_object_count 3.bdl 4 &&
373
+ test_bundle_object_count stdin-3.bdl 4
374
'
375
376
test_expect_success 'create bundle 4 - with tags' '
377
+ # create bundle from args
378
git bundle create 4.bdl \
379
^main \
380
^release \
@@ -330,6 +382,18 @@ test_expect_success 'create bundle 4 - with tags' '
382
^topic/2 \
383
--all &&
384
385
+ # create bundle from stdin
386
+ cat >input <<-\EOF &&
387
+ ^main
388
+ ^release
389
+ ^topic/1
390
+ ^topic/2
391
+ EOF
392
+ git bundle create stdin-4.bdl \
393
+ --ignore-missing \
394
+ --stdin \
395
+ --all <input &&
396
+
397
cat >expect <<-\EOF &&
398
The bundle contains these 3 refs:
399
<TAG-1> refs/tags/v1
@@ -342,7 +406,12 @@ test_expect_success 'create bundle 4 - with tags' '
406
make_user_friendly_and_stable_output >actual &&
407
test_i18ncmp expect actual &&
408
345
- test_bundle_object_count 4.bdl 3
409
+ git bundle verify stdin-4.bdl |
410
+ make_user_friendly_and_stable_output >actual &&
411
+ test_i18ncmp expect actual &&
412
+
413
+ test_bundle_object_count 4.bdl 3 &&
414
+ test_bundle_object_count stdin-4.bdl 3
415
'
416
417
test_expect_success 'clone from bundle' '