fetch: ensure submodule objects fetched
Currently when git-fetch is asked to recurse into submodules, it dispatches a plain "git-fetch -C <submodule-dir>" (with some submodule related options such as prefix and recusing strategy, but) without any information of the remote or the tip that should be fetched. But this default fetch is not sufficient, as a newly fetched commit in the superproject could point to a commit in the submodule that is not in the default refspec. This is common in workflows like Gerrit's. When fetching a Gerrit change under review (from refs/changes/??), the commits in that change likely point to submodule commits that have not been merged to a branch yet. Fetch a submodule object by id if the object that the superproject points to, cannot be found. For now this object is fetched from the 'origin' remote as we defer getting the default remote to a later patch. A list of new submodule commits are already generated in certain conditions (by check_for_new_submodule_commits()); this new feature invokes that function in more situations. The submodule checks were done only when a ref in the superproject changed, these checks were extended to also be performed when fetching into FETCH_HEAD for completeness, and add a test for that too. Signed-off-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Stefan Beller committed
Dec 6, 2018 at 13:26 UTC
be76c2128234d94b47f7087152ee55d08bb65d88
3 files changed
+296
-38
builtin/fetch.c
+2
-9
@@ -763,9 +763,6 @@ static int update_local_ref(struct ref *ref,
763
what = _("[new ref]");
764
}
765
766
- if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
767
- (recurse_submodules != RECURSE_SUBMODULES_ON))
768
- check_for_new_submodule_commits(&ref->new_oid);
766
r = s_update_ref(msg, ref, 0);
767
format_display(display, r ? '!' : '*', what,
768
r ? _("unable to update local ref") : NULL,
@@ -779,9 +776,6 @@ static int update_local_ref(struct ref *ref,
776
strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV);
777
strbuf_addstr(&quickref, "..");
778
strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
782
- if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
783
- (recurse_submodules != RECURSE_SUBMODULES_ON))
784
- check_for_new_submodule_commits(&ref->new_oid);
779
r = s_update_ref("fast-forward", ref, 1);
780
format_display(display, r ? '!' : ' ', quickref.buf,
781
r ? _("unable to update local ref") : NULL,
@@ -794,9 +788,6 @@ static int update_local_ref(struct ref *ref,
788
strbuf_add_unique_abbrev(&quickref, ¤t->object.oid, DEFAULT_ABBREV);
789
strbuf_addstr(&quickref, "...");
790
strbuf_add_unique_abbrev(&quickref, &ref->new_oid, DEFAULT_ABBREV);
797
- if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
798
- (recurse_submodules != RECURSE_SUBMODULES_ON))
799
- check_for_new_submodule_commits(&ref->new_oid);
791
r = s_update_ref("forced-update", ref, 1);
792
format_display(display, r ? '!' : '+', quickref.buf,
793
r ? _("unable to update local ref") : _("forced update"),
@@ -892,6 +883,8 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,
883
ref->force = rm->peer_ref->force;
884
}
885
886
+ if (recurse_submodules != RECURSE_SUBMODULES_OFF)
887
+ check_for_new_submodule_commits(&rm->old_oid);
888
889
if (!strcmp(rm->name, "HEAD")) {
890
kind = "";
submodule.c
+177
-29
@@ -1231,8 +1231,14 @@ struct submodule_parallel_fetch {
1231
int result;
1232
1233
struct string_list changed_submodule_names;
1234
+
1235
+ /* Pending fetches by OIDs */
1236
+ struct fetch_task **oid_fetch_tasks;
1237
+ int oid_fetch_tasks_nr, oid_fetch_tasks_alloc;
1238
};
1235
-#define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0, 0, STRING_LIST_INIT_DUP }
1239
+#define SPF_INIT {0, ARGV_ARRAY_INIT, NULL, NULL, 0, 0, 0, 0, \
1240
+ STRING_LIST_INIT_DUP, \
1241
+ NULL, 0, 0}
1242
1243
static int get_fetch_recurse_config(const struct submodule *submodule,
1244
struct submodule_parallel_fetch *spf)
@@ -1259,6 +1265,76 @@ static int get_fetch_recurse_config(const struct submodule *submodule,
1265
return spf->default_option;
1266
}
1267
1268
+/*
1269
+ * Fetch in progress (if callback data) or
1270
+ * pending (if in oid_fetch_tasks in struct submodule_parallel_fetch)
1271
+ */
1272
+struct fetch_task {
1273
+ struct repository *repo;
1274
+ const struct submodule *sub;
1275
+ unsigned free_sub : 1; /* Do we need to free the submodule? */
1276
+
1277
+ struct oid_array *commits; /* Ensure these commits are fetched */
1278
+};
1279
+
1280
+/**
1281
+ * When a submodule is not defined in .gitmodules, we cannot access it
1282
+ * via the regular submodule-config. Create a fake submodule, which we can
1283
+ * work on.
1284
+ */
1285
+static const struct submodule *get_non_gitmodules_submodule(const char *path)
1286
+{
1287
+ struct submodule *ret = NULL;
1288
+ const char *name = default_name_or_path(path);
1289
+
1290
+ if (!name)
1291
+ return NULL;
1292
+
1293
+ ret = xmalloc(sizeof(*ret));
1294
+ memset(ret, 0, sizeof(*ret));
1295
+ ret->path = name;
1296
+ ret->name = name;
1297
+
1298
+ return (const struct submodule *) ret;
1299
+}
1300
+
1301
+static struct fetch_task *fetch_task_create(struct repository *r,
1302
+ const char *path)
1303
+{
1304
+ struct fetch_task *task = xmalloc(sizeof(*task));
1305
+ memset(task, 0, sizeof(*task));
1306
+
1307
+ task->sub = submodule_from_path(r, &null_oid, path);
1308
+ if (!task->sub) {
1309
+ /*
1310
+ * No entry in .gitmodules? Technically not a submodule,
1311
+ * but historically we supported repositories that happen to be
1312
+ * in-place where a gitlink is. Keep supporting them.
1313
+ */
1314
+ task->sub = get_non_gitmodules_submodule(path);
1315
+ if (!task->sub) {
1316
+ free(task);
1317
+ return NULL;
1318
+ }
1319
+
1320
+ task->free_sub = 1;
1321
+ }
1322
+
1323
+ return task;
1324
+}
1325
+
1326
+static void fetch_task_release(struct fetch_task *p)
1327
+{
1328
+ if (p->free_sub)
1329
+ free((void*)p->sub);
1330
+ p->free_sub = 0;
1331
+ p->sub = NULL;
1332
+
1333
+ if (p->repo)
1334
+ repo_clear(p->repo);
1335
+ FREE_AND_NULL(p->repo);
1336
+}
1337
+
1338
static struct repository *get_submodule_repo_for(struct repository *r,
1339
const struct submodule *sub)
1340
{
@@ -1286,39 +1362,29 @@ static struct repository *get_submodule_repo_for(struct repository *r,
1362
static int get_next_submodule(struct child_process *cp,
1363
struct strbuf *err, void *data, void **task_cb)
1364
{
1289
- int ret = 0;
1365
struct submodule_parallel_fetch *spf = data;
1366
1367
for (; spf->count < spf->r->index->cache_nr; spf->count++) {
1293
- struct strbuf submodule_prefix = STRBUF_INIT;
1368
const struct cache_entry *ce = spf->r->index->cache[spf->count];
1369
const char *default_argv;
1296
- const struct submodule *submodule;
1297
- struct repository *repo;
1298
- struct submodule default_submodule = SUBMODULE_INIT;
1370
+ struct fetch_task *task;
1371
1372
if (!S_ISGITLINK(ce->ce_mode))
1373
continue;
1374
1303
- submodule = submodule_from_path(spf->r, &null_oid, ce->name);
1304
- if (!submodule) {
1305
- const char *name = default_name_or_path(ce->name);
1306
- if (name) {
1307
- default_submodule.path = name;
1308
- default_submodule.name = name;
1309
- submodule = &default_submodule;
1310
- }
1311
- }
1375
+ task = fetch_task_create(spf->r, ce->name);
1376
+ if (!task)
1377
+ continue;
1378
1313
- switch (get_fetch_recurse_config(submodule, spf))
1379
+ switch (get_fetch_recurse_config(task->sub, spf))
1380
{
1381
default:
1382
case RECURSE_SUBMODULES_DEFAULT:
1383
case RECURSE_SUBMODULES_ON_DEMAND:
1318
- if (!submodule ||
1384
+ if (!task->sub ||
1385
!string_list_lookup(
1386
&spf->changed_submodule_names,
1321
- submodule->name))
1387
+ task->sub->name))
1388
continue;
1389
default_argv = "on-demand";
1390
break;
@@ -1329,11 +1395,11 @@ static int get_next_submodule(struct child_process *cp,
1395
continue;
1396
}
1397
1332
- strbuf_addf(&submodule_prefix, "%s%s/", spf->prefix, ce->name);
1333
- repo = get_submodule_repo_for(spf->r, submodule);
1334
- if (repo) {
1398
+ task->repo = get_submodule_repo_for(spf->r, task->sub);
1399
+ if (task->repo) {
1400
+ struct strbuf submodule_prefix = STRBUF_INIT;
1401
child_process_init(cp);
1336
- cp->dir = xstrdup(repo->gitdir);
1402
+ cp->dir = task->repo->gitdir;
1403
prepare_submodule_repo_env_in_gitdir(&cp->env_array);
1404
cp->git_cmd = 1;
1405
if (!spf->quiet)
@@ -1343,12 +1409,22 @@ static int get_next_submodule(struct child_process *cp,
1409
argv_array_pushv(&cp->args, spf->args.argv);
1410
argv_array_push(&cp->args, default_argv);
1411
argv_array_push(&cp->args, "--submodule-prefix");
1412
+
1413
+ strbuf_addf(&submodule_prefix, "%s%s/",
1414
+ spf->prefix,
1415
+ task->sub->path);
1416
argv_array_push(&cp->args, submodule_prefix.buf);
1417
1348
- repo_clear(repo);
1349
- free(repo);
1350
- ret = 1;
1418
+ spf->count++;
1419
+ *task_cb = task;
1420
+
1421
+ strbuf_release(&submodule_prefix);
1422
+ return 1;
1423
} else {
1424
+
1425
+ fetch_task_release(task);
1426
+ free(task);
1427
+
1428
/*
1429
* An empty directory is normal,
1430
* the submodule is not initialized
@@ -1361,12 +1437,38 @@ static int get_next_submodule(struct child_process *cp,
1437
ce->name);
1438
}
1439
}
1440
+ }
1441
+
1442
+ if (spf->oid_fetch_tasks_nr) {
1443
+ struct fetch_task *task =
1444
+ spf->oid_fetch_tasks[spf->oid_fetch_tasks_nr - 1];
1445
+ struct strbuf submodule_prefix = STRBUF_INIT;
1446
+ spf->oid_fetch_tasks_nr--;
1447
+
1448
+ strbuf_addf(&submodule_prefix, "%s%s/",
1449
+ spf->prefix, task->sub->path);
1450
+
1451
+ child_process_init(cp);
1452
+ prepare_submodule_repo_env_in_gitdir(&cp->env_array);
1453
+ cp->git_cmd = 1;
1454
+ cp->dir = task->repo->gitdir;
1455
+
1456
+ argv_array_init(&cp->args);
1457
+ argv_array_pushv(&cp->args, spf->args.argv);
1458
+ argv_array_push(&cp->args, "on-demand");
1459
+ argv_array_push(&cp->args, "--submodule-prefix");
1460
+ argv_array_push(&cp->args, submodule_prefix.buf);
1461
+
1462
+ /* NEEDSWORK: have get_default_remote from submodule--helper */
1463
+ argv_array_push(&cp->args, "origin");
1464
+ oid_array_for_each_unique(task->commits,
1465
+ append_oid_to_argv, &cp->args);
1466
+
1467
+ *task_cb = task;
1468
strbuf_release(&submodule_prefix);
1365
- if (ret) {
1366
- spf->count++;
1367
- return 1;
1368
- }
1469
+ return 1;
1470
}
1471
+
1472
return 0;
1473
}
1474
@@ -1374,20 +1476,66 @@ static int fetch_start_failure(struct strbuf *err,
1476
void *cb, void *task_cb)
1477
{
1478
struct submodule_parallel_fetch *spf = cb;
1479
+ struct fetch_task *task = task_cb;
1480
1481
spf->result = 1;
1482
1483
+ fetch_task_release(task);
1484
return 0;
1485
}
1486
1487
+static int commit_missing_in_sub(const struct object_id *oid, void *data)
1488
+{
1489
+ struct repository *subrepo = data;
1490
+
1491
+ enum object_type type = oid_object_info(subrepo, oid, NULL);
1492
+
1493
+ return type != OBJ_COMMIT;
1494
+}
1495
+
1496
static int fetch_finish(int retvalue, struct strbuf *err,
1497
void *cb, void *task_cb)
1498
{
1499
struct submodule_parallel_fetch *spf = cb;
1500
+ struct fetch_task *task = task_cb;
1501
+
1502
+ struct string_list_item *it;
1503
+ struct oid_array *commits;
1504
1505
if (retvalue)
1506
spf->result = 1;
1507
1508
+ if (!task || !task->sub)
1509
+ BUG("callback cookie bogus");
1510
+
1511
+ /* Is this the second time we process this submodule? */
1512
+ if (task->commits)
1513
+ goto out;
1514
+
1515
+ it = string_list_lookup(&spf->changed_submodule_names, task->sub->name);
1516
+ if (!it)
1517
+ /* Could be an unchanged submodule, not contained in the list */
1518
+ goto out;
1519
+
1520
+ commits = it->util;
1521
+ oid_array_filter(commits,
1522
+ commit_missing_in_sub,
1523
+ task->repo);
1524
+
1525
+ /* Are there commits we want, but do not exist? */
1526
+ if (commits->nr) {
1527
+ task->commits = commits;
1528
+ ALLOC_GROW(spf->oid_fetch_tasks,
1529
+ spf->oid_fetch_tasks_nr + 1,
1530
+ spf->oid_fetch_tasks_alloc);
1531
+ spf->oid_fetch_tasks[spf->oid_fetch_tasks_nr] = task;
1532
+ spf->oid_fetch_tasks_nr++;
1533
+ return 0;
1534
+ }
1535
+
1536
+out:
1537
+ fetch_task_release(task);
1538
+
1539
return 0;
1540
}
1541
t/t5526-fetch-submodules.sh
+117
@@ -600,4 +600,121 @@ test_expect_success "fetch new commits when submodule got renamed" '
600
test_cmp expect actual
601
'
602
603
+test_expect_success "fetch new submodule commits on-demand outside standard refspec" '
604
+ # add a second submodule and ensure it is around in downstream first
605
+ git clone submodule sub1 &&
606
+ git submodule add ./sub1 &&
607
+ git commit -m "adding a second submodule" &&
608
+ git -C downstream pull &&
609
+ git -C downstream submodule update --init --recursive &&
610
+
611
+ git checkout --detach &&
612
+
613
+ C=$(git -C submodule commit-tree -m "new change outside refs/heads" HEAD^{tree}) &&
614
+ git -C submodule update-ref refs/changes/1 $C &&
615
+ git update-index --cacheinfo 160000 $C submodule &&
616
+ test_tick &&
617
+
618
+ D=$(git -C sub1 commit-tree -m "new change outside refs/heads" HEAD^{tree}) &&
619
+ git -C sub1 update-ref refs/changes/2 $D &&
620
+ git update-index --cacheinfo 160000 $D sub1 &&
621
+
622
+ git commit -m "updated submodules outside of refs/heads" &&
623
+ E=$(git rev-parse HEAD) &&
624
+ git update-ref refs/changes/3 $E &&
625
+ (
626
+ cd downstream &&
627
+ git fetch --recurse-submodules origin refs/changes/3:refs/heads/my_branch &&
628
+ git -C submodule cat-file -t $C &&
629
+ git -C sub1 cat-file -t $D &&
630
+ git checkout --recurse-submodules FETCH_HEAD
631
+ )
632
+'
633
+
634
+test_expect_success 'fetch new submodule commit on-demand in FETCH_HEAD' '
635
+ # depends on the previous test for setup
636
+
637
+ C=$(git -C submodule commit-tree -m "another change outside refs/heads" HEAD^{tree}) &&
638
+ git -C submodule update-ref refs/changes/4 $C &&
639
+ git update-index --cacheinfo 160000 $C submodule &&
640
+ test_tick &&
641
+
642
+ D=$(git -C sub1 commit-tree -m "another change outside refs/heads" HEAD^{tree}) &&
643
+ git -C sub1 update-ref refs/changes/5 $D &&
644
+ git update-index --cacheinfo 160000 $D sub1 &&
645
+
646
+ git commit -m "updated submodules outside of refs/heads" &&
647
+ E=$(git rev-parse HEAD) &&
648
+ git update-ref refs/changes/6 $E &&
649
+ (
650
+ cd downstream &&
651
+ git fetch --recurse-submodules origin refs/changes/6 &&
652
+ git -C submodule cat-file -t $C &&
653
+ git -C sub1 cat-file -t $D &&
654
+ git checkout --recurse-submodules FETCH_HEAD
655
+ )
656
+'
657
+
658
+test_expect_success 'fetch new submodule commits on-demand without .gitmodules entry' '
659
+ # depends on the previous test for setup
660
+
661
+ git config -f .gitmodules --remove-section submodule.sub1 &&
662
+ git add .gitmodules &&
663
+ git commit -m "delete gitmodules file" &&
664
+ git checkout -B master &&
665
+ git -C downstream fetch &&
666
+ git -C downstream checkout origin/master &&
667
+
668
+ C=$(git -C submodule commit-tree -m "yet another change outside refs/heads" HEAD^{tree}) &&
669
+ git -C submodule update-ref refs/changes/7 $C &&
670
+ git update-index --cacheinfo 160000 $C submodule &&
671
+ test_tick &&
672
+
673
+ D=$(git -C sub1 commit-tree -m "yet another change outside refs/heads" HEAD^{tree}) &&
674
+ git -C sub1 update-ref refs/changes/8 $D &&
675
+ git update-index --cacheinfo 160000 $D sub1 &&
676
+
677
+ git commit -m "updated submodules outside of refs/heads" &&
678
+ E=$(git rev-parse HEAD) &&
679
+ git update-ref refs/changes/9 $E &&
680
+ (
681
+ cd downstream &&
682
+ git fetch --recurse-submodules origin refs/changes/9 &&
683
+ git -C submodule cat-file -t $C &&
684
+ git -C sub1 cat-file -t $D &&
685
+ git checkout --recurse-submodules FETCH_HEAD
686
+ )
687
+'
688
+
689
+test_expect_success 'fetch new submodule commit intermittently referenced by superproject' '
690
+ # depends on the previous test for setup
691
+
692
+ D=$(git -C sub1 commit-tree -m "change 10 outside refs/heads" HEAD^{tree}) &&
693
+ E=$(git -C sub1 commit-tree -m "change 11 outside refs/heads" HEAD^{tree}) &&
694
+ F=$(git -C sub1 commit-tree -m "change 12 outside refs/heads" HEAD^{tree}) &&
695
+
696
+ git -C sub1 update-ref refs/changes/10 $D &&
697
+ git update-index --cacheinfo 160000 $D sub1 &&
698
+ git commit -m "updated submodules outside of refs/heads" &&
699
+
700
+ git -C sub1 update-ref refs/changes/11 $E &&
701
+ git update-index --cacheinfo 160000 $E sub1 &&
702
+ git commit -m "updated submodules outside of refs/heads" &&
703
+
704
+ git -C sub1 update-ref refs/changes/12 $F &&
705
+ git update-index --cacheinfo 160000 $F sub1 &&
706
+ git commit -m "updated submodules outside of refs/heads" &&
707
+
708
+ G=$(git rev-parse HEAD) &&
709
+ git update-ref refs/changes/13 $G &&
710
+ (
711
+ cd downstream &&
712
+ git fetch --recurse-submodules origin refs/changes/13 &&
713
+
714
+ git -C sub1 cat-file -t $D &&
715
+ git -C sub1 cat-file -t $E &&
716
+ git -C sub1 cat-file -t $F
717
+ )
718
+'
719
+
720
test_done