Git development
 help / color / mirror / Atom feed
* [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability
@ 2008-03-26 17:34 Gerrit Pape
  2008-03-26 17:55 ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: Gerrit Pape @ 2008-03-26 17:34 UTC (permalink / raw)
  To: git, Junio C Hamano

From: Frank Lichtenheld <djpig@debian.org>

This actually sounds like a bug in cvsps, which requires an existing
home directory when asked for the usage through -h

 $ HOME=/nonexistent cvsps -h
 Cannot create the cvsps directory '.cvsps': No such file or directory

This made t9600 think that cvsps is not available if HOME did not exist,
causing the tests to be skipped

 $ HOME=/nonexistent sh t9600-cvsimport.sh
 * skipping cvsimport tests, cvsps not found
 * passed all 0 test(s)

Now t9600 sets HOME to the current working directory before checking for
the availability of the cvsps program.

This issue has been discovered by Marco Rodrigues, and fixed by Frank
Lichtenheld through
 http://bugs.debian.org/471969

Signed-off-by: Gerrit Pape <pape@smarden.org>
---
 t/t9600-cvsimport.sh |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/t/t9600-cvsimport.sh b/t/t9600-cvsimport.sh
index 7706430..00a74ee 100755
--- a/t/t9600-cvsimport.sh
+++ b/t/t9600-cvsimport.sh
@@ -3,6 +3,12 @@
 test_description='git-cvsimport basic tests'
 . ./test-lib.sh
 
+CVSROOT=$(pwd)/cvsroot
+export CVSROOT
+# for clean cvsps cache
+HOME=$(pwd)
+export HOME
+
 if ! type cvs >/dev/null 2>&1
 then
 	say 'skipping cvsimport tests, cvs not found'
@@ -26,12 +32,6 @@ case "$cvsps_version" in
 	;;
 esac
 
-CVSROOT=$(pwd)/cvsroot
-export CVSROOT
-# for clean cvsps cache
-HOME=$(pwd)
-export HOME
-
 test_expect_success 'setup cvsroot' 'cvs init'
 
 test_expect_success 'setup a cvs module' '
-- 
1.5.4.4

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability
  2008-03-26 17:34 [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability Gerrit Pape
@ 2008-03-26 17:55 ` Junio C Hamano
  2008-03-27  3:28   ` Jeff King
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2008-03-26 17:55 UTC (permalink / raw)
  To: Gerrit Pape; +Cc: git

Gerrit Pape <pape@smarden.org> writes:

> From: Frank Lichtenheld <djpig@debian.org>
>
> This actually sounds like a bug in cvsps, which requires an existing
> home directory when asked for the usage through -h
>
>  $ HOME=/nonexistent cvsps -h
>  Cannot create the cvsps directory '.cvsps': No such file or directory

Thanks.

In the longer run, we probably would want to do something similar for all
tests to ensure a stable testing environment, don't we?  $HOME/.gitconfig
may affect the way tested programs behave otherwise.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability
  2008-03-26 17:55 ` Junio C Hamano
@ 2008-03-27  3:28   ` Jeff King
  2008-03-27 11:07     ` Johannes Schindelin
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff King @ 2008-03-27  3:28 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Gerrit Pape, git

On Wed, Mar 26, 2008 at 10:55:21AM -0700, Junio C Hamano wrote:

> In the longer run, we probably would want to do something similar for all
> tests to ensure a stable testing environment, don't we?  $HOME/.gitconfig
> may affect the way tested programs behave otherwise.

We used to, as per a patch from Gerrit long ago:

  2eb10ac... Set $HOME for selftests

but it was removed during Dscho's config fix:

  8565d2d... Make tests independent of global config files

which used GIT_CONFIG instead. But that had some problems, which I fixed
in:

  8bfa6bd... fix config reading in tests

and that patch explicitly suppresses lookup in the ~/.gitconfig and
/etc/gitconfig files (implemented by ab88c363).

So I think all is correct with .gitconfig files. However, I still think
setting HOME is a good idea, because it eliminates one more variable in
test runs. In particular, I know I have gotten cvsps into a funny state
that was resolved by removing the ~/.cvsps cache. So it's probably worth
doing as a preventative measure, even if it doesn't fix a specific bug.

-Peff

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability
  2008-03-27  3:28   ` Jeff King
@ 2008-03-27 11:07     ` Johannes Schindelin
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Schindelin @ 2008-03-27 11:07 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Gerrit Pape, git

Hi,

On Wed, 26 Mar 2008, Jeff King wrote:

> However, I still think setting HOME is a good idea, because it 
> eliminates one more variable in test runs. In particular, I know I have 
> gotten cvsps into a funny state that was resolved by removing the 
> ~/.cvsps cache. So it's probably worth doing as a preventative measure, 
> even if it doesn't fix a specific bug.

Yes, I did not think about 3rd party configurations when I removed that 
HOME setting.  Bad me.

Ciao,
Dscho

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2008-03-27 11:08 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-03-26 17:34 [PATCH] t9600-cvsimport.sh: set HOME before checking for cvsps availability Gerrit Pape
2008-03-26 17:55 ` Junio C Hamano
2008-03-27  3:28   ` Jeff King
2008-03-27 11:07     ` Johannes Schindelin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox