[PATCH 0/7] remote-hg, remote-bzr fixes

[PATCH 0/7] remote-hg, remote-bzr fixes

[Richard Hansen]

A handful of fixes for the git-remote-hg and git-remote-bzr remote
helpers and their unit tests.

Richard Hansen (7):
  remote-hg:  don't decode UTF-8 paths into Unicode objects
  test-bzr.sh, test-hg.sh: allow running from any dir
  test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
  test-hg.sh: eliminate 'local' bashism
  test-hg.sh: avoid obsolete 'test' syntax
  test-hg.sh: help user correlate verbose output with email test
  remote-bzr, remote-hg: fix email address regular expression

 contrib/remote-helpers/git-remote-bzr |  7 +++----
 contrib/remote-helpers/git-remote-hg  |  9 ++++-----
 contrib/remote-helpers/test-bzr.sh    |  6 +++++-
 contrib/remote-helpers/test-hg.sh     | 31 ++++++++++++++++++-------------
 4 files changed, 30 insertions(+), 23 deletions(-)

-- 
1.8.5.rc1.207.gc17dd22

[PATCH 1/7] remote-hg: don't decode UTF-8 paths into Unicode objects

[Richard Hansen]

The internal mercurial API expects ordinary 8-bit string objects, not
Unicode string objects.  With this change, the test-hg.sh unit tests
pass again.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 3222afd..c6026b9 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -747,7 +747,7 @@ def parse_commit(parser):
             f = { 'deleted' : True }
         else:
             die('Unknown file command: %s' % line)
-        path = c_style_unescape(path).decode('utf-8')
+        path = c_style_unescape(path)
         files[path] = f
 
     # only export the commits if we are on an internal proxy repo
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 1/7] remote-hg: don't decode UTF-8 paths into Unicode objects

[Felipe Contreras]

Richard Hansen wrote:
> The internal mercurial API expects ordinary 8-bit string objects, not
> Unicode string objects.  With this change, the test-hg.sh unit tests
> pass again.

This makes sense to me, but the tests are already passing for me. How are they
failing for you?

-- 
Felipe Contreras

Re: [PATCH 1/7] remote-hg: don't decode UTF-8 paths into Unicode objects

[Richard Hansen]

On 2013-11-11 06:04, Felipe Contreras wrote:
> Richard Hansen wrote:
>> The internal mercurial API expects ordinary 8-bit string objects, not
>> Unicode string objects.  With this change, the test-hg.sh unit tests
>> pass again.
> 
> This makes sense to me, but the tests are already passing for me. How are they
> failing for you?

$ hg --version | head -n 1
Mercurial Distributed SCM (version 2.2.2)
$ cd ~/git/t
$ ../contrib/remote-helpers/test-hg.sh --verbose --immediate
...
Traceback (most recent call last):
  File "~/git/contrib/remote-helpers/git-remote-hg", line 1246, in <module>
    sys.exit(main(sys.argv))
  File "~/git/contrib/remote-helpers/git-remote-hg", line 1230, in main
    do_export(parser)
  File "~/git/contrib/remote-helpers/git-remote-hg", line 1031, in do_export
    parse_commit(parser)
  File "~/git/contrib/remote-helpers/git-remote-hg", line 822, in
parse_commit
    node = hghex(repo.commitctx(ctx))
  File "/usr/lib/python2.7/dist-packages/mercurial/localrepo.py", line
1270, in commitctx
    p2.manifestnode(), (new, drop))
  File "/usr/lib/python2.7/dist-packages/mercurial/manifest.py", line
197, in add
    cachedelta = (self.rev(p1), addlistdelta(addlist, delta))
  File "/usr/lib/python2.7/dist-packages/mercurial/manifest.py", line
124, in addlistdelta
    addlist[start:end] = array.array('c', content)
TypeError: array item must be char
not ok 4 - update bookmark

I can put the above in the commit message if people would like it there.

-Richard

Re: [PATCH 1/7] remote-hg: don't decode UTF-8 paths into Unicode objects

[Felipe Contreras]

On Mon, Nov 11, 2013 at 12:30 PM, Richard Hansen <rhansen@bbn.com> wrote:
> On 2013-11-11 06:04, Felipe Contreras wrote:
>> Richard Hansen wrote:
>>> The internal mercurial API expects ordinary 8-bit string objects, not
>>> Unicode string objects.  With this change, the test-hg.sh unit tests
>>> pass again.
>>
>> This makes sense to me, but the tests are already passing for me. How are they
>> failing for you?
>
> $ hg --version | head -n 1
> Mercurial Distributed SCM (version 2.2.2)
> $ cd ~/git/t
> $ ../contrib/remote-helpers/test-hg.sh --verbose --immediate

Ah, I see they are failing now (v 2.8). I don't know what I was testing.

FWIW my tree doesn't have this problem [1].

> I can put the above in the commit message if people would like it there.

Personally I think it's overkill. You mentioned the tests failed,
that's enough explanation. I just wanted to see if that was actually
the case.

[1] https://travis-ci.org/felipec/git-travis

-- 
Felipe Contreras

[PATCH 2/7] test-bzr.sh, test-hg.sh: allow running from any dir

[Richard Hansen]

cd to the t/ subdirectory so that the user doesn't already have to be
in the test directory to run these test scripts.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-bzr.sh | 1 +
 contrib/remote-helpers/test-hg.sh  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 5c50251..094062c 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -5,6 +5,7 @@
 
 test_description='Test remote-bzr'
 
+cd "${0%/*}"/../../t || exit 1
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 72f745d..dbe0eec 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -8,6 +8,7 @@
 
 test_description='Test remote-hg'
 
+cd "${0%/*}"/../../t || exit 1
 . ./test-lib.sh
 
 if ! test_have_prereq PYTHON
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 2/7] test-bzr.sh, test-hg.sh: allow running from any dir

[Felipe Contreras]

Richard Hansen wrote:
> cd to the t/ subdirectory so that the user doesn't already have to be
> in the test directory to run these test scripts.
> 
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
>  contrib/remote-helpers/test-bzr.sh | 1 +
>  contrib/remote-helpers/test-hg.sh  | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
> index 5c50251..094062c 100755
> --- a/contrib/remote-helpers/test-bzr.sh
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -5,6 +5,7 @@
>  
>  test_description='Test remote-bzr'
>  
> +cd "${0%/*}"/../../t || exit 1

I think this should do the trick:

  test -z "$TEST_DIRECTORY" && TEST_DIRECTORY="$(realpath ${0%/*}/../../t)"
  . "$TEST_DIRECTORY"/test-lib.sh

-- 
Felipe Contreras

Re: [PATCH 2/7] test-bzr.sh, test-hg.sh: allow running from any dir

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Richard Hansen wrote:
>> cd to the t/ subdirectory so that the user doesn't already have to be
>> in the test directory to run these test scripts.
>> 
>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>> ---
>>  contrib/remote-helpers/test-bzr.sh | 1 +
>>  contrib/remote-helpers/test-hg.sh  | 1 +
>>  2 files changed, 2 insertions(+)
>> 
>> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
>> index 5c50251..094062c 100755
>> --- a/contrib/remote-helpers/test-bzr.sh
>> +++ b/contrib/remote-helpers/test-bzr.sh
>> @@ -5,6 +5,7 @@
>>  
>>  test_description='Test remote-bzr'
>>  
>> +cd "${0%/*}"/../../t || exit 1
>
> I think this should do the trick:
>
>   test -z "$TEST_DIRECTORY" && TEST_DIRECTORY="$(realpath ${0%/*}/../../t)"
>   . "$TEST_DIRECTORY"/test-lib.sh

Can we do that without using realpath(1)? I do not think we use it
anywhere in the main part of the project.

