Re: [PATCH v3 8/8] clone, submodule update: create and check out branches

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 15, 2022, 18:15 UTC
Message-ID: <20221115181524.164472-1-jonathantanmy@google.com>
In-Reply-To: <7cdd6c4184da2d3109498589167f10ecf972edc9.1666988096.git.gitgitgadget@gmail.com>

"Glen Choo via GitGitGadget" <gitgitgadget@gmail.com> writes:

Show 10 quoted lines

> +# Test the behavior of an already-cloned submodule.
> +# NEEDSWORK When updating with branches, we always use the branch instead of the
> +# gitlink's OID. This results in some imperfect behavior:
> +#
> +# - If the gitlink's OID disagrees with the branch OID, updating with branches
> +#   may result in a dirty worktree
> +# - If the branch does not exist, the update fails.
> +#
> +# We will reevaluate when "git checkout --recurse-submodules" supports branches
> +# For now, just test for this imperfect behavior.

I think the rationale for this behavior is as follows:

We want a world in which submodules have branches and Git commands use them wherever possible. There are a few options for "git submodule update" when the superproject has a branch checked out:

1. Checkout the branch, ignoring OID (as in this patch).
2. Checkout the branch, erroring out if the OID is wrong.
3. 1 + creating the branch if it does not exist.
4. 2 + creating the branch if it does not exist.
5. Always forcibly create the branch at the gitlink's OID and then checking
   it out.

At this point in the discussion, for a low-level command like "git submodule update", doing as little as possible makes sense to me, which is 1.

But since we do not automatically create the branch if it does not exist, this means that we have to do it when we clone the submodule. Our options are:

A. Create only the branch that is checked out in the superproject (as in this
   patch).
B. Create all branches that are present in the superproject.
C. Go back on our previous decision, switching to 3.

My instinct is that we want to maintain, as much as possible, the invariant that for each branch in the superproject, if the branch tip has a gitlink pointing to a submodule, that submodule has a branch of the same name. And I think that this invariant can only be maintained by "git submodule update" if we use B or C.

Show 23 quoted lines

> +test_expect_success 'branches - other branch checked out, correct branch exists, OIDs disagree' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch --recurse-submodules new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	git -C branch-super-cloned/sub1 checkout new-branch &&
> +	test_commit -C branch-super-cloned/sub1 new-commit &&
> +	git -C branch-super-cloned/sub1 checkout main &&
> +	git -C branch-super-cloned submodule update &&
> +
> +	HEAD_BRANCH1=$(git -C branch-super-cloned/sub1 symbolic-ref HEAD) &&
> +	test $HEAD_BRANCH1 = "refs/heads/new-branch" &&
> +	test_clean_submodule ! branch-super-cloned sub1
> +'
> +
> +test_expect_success 'branches - other branch checked out, correct branch does not exist' '
> +	test_when_finished "rm -fr branch-super-cloned" &&
> +	cp -r branch-super-clean branch-super-cloned &&
> +
> +	git -C branch-super-cloned branch new-branch &&
> +	git -C branch-super-cloned checkout new-branch &&
> +	test_must_fail git -C branch-super-cloned submodule update

Can we also check what error message is being printed?