threads / patch / 33383

v2, 4 partsremote-bzr: avoid echo -n

Subject: [PATCH v2 1/4] remote-bzr: avoid echo -n

## tl;dr

11 messages between Apr 4, 2013 and Apr 6, 2013. Diffs are folded; open one to read it.

replies: 10people: 2as markdown or json

Felipe Contreras· Apr 4, 2013, 15:36 UTC · lore

[PATCH v2 0/4] remote-helpers: trivial test fixes

Hi,

A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the outsanding shell portability issue. The rest is the same.

Felipe Contreras (4):
  remote-bzr: avoid echo -n
  remote-helpers: fix the run of all tests
  remote-bzr: remove stale check code for tests
  remote-hg: fix hg-git test-case
 contrib/remote-helpers/Makefile          |  1 +
 contrib/remote-helpers/test-bzr.sh       | 16 +---------------
 contrib/remote-helpers/test-hg-hg-git.sh |  1 -
 3 files changed, 2 insertions(+), 16 deletions(-)
-- 
1.8.2
Felipe Contreras· Apr 4, 2013, 15:36 UTC · re: Felipe Contreras · lore
It's not portable, as reported by test-lint.
Signed-off-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 70aa8a0..b81052b 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -136,7 +136,7 @@ test_expect_success 'special modes' '
   (cd gitrepo &&
   git cat-file -p HEAD:link > ../actual) &&
 
-  echo -n content > expected &&
+  printf content > expected &&
   test_cmp expected actual
 '
 
-- 
1.8.2
Felipe Contreras· Apr 4, 2013, 15:36 UTC · re: Felipe Contreras · lore

[PATCH v2 2/4] remote-helpers: fix the run of all tests

We don't need to check for duplicate test numbers, we don't have them, and either way test-lint-duplicates doesn't work in this situation.

Also, while we are on it, enable test-lint-shell-syntax to check for sh errors.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/Makefile | 1 +
 1 file changed, 1 insertion(+)
Show changes to contrib/remote-helpers/Makefile +1 −0
diff --git a/contrib/remote-helpers/Makefile b/contrib/remote-helpers/Makefile
index 9a76575..239161d 100644
--- a/contrib/remote-helpers/Makefile
+++ b/contrib/remote-helpers/Makefile
@@ -3,6 +3,7 @@ TESTS := $(wildcard test*.sh)
 export T := $(addprefix $(CURDIR)/,$(TESTS))
 export MAKE := $(MAKE) -e
 export PATH := $(CURDIR):$(PATH)
+export TEST_LINT := test-lint-executable test-lint-shell-syntax
 
 test:
 	$(MAKE) -C ../../t $@
-- 
1.8.2
Felipe Contreras· Apr 4, 2013, 15:36 UTC · re: Felipe Contreras · lore

[PATCH v2 3/4] remote-bzr: remove stale check code for tests

The fastimport plugin was only required in the early stage of development.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-bzr.sh | 14 --------------
 1 file changed, 14 deletions(-)
Show changes to contrib/remote-helpers/test-bzr.sh +0 −14
diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index b81052b..8450432 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -17,20 +17,6 @@ if ! "$PYTHON_PATH" -c 'import bzrlib'; then
 	test_done
 fi
 
