Glen Choo <chooglen@xxxxxxxxxx> 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?
> @@ -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.
> @@ -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().
> @@ -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.
> @@ -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.