Re: [PATCH 2/7] test-bzr.sh, test-hg.sh: allow running from any dir

[Felipe Contreras]

On Mon, Nov 11, 2013 at 12:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Richard Hansen wrote:
>>> cd to the t/ subdirectory so that the user doesn't already have to be
>>> in the test directory to run these test scripts.
>>>
>>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>>> ---
>>>  contrib/remote-helpers/test-bzr.sh | 1 +
>>>  contrib/remote-helpers/test-hg.sh  | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
>>> index 5c50251..094062c 100755
>>> --- a/contrib/remote-helpers/test-bzr.sh
>>> +++ b/contrib/remote-helpers/test-bzr.sh
>>> @@ -5,6 +5,7 @@
>>>
>>>  test_description='Test remote-bzr'
>>>
>>> +cd "${0%/*}"/../../t || exit 1
>>
>> I think this should do the trick:
>>
>>   test -z "$TEST_DIRECTORY" && TEST_DIRECTORY="$(realpath ${0%/*}/../../t)"
>>   . "$TEST_DIRECTORY"/test-lib.sh
>
> Can we do that without using realpath(1)? I do not think we use it
> anywhere in the main part of the project.

Something like this, probably:

--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -26,6 +26,8 @@ then
        # outside of t/, e.g. for running tests on the test library
        # itself.
        TEST_DIRECTORY=$(pwd)
+else
+       TEST_DIRECTORY=$(cd $TEST_DIRECTORY && pwd)
 fi
 if test -z "$TEST_OUTPUT_DIRECTORY"
 then


Then we can do:

 test -z "$TEST_DIRECTORY" && TEST_DIRECTORY="${0%/*}/../../t"
 . "$TEST_DIRECTORY"/test-lib.sh

-- 
Felipe Contreras

[PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Richard Hansen]

Change 'git push' to 'git push -u <remote> <branch>' in one of the
test-bzr.sh tests to ensure that the test continues to pass when the
default value of push.default changes to simple.

Also, explicitly set push.default to simple to silence warnings when
using --verbose.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-bzr.sh | 5 ++++-
 contrib/remote-helpers/test-hg.sh  | 3 +++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 094062c..ea597b0 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -28,6 +28,9 @@ check () {
 
 bzr whoami "A U Thor <author@example.com>"
 
+# silence warnings
+git config --global push.default simple
+
 test_expect_success 'cloning' '
 	(
 	bzr init bzrrepo &&
@@ -379,7 +382,7 @@ test_expect_success 'export utf-8 authors' '
 	git add content &&
 	git commit -m one &&
 	git remote add bzr "bzr::../bzrrepo" &&
-	git push bzr
+	git push -u bzr master
 	) &&
 
 	(
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index dbe0eec..53f2bba 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -102,6 +102,9 @@ setup () {
 	GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230" &&
 	GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE" &&
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+
+	# silence warnings
+	git config --global push.default simple
 }
 
 setup
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Felipe Contreras]

Richard Hansen wrote:
> Change 'git push' to 'git push -u <remote> <branch>' in one of the
> test-bzr.sh tests to ensure that the test continues to pass when the
> default value of push.default changes to simple.

This makes sense.

> Also, explicitly set push.default to simple to silence warnings when
> using --verbose.

This doesn't. Run the tests in t/* and you would seen tons and tons of those
warnings, if they should be avoided, they should be avoided for all the tests,
why only these?

I say drop the second part. Yes it's annoying, but we have to deal with it.

-- 
Felipe Contreras

Re: [PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Richard Hansen]

On 2013-11-11 06:37, Felipe Contreras wrote:
> Richard Hansen wrote:
>> Change 'git push' to 'git push -u <remote> <branch>' in one of the
>> test-bzr.sh tests to ensure that the test continues to pass when the
>> default value of push.default changes to simple.
> 
> This makes sense.
> 
>> Also, explicitly set push.default to simple to silence warnings when
>> using --verbose.
> 
> This doesn't. Run the tests in t/* and you would seen tons and tons of those
> warnings, if they should be avoided, they should be avoided for all the tests,
> why only these?
> 
> I say drop the second part. Yes it's annoying, but we have to deal with it.

OK, will do.

Thanks,
Richard

Re: [PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Felipe Contreras]

On Sun, Nov 10, 2013 at 10:05 PM, Richard Hansen <rhansen@bbn.com> wrote:

> @@ -379,7 +382,7 @@ test_expect_success 'export utf-8 authors' '
>         git add content &&
>         git commit -m one &&
>         git remote add bzr "bzr::../bzrrepo" &&
> -       git push bzr
> +       git push -u bzr master
>         ) &&

Actually, why -u? Isn't 'git push bzr master' enough?

-- 
Felipe Contreras

Re: [PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Richard Hansen]

On 2013-11-11 14:31, Felipe Contreras wrote:
> On Sun, Nov 10, 2013 at 10:05 PM, Richard Hansen <rhansen@bbn.com> wrote:
> 
>> @@ -379,7 +382,7 @@ test_expect_success 'export utf-8 authors' '
>>         git add content &&
>>         git commit -m one &&
>>         git remote add bzr "bzr::../bzrrepo" &&
>> -       git push bzr
>> +       git push -u bzr master
>>         ) &&
> 
> Actually, why -u? Isn't 'git push bzr master' enough?

It's defensive in case that test is ever updated to do more pushing.  I
can leave it out in the reroll.

-Richard

Re: [PATCH 3/7] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Felipe Contreras]

On Mon, Nov 11, 2013 at 3:16 PM, Richard Hansen <rhansen@bbn.com> wrote:
> On 2013-11-11 14:31, Felipe Contreras wrote:
>> On Sun, Nov 10, 2013 at 10:05 PM, Richard Hansen <rhansen@bbn.com> wrote:
>>
>>> @@ -379,7 +382,7 @@ test_expect_success 'export utf-8 authors' '
>>>         git add content &&
>>>         git commit -m one &&
>>>         git remote add bzr "bzr::../bzrrepo" &&
>>> -       git push bzr
>>> +       git push -u bzr master
>>>         ) &&
>>
>> Actually, why -u? Isn't 'git push bzr master' enough?
>
> It's defensive in case that test is ever updated to do more pushing.  I
> can leave it out in the reroll.

Please do. If there's any need for that we can add it later, but even
then I would prefer that the push explicit again, like this one. And
suspect we will not need to update this in that direction.

-- 
Felipe Contreras

[PATCH 4/7] test-hg.sh: eliminate 'local' bashism

[Richard Hansen]

Unlike bash, POSIX shell does not specify a 'local' command for
declaring function-local variable scope.  Except for IFS, the variable
names are not used anywhere else in the script so simply remove the
'local'.  For IFS, move the assignment to the 'read' command to
prevent it from affecting code outside the function.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 53f2bba..558a656 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -54,14 +54,14 @@ check_bookmark () {
 }
 
 check_push () {
-	local expected_ret=$1 ret=0 ref_ret=0 IFS=':'
+	expected_ret=$1 ret=0 ref_ret=0
 
 	shift
 	git push origin "$@" 2>error
 	ret=$?
 	cat error
 
-	while read branch kind
+	while IFS=':' read branch kind
 	do
 		case "$kind" in
 		'new')
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 4/7] test-hg.sh: eliminate 'local' bashism

[Felipe Contreras]

Richard Hansen wrote:
> Unlike bash, POSIX shell does not specify a 'local' command for
> declaring function-local variable scope.  Except for IFS, the variable
> names are not used anywhere else in the script so simply remove the
> 'local'.  For IFS, move the assignment to the 'read' command to
> prevent it from affecting code outside the function.

Makes sense.

-- 
Felipe Contreras

[PATCH 5/7] test-hg.sh: avoid obsolete 'test' syntax

[Richard Hansen]

The POSIX spec says that the '-a', '-o', and parentheses operands to
the 'test' utility are obsolete extensions due to the potential for
ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 558a656..84c67ff 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -83,7 +83,7 @@ check_push () {
 		test $ref_ret -ne 0 && echo "match for '$branch' failed" && break
 	done
 
-	if test $expected_ret -ne $ret -o $ref_ret -ne 0
+	if test $expected_ret -ne $ret || test $ref_ret -ne 0
 	then
 		return 1
 	fi
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 5/7] test-hg.sh: avoid obsolete 'test' syntax

[Felipe Contreras]

Richard Hansen wrote:
> The POSIX spec says that the '-a', '-o', and parentheses operands to
> the 'test' utility are obsolete extensions due to the potential for
> ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.

All right, if you say so.

-- 
Felipe Contreras

Re: [PATCH 5/7] test-hg.sh: avoid obsolete 'test' syntax

[Richard Hansen]

On 2013-11-11 06:44, Felipe Contreras wrote:
> Richard Hansen wrote:
>> The POSIX spec says that the '-a', '-o', and parentheses operands to
>> the 'test' utility are obsolete extensions due to the potential for
>> ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.
> 
> All right, if you say so.

In case you're curious about what the spec says:

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html#tag_20_128_16

-Richard

Re: [PATCH 5/7] test-hg.sh: avoid obsolete 'test' syntax

[Felipe Contreras]

Richard Hansen wrote:
> On 2013-11-11 06:44, Felipe Contreras wrote:
> > Richard Hansen wrote:
> >> The POSIX spec says that the '-a', '-o', and parentheses operands to
> >> the 'test' utility are obsolete extensions due to the potential for
> >> ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.
> > 
> > All right, if you say so.
> 
> In case you're curious about what the spec says:
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/utilities/test.html#tag_20_128_16

Thanks, I didn't know the POSIX spec was available publicly.

-- 
Felipe Contreras

[PATCH 6/7] test-hg.sh: help user correlate verbose output with email test

[Richard Hansen]

It's hard to tell which author conversion test failed when the email
addresses look similar.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 84c67ff..5eda265 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -209,16 +209,16 @@ test_expect_success 'authors' '
 
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
-	author_test beta "test" "test <unknown>" &&
-	author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
-	author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
-	author_test delta "name<test@example.com>" "name <test@example.com>" &&
-	author_test epsilon "name <test@example.com" "name <test@example.com>" &&
-	author_test zeta " test " "test <unknown>" &&
-	author_test eta "test < test@example.com >" "test <test@example.com>" &&
-	author_test theta "test >test@example.com>" "test <test@example.com>" &&
-	author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
-	author_test kappa "test@example.com" "Unknown <test@example.com>"
+	author_test beta "beta" "beta <unknown>" &&
+	author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&
+	author_test gamma "<gamma@example.com>" "Unknown <gamma@example.com>" &&
+	author_test delta "delta<test@example.com>" "delta <test@example.com>" &&
+	author_test epsilon "epsilon <test@example.com" "epsilon <test@example.com>" &&
+	author_test zeta " zeta " "zeta <unknown>" &&
+	author_test eta "eta < test@example.com >" "eta <test@example.com>" &&
+	author_test theta "theta >test@example.com>" "theta <test@example.com>" &&
+	author_test iota "iota < test <at> example <dot> com>" "iota <unknown>" &&
+	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 6/7] test-hg.sh: help user correlate verbose output with email test

[Felipe Contreras]

Richard Hansen wrote:
> It's hard to tell which author conversion test failed when the email
> addresses look similar.
> 
> Signed-off-by: Richard Hansen <rhansen@bbn.com>
> ---
>  contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index 84c67ff..5eda265 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -209,16 +209,16 @@ test_expect_success 'authors' '
>  
>  	>../expected &&
>  	author_test alpha "" "H G Wells <wells@example.com>" &&
> -	author_test beta "test" "test <unknown>" &&
> -	author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
> -	author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
> -	author_test delta "name<test@example.com>" "name <test@example.com>" &&
> -	author_test epsilon "name <test@example.com" "name <test@example.com>" &&
> -	author_test zeta " test " "test <unknown>" &&
> -	author_test eta "test < test@example.com >" "test <test@example.com>" &&
> -	author_test theta "test >test@example.com>" "test <test@example.com>" &&
> -	author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
> -	author_test kappa "test@example.com" "Unknown <test@example.com>"
> +	author_test beta "beta" "beta <unknown>" &&
> +	author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&

Two betas?

> +	author_test gamma "<gamma@example.com>" "Unknown <gamma@example.com>" &&
> +	author_test delta "delta<test@example.com>" "delta <test@example.com>" &&
> +	author_test epsilon "epsilon <test@example.com" "epsilon <test@example.com>" &&
> +	author_test zeta " zeta " "zeta <unknown>" &&
> +	author_test eta "eta < test@example.com >" "eta <test@example.com>" &&
> +	author_test theta "theta >test@example.com>" "theta <test@example.com>" &&
> +	author_test iota "iota < test <at> example <dot> com>" "iota <unknown>" &&
> +	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>"
>  	) &&
>  
>  	git clone "hg::hgrepo" gitrepo &&

-- 
Felipe Contreras

Re: [PATCH 6/7] test-hg.sh: help user correlate verbose output with email test

[Richard Hansen]

On 2013-11-11 06:42, Felipe Contreras wrote:
> Richard Hansen wrote:
>> It's hard to tell which author conversion test failed when the email
>> addresses look similar.
>>
>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>> ---
>>  contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
>> index 84c67ff..5eda265 100755
>> --- a/contrib/remote-helpers/test-hg.sh
>> +++ b/contrib/remote-helpers/test-hg.sh
>> @@ -209,16 +209,16 @@ test_expect_success 'authors' '
>>  
>>  	>../expected &&
>>  	author_test alpha "" "H G Wells <wells@example.com>" &&
>> -	author_test beta "test" "test <unknown>" &&
>> -	author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&

Notice the two betas here in the original code.

>> -	author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
>> -	author_test delta "name<test@example.com>" "name <test@example.com>" &&
>> -	author_test epsilon "name <test@example.com" "name <test@example.com>" &&
>> -	author_test zeta " test " "test <unknown>" &&
>> -	author_test eta "test < test@example.com >" "test <test@example.com>" &&
>> -	author_test theta "test >test@example.com>" "test <test@example.com>" &&
>> -	author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
>> -	author_test kappa "test@example.com" "Unknown <test@example.com>"
>> +	author_test beta "beta" "beta <unknown>" &&
>> +	author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&
> 
> Two betas?

See above.  I can change them to beta1 and beta2, or if you'd prefer I
can change them to beta and gamma and increment the subsequent entries.

Thanks,
Richard

> 
>> +	author_test gamma "<gamma@example.com>" "Unknown <gamma@example.com>" &&
>> +	author_test delta "delta<test@example.com>" "delta <test@example.com>" &&
>> +	author_test epsilon "epsilon <test@example.com" "epsilon <test@example.com>" &&
>> +	author_test zeta " zeta " "zeta <unknown>" &&
>> +	author_test eta "eta < test@example.com >" "eta <test@example.com>" &&
>> +	author_test theta "theta >test@example.com>" "theta <test@example.com>" &&
>> +	author_test iota "iota < test <at> example <dot> com>" "iota <unknown>" &&
>> +	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>"
>>  	) &&
>>  
>>  	git clone "hg::hgrepo" gitrepo &&
> 

Re: [PATCH 6/7] test-hg.sh: help user correlate verbose output with email test