-cmd='
-import bzrlib
-bzrlib.initialize()
-import bzrlib.plugin
-bzrlib.plugin.load_plugins()
-import bzrlib.plugins.fastimport
-'
-
-if ! "$PYTHON_PATH" -c "$cmd"; then
-	echo "consider setting BZR_PLUGIN_PATH=$HOME/.bazaar/plugins" 1>&2
-	skip_all='skipping remote-bzr tests; bzr-fastimport not available'
-	test_done
-fi
-
 check () {
 	(cd $1 &&
 	git log --format='%s' -1 &&
-- 
1.8.2
Felipe Contreras· Apr 4, 2013, 15:36 UTC · re: Felipe Contreras · lore

[PATCH v2 4/4] remote-hg: fix hg-git test-case

There was some lingering code that shouldn't have been there in the first place.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/remote-helpers/test-hg-hg-git.sh | 1 -
 1 file changed, 1 deletion(-)
Show changes to contrib/remote-helpers/test-hg-hg-git.sh +0 −1
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 7e3967f..3f253b7 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -140,7 +140,6 @@ test_expect_success 'executable bit' '
 		git_clone_$x hgrepo-$x gitrepo2-$x &&
 		git_log gitrepo2-$x > log-$x
 	done &&
-	cp -r log-* output-* /tmp/foo/ &&
 
 	test_cmp output-hg output-git &&
 	test_cmp log-hg log-git
-- 
1.8.2
Torsten Bögershausen· Apr 6, 2013, 17:03 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On 04.04.13 17:36, Felipe Contreras wrote:
Show 16 quoted lines
> Hi,
> 
> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
> outsanding shell portability issue. The rest is the same.
> 
> Felipe Contreras (4):
>   remote-bzr: avoid echo -n
>   remote-helpers: fix the run of all tests
>   remote-bzr: remove stale check code for tests
>   remote-hg: fix hg-git test-case
> 
>  contrib/remote-helpers/Makefile          |  1 +
>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>  3 files changed, 2 insertions(+), 16 deletions(-)
> 
Sorry being late, now I installed bzr and hg on one of my machines 

One defect found: "\s" is not portable on all grep versions A "*" is not a "basic regular expression", so we need to use egrep

Show changes to 3 files +3 −3

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

diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
index 5f81dfa..2e80c11 100755
--- a/contrib/remote-helpers/test-hg.sh
+++ b/contrib/remote-helpers/test-hg.sh
@@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
   git push
   ) &&
 
-  hg -R hgrepo bookmarks | grep "devel\s\+3:"
+  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
 '
2 mninor nits:

diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
index 8450432..7970f9e 100755
--- a/contrib/remote-helpers/test-bzr.sh
+++ b/contrib/remote-helpers/test-bzr.sh
@@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then
 fi
 
 if ! "$PYTHON_PATH" -c 'import bzrlib'; then
-       skip_all='skipping remote-bzr tests; bzr not available'
+       skip_all='skipping remote-bzr tests; python bzrlib not available'
        test_done
 fi
 
diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
index 3f253b7..5d87282 100755
--- a/contrib/remote-helpers/test-hg-hg-git.sh
+++ b/contrib/remote-helpers/test-hg-hg-git.sh
@@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then
 fi
 
 if ! "$PYTHON_PATH" -c 'import hggit'; then
-       skip_all='skipping remote-hg tests; hg-git not available'
+       skip_all='skipping remote-hg tests; python hggit not available'
        test_done
 fi

(And as a micro-nit: the indenting deserves some better indentation:
TAB could be used for *.sh, and sub-shells could be indentented one TAB:


test_expect_success 'update bookmark' '
	test_when_finished "rm -rf gitrepo*" &&
	(
		cd hgrepo &&
		hg bookmark devel

/Torsten
Torsten Bögershausen· Apr 6, 2013, 17:09 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On 06.04.13 19:03, Torsten Bögershausen wrote:
Show 21 quoted lines
> On 04.04.13 17:36, Felipe Contreras wrote:
>> Hi,
>>
>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>> outsanding shell portability issue. The rest is the same.
>>
>> Felipe Contreras (4):
>>   remote-bzr: avoid echo -n
>>   remote-helpers: fix the run of all tests
>>   remote-bzr: remove stale check code for tests
>>   remote-hg: fix hg-git test-case
>>
>>  contrib/remote-helpers/Makefile          |  1 +
>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>
> Sorry being late, now I installed bzr and hg on one of my machines 
> 
> One defect found: "\s" is not portable on all grep versions
> A "*" is not a "basic regular expression", so we need to use egrep

Sorry for confusion: A "*" is a basic regular expression, but not the "+" "+" is an extended regular expression, which is understood by some grep versions (gnu ?).

Felipe: Should I send a patch, or wait for a re-roll?
Felipe Contreras· Apr 6, 2013, 17:29 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 34 quoted lines
> On 04.04.13 17:36, Felipe Contreras wrote:
>> Hi,
>>
>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>> outsanding shell portability issue. The rest is the same.
>>
>> Felipe Contreras (4):
>>   remote-bzr: avoid echo -n
>>   remote-helpers: fix the run of all tests
>>   remote-bzr: remove stale check code for tests
>>   remote-hg: fix hg-git test-case
>>
>>  contrib/remote-helpers/Makefile          |  1 +
>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>
> Sorry being late, now I installed bzr and hg on one of my machines
>
> One defect found: "\s" is not portable on all grep versions
> A "*" is not a "basic regular expression", so we need to use egrep
>
>
> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
> index 5f81dfa..2e80c11 100755
> --- a/contrib/remote-helpers/test-hg.sh
> +++ b/contrib/remote-helpers/test-hg.sh
> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>    git push
>    ) &&
>
> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>  '
I would rather use [ \t] instead.
Show 12 quoted lines
> 2 mninor nits:
>
> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh
> index 8450432..7970f9e 100755
> --- a/contrib/remote-helpers/test-bzr.sh
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -13,7 +13,7 @@ if ! test_have_prereq PYTHON; then
>  fi
>
>  if ! "$PYTHON_PATH" -c 'import bzrlib'; then
> -       skip_all='skipping remote-bzr tests; bzr not available'
> +       skip_all='skipping remote-bzr tests; python bzrlib not available'

"python bzrlib" is meaningless to the user; bazaar provides it; basically all the code is there.

Show 13 quoted lines
>         test_done
>  fi
>
> diff --git a/contrib/remote-helpers/test-hg-hg-git.sh b/contrib/remote-helpers/test-hg-hg-git.sh
> index 3f253b7..5d87282 100755
> --- a/contrib/remote-helpers/test-hg-hg-git.sh
> +++ b/contrib/remote-helpers/test-hg-hg-git.sh
> @@ -21,7 +21,7 @@ if ! "$PYTHON_PATH" -c 'import mercurial'; then
>  fi
>
>  if ! "$PYTHON_PATH" -c 'import hggit'; then
> -       skip_all='skipping remote-hg tests; hg-git not available'
> +       skip_all='skipping remote-hg tests; python hggit not available'

Same. Google for 'python hggit' and the result is hg-git; and that's what you would actually tell your package manager to install.

Cheers.
-- 
Felipe Contreras
Torsten Bögershausen· Apr 6, 2013, 17:45 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On 06.04.13 19:29, Felipe Contreras wrote:
Show 37 quoted lines
> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 04.04.13 17:36, Felipe Contreras wrote:
>>> Hi,
>>>
>>> A reroll, now we do some checks, just avoid test-lint-duplicates, and fix the
>>> outsanding shell portability issue. The rest is the same.
>>>
>>> Felipe Contreras (4):
>>>   remote-bzr: avoid echo -n
>>>   remote-helpers: fix the run of all tests
>>>   remote-bzr: remove stale check code for tests
>>>   remote-hg: fix hg-git test-case
>>>
>>>  contrib/remote-helpers/Makefile          |  1 +
>>>  contrib/remote-helpers/test-bzr.sh       | 16 +---------------
>>>  contrib/remote-helpers/test-hg-hg-git.sh |  1 -
>>>  3 files changed, 2 insertions(+), 16 deletions(-)
>>>
>> Sorry being late, now I installed bzr and hg on one of my machines
>>
>> One defect found: "\s" is not portable on all grep versions
>> A "*" is not a "basic regular expression", so we need to use egrep
>>
>>
>> diff --git a/contrib/remote-helpers/test-hg.sh b/contrib/remote-helpers/test-hg.sh
>> index 5f81dfa..2e80c11 100755
>> --- a/contrib/remote-helpers/test-hg.sh
>> +++ b/contrib/remote-helpers/test-hg.sh
>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>    git push
>>    ) &&
>>
>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>  '
> 
> I would rather use [ \t] instead.

That doesn't work on e.g. Mac OS. [:space:] is actually portable /Torsten

Felipe Contreras· Apr 6, 2013, 17:58 UTC · re: Torsten Bögershausen · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 06.04.13 19:29, Felipe Contreras wrote:
>> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
Show 13 quoted lines
>>> --- a/contrib/remote-helpers/test-hg.sh
>>> +++ b/contrib/remote-helpers/test-hg.sh
>>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>>    git push
>>>    ) &&
>>>
>>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>>  '
>>
>> I would rather use [ \t] instead.
> That doesn't work on e.g. Mac OS.
> [:space:] is actually portable
Why wouldn't it work? This is from their manpage:

