[PATCH next] test-lib: set $DIFF to diff if it is unset

[PATCH next] test-lib: set $DIFF to diff if it is unset

[Thomas Rast]

Since 7b3bdbb (fixup: do not unconditionally disable "diff -u",
2010-05-31), test-lib.sh depends on having $DIFF set in the
environment for the construction of $GIT_TEST_CMP.  While this works
when called from the main Makefile, it fails if the tests are called
on their own and the user does not have $DIFF set.

Set it to 'diff' if it is unset, like the Makefile does.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

I have an uneasy feeling that I must be missing something, seeing as
this went unnoticed for 10 days...

 t/test-lib.sh |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b23a61d..4d89049 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -78,6 +78,7 @@ export EDITOR
 
 if test -z "$GIT_TEST_CMP"
 then
+	DIFF=${DIFF:-diff}
 	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"
 	then
 		GIT_TEST_CMP="$DIFF -c"
-- 
1.7.1.550.g553ab5

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Ævar Arnfjörð Bjarmason]

On Thu, Jun 10, 2010 at 09:39, Thomas Rast <trast@student.ethz.ch> wrote:
> Since 7b3bdbb (fixup: do not unconditionally disable "diff -u",
> 2010-05-31), test-lib.sh depends on having $DIFF set in the
> environment for the construction of $GIT_TEST_CMP.  While this works
> when called from the main Makefile, it fails if the tests are called
> on their own and the user does not have $DIFF set.
>
> Set it to 'diff' if it is unset, like the Makefile does.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>
> I have an uneasy feeling that I must be missing something, seeing as
> this went unnoticed for 10 days...

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>

I spotted it too when preparing the TAP series, I just didn't submit a
fix for it.

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Brandon Casey]

On 06/10/2010 04:39 AM, Thomas Rast wrote:
> Since 7b3bdbb (fixup: do not unconditionally disable "diff -u",
> 2010-05-31), test-lib.sh depends on having $DIFF set in the
> environment for the construction of $GIT_TEST_CMP.  While this works
> when called from the main Makefile, it fails if the tests are called
> on their own and the user does not have $DIFF set.
> 
> Set it to 'diff' if it is unset, like the Makefile does.
> 
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
> 
> I have an uneasy feeling that I must be missing something, seeing as
> this went unnoticed for 10 days...
> 
>  t/test-lib.sh |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index b23a61d..4d89049 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -78,6 +78,7 @@ export EDITOR
>  
>  if test -z "$GIT_TEST_CMP"
>  then
> +	DIFF=${DIFF:-diff}
>  	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"
>  	then
>  		GIT_TEST_CMP="$DIFF -c"

I think what should be done instead, is to move this section
down below the line where GIT-BUILD-OPTIONS is sourced.  That
way, the value of $DIFF can be gotten from that file.

I guess GIT_TEST_CMP and GIT_TEST_CMP_USE_COPIED_CONTEXT
should be written into GIT-BUILD-OPTIONS too.

-brandon

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Junio C Hamano]

Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:

> I think what should be done instead, is to move this section
> down below the line where GIT-BUILD-OPTIONS is sourced.  That
> way, the value of $DIFF can be gotten from that file.
>
> I guess GIT_TEST_CMP and GIT_TEST_CMP_USE_COPIED_CONTEXT
> should be written into GIT-BUILD-OPTIONS too.

I like that.  Something like this?

 Makefile      |    6 ++++++
 t/test-lib.sh |   20 ++++++++++----------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index 0367e8a..6b3b59b 100644
--- a/Makefile
+++ b/Makefile
@@ -1944,6 +1944,12 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo NO_CURL=\''$(subst ','\'',$(subst ','\'',$(NO_CURL)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+ifdef GIT_TEST_CMP
+	@echo GIT_TEST_CMP=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_CMP)))'\' >>$@
+endif
+ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
+	@echo GIT_TEST_CMP_USE_COPIED_CONTEXT=YesPlease >>$@
+endif
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
diff --git a/t/test-lib.sh b/t/test-lib.sh
index a290011..eafe146 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -64,16 +64,6 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
 
-if test -z "$GIT_TEST_CMP"
-then
-	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"
-	then
-		GIT_TEST_CMP="$DIFF -c"
-	else
-		GIT_TEST_CMP="$DIFF -u"
-	fi
-fi
-
 # Protect ourselves from common misconfiguration to export
 # CDPATH into the environment
 unset CDPATH
@@ -691,6 +681,16 @@ export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_CONFIG_NOGLOB
 
 . ../GIT-BUILD-OPTIONS
 
+if test -z "$GIT_TEST_CMP"
+then
+	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"
+	then
+		GIT_TEST_CMP="$DIFF -c"
+	else
+		GIT_TEST_CMP="$DIFF -u"
+	fi
+fi
+
 GITPERLLIB=$(pwd)/../perl/blib/lib:$(pwd)/../perl/blib/arch/auto/Git
 export GITPERLLIB
 test -d ../templates/blt || {

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Brandon Casey]

On 06/11/2010 11:15 AM, Junio C Hamano wrote:
> Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:
> 
>> I think what should be done instead, is to move this section
>> down below the line where GIT-BUILD-OPTIONS is sourced.  That
>> way, the value of $DIFF can be gotten from that file.
>>
>> I guess GIT_TEST_CMP and GIT_TEST_CMP_USE_COPIED_CONTEXT
>> should be written into GIT-BUILD-OPTIONS too.
> 
> I like that.  Something like this?

Looks good, will test.

-brandon

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Thomas Rast]

Junio C Hamano wrote:
> Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:
> 
> > I think what should be done instead, is to move this section
> > down below the line where GIT-BUILD-OPTIONS is sourced.  That
> > way, the value of $DIFF can be gotten from that file.
> >
> > I guess GIT_TEST_CMP and GIT_TEST_CMP_USE_COPIED_CONTEXT
> > should be written into GIT-BUILD-OPTIONS too.
> 
> I like that.  Something like this?

Fine by me, and tested.

Acked-by: Thomas Rast <trast@student.ethz.ch>

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH next] test-lib: set $DIFF to diff if it is unset

[Brandon Casey]

On 06/11/2010 11:26 AM, Brandon Casey wrote:
> On 06/11/2010 11:15 AM, Junio C Hamano wrote:
>> Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:
>>
>>> I think what should be done instead, is to move this section
>>> down below the line where GIT-BUILD-OPTIONS is sourced.  That
>>> way, the value of $DIFF can be gotten from that file.
>>>
>>> I guess GIT_TEST_CMP and GIT_TEST_CMP_USE_COPIED_CONTEXT
>>> should be written into GIT-BUILD-OPTIONS too.
>> I like that.  Something like this?
> 
> Looks good, will test.

Works for me.  It's kind of belated, I know.