Re: [PATCH 3/8] submodule: make static functions read submodules from commits
From: Jonathan Tan <hidden>
Date: 2022-02-10 19:15:47
Glen Choo [off-list ref] writes:
As a result, "git fetch" now reads changed submodules using the `.gitmodules` and path from super_oid's tree (which is where "git fetch" actually noticed the changed submodule) instead of the filesystem.
Could we have a test showing what has changed?
quoted hunk ↗ jump to hunk
@@ -1029,7 +1044,7 @@ static int submodule_needs_pushing(struct repository *r, const char *path, struct oid_array *commits) { - if (!submodule_has_commits(r, path, commits)) + if (!submodule_has_commits(r, path, null_oid(), commits))
This confused me at first, but I see that this code is not for fetching, but for pushing. This patch set concerns itself with fetching, so passing null_oid() to preserve existing behavior is good.
quoted hunk ↗ jump to hunk
@@ -1414,12 +1429,13 @@ static const struct submodule *get_non_gitmodules_submodule(const char *path) } static struct fetch_task *fetch_task_create(struct repository *r, - const char *path) + const char *path, + const struct object_id *treeish_name) { struct fetch_task *task = xmalloc(sizeof(*task)); memset(task, 0, sizeof(*task)); - task->sub = submodule_from_path(r, null_oid(), path); + task->sub = submodule_from_path(r, treeish_name, path);
If there is not a good reason to have "path" before "treeish_name", probably best to maintain the same parameter order as submodule_from_path().
quoted hunk ↗ jump to hunk
@@ -1476,7 +1493,7 @@ static int get_next_submodule(struct child_process *cp, if (!S_ISGITLINK(ce->ce_mode)) continue; - task = fetch_task_create(spf->r, ce->name); + task = fetch_task_create(spf->r, ce->name, null_oid());
Hmm...is the plumbing incomplete? This code is about fetching, but we're not passing any superproject commit OID here. If this will be fixed in a future commit, maybe the distribution of what goes into each commit needs to be revised.
quoted hunk ↗ jump to hunk
@@ -1499,7 +1516,7 @@ static int get_next_submodule(struct child_process *cp, continue; } - task->repo = get_submodule_repo_for(spf->r, task->sub->path); + task->repo = get_submodule_repo_for(spf->r, task->sub->path, null_oid());
Same comment here.