Re: [PATCH v2 2/2] submodule: drop the top-level requirement

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Apr 18, 2013, 14:46 UTC
Message-ID: <CALkWK0==bnrRJDk1daANoZOOVLwZOPM+YhwOZs7yimoYBoyXyA@mail.gmail.com>
In-Reply-To: <4a6394e9e7124c2ad5a3ce232dc746e85cb1c600.1365539059.git.john@keeping.me.uk>

John Keeping wrote:

> Use the new rev-parse --prefix option to process all paths given to the
> submodule command, dropping the requirement that it be run from the
> top-level of the repository.

Yay!

Show 9 quoted lines

> diff --git a/git-submodule.sh b/git-submodule.sh
> index 79bfaac..bbf7983 100755
> --- a/git-submodule.sh
> +++ b/git-submodule.sh
> @@ -112,6 +115,7 @@ resolve_relative_url ()
>  #
>  module_list()
>  {
> +       eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"

Nit: Why not use "--" to disambiguate between options and arguments,
like you showed in your intended usage?

So, this essentially converts all the paths specified in "$@" to toplevel-relative paths. Beautiful, as this is practically the only change you require in each function.

> +       sm_path="$wt_prefix$sm_path"

Wait, isn't sm_path the value of submodule.<name>.path in .gitmodules?
 Why does it need to be prefixed?

Show 5 quoted lines

> @@ -942,6 +948,7 @@ cmd_summary() {
>         fi
>
>         cd_to_toplevel
> +       eval "set $(git rev-parse --sq --prefix "$wt_prefix" -- "$@")"

Um, what about other functions? Yeah, it looks like all of them except this one call module_list().

Show 11 quoted lines

> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index ff26535..7795f21 100755
> --- a/t/t7400-submodule-basic.sh
> +++ b/t/t7400-submodule-basic.sh
> @@ -212,6 +212,23 @@ test_expect_success 'submodule add with ./, /.. and // in path' '
>         test_cmp empty untracked
>  '
>
> +test_expect_success 'submodule add in subdir' '
> +       echo "refs/heads/master" >expect &&
> +       >empty &&

Nit: Use : >empty.  It's clearer.

> +       (
> +               mkdir addtest/sub &&

Why is the mkdir inside the subshell? The next statement (cd) onwards should be in the subshell, to save you cd'ing back.

Show 6 quoted lines

> +               cd addtest/sub &&
> +               git submodule add "$submodurl" ../realsubmod3 &&
> +               git submodule init
> +       ) &&
> +
> +       rm -f heads head untracked &&

Huh? What is this in the middle? The next statement (call to inspect) write-truncates them anyway, so this is unnecessary.