threads / patch / 35318

patch, 7 partsremote-hg, remote-bzr fixes

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

## tl;dr

56 messages between Nov 11, 2013 and Nov 18, 2013. Diffs are folded; open one to read it.

replies: 55people: 3as markdown or json

Richard Hansen· Nov 11, 2013, 04:05 UTC · lore

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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/git-remote-hg +1 −1
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
Felipe Contreras· Nov 11, 2013, 11:04 UTC · re: Richard Hansen · lore

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

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
Richard Hansen· Nov 11, 2013, 18:30 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 06:04, Felipe Contreras wrote:
Show 7 quoted lines
> 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
Felipe Contreras· Nov 11, 2013, 19:10 UTC · re: Richard Hansen · lore

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

On Mon, Nov 11, 2013 at 12:30 PM, Richard Hansen <rhansen@bbn.com> wrote:
Show 13 quoted lines
> 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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(+)
Show changes to 2 files +2 −0

contrib/remote-helpers/test-bzr.sh, contrib/remote-helpers/test-hg.sh

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
Felipe Contreras· Nov 11, 2013, 11:35 UTC · re: Richard Hansen · lore

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

Richard Hansen wrote:
Show 18 quoted lines
> 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
Junio C Hamano· Nov 11, 2013, 18:31 UTC · re: Felipe Contreras · lore

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

Felipe Contreras <felipe.contreras@gmail.com> writes:
Show 24 quoted lines
> 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.

Felipe Contreras· Nov 11, 2013, 18:58 UTC · re: Junio C Hamano · lore

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

On Mon, Nov 11, 2013 at 12:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 29 quoted lines
> 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:
Show changes to t/test-lib.sh +2 −0
--- 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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 2 files +7 −1

contrib/remote-helpers/test-bzr.sh, contrib/remote-helpers/test-hg.sh

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
Felipe Contreras· Nov 11, 2013, 11:37 UTC · re: Richard Hansen · lore

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

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
Richard Hansen· Nov 11, 2013, 19:20 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 06:37, Felipe Contreras wrote:
Show 15 quoted lines
> 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

Felipe Contreras· Nov 11, 2013, 19:31 UTC · re: Richard Hansen · lore

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

On Sun, Nov 10, 2013 at 10:05 PM, Richard Hansen <rhansen@bbn.com> wrote:
Show 7 quoted lines
> @@ -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
Richard Hansen· Nov 11, 2013, 21:16 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 14:31, Felipe Contreras wrote:
Show 11 quoted lines
> 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
Felipe Contreras· Nov 11, 2013, 21:19 UTC · re: Richard Hansen · lore

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

On Mon, Nov 11, 2013 at 3:16 PM, Richard Hansen <rhansen@bbn.com> wrote:
Show 15 quoted lines
> 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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +2 −2
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
Felipe Contreras· Nov 11, 2013, 11:40 UTC · re: Richard Hansen · lore

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

Richard Hansen wrote:
Show 5 quoted lines
> 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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +1 −1
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
Felipe Contreras· Nov 11, 2013, 11:44 UTC · re: Richard Hansen · lore

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

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
Richard Hansen· Nov 12, 2013, 05:34 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 06:44, Felipe Contreras wrote:
Show 6 quoted lines
> 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
Felipe Contreras· Nov 12, 2013, 20:23 UTC · re: Richard Hansen · lore

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

Richard Hansen wrote:
Show 11 quoted lines
> 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
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +10 −10
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
Felipe Contreras· Nov 11, 2013, 11:42 UTC · re: Richard Hansen · lore

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

Richard Hansen wrote:
Show 28 quoted lines
> 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?
Show 11 quoted lines
> +	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
Richard Hansen· Nov 11, 2013, 19:19 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 06:42, Felipe Contreras wrote:
Show 19 quoted lines
> 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.
Show 12 quoted lines
>> -	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

Show 13 quoted lines
> 
>> +	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· Nov 11, 2013, 19:29 UTC · re: Richard Hansen · lore

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

On Mon, Nov 11, 2013 at 1:19 PM, Richard Hansen <rhansen@bbn.com> wrote:
Show 22 quoted lines
> 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.
Show 15 quoted lines
>>> -    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
Richard Hansen· Nov 11, 2013, 21:17 UTC · re: Felipe Contreras · lore

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

On 2013-11-11 14:29, Felipe Contreras wrote:
Show 44 quoted lines
> 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

Show 6 quoted lines
> 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.
> 
Richard Hansen· Nov 11, 2013, 04:05 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 3 files +8 −9

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/git-remote-hg, contrib/remote-helpers/test-hg.sh

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
Felipe Contreras· Nov 11, 2013, 11:43 UTC · re: Richard Hansen · lore

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

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
Junio C Hamano· Nov 11, 2013, 18:35 UTC · re: Richard Hansen · lore

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

Richard Hansen <rhansen@bbn.com> writes:
Show 17 quoted lines
> 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.
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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

Changes from v1:
Show changes to 3 files +19 −21

contrib/remote-helpers/test-bzr.sh, contrib/remote-helpers/test-hg.sh, t/test-lib.sh

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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/git-remote-hg +1 −1
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(+)
Show changes to t/test-lib.sh +4 −0
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
Richard Hansen· Nov 12, 2013, 06:05 UTC · re: Richard Hansen · lore

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

On 2013-11-12 00:54, Richard Hansen wrote:
Show 5 quoted lines
> 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

Show 19 quoted lines
> ---
>  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
Junio C Hamano· Nov 12, 2013, 20:06 UTC · re: Richard Hansen · lore

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

Richard Hansen <rhansen@bbn.com> writes:
Show 9 quoted lines
> 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@...>
	
Show 24 quoted lines
>
> 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
Felipe Contreras· Nov 12, 2013, 20:25 UTC · re: Richard Hansen · lore

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

Richard Hansen wrote:
Show 9 quoted lines
> 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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 2 files +4 −2

contrib/remote-helpers/test-bzr.sh, contrib/remote-helpers/test-hg.sh

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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-bzr.sh +1 −1
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +2 −2
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +1 −1
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

"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(-)
Show changes to contrib/remote-helpers/test-hg.sh +9 −9
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +10 −10
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
Richard Hansen· Nov 12, 2013, 05:54 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 3 files +8 −9

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/git-remote-hg, contrib/remote-helpers/test-hg.sh

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
Junio C Hamano· Nov 12, 2013, 17:59 UTC · re: Richard Hansen · lore

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

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?
Felipe Contreras· Nov 12, 2013, 20:27 UTC · re: Richard Hansen · lore

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

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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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:
Show changes to t/test-lib.sh +1 −1
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/git-remote-hg +1 −1
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(+)
Show changes to t/test-lib.sh +4 −0
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 2 files +4 −2

contrib/remote-helpers/test-bzr.sh, contrib/remote-helpers/test-hg.sh

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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-bzr.sh +1 −1
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +2 −2
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +1 −1
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

"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(-)
Show changes to contrib/remote-helpers/test-hg.sh +9 −9
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to contrib/remote-helpers/test-hg.sh +10 −10
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
Richard Hansen· Nov 18, 2013, 04:12 UTC · re: Richard Hansen · lore

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

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(-)
Show changes to 3 files +8 −9

contrib/remote-helpers/git-remote-bzr, contrib/remote-helpers/git-remote-hg, contrib/remote-helpers/test-hg.sh

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
Junio C Hamano· Nov 18, 2013, 16:13 UTC · re: Richard Hansen · lore

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

Richard Hansen <rhansen@bbn.com> writes:
Show 10 quoted lines
> 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.

← back to recent threads