Re: [PATCH v3 2/2] submodule: drop the top-level requirement
On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
Could $curdir have glob wildcard to throw this part of the logic
off? It is OK to have limitations like you cannot have a glob
characters in your path to submodule working tree (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.
I think the use of # instead of ## saves us here because even with a
wildcard in $curdir the case statement matches literally, so we know
that $target starts with $curdir/, so ${target#$curdir/} can't
match anything longer than the literal $curdir prefix.
module_list()
{
+ eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
An efficient reuse of -- ;-)
+test_expect_success 'run summary from subdir' '
+ mkdir sub
+ (
+ cd sub
+ git submodule summary ../actual
+ )
+ cat expected -EOF
+* ../sm1 000...$head1 (2):
+ Add foo2
+
+EOF
It somewhat looks strange to start with -EOF and then not to
indent the body nor EOF.
Yes, but I copied this from the preceding test. I'd rather leave this
as it is for consistency and let later style fixes change all of the
tests in this file.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
John Keeping j...@keeping.me.uk writes:
On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
Could $curdir have glob wildcard to throw this part of the logic
off? It is OK to have limitations like you cannot have a glob
characters in your path to submodule working tree (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.
I think the use of # instead of ## saves us here because even with a
wildcard in $curdir the case statement matches literally,
If you have curdir=a*b and target=adropb/c/d/e, the chopping itself
target=${target#$curdir/}
would happily chop adropb/ from the target, but because the dq
around $curdir/* in the case arm label enforces that target must
literally match curdir followed by a slash, we do not even come to
the chomping part.
I still have not convinced myself that it is impossible for somebody
more clever than I to craft a pair of target and curdir that breaks
it, though. (target=a*b/c/d, curdir=a*b) is correctly chopped,
so that is not it.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
Am 19.04.2013 18:45, schrieb Junio C Hamano:
John Keeping j...@keeping.me.uk writes:
On Thu, Apr 18, 2013 at 03:40:41PM -0700, Junio C Hamano wrote:
John Keeping j...@keeping.me.uk writes:
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
Could $curdir have glob wildcard to throw this part of the logic
off? It is OK to have limitations like you cannot have a glob
characters in your path to submodule working tree (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.
I think the use of # instead of ## saves us here because even with a
wildcard in $curdir the case statement matches literally,
If you have curdir=a*b and target=adropb/c/d/e, the chopping itself
target=${target#$curdir/}
would happily chop adropb/ from the target, but because the dq
around $curdir/* in the case arm label enforces that target must
literally match curdir followed by a slash, we do not even come to
the chomping part.
I still have not convinced myself that it is impossible for somebody
more clever than I to craft a pair of target and curdir that breaks
it, though. (target=a*b/c/d, curdir=a*b) is correctly chopped,
so that is not it.
Why not just replace the six-liner by this one-liner:
target=${target#$curdir/}
-- Hannes
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
Johannes Sixt j...@kdbg.org writes:
Why not just replace the six-liner by this one-liner:
target=${target#$curdir/}
Simple enough ;-)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
John Keeping j...@keeping.me.uk writes:
+relative_path ()
+{
+ local target curdir result
+ target=$1
+ curdir=${2-$wt_prefix}
+ curdir=${curdir%/}
+ result=
+
+ while test -n $curdir
+ do
+ case $target in
+ $curdir/*)
+ target=${target#$curdir/}
+ break
+ ;;
+ esac
Could $curdir have glob wildcard to throw this part of the logic
off? It is OK to have limitations like you cannot have a glob
characters in your path to submodule working tree (at least until
we start rewriting these in C or Perl or Python), but we need to be
aware of them.
module_list()
{
+ eval set $(git rev-parse --sq --prefix $wt_prefix -- $@)
An efficient reuse of -- ;-)
+test_expect_success 'run summary from subdir' '
+ mkdir sub
+ (
+ cd sub
+ git submodule summary ../actual
+ )
+ cat expected -EOF
+* ../sm1 000...$head1 (2):
+ Add foo2
+
+EOF
It somewhat looks strange to start with -EOF and then not to
indent the body nor EOF.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/2] submodule: drop the top-level requirement
On Thu, Apr 18, 2013 at 3:50 PM, John Keeping j...@keeping.me.uk 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. Signed-off-by: John Keeping j...@keeping.me.uk --- diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh index ff26535..ca0a6ab 100755 --- a/t/t7400-submodule-basic.sh +++ b/t/t7400-submodule-basic.sh @@ -212,6 +212,24 @@ test_expect_success 'submodule add with ./, /.. and // in path' ' test_cmp empty untracked ' +test_expect_success 'submodule add in subdir' ' A particularly minor nit. Existing subdirectory-related tests in t7400 spell out subdirectory fully, so perhaps for consistency: s/subdir/subdirectory/ + echo refs/heads/master expect + empty + + mkdir addtest/sub + ( + cd addtest/sub + git submodule add $submodurl ../realsubmod3 + git submodule init + ) + + rm -f heads head untracked + inspect addtest/realsubmod3 ../.. + test_cmp expect heads + test_cmp expect head + test_cmp empty untracked +' + test_expect_success 'setup - add an example entry to .gitmodules' ' GIT_CONFIG=.gitmodules \ git config submodule.example.url git://example.com/init.git @@ -319,6 +337,15 @@ test_expect_success 'status should be up-to-date after update' ' grep ^ $rev1 list ' +test_expect_success 'status works correctly from a subdirectory' ' Good: subdirectory + mkdir sub + ( + cd sub + git submodule status ../list + ) + grep ^ $rev1 list +' + test_expect_success 'status should be modified after submodule commit' ' ( cd init diff --git a/t/t7401-submodule-summary.sh b/t/t7401-submodule-summary.sh index 30b429e..992b66b 100755 --- a/t/t7401-submodule-summary.sh +++ b/t/t7401-submodule-summary.sh @@ -45,6 +45,20 @@ EOF test_cmp expected actual +test_expect_success 'run summary from subdir' ' t7401 does not have any existing subdirectory-related tests, but for consistency with t7400, perhaps: s/subdir/subdirectory/ + mkdir sub + ( + cd sub + git submodule summary ../actual + ) + cat expected -EOF +* ../sm1 000...$head1 (2): + Add foo2 + +EOF + test_cmp expected actual +' + commit_file sm1 head2=$(add_file sm1 foo3) -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
