[PATCH] t/perf: add "trash directory" to .gitignore

[PATCH] t/perf: add "trash directory" to .gitignore

[Ramkumar Ramachandra]

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/perf/.gitignore |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/t/perf/.gitignore b/t/perf/.gitignore
index 50f5cc1..0061cbc 100644
--- a/t/perf/.gitignore
+++ b/t/perf/.gitignore
@@ -1,2 +1,3 @@
-build/
-test-results/
+/build
+/test-results
+/trash directory*
-- 
1.7.8.1.362.g5d6df.dirty

[PATCH] t/test-lib: print pretty msg when git isn't built

[Ramkumar Ramachandra]

When tests were run without building git, the following error message
was displayed:

    .: 54: Can't open /path/to/git/source/t/../GIT-BUILD-OPTIONS

Change this to display a more user-friendly error message:

     error: you do not seem to have built git yet.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 t/t0000-basic.sh |   10 ----------
 t/test-lib.sh    |    6 ++++++
 2 files changed, 6 insertions(+), 10 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae6a3f0..08677df 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -18,16 +18,6 @@ swapping compression and hashing order, the person who is making the
 modification *should* take notice and update the test vectors here.
 '
 
-################################################################
-# It appears that people try to run tests without building...
-
-../git >/dev/null
-if test $? != 1
-then
-	echo >&2 'You do not seem to have built git yet.'
-	exit 1
-fi
-
 . ./test-lib.sh
 
 ################################################################
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f8e3733..c00452a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -51,6 +51,12 @@ then
 fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
+if ! test -r "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+then
+	echo 'error: you do not seem to have built git yet.' >&2
+	exit 1
+fi
+
 . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 export PERL_PATH SHELL_PATH
 
-- 
1.7.8.1.362.g5d6df.dirty

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> When tests were run without building git, the following error message
> was displayed:
>
>     .: 54: Can't open /path/to/git/source/t/../GIT-BUILD-OPTIONS
>
> Change this to display a more user-friendly error message:
>
>      error: you do not seem to have built git yet.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t0000-basic.sh |   10 ----------
>  t/test-lib.sh    |    6 ++++++
>  2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index ae6a3f0..08677df 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -18,16 +18,6 @@ swapping compression and hashing order, the person who is making the
>  modification *should* take notice and update the test vectors here.
>  '
>  
> -################################################################
> -# It appears that people try to run tests without building...
> -
> -../git >/dev/null
> -if test $? != 1
> -then
> -	echo >&2 'You do not seem to have built git yet.'
> -	exit 1
> -fi
> -
>  . ./test-lib.sh
>  
>  ################################################################
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f8e3733..c00452a 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -51,6 +51,12 @@ then
>  fi
>  GIT_BUILD_DIR="$TEST_DIRECTORY"/..
>  
> +if ! test -r "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> +then
> +	echo 'error: you do not seem to have built git yet.' >&2
> +	exit 1
> +fi
> +

Is this a sufficient replacement for what you removed from 0000?
Can the BUILD-OPTIONS file exist when your build of git failed?

>  . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
>  export PERL_PATH SHELL_PATH

Re: [PATCH] t/perf: add "trash directory" to .gitignore

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/perf/.gitignore |    5 +++--
>  1 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/t/perf/.gitignore b/t/perf/.gitignore
> index 50f5cc1..0061cbc 100644
> --- a/t/perf/.gitignore
> +++ b/t/perf/.gitignore
> @@ -1,2 +1,3 @@
> -build/
> -test-results/
> +/build
> +/test-results
> +/trash directory*

Thanks.  I _think_ you still want to make sure these are
directories, so instead of losing the trailing slash, you would want
to keep it and add a leading slash to anchor them to the t/perf
directory, i.e.

	/build/
        /test-results/
        /trash directory*/

Re: [PATCH] t/perf: add "trash directory" to .gitignore

[Ramkumar Ramachandra]

Hi,

Junio C Hamano wrote:
> Thanks.  I _think_ you still want to make sure these are
> directories, so instead of losing the trailing slash, you would want
> to keep it and add a leading slash to anchor them to the t/perf
> directory, i.e.
>
>         /build/
>         /test-results/
>         /trash directory*/

Right.  Thanks for taking care of this.

Ram

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Ramkumar Ramachandra]

Hi Junio,

