Re: [PATCH 03/10] submodule add: support multiple worktrees

From: Stefan Beller <sbeller@google.com>
Date: Jan 16, 2019, 22:27 UTC
Message-ID: <CAGZ79kY73eYTSDqyQAYcqVJn4t9kMNW00_=uPN1yCEwaOij3MA@mail.gmail.com>
In-Reply-To: <20190116103159.9305-4-pclouds@gmail.com>

Show 15 quoted lines

> --- /dev/null
> +++ b/t/t2405-worktree-submodules.sh
> @@ -0,0 +1,42 @@
> +#!/bin/sh
> +
> +test_description='multiple worktrees as superprojects'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up submodule source' '
> +       test_create_repo submodsrc &&
> +       (
> +               cd submodsrc &&
> +               test_commit one
> +       ) &&

Just like git itself, test_commit now supports the -C <dir> argument, so you could replace the whole subshell with

    test_commit -C submodsrc one &&

> +       test_commit initial &&
> +       git worktree add -b secondary secondary HEAD &&

Could we have multiple things not named the same? (i.e. have the the branch and worktree be spelled differently) That way it is less confusing and we'd catch errors of swapping these two in the implementation (unlikely, but...)

Show 6 quoted lines

> +       git config extensions.worktreeConfig true
> +'
> +
> +test_expect_success 'add submodules' '
> +       SRC="$(pwd)/submodsrc" &&
> +       git submodule add "$SRC" sub1 &&

you can also use relative paths in submodules:

    git submodule add ./submodsrc sub1 &&

if you want to.

> +       git commit -m sub1 &&
> +       git -C secondary submodule add "$SRC" sub2 &&

Oh never mind, by having the absolute path we add the submodule with the same setting. (When using relative path we'd have to use ../submodusrc here, but would that matter?)

Show 17 quoted lines

> +       git -C secondary commit -m sub2 &&
> +
> +       git config --get-regexp "submodule.*" | sort >actual1 &&
> +       cat >expected1 <<-EOF &&
> +       submodule.sub1.active true
> +       submodule.sub1.url $(pwd)/submodsrc
> +       EOF
> +       test_cmp expected1 actual1 &&
> +       test -d .git/modules/sub1 &&
> +
> +       git -C secondary config --get-regexp "submodule.*" | sort >actual2 &&
> +       cat >expected2 <<-EOF &&
> +       submodule.sub2.active true
> +       submodule.sub2.url $(pwd)/submodsrc
> +       EOF
> +       test_cmp expected2 actual2 &&
> +       test -d .git/worktrees/secondary/modules/sub2

This section is very brittle. For example the way how submodules are active changed not so long ago, and might change again, which seems unrelated to the thing tested here?