[Felipe Contreras]

On Mon, Nov 11, 2013 at 1:19 PM, Richard Hansen <rhansen@bbn.com> wrote:
> On 2013-11-11 06:42, Felipe Contreras wrote:
>> Richard Hansen wrote:
>>> It's hard to tell which author conversion test failed when the email
>>> addresses look similar.
>>>
>>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>>> ---
>>>  contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
>>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
>>> index 84c67ff..5eda265 100755
>>> --- a/contrib/remote-helpers/test-hg.sh
>>> +++ b/contrib/remote-helpers/test-hg.sh
>>> @@ -209,16 +209,16 @@ test_expect_success 'authors' '
>>>
>>>      >../expected &&
>>>      author_test alpha "" "H G Wells <wells@example.com>" &&
>>> -    author_test beta "test" "test <unknown>" &&
>>> -    author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
>
> Notice the two betas here in the original code.

Ahh, that's a bug.

>>> -    author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
>>> -    author_test delta "name<test@example.com>" "name <test@example.com>" &&
>>> -    author_test epsilon "name <test@example.com" "name <test@example.com>" &&
>>> -    author_test zeta " test " "test <unknown>" &&
>>> -    author_test eta "test < test@example.com >" "test <test@example.com>" &&
>>> -    author_test theta "test >test@example.com>" "test <test@example.com>" &&
>>> -    author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
>>> -    author_test kappa "test@example.com" "Unknown <test@example.com>"
>>> +    author_test beta "beta" "beta <unknown>" &&
>>> +    author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&
>>
>> Two betas?
>
> See above.  I can change them to beta1 and beta2, or if you'd prefer I
> can change them to beta and gamma and increment the subsequent entries.

Yeah, I would prefer that in two patches, one that fixes the sequence,
and the other one that changes the emails. If you don't have time for
that the original patch is OK by me. The problem with the sequence can
be fixed later.

Cheers.

-- 
Felipe Contreras

Re: [PATCH 6/7] test-hg.sh: help user correlate verbose output with email test

[Richard Hansen]

On 2013-11-11 14:29, Felipe Contreras wrote:
> On Mon, Nov 11, 2013 at 1:19 PM, Richard Hansen <rhansen@bbn.com> wrote:
>> On 2013-11-11 06:42, Felipe Contreras wrote:
>>> Richard Hansen wrote:
>>>> It's hard to tell which author conversion test failed when the email
>>>> addresses look similar.
>>>>
>>>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>>>> ---
>>>>  contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
>>>>  1 file changed, 10 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
>>>> index 84c67ff..5eda265 100755
>>>> --- a/contrib/remote-helpers/test-hg.sh
>>>> +++ b/contrib/remote-helpers/test-hg.sh
>>>> @@ -209,16 +209,16 @@ test_expect_success 'authors' '
>>>>
>>>>      >../expected &&
>>>>      author_test alpha "" "H G Wells <wells@example.com>" &&
>>>> -    author_test beta "test" "test <unknown>" &&
>>>> -    author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
>>
>> Notice the two betas here in the original code.
> 
> Ahh, that's a bug.
> 
>>>> -    author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
>>>> -    author_test delta "name<test@example.com>" "name <test@example.com>" &&
>>>> -    author_test epsilon "name <test@example.com" "name <test@example.com>" &&
>>>> -    author_test zeta " test " "test <unknown>" &&
>>>> -    author_test eta "test < test@example.com >" "test <test@example.com>" &&
>>>> -    author_test theta "test >test@example.com>" "test <test@example.com>" &&
>>>> -    author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
>>>> -    author_test kappa "test@example.com" "Unknown <test@example.com>"
>>>> +    author_test beta "beta" "beta <unknown>" &&
>>>> +    author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&
>>>
>>> Two betas?
>>
>> See above.  I can change them to beta1 and beta2, or if you'd prefer I
>> can change them to beta and gamma and increment the subsequent entries.
> 
> Yeah, I would prefer that in two patches, one that fixes the sequence,
> and the other one that changes the emails.

Will do.

Thanks,
Richard

> If you don't have time for
> that the original patch is OK by me. The problem with the sequence can
> be fixed later.
> 
> Cheers.
> 

[PATCH 7/7] remote-bzr, remote-hg: fix email address regular expression

[Richard Hansen]

Before, strings like "foo.bar@example.com" would be converted to
"foo. <bar@example.com>" when they should be "unknown
<foo.bar@example.com>".

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/git-remote-bzr | 7 +++----
 contrib/remote-helpers/git-remote-hg  | 7 +++----
 contrib/remote-helpers/test-hg.sh     | 3 ++-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 054161a..7e34532 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -44,8 +44,8 @@ import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 RAW_AUTHOR_RE = re.compile('^(\w+) (.+)? <(.*)> (\d+) ([+-]\d+)')
 
 def die(msg, *args):