A bracket expression is a list of characters enclosed by [ and ]. It matches any single character in that list; if the first character of the list is the caret ^ then it matches any character not in the list. For example, the regular expression [0123456789] matches any single digit.

-- 
Felipe Contreras
Torsten Bögershausen· Apr 6, 2013, 18:41 UTC · re: Felipe Contreras · lore

Re: [PATCH v2 0/4] remote-helpers: trivial test fixes

On 06.04.13 19:58, Felipe Contreras wrote:
Show 27 quoted lines
> On Sat, Apr 6, 2013 at 11:45 AM, Torsten Bögershausen <tboegi@web.de> wrote:
>> On 06.04.13 19:29, Felipe Contreras wrote:
>>> On Sat, Apr 6, 2013 at 11:03 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> 
>>>> --- a/contrib/remote-helpers/test-hg.sh
>>>> +++ b/contrib/remote-helpers/test-hg.sh
>>>> @@ -115,7 +115,7 @@ test_expect_success 'update bookmark' '
>>>>    git push
>>>>    ) &&
>>>>
>>>> -  hg -R hgrepo bookmarks | grep "devel\s\+3:"
>>>> +  hg -R hgrepo bookmarks | egrep "devel[[:space:]]+3:"
>>>>  '
>>>
>>> I would rather use [ \t] instead.
>> That doesn't work on e.g. Mac OS.
>> [:space:] is actually portable
> 
> Why wouldn't it work? This is from their manpage:
> 
> A bracket expression is a list of characters enclosed by [ and ].  It
> matches any single character in
> that list; if the first character of the list is the caret ^ then it
> matches any character not in the
> list.  For example, the regular expression [0123456789] matches any
> single digit.
> 

It's not about the "bracket list". It's about using \t as an abreviation for TAB. The "backslash n" as an replacement for TAB is quite often understood by many programs. It is not demanded to be understood by all grep implementations, please see below. Instead of using \t you can use a literal TAB. Contact your local editor how to put that inte source code ;-)

 
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/xbd_chap09.html#tag_09_03
9.3.2 BRE Ordinary Characters
An ordinary character is a BRE that matches itself: any character in the supported character set, except for the BRE special characters listed in BRE Special Characters.
The interpretation of an ordinary character preceded by a backslash ( '\' ) is undefined, except for:
    The characters ')', '(', '{', and '}'
    The digits 1 to 9 inclusive (see BREs Matching Multiple Characters)
    A character inside a bracket expression

← back to recent threads