Re: [PATCH v2 2/2] submodule: drop the top-level requirement
From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:53
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!
quoted hunk ↗ jump to hunk
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?
quoted hunk ↗ jump to hunk
@@ -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().
quoted hunk ↗ jump to hunk
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.
+ 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.