Junio C Hamano wrote:
> Is this a sufficient replacement for what you removed from 0000?
> Can the BUILD-OPTIONS file exist when your build of git failed?

Oops, I didn't realize that BUILD-OPTIONS would be written when the
build fails.  How about something like this instead:

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ae6a3f0..08677df 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -18,16 +18,6 @@ swapping compression and hashing order, the person
who is making the
 modification *should* take notice and update the test vectors here.
 '

-################################################################
-# It appears that people try to run tests without building...
-
-../git >/dev/null
-if test $? != 1
-then
-	echo >&2 'You do not seem to have built git yet.'
-	exit 1
-fi
-
 . ./test-lib.sh

 ################################################################
	Modified   t/test-lib.sh
diff --git a/t/test-lib.sh b/t/test-lib.sh
index f8e3733..8cc7755 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -51,6 +51,12 @@ then
 fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..

+if ! test -x "$GIT_BUILD_DIR"/git || "$GIT_BUILD_DIR"/git && test $? != 1
+then
+	echo 'error: you do not seem to have built git yet.' >&2
+	exit 1
+fi
+
 . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 export PERL_PATH SHELL_PATH

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Hi Junio,
>
> Junio C Hamano wrote:
>> Is this a sufficient replacement for what you removed from 0000?
>> Can the BUILD-OPTIONS file exist when your build of git failed?
>
> Oops, I didn't realize that BUILD-OPTIONS would be written when the
> build fails.  How about something like this instead:

Yeah, but why change it so much?  Wouldn't writing

	"$GIT_BUILD_DIR/git" >/dev/null
	if test $? != 1
        then
		: You haven't built git!
	fi

just like the original in 0000 be sufficient??

> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index ae6a3f0..08677df 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -18,16 +18,6 @@ swapping compression and hashing order, the person
> who is making the
>  modification *should* take notice and update the test vectors here.
>  '
>
> -################################################################
> -# It appears that people try to run tests without building...
> -
> -../git >/dev/null
> -if test $? != 1
> -then
> -	echo >&2 'You do not seem to have built git yet.'
> -	exit 1
> -fi
> -
>  . ./test-lib.sh
>
>  ################################################################
> 	Modified   t/test-lib.sh
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f8e3733..8cc7755 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -51,6 +51,12 @@ then
>  fi
>  GIT_BUILD_DIR="$TEST_DIRECTORY"/..
>
> +if ! test -x "$GIT_BUILD_DIR"/git || "$GIT_BUILD_DIR"/git && test $? != 1
> +then
> +	echo 'error: you do not seem to have built git yet.' >&2
> +	exit 1
> +fi
> +
>  . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
>  export PERL_PATH SHELL_PATH

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Ramkumar Ramachandra]

Hi,

Junio C Hamano wrote:
> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Hi Junio,
>>
>> Junio C Hamano wrote:
>>> Is this a sufficient replacement for what you removed from 0000?
>>> Can the BUILD-OPTIONS file exist when your build of git failed?
>>
>> Oops, I didn't realize that BUILD-OPTIONS would be written when the
>> build fails.  How about something like this instead:
>
> Yeah, but why change it so much?  Wouldn't writing
>
>         "$GIT_BUILD_DIR/git" >/dev/null
>         if test $? != 1
>         then
>                 : You haven't built git!
>         fi
>
> just like the original in 0000 be sufficient??

Because that emits an ugly
./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found

Ram

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Hi,
>
> Junio C Hamano wrote:
>> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>>
>>> Hi Junio,
>>>
>>> Junio C Hamano wrote:
>>>> Is this a sufficient replacement for what you removed from 0000?
>>>> Can the BUILD-OPTIONS file exist when your build of git failed?
>>>
>>> Oops, I didn't realize that BUILD-OPTIONS would be written when the
>>> build fails.  How about something like this instead:
>>
>> Yeah, but why change it so much?  Wouldn't writing
>>
>>         "$GIT_BUILD_DIR/git" >/dev/null
>>         if test $? != 1
>>         then
>>                 : You haven't built git!
>>         fi
>>
>> just like the original in 0000 be sufficient??
>
> Because that emits an ugly
> ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found

Don't you deserve it? ;-)

The full message would read

    ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found
    error: you do not seem to have built git yet.

which looks perfectly sensible to me.  It makes it clear where on
the filesystem the test script expects your "git", which is an added
benefit.

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Ramkumar Ramachandra]

Hi Junio,