@@ -193,8 +193,7 @@ def fixup_user(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index c6026b9..30402d5 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -51,8 +51,8 @@ import time as ptime
 #
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 AUTHOR_HG_RE = re.compile('^(.*?) ?<(.*?)(?:>(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?<(.*)> (\d+) ([+-]\d+)')
 
@@ -316,8 +316,7 @@ def fixup_user_git(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 5eda265..9f5066b 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -218,7 +218,8 @@ test_expect_success 'authors' '
 	author_test eta "eta < test@example.com >" "eta <test@example.com>" &&
 	author_test theta "theta >test@example.com>" "theta <test@example.com>" &&
 	author_test iota "iota < test <at> example <dot> com>" "iota <unknown>" &&
-	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>"
+	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>" &&
+	author_test lambda "lambda.lambda@example.com" "Unknown <lambda.lambda@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.207.gc17dd22

RE: [PATCH 7/7] remote-bzr, remote-hg: fix email address regular expression

[Felipe Contreras]

Richard Hansen wrote:
> Before, strings like "foo.bar@example.com" would be converted to
> "foo. <bar@example.com>" when they should be "unknown
> <foo.bar@example.com>".

Indeed. Thanks.

-- 
Felipe Contreras

Re: [PATCH 0/7] remote-hg, remote-bzr fixes

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> A handful of fixes for the git-remote-hg and git-remote-bzr remote
> helpers and their unit tests.
>
> Richard Hansen (7):
>   remote-hg:  don't decode UTF-8 paths into Unicode objects
>   test-bzr.sh, test-hg.sh: allow running from any dir
>   test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
>   test-hg.sh: eliminate 'local' bashism
>   test-hg.sh: avoid obsolete 'test' syntax
>   test-hg.sh: help user correlate verbose output with email test
>   remote-bzr, remote-hg: fix email address regular expression
>
>  contrib/remote-helpers/git-remote-bzr |  7 +++----
>  contrib/remote-helpers/git-remote-hg  |  9 ++++-----
>  contrib/remote-helpers/test-bzr.sh    |  6 +++++-
>  contrib/remote-helpers/test-hg.sh     | 31 ++++++++++++++++++-------------
>  4 files changed, 30 insertions(+), 23 deletions(-)

I'll defer to Felipe for the meat of the logic in these scripts; the
POSIXify part of the fixes look all good to me.

I see there already are review comments, so let me know when the
reviews have settled with a final reroll.

Thanks.

[PATCH v2 0/9] remote-hg, remote-bzr fixes

[Richard Hansen]

A handful of fixes for the git-remote-hg and git-remote-bzr remote
helpers and their unit tests.

Changes from v1:

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index ea597b0..1e53ff9 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -5,8 +5,8 @@
 
 test_description='Test remote-bzr'
 
-cd "${0%/*}"/../../t || exit 1
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
@@ -28,9 +28,6 @@ check () {
 
 bzr whoami "A U Thor <author@example.com>"
 
-# silence warnings
-git config --global push.default simple
-
 test_expect_success 'cloning' '
 	(
 	bzr init bzrrepo &&
@@ -382,7 +379,7 @@ test_expect_success 'export utf-8 authors' '
 	git add content &&
 	git commit -m one &&
 	git remote add bzr "bzr::../bzrrepo" &&
-	git push -u bzr master
+	git push bzr master
 	) &&
 
 	(
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 9f5066b..347e812 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -8,8 +8,8 @@
 
 test_description='Test remote-hg'
 
-cd "${0%/*}"/../../t || exit 1
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
@@ -102,9 +102,6 @@ setup () {
 	GIT_AUTHOR_DATE="2007-01-01 00:00:00 +0230" &&
 	GIT_COMMITTER_DATE="$GIT_AUTHOR_DATE" &&
 	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
-
-	# silence warnings
-	git config --global push.default simple
 }
 
 setup
@@ -210,16 +207,16 @@ test_expect_success 'authors' '
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
 	author_test beta "beta" "beta <unknown>" &&
-	author_test beta "beta <test@example.com> (comment)" "beta <test@example.com>" &&
-	author_test gamma "<gamma@example.com>" "Unknown <gamma@example.com>" &&
-	author_test delta "delta<test@example.com>" "delta <test@example.com>" &&
-	author_test epsilon "epsilon <test@example.com" "epsilon <test@example.com>" &&
-	author_test zeta " zeta " "zeta <unknown>" &&
-	author_test eta "eta < test@example.com >" "eta <test@example.com>" &&
-	author_test theta "theta >test@example.com>" "theta <test@example.com>" &&
-	author_test iota "iota < test <at> example <dot> com>" "iota <unknown>" &&
-	author_test kappa "kappa@example.com" "Unknown <kappa@example.com>" &&
-	author_test lambda "lambda.lambda@example.com" "Unknown <lambda.lambda@example.com>"
+	author_test gamma "gamma <test@example.com> (comment)" "gamma <test@example.com>" &&
+	author_test delta "<delta@example.com>" "Unknown <delta@example.com>" &&
+	author_test epsilon "epsilon<test@example.com>" "epsilon <test@example.com>" &&
+	author_test zeta "zeta <test@example.com" "zeta <test@example.com>" &&
+	author_test eta " eta " "eta <unknown>" &&
+	author_test theta "theta < test@example.com >" "theta <test@example.com>" &&
+	author_test iota "iota >test@example.com>" "iota <test@example.com>" &&
+	author_test kappa "kappa < test <at> example <dot> com>" "kappa <unknown>" &&
+	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>" &&
+	author_test mu "mu.mu@example.com" "Unknown <mu.mu@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
diff --git a/t/test-lib.sh b/t/test-lib.sh
index b25249e..af172d9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -26,6 +26,10 @@ then
 	# outside of t/, e.g. for running tests on the test library
 	# itself.
 	TEST_DIRECTORY=$(pwd)
+else
+	# ensure that TEST_DIRECTORY is an absolute path so that it
+	# works even if the current working directory is changed
+	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
 fi
 if test -z "$TEST_OUTPUT_DIRECTORY"
 then

Richard Hansen (9):
  remote-hg:  don't decode UTF-8 paths into Unicode objects
  test-lib.sh: convert $TEST_DIRECTORY to an absolute path
  test-bzr.sh, test-hg.sh: allow running from any dir
  test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
  test-hg.sh: eliminate 'local' bashism
  test-hg.sh: avoid obsolete 'test' syntax
  test-hg.sh: fix duplicate content strings in author tests
  test-hg.sh: help user correlate verbose output with email test
  remote-bzr, remote-hg: fix email address regular expression

 contrib/remote-helpers/git-remote-bzr |  7 +++----
 contrib/remote-helpers/git-remote-hg  |  9 ++++-----
 contrib/remote-helpers/test-bzr.sh    |  5 +++--
 contrib/remote-helpers/test-hg.sh     | 30 ++++++++++++++++--------------
 t/test-lib.sh                         |  4 ++++
 5 files changed, 30 insertions(+), 25 deletions(-)

-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 1/9] remote-hg: don't decode UTF-8 paths into Unicode objects

[Richard Hansen]

The internal mercurial API expects ordinary 8-bit string objects, not
Unicode string objects.  With this change, the test-hg.sh unit tests
pass again.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 3222afd..c6026b9 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -747,7 +747,7 @@ def parse_commit(parser):
             f = { 'deleted' : True }
         else:
             die('Unknown file command: %s' % line)
-        path = c_style_unescape(path).decode('utf-8')
+        path = c_style_unescape(path)
         files[path] = f
 
     # only export the commits if we are on an internal proxy repo
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path

[Richard Hansen]

If $TEST_DIRECTORY is specified in the environment, convert the value
to an absolute path to ensure that it remains valid even when 'cd' is
used.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/test-lib.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b25249e..af172d9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -26,6 +26,10 @@ then
 	# outside of t/, e.g. for running tests on the test library
 	# itself.
 	TEST_DIRECTORY=$(pwd)
+else
+	# ensure that TEST_DIRECTORY is an absolute path so that it
+	# works even if the current working directory is changed
+	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
 fi
 if test -z "$TEST_OUTPUT_DIRECTORY"
 then
-- 
1.8.5.rc1.208.g8ff7964

Re: [PATCH v2 2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path

[Richard Hansen]

On 2013-11-12 00:54, Richard Hansen wrote:
> If $TEST_DIRECTORY is specified in the environment, convert the value
> to an absolute path to ensure that it remains valid even when 'cd' is
> used.
> 
> Signed-off-by: Richard Hansen <rhansen@bbn.com>

Actually, credit for this and the next patch should go to Felipe.  How
should I note that?

Thanks,
Richard


> ---
>  t/test-lib.sh | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index b25249e..af172d9 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -26,6 +26,10 @@ then
>  	# outside of t/, e.g. for running tests on the test library
>  	# itself.
>  	TEST_DIRECTORY=$(pwd)
> +else
> +	# ensure that TEST_DIRECTORY is an absolute path so that it
> +	# works even if the current working directory is changed
> +	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
>  fi
>  if test -z "$TEST_OUTPUT_DIRECTORY"
>  then

Re: [PATCH v2 2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> On 2013-11-12 00:54, Richard Hansen wrote:
>> If $TEST_DIRECTORY is specified in the environment, convert the value
>> to an absolute path to ensure that it remains valid even when 'cd' is
>> used.
>> 
>> Signed-off-by: Richard Hansen <rhansen@bbn.com>
>
> Actually, credit for this and the next patch should go to Felipe.  How
> should I note that?

If the patch text was copied from his response message, you would start
the _body_ of your e-mail as:

	From: F.. C.. <felipe.contreras@gmail.com>
        
	If $TEST_DIRECTORY is specified ...

	Signed-off-by: F.. C.. <felipe.contreras@gmail.com>
	Signed-off-by: R Hansen <rhansen@...>
        
_after_ getting him say it is OK to add his Sign-off.  The first
line in the body of your e-mail, "From: Real Author", followed by a
blank line, will signal to "git am" that you are forwarding a patch
by somebody else, and we record that real author on the "author"
line of the resulting commit object.

On the other hand, if the patch is based on the _idea_ you gained by
discussing with him, you would just mention it near your sign-off,
like this:

	If $TEST_DIRECTORY is specified ...

	Helped-by: F... C... <felipe.contreras@gmail.com>
	Signed-off-by: R... Hansen <rhansen@...>


	

>
> Thanks,
> Richard
>
>
>> ---
>>  t/test-lib.sh | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/t/test-lib.sh b/t/test-lib.sh
>> index b25249e..af172d9 100644
>> --- a/t/test-lib.sh
>> +++ b/t/test-lib.sh
>> @@ -26,6 +26,10 @@ then
>>  	# outside of t/, e.g. for running tests on the test library
>>  	# itself.
>>  	TEST_DIRECTORY=$(pwd)
>> +else
>> +	# ensure that TEST_DIRECTORY is an absolute path so that it
>> +	# works even if the current working directory is changed
>> +	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
>>  fi
>>  if test -z "$TEST_OUTPUT_DIRECTORY"
>>  then

Re: [PATCH v2 2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path

[Felipe Contreras]

Richard Hansen wrote:
> On 2013-11-12 00:54, Richard Hansen wrote:
> > If $TEST_DIRECTORY is specified in the environment, convert the value
> > to an absolute path to ensure that it remains valid even when 'cd' is
> > used.
> > 
> > Signed-off-by: Richard Hansen <rhansen@bbn.com>
> 
> Actually, credit for this and the next patch should go to Felipe.  How
> should I note that?

Normally what I would do is put myself as the author, but not s-o-b, but since
now I'm telling you you can add my s-o-b, you can do that.

I would have sent those patches myself, if Junio agreed that was the way to go,
but I didn't see that.

Cheers.

-- 
Felipe Contreras

[PATCH v2 3/9] test-bzr.sh, test-hg.sh: allow running from any dir

[Richard Hansen]

Set TEST_DIRECTORY to the t/ directory (if TEST_DIRECTORY is not
already set) so that the user doesn't already have to be in the test
directory to run these test scripts.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-bzr.sh | 3 ++-
 contrib/remote-helpers/test-hg.sh  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 5c50251..1e850c3 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -5,7 +5,8 @@
 
 test_description='Test remote-bzr'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 72f745d..fa7d17b 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -8,7 +8,8 @@
 
 test_description='Test remote-hg'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 4/9] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Richard Hansen]

Change 'git push <remote>' to 'git push <remote> <branch>' in one of
the test-bzr.sh tests to ensure that the test continues to pass when
the default value of push.default changes to simple.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-bzr.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 1e850c3..1e53ff9 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -379,7 +379,7 @@ test_expect_success 'export utf-8 authors' '
 	git add content &&
 	git commit -m one &&
 	git remote add bzr "bzr::../bzrrepo" &&
-	git push bzr
+	git push bzr master
 	) &&
 
 	(
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 5/9] test-hg.sh: eliminate 'local' bashism

[Richard Hansen]

Unlike bash, POSIX shell does not specify a 'local' command for
declaring function-local variable scope.  Except for IFS, the variable
names are not used anywhere else in the script so simply remove the
'local'.  For IFS, move the assignment to the 'read' command to
prevent it from affecting code outside the function.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index fa7d17b..ceef6b1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -54,14 +54,14 @@ check_bookmark () {
 }
 
 check_push () {
-	local expected_ret=$1 ret=0 ref_ret=0 IFS=':'
+	expected_ret=$1 ret=0 ref_ret=0
 
 	shift
 	git push origin "$@" 2>error
 	ret=$?
 	cat error
 
-	while read branch kind
+	while IFS=':' read branch kind
 	do
 		case "$kind" in
 		'new')
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 6/9] test-hg.sh: avoid obsolete 'test' syntax

[Richard Hansen]

The POSIX spec says that the '-a', '-o', and parentheses operands to
the 'test' utility are obsolete extensions due to the potential for
ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index ceef6b1..5535e8c 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -83,7 +83,7 @@ check_push () {
 		test $ref_ret -ne 0 && echo "match for '$branch' failed" && break
 	done
 
-	if test $expected_ret -ne $ret -o $ref_ret -ne 0
+	if test $expected_ret -ne $ret || test $ref_ret -ne 0
 	then
 		return 1
 	fi
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 7/9] test-hg.sh: fix duplicate content strings in author tests

[Richard Hansen]

"beta" was used twice.  Change the second copy to "gamma" and
increment the remaining content strings.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 5535e8c..eb72db8 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -207,15 +207,15 @@ test_expect_success 'authors' '
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
 	author_test beta "test" "test <unknown>" &&
-	author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
-	author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
-	author_test delta "name<test@example.com>" "name <test@example.com>" &&
-	author_test epsilon "name <test@example.com" "name <test@example.com>" &&
-	author_test zeta " test " "test <unknown>" &&
-	author_test eta "test < test@example.com >" "test <test@example.com>" &&
-	author_test theta "test >test@example.com>" "test <test@example.com>" &&
-	author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
-	author_test kappa "test@example.com" "Unknown <test@example.com>"
+	author_test gamma "test <test@example.com> (comment)" "test <test@example.com>" &&
+	author_test delta "<test@example.com>" "Unknown <test@example.com>" &&
+	author_test epsilon "name<test@example.com>" "name <test@example.com>" &&
+	author_test zeta "name <test@example.com" "name <test@example.com>" &&
+	author_test eta " test " "test <unknown>" &&
+	author_test theta "test < test@example.com >" "test <test@example.com>" &&
+	author_test iota "test >test@example.com>" "test <test@example.com>" &&
+	author_test kappa "test < test <at> example <dot> com>" "test <unknown>" &&
+	author_test lambda "test@example.com" "Unknown <test@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 8/9] test-hg.sh: help user correlate verbose output with email test

[Richard Hansen]

It's hard to tell which author conversion test failed when the email
addresses look similar.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index eb72db8..642ad93 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -206,16 +206,16 @@ test_expect_success 'authors' '
 
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
-	author_test beta "test" "test <unknown>" &&
-	author_test gamma "test <test@example.com> (comment)" "test <test@example.com>" &&
-	author_test delta "<test@example.com>" "Unknown <test@example.com>" &&
-	author_test epsilon "name<test@example.com>" "name <test@example.com>" &&
-	author_test zeta "name <test@example.com" "name <test@example.com>" &&
-	author_test eta " test " "test <unknown>" &&
-	author_test theta "test < test@example.com >" "test <test@example.com>" &&
-	author_test iota "test >test@example.com>" "test <test@example.com>" &&
-	author_test kappa "test < test <at> example <dot> com>" "test <unknown>" &&
-	author_test lambda "test@example.com" "Unknown <test@example.com>"
+	author_test beta "beta" "beta <unknown>" &&
+	author_test gamma "gamma <test@example.com> (comment)" "gamma <test@example.com>" &&
+	author_test delta "<delta@example.com>" "Unknown <delta@example.com>" &&
+	author_test epsilon "epsilon<test@example.com>" "epsilon <test@example.com>" &&
+	author_test zeta "zeta <test@example.com" "zeta <test@example.com>" &&
+	author_test eta " eta " "eta <unknown>" &&
+	author_test theta "theta < test@example.com >" "theta <test@example.com>" &&
+	author_test iota "iota >test@example.com>" "iota <test@example.com>" &&
+	author_test kappa "kappa < test <at> example <dot> com>" "kappa <unknown>" &&
+	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v2 9/9] remote-bzr, remote-hg: fix email address regular expression

[Richard Hansen]

Before, strings like "foo.bar@example.com" would be converted to
"foo. <bar@example.com>" when they should be "unknown
<foo.bar@example.com>".

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/git-remote-bzr | 7 +++----
 contrib/remote-helpers/git-remote-hg  | 7 +++----
 contrib/remote-helpers/test-hg.sh     | 3 ++-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 054161a..7e34532 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -44,8 +44,8 @@ import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 RAW_AUTHOR_RE = re.compile('^(\w+) (.+)? <(.*)> (\d+) ([+-]\d+)')
 
 def die(msg, *args):
@@ -193,8 +193,7 @@ def fixup_user(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index c6026b9..30402d5 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -51,8 +51,8 @@ import time as ptime
 #
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 AUTHOR_HG_RE = re.compile('^(.*?) ?<(.*?)(?:>(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?<(.*)> (\d+) ([+-]\d+)')
 
@@ -316,8 +316,7 @@ def fixup_user_git(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 642ad93..347e812 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -215,7 +215,8 @@ test_expect_success 'authors' '
 	author_test theta "theta < test@example.com >" "theta <test@example.com>" &&
 	author_test iota "iota >test@example.com>" "iota <test@example.com>" &&
 	author_test kappa "kappa < test <at> example <dot> com>" "kappa <unknown>" &&
-	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>"
+	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>" &&
+	author_test mu "mu.mu@example.com" "Unknown <mu.mu@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

Re: [PATCH v2 0/9] remote-hg, remote-bzr fixes

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> A handful of fixes for the git-remote-hg and git-remote-bzr remote
> helpers and their unit tests.

Thanks; could you arrange with Felipe so that we can have
Acked-by: or Reviewed-by: after your Signed-off-by: lines
of these patches?

RE: [PATCH v2 0/9] remote-hg, remote-bzr fixes

[Felipe Contreras]

Richard Hansen wrote:
> A handful of fixes for the git-remote-hg and git-remote-bzr remote
> helpers and their unit tests.

I'm OK with the whole series.

-- 
Felipe Contreras

[PATCH v3 0/9] remote-hg, remote-bzr fixes

[Richard Hansen]

A handful of fixes for the git-remote-hg and git-remote-bzr remote
helpers and their unit tests.

Changes from v2:
  * changed the author on the following patches to Felipe Contreras:
    - [2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path
    - [3/9] test-bzr.sh, test-hg.sh: allow running from any dir
  * added 'Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>'
    to the other patches
  * made the following comment change:

diff --git a/t/test-lib.sh b/t/test-lib.sh
index af172d9..d303e6c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -28,7 +28,7 @@ then
 	TEST_DIRECTORY=$(pwd)
 else
 	# ensure that TEST_DIRECTORY is an absolute path so that it
-	# works even if the current working directory is changed
+	# is valid even if the current working directory is changed
 	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
 fi
 if test -z "$TEST_OUTPUT_DIRECTORY"

Felipe Contreras (2):
  test-lib.sh: convert $TEST_DIRECTORY to an absolute path
  test-bzr.sh, test-hg.sh: allow running from any dir

Richard Hansen (7):
  remote-hg:  don't decode UTF-8 paths into Unicode objects
  test-bzr.sh, test-hg.sh: prepare for change to push.default=simple
  test-hg.sh: eliminate 'local' bashism
  test-hg.sh: avoid obsolete 'test' syntax
  test-hg.sh: fix duplicate content strings in author tests
  test-hg.sh: help user correlate verbose output with email test
  remote-bzr, remote-hg: fix email address regular expression

 contrib/remote-helpers/git-remote-bzr |  7 +++----
 contrib/remote-helpers/git-remote-hg  |  9 ++++-----
 contrib/remote-helpers/test-bzr.sh    |  5 +++--
 contrib/remote-helpers/test-hg.sh     | 30 ++++++++++++++++--------------
 t/test-lib.sh                         |  4 ++++
 5 files changed, 30 insertions(+), 25 deletions(-)

-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 1/9] remote-hg: don't decode UTF-8 paths into Unicode objects

[Richard Hansen]

The internal mercurial API expects ordinary 8-bit string objects, not
Unicode string objects.  With this change, the test-hg.sh unit tests
pass again.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-hg | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index 3222afd..c6026b9 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -747,7 +747,7 @@ def parse_commit(parser):
             f = { 'deleted' : True }
         else:
             die('Unknown file command: %s' % line)
-        path = c_style_unescape(path).decode('utf-8')
+        path = c_style_unescape(path)
         files[path] = f
 
     # only export the commits if we are on an internal proxy repo
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path

[Richard Hansen]

From: Felipe Contreras <felipe.contreras@gmail.com>

If $TEST_DIRECTORY is specified in the environment, convert the value
to an absolute path to ensure that it remains valid even when 'cd' is
used.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Reviewed-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/test-lib.sh | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b25249e..d303e6c 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -26,6 +26,10 @@ then
 	# outside of t/, e.g. for running tests on the test library
 	# itself.
 	TEST_DIRECTORY=$(pwd)
+else
+	# ensure that TEST_DIRECTORY is an absolute path so that it
+	# is valid even if the current working directory is changed
+	TEST_DIRECTORY=$(cd "$TEST_DIRECTORY" && pwd) || exit 1
 fi
 if test -z "$TEST_OUTPUT_DIRECTORY"
 then
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 3/9] test-bzr.sh, test-hg.sh: allow running from any dir

[Richard Hansen]

From: Felipe Contreras <felipe.contreras@gmail.com>

Set TEST_DIRECTORY to the t/ directory (if TEST_DIRECTORY is not
already set) so that the user doesn't already have to be in the test
directory to run these test scripts.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
Based-on-patch-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Richard Hansen <rhansen@bbn.com>
Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 contrib/remote-helpers/test-bzr.sh | 3 ++-
 contrib/remote-helpers/test-hg.sh  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 5c50251..1e850c3 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -5,7 +5,8 @@
 
 test_description='Test remote-bzr'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 72f745d..fa7d17b 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -8,7 +8,8 @@
 
 test_description='Test remote-hg'
 
-. ./test-lib.sh
+test -n "$TEST_DIRECTORY" || TEST_DIRECTORY=${0%/*}/../../t
+. "$TEST_DIRECTORY"/test-lib.sh
 
 if ! test_have_prereq PYTHON
 then
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 4/9] test-bzr.sh, test-hg.sh: prepare for change to push.default=simple

[Richard Hansen]

Change 'git push <remote>' to 'git push <remote> <branch>' in one of
the test-bzr.sh tests to ensure that the test continues to pass when
the default value of push.default changes to simple.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 1e850c3..1e53ff9 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -379,7 +379,7 @@ test_expect_success 'export utf-8 authors' '
 	git add content &&
 	git commit -m one &&
 	git remote add bzr "bzr::../bzrrepo" &&
-	git push bzr
+	git push bzr master
 	) &&
 
 	(
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 5/9] test-hg.sh: eliminate 'local' bashism

[Richard Hansen]

Unlike bash, POSIX shell does not specify a 'local' command for
declaring function-local variable scope.  Except for IFS, the variable
names are not used anywhere else in the script so simply remove the
'local'.  For IFS, move the assignment to the 'read' command to
prevent it from affecting code outside the function.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index fa7d17b..ceef6b1 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -54,14 +54,14 @@ check_bookmark () {
 }
 
 check_push () {
-	local expected_ret=$1 ret=0 ref_ret=0 IFS=':'
+	expected_ret=$1 ret=0 ref_ret=0
 
 	shift
 	git push origin "$@" 2>error
 	ret=$?
 	cat error
 
-	while read branch kind
+	while IFS=':' read branch kind
 	do
 		case "$kind" in
 		'new')
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 6/9] test-hg.sh: avoid obsolete 'test' syntax

[Richard Hansen]

The POSIX spec says that the '-a', '-o', and parentheses operands to
the 'test' utility are obsolete extensions due to the potential for
ambiguity.  Replace '-o' with '|| test' to avoid unspecified behavior.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index ceef6b1..5535e8c 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -83,7 +83,7 @@ check_push () {
 		test $ref_ret -ne 0 && echo "match for '$branch' failed" && break
 	done
 
-	if test $expected_ret -ne $ret -o $ref_ret -ne 0
+	if test $expected_ret -ne $ret || test $ref_ret -ne 0
 	then
 		return 1
 	fi
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 7/9] test-hg.sh: fix duplicate content strings in author tests

[Richard Hansen]

"beta" was used twice.  Change the second copy to "gamma" and
increment the remaining content strings.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg.sh | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 5535e8c..eb72db8 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -207,15 +207,15 @@ test_expect_success 'authors' '
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
 	author_test beta "test" "test <unknown>" &&
-	author_test beta "test <test@example.com> (comment)" "test <test@example.com>" &&
-	author_test gamma "<test@example.com>" "Unknown <test@example.com>" &&
-	author_test delta "name<test@example.com>" "name <test@example.com>" &&
-	author_test epsilon "name <test@example.com" "name <test@example.com>" &&
-	author_test zeta " test " "test <unknown>" &&
-	author_test eta "test < test@example.com >" "test <test@example.com>" &&
-	author_test theta "test >test@example.com>" "test <test@example.com>" &&
-	author_test iota "test < test <at> example <dot> com>" "test <unknown>" &&
-	author_test kappa "test@example.com" "Unknown <test@example.com>"
+	author_test gamma "test <test@example.com> (comment)" "test <test@example.com>" &&
+	author_test delta "<test@example.com>" "Unknown <test@example.com>" &&
+	author_test epsilon "name<test@example.com>" "name <test@example.com>" &&
+	author_test zeta "name <test@example.com" "name <test@example.com>" &&
+	author_test eta " test " "test <unknown>" &&
+	author_test theta "test < test@example.com >" "test <test@example.com>" &&
+	author_test iota "test >test@example.com>" "test <test@example.com>" &&
+	author_test kappa "test < test <at> example <dot> com>" "test <unknown>" &&
+	author_test lambda "test@example.com" "Unknown <test@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 8/9] test-hg.sh: help user correlate verbose output with email test

[Richard Hansen]

It's hard to tell which author conversion test failed when the email
addresses look similar.

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg.sh | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index eb72db8..642ad93 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -206,16 +206,16 @@ test_expect_success 'authors' '
 
 	>../expected &&
 	author_test alpha "" "H G Wells <wells@example.com>" &&
-	author_test beta "test" "test <unknown>" &&
-	author_test gamma "test <test@example.com> (comment)" "test <test@example.com>" &&
-	author_test delta "<test@example.com>" "Unknown <test@example.com>" &&
-	author_test epsilon "name<test@example.com>" "name <test@example.com>" &&
-	author_test zeta "name <test@example.com" "name <test@example.com>" &&
-	author_test eta " test " "test <unknown>" &&
-	author_test theta "test < test@example.com >" "test <test@example.com>" &&
-	author_test iota "test >test@example.com>" "test <test@example.com>" &&
-	author_test kappa "test < test <at> example <dot> com>" "test <unknown>" &&
-	author_test lambda "test@example.com" "Unknown <test@example.com>"
+	author_test beta "beta" "beta <unknown>" &&
+	author_test gamma "gamma <test@example.com> (comment)" "gamma <test@example.com>" &&
+	author_test delta "<delta@example.com>" "Unknown <delta@example.com>" &&
+	author_test epsilon "epsilon<test@example.com>" "epsilon <test@example.com>" &&
+	author_test zeta "zeta <test@example.com" "zeta <test@example.com>" &&
+	author_test eta " eta " "eta <unknown>" &&
+	author_test theta "theta < test@example.com >" "theta <test@example.com>" &&
+	author_test iota "iota >test@example.com>" "iota <test@example.com>" &&
+	author_test kappa "kappa < test <at> example <dot> com>" "kappa <unknown>" &&
+	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

[PATCH v3 9/9] remote-bzr, remote-hg: fix email address regular expression

[Richard Hansen]

Before, strings like "foo.bar@example.com" would be converted to
"foo. <bar@example.com>" when they should be "unknown
<foo.bar@example.com>".

Signed-off-by: Richard Hansen <rhansen@bbn.com>
Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/git-remote-bzr | 7 +++----
 contrib/remote-helpers/git-remote-hg  | 7 +++----
 contrib/remote-helpers/test-hg.sh     | 3 ++-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/contrib/remote-helpers/git-remote-bzr b/contrib/remote-helpers/git-remote-bzr
index 054161a..7e34532 100755
--- a/contrib/remote-helpers/git-remote-bzr
+++ b/contrib/remote-helpers/git-remote-bzr
@@ -44,8 +44,8 @@ import StringIO
 import atexit, shutil, hashlib, urlparse, subprocess
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 RAW_AUTHOR_RE = re.compile('^(\w+) (.+)? <(.*)> (\d+) ([+-]\d+)')
 
 def die(msg, *args):
@@ -193,8 +193,7 @@ def fixup_user(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/git-remote-hg b/contrib/remote-helpers/git-remote-hg
index c6026b9..30402d5 100755
--- a/contrib/remote-helpers/git-remote-hg
+++ b/contrib/remote-helpers/git-remote-hg
@@ -51,8 +51,8 @@ import time as ptime
 #
 
 NAME_RE = re.compile('^([^<>]+)')
-AUTHOR_RE = re.compile('^([^<>]+?)? ?<([^<>]*)>$')
-EMAIL_RE = re.compile('^([^<>]+[^ \\\t<>])?\\b(?:[ \\t<>]*?)\\b([^ \\t<>]+@[^ \\t<>]+)')
+AUTHOR_RE = re.compile('^([^<>]+?)? ?[<>]([^<>]*)(?:$|>)')
+EMAIL_RE = re.compile(r'([^ \t<>]+@[^ \t<>]+)')
 AUTHOR_HG_RE = re.compile('^(.*?) ?<(.*?)(?:>(.+)?)?$')
 RAW_AUTHOR_RE = re.compile('^(\w+) (?:(.+)? )?<(.*)> (\d+) ([+-]\d+)')
 
@@ -316,8 +316,7 @@ def fixup_user_git(user):
     else:
         m = EMAIL_RE.match(user)
         if m:
-            name = m.group(1)
-            mail = m.group(2)
+            mail = m.group(1)
         else:
             m = NAME_RE.match(user)
             if m:
diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 642ad93..347e812 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -215,7 +215,8 @@ test_expect_success 'authors' '
 	author_test theta "theta < test@example.com >" "theta <test@example.com>" &&
 	author_test iota "iota >test@example.com>" "iota <test@example.com>" &&
 	author_test kappa "kappa < test <at> example <dot> com>" "kappa <unknown>" &&
-	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>"
+	author_test lambda "lambda@example.com" "Unknown <lambda@example.com>" &&
+	author_test mu "mu.mu@example.com" "Unknown <mu.mu@example.com>"
 	) &&
 
 	git clone "hg::hgrepo" gitrepo &&
-- 
1.8.5.rc1.208.g8ff7964

Re: [PATCH v3 0/9] remote-hg, remote-bzr fixes

[Junio C Hamano]

Richard Hansen <rhansen@bbn.com> writes:

> A handful of fixes for the git-remote-hg and git-remote-bzr remote
> helpers and their unit tests.
>
> Changes from v2:
>   * changed the author on the following patches to Felipe Contreras:
>     - [2/9] test-lib.sh: convert $TEST_DIRECTORY to an absolute path
>     - [3/9] test-bzr.sh, test-hg.sh: allow running from any dir
>   * added 'Reviewed-by: Felipe Contreras <felipe.contreras@gmail.com>'
>     to the other patches
>   * made the following comment change:

Thanks, both; will requeue.

Re: [PATCH v3 0/9] remote-hg, remote-bzr fixes

[Junio C Hamano]

Thanks, will requeue.