threads / patch / 26064

patch, 14 partst{3032,6038}-*.sh: Allow SED_BIN_OPT to override the -b sed option

Subject: [PATCH 14/14] t{3032,6038}-*.sh: Allow SED_BIN_OPT to override the -b sed option

## tl;dr

3 messages between Dec 14, 2010 and Dec 16, 2010. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Ramsay Jones· Dec 14, 2010, 18:43 UTC · lore

Some older versions of sed (eg. v3.02) do not understand the -b (--binary) option. However, these versions of sed may have an equivalent option we can use to request binary mode processing (eg. -c or --nocr).

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

I nearly didn't send this patch, since my MinGW sed is somewhat old, and most people would not have this problem. (Note I *think* I installed using msysGit-fullinstall-1.6.4-preview20090729.exe) So, feel free to drop this patch...

 t/t3032-merge-recursive-options.sh |    2 +-
 t/t6038-merge-text-auto.sh         |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Show changes to 2 files +2 −2

t/t3032-merge-recursive-options.sh, t/t6038-merge-text-auto.sh

diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-options.sh
index 29e049a..22d9988 100755
--- a/t/t3032-merge-recursive-options.sh
+++ b/t/t3032-merge-recursive-options.sh
@@ -13,7 +13,7 @@ test_description='merge-recursive options
 
 . ./test-lib.sh
 
-test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
+test_have_prereq SED_STRIPS_CR && SED_OPTIONS=${SED_BIN_OPT--b}
 test_have_prereq MINGW && export GREP_OPTIONS=-U
 
 test_expect_success 'setup' '
diff --git a/t/t6038-merge-text-auto.sh b/t/t6038-merge-text-auto.sh
index d9c2d38..4245d6e 100755
--- a/t/t6038-merge-text-auto.sh
+++ b/t/t6038-merge-text-auto.sh
@@ -14,7 +14,7 @@ test_description='CRLF merge conflict across text=auto change
 
 . ./test-lib.sh
 
-test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
+test_have_prereq SED_STRIPS_CR && SED_OPTIONS=${SED_BIN_OPT--b}
 
 test_expect_success setup '
 	git config core.autocrlf false &&

-- 
1.7.3

Junio C Hamano· Dec 14, 2010, 19:31 UTC · re: Ramsay Jones · lore

Re: [PATCH 14/14] t{3032,6038}-*.sh: Allow SED_BIN_OPT to override the -b sed option

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

Show 27 quoted lines

> Some older versions of sed (eg. v3.02) do not understand the -b
> (--binary) option. However, these versions of sed may have an
> equivalent option we can use to request binary mode processing
> (eg. -c or --nocr).
>
> Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
> ---
>
> I nearly didn't send this patch, since my MinGW sed is somewhat old, and
> most people would not have this problem. (Note I *think* I installed
> using msysGit-fullinstall-1.6.4-preview20090729.exe)
> So, feel free to drop this patch...
>
>  t/t3032-merge-recursive-options.sh |    2 +-
>  t/t6038-merge-text-auto.sh         |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/t/t3032-merge-recursive-options.sh b/t/t3032-merge-recursive-options.sh
> index 29e049a..22d9988 100755
> --- a/t/t3032-merge-recursive-options.sh
> +++ b/t/t3032-merge-recursive-options.sh
> @@ -13,7 +13,7 @@ test_description='merge-recursive options
>  
>  . ./test-lib.sh
>  
> -test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
> +test_have_prereq SED_STRIPS_CR && SED_OPTIONS=${SED_BIN_OPT--b}

It is unclear who is supposed to feed you SED_BIN_OPT.

Perhaps a patch to add an insn like "If you have an ancient sed, export SED_BIN_OPT=-c before running tests" to t/README is also necessary to go together with this change.

Ramsay Jones· Dec 16, 2010, 20:15 UTC · re: Junio C Hamano · lore

Re: [PATCH 14/14] t{3032,6038}-*.sh: Allow SED_BIN_OPT to override the -b sed option

Junio C Hamano wrote:

Show 9 quoted lines

>>  
>> -test_have_prereq SED_STRIPS_CR && SED_OPTIONS=-b
>> +test_have_prereq SED_STRIPS_CR && SED_OPTIONS=${SED_BIN_OPT--b}
> 
> It is unclear who is supposed to feed you SED_BIN_OPT.
> 
> Perhaps a patch to add an insn like "If you have an ancient sed, export
> SED_BIN_OPT=-c before running tests" to t/README is also necessary to go
> together with this change.

Yes, you are right. I had such a change to t/README but it was mixed in with another patch to document other such variables which was incomplete. (see my previous mail in response to your comments on patch 4/14).

Also, I was half expecting you to drop the patch, because I thought I was in the minority with an old sed; however, it seems I'm not that unusual after all ... ;-)

ATB, Ramsay Jones