Skip to content

Commit be76c21

Browse files
stefanbellergitster
stefanbeller
authored and
gitster
committed
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>
1 parent a62387b commit be76c21

File tree

3 files changed

+296
-38
lines changed

3 files changed

+296
-38
lines changed

‎builtin/fetch.c‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -763,9 +763,6 @@ static int update_local_ref(struct ref *ref,
763763
what = _("[new ref]");
764764
}
765765

766-
if ((recurse_submodules != RECURSE_SUBMODULES_OFF) &&
767-
(recurse_submodules != RECURSE_SUBMODULES_ON))
768-
check_for_new_submodule_commits(&ref->new_oid);
769766
r = s_update_ref(msg, ref, 0);
770767
format_display(display, r ? '!' : '*', what,
771768
r ? _("unable to update local ref") : NULL,
@@ -779,9 +776,6 @@ static int update_local_ref(struct ref *ref,
779776
strbuf_add_unique_abbrev(&quickref, &current->object.oid, DEFAULT_ABBREV);
780777
strbuf_addstr(&quickref, "..");
781778
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);
785779
r = s_update_ref("fast-forward", ref, 1);
786780
format_display(display, r ? '!' : ' ', quickref.buf,
787781
r ? _("unable to update local ref") : NULL,
@@ -794,9 +788,6 @@ static int update_local_ref(struct ref *ref,
794788
strbuf_add_unique_abbrev(&quickref, &current->object.oid, DEFAULT_ABBREV);
795789
strbuf_addstr(&quickref, "...");
796790
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);
800791
r = s_update_ref("forced-update", ref, 1);
801792
format_display(display, r ? '!' : '+', quickref.buf,
802793
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,
892883
ref->force = rm->peer_ref->force;
893884
}
894885

886+
if (recurse_submodules != RECURSE_SUBMODULES_OFF)
887+
check_for_new_submodule_commits(&rm->old_oid);
895888

896889
if (!strcmp(rm->name, "HEAD")) {
897890
kind = "";

‎submodule.c‎

Lines changed: 177 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1231,8 +1231,14 @@ struct submodule_parallel_fetch {
12311231
int result;
12321232

12331233
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;
12341238
};
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}
12361242

12371243
static int get_fetch_recurse_config(const struct submodule *submodule,
12381244
struct submodule_parallel_fetch *spf)
@@ -1259,6 +1265,76 @@ static int get_fetch_recurse_config(const struct submodule *submodule,
12591265
return spf->default_option;
12601266
}
12611267

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+
12621338
static struct repository *get_submodule_repo_for(struct repository *r,
12631339
const struct submodule *sub)
12641340
{
@@ -1286,39 +1362,29 @@ static struct repository *get_submodule_repo_for(struct repository *r,
12861362
static int get_next_submodule(struct child_process *cp,
12871363
struct strbuf *err, void *data, void **task_cb)
12881364
{
1289-
int ret = 0;
12901365
struct submodule_parallel_fetch *spf = data;
12911366

12921367
for (; spf->count < spf->r->index->cache_nr; spf->count++) {
1293-
struct strbuf submodule_prefix = STRBUF_INIT;
12941368
const struct cache_entry *ce = spf->r->index->cache[spf->count];
12951369
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;
12991371

13001372
if (!S_ISGITLINK(ce->ce_mode))
13011373
continue;
13021374

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;
13121378

1313-
switch (get_fetch_recurse_config(submodule, spf))
1379+
switch (get_fetch_recurse_config(task->sub, spf))
13141380
{
13151381
default:
13161382
case RECURSE_SUBMODULES_DEFAULT:
13171383
case RECURSE_SUBMODULES_ON_DEMAND:
1318-
if (!submodule ||
1384+
if (!task->sub ||
13191385
!string_list_lookup(
13201386
&spf->changed_submodule_names,
1321-
submodule->name))
1387+
task->sub->name))
13221388
continue;
13231389
default_argv = "on-demand";
13241390
break;
@@ -1329,11 +1395,11 @@ static int get_next_submodule(struct child_process *cp,
13291395
continue;
13301396
}
13311397

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;
13351401
child_process_init(cp);
1336-
cp->dir = xstrdup(repo->gitdir);
1402+
cp->dir = task->repo->gitdir;
13371403
prepare_submodule_repo_env_in_gitdir(&cp->env_array);
13381404
cp->git_cmd = 1;
13391405
if (!spf->quiet)
@@ -1343,12 +1409,22 @@ static int get_next_submodule(struct child_process *cp,
13431409
argv_array_pushv(&cp->args, spf->args.argv);
13441410
argv_array_push(&cp->args, default_argv);
13451411
argv_array_push(&cp->args, "--submodule-prefix");
1412+
1413+
strbuf_addf(&submodule_prefix, "%s%s/",
1414+
spf->prefix,
1415+
task->sub->path);
13461416
argv_array_push(&cp->args, submodule_prefix.buf);
13471417

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;
13511423
} else {
1424+
1425+
fetch_task_release(task);
1426+
free(task);
1427+
13521428
/*
13531429
* An empty directory is normal,
13541430
* the submodule is not initialized
@@ -1361,33 +1437,105 @@ static int get_next_submodule(struct child_process *cp,
13611437
ce->name);
13621438
}
13631439
}
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;
13641468
strbuf_release(&submodule_prefix);
1365-
if (ret) {
1366-
spf->count++;
1367-
return 1;
1368-
}
1469+
return 1;
13691470
}
1471+
13701472
return 0;
13711473
}
13721474

13731475
static int fetch_start_failure(struct strbuf *err,
13741476
void *cb, void *task_cb)
13751477
{
13761478
struct submodule_parallel_fetch *spf = cb;
1479+
struct fetch_task *task = task_cb;
13771480

13781481
spf->result = 1;
13791482

1483+
fetch_task_release(task);
13801484
return 0;
13811485
}
13821486

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+
13831496
static int fetch_finish(int retvalue, struct strbuf *err,
13841497
void *cb, void *task_cb)
13851498
{
13861499
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;
13871504

13881505
if (retvalue)
13891506
spf->result = 1;
13901507

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+
13911539
return 0;
13921540
}
13931541

0 commit comments

Comments
 (0)