threads / patch / 24064

patchtest-lib: set $DIFF to diff if it is unset

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

## tl;dr

7 messages between Jun 10, 2010 and Jun 15, 2010. Diffs are folded; open one to read it.

replies: 6people: 4as markdown or json

Thomas Rast· Jun 10, 2010, 09:39 UTC · lore

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(-)
Show changes to t/test-lib.sh +1 −0
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
Ævar Arnfjörð Bjarmason· Jun 10, 2010, 09:46 UTC · re: Thomas Rast · lore

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

On Thu, Jun 10, 2010 at 09:39, Thomas Rast <trast@student.ethz.ch> wrote:
Show 13 quoted lines
> 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.

Brandon Casey· Jun 10, 2010, 17:23 UTC · re: Thomas Rast · lore

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

On 06/10/2010 04:39 AM, Thomas Rast wrote:
Show 29 quoted lines
> 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
Junio C Hamano· Jun 11, 2010, 16:15 UTC · re: Brandon Casey · lore

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

Brandon Casey <brandon.casey.ctr@nrlssc.navy.mil> writes:
Show 6 quoted lines
> 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(-)
Show changes to 2 files +16 −10

Makefile, t/test-lib.sh

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 || {
Brandon Casey· Jun 11, 2010, 16:26 UTC · re: Junio C Hamano · lore

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

On 06/11/2010 11:15 AM, Junio C Hamano wrote:
Show 10 quoted lines
> 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
Brandon Casey· Jun 15, 2010, 15:19 UTC · re: Brandon Casey · lore

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

On 06/11/2010 11:26 AM, Brandon Casey wrote:
Show 12 quoted lines
> 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.
Thomas Rast· Jun 11, 2010, 16:34 UTC · re: Junio C Hamano · lore

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

Junio C Hamano wrote:
Show 10 quoted lines
> 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

← back to recent threads