Junio C Hamano wrote:
>> Junio C Hamano wrote:
>>> Yeah, but why change it so much?  Wouldn't writing
>>>
>>>         "$GIT_BUILD_DIR/git" >/dev/null
>>>         if test $? != 1
>>>         then
>>>                 : You haven't built git!
>>>         fi
>>>
>>> just like the original in 0000 be sufficient??
>>
>> Because that emits an ugly
>> ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found
>
> Don't you deserve it? ;-)
>
> The full message would read
>
>     ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found
>     error: you do not seem to have built git yet.
>
> which looks perfectly sensible to me.  It makes it clear where on
> the filesystem the test script expects your "git", which is an added
> benefit.

Fair enough- use your version then.  Let me know if you want me to
submit a revised patch.

Ram

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> When tests were run without building git, the following error message
> was displayed:
>
>     .: 54: Can't open /path/to/git/source/t/../GIT-BUILD-OPTIONS

Does the test stop due to this error, or it just goes on and hit
another error?  I am guessing that it is the latter, and if that is
the case, a more important change to describe here is

> Change this to display a more user-friendly error message:
>
>      error: you do not seem to have built git yet.

... "and stop the execution."

> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  t/t0000-basic.sh |   10 ----------
>  t/test-lib.sh    |    6 ++++++
>  2 files changed, 6 insertions(+), 10 deletions(-)
>
> diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
> index ae6a3f0..08677df 100755
> --- a/t/t0000-basic.sh
> +++ b/t/t0000-basic.sh
> @@ -18,16 +18,6 @@ swapping compression and hashing order, the person who is making the
>  modification *should* take notice and update the test vectors here.
>  '
>  
> -################################################################
> -# It appears that people try to run tests without building...
> -
> -../git >/dev/null
> -if test $? != 1
> -then
> -	echo >&2 'You do not seem to have built git yet.'
> -	exit 1
> -fi
> -
>  . ./test-lib.sh
>  
>  ################################################################
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index f8e3733..c00452a 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -51,6 +51,12 @@ then
>  fi
>  GIT_BUILD_DIR="$TEST_DIRECTORY"/..
>  
> +if ! test -r "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
> +then
> +	echo 'error: you do not seem to have built git yet.' >&2
> +	exit 1
> +fi
> +
>  . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
>  export PERL_PATH SHELL_PATH

Re: [PATCH] t/test-lib: print pretty msg when git isn't built

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

>>> Because that emits an ugly
>>> ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found
>>
>> Don't you deserve it? ;-)
>>
>> The full message would read
>>
>>     ./test-lib.sh: 54: /home/artagnon/src/git/t/../git: not found
>>     error: you do not seem to have built git yet.
>>
>> which looks perfectly sensible to me.  It makes it clear where on
>> the filesystem the test script expects your "git", which is an added
>> benefit.
>
> Fair enough- use your version then.  Let me know if you want me to
> submit a revised patch.

OK, let's do this.

-- >8 --
From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Mon, 17 Sep 2012 22:36:19 +0530
Subject: [PATCH] t/test-lib: print pretty msg when git isn't built

When tests were run without building git, they stopped with:

    .: 54: Can't open /path/to/git/source/t/../GIT-BUILD-OPTIONS

Move the check that makes sure that git has already been built from
t0000 to test-lib, so that any test will do so before it runs.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t0000-basic.sh | 10 ----------
 t/test-lib.sh    |  7 +++++++
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/t/t0000-basic.sh b/t/t0000-basic.sh
index ccb5435..741b6b6 100755
--- a/t/t0000-basic.sh
+++ b/t/t0000-basic.sh
@@ -18,16 +18,6 @@ swapping compression and hashing order, the person who is making the
 modification *should* take notice and update the test vectors here.
 '
 
-################################################################
-# It appears that people try to run tests without building...
-
-../git >/dev/null
-if test $? != 1
-then
-	echo >&2 'You do not seem to have built git yet.'
-	exit 1
-fi
-
 . ./test-lib.sh
 
 ################################################################
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 78c4286..61930fb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -51,6 +51,13 @@ then
 fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
+"$GIT_BUILD_DIR/git" >/dev/null
+if test $? != 1
+then
+	echo >&2 'error: you do not seem to have built git yet.'
+	exit 1
+fi
+
 . "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
 export PERL_PATH SHELL_PATH
 
-- 
1.7.12.563.g54818e4