Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-25 Thread Ramkumar Ramachandra
Hi Junio, Junio C Hamano wrote: Ramkumar Ramachandra artag...@gmail.com writes: Here's a patch. -- 8 -- From: Ramkumar Ramachandra artag...@gmail.com Date: Sat, 22 Sep 2012 10:25:10 +0530 Subject: [PATCH] test-lib: do not trust $SHELL Do not trust $SHELL to be a bourne-compatible shell.

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-22 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Here's a patch. -- 8 -- From: Ramkumar Ramachandra artag...@gmail.com Date: Sat, 22 Sep 2012 10:25:10 +0530 Subject: [PATCH] test-lib: do not trust $SHELL Do not trust $SHELL to be a bourne-compatible shell. Instead, use the Makefile

[PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Ramkumar Ramachandra
Replace $SHELL with an explicit `/bin/sh`, as some shells do not support all the features used in the script. For example, ZSH does not respect IFS, which is used in line 478. Signed-off-by: Ramkumar Ramachandra artag...@gmail.com --- t/test-lib.sh |2 +- 1 files changed, 1 insertions(+), 1

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Jeff King
On Sat, Sep 22, 2012 at 02:22:46AM +0530, Ramkumar Ramachandra wrote: Replace $SHELL with an explicit `/bin/sh`, as some shells do not support all the features used in the script. For example, ZSH does not respect IFS, which is used in line 478. I don't think that is the right thing to do.

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Andreas Schwab
Jeff King p...@peff.net writes: A bigger question is: why are you setting SHELL=zsh in the first place? SHELL is set to the login shell by default. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Jeff King
On Sat, Sep 22, 2012 at 02:37:38AM +0530, Ramkumar Ramachandra wrote: I don't think that is the right thing to do. The point of SHELL is to point at a bourne-compatible shell. On some systems, the main reason to set it is that /bin/sh is _broken_, and we are trying to avoid it. But

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Jeff King
On Fri, Sep 21, 2012 at 11:08:34PM +0200, Andreas Schwab wrote: Jeff King p...@peff.net writes: A bigger question is: why are you setting SHELL=zsh in the first place? SHELL is set to the login shell by default. Yeah, sorry, I was thinking this was coming from our $SHELL_PATH Makefile

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Ramkumar Ramachandra
Hi Peff, Jeff King wrote: On Sat, Sep 22, 2012 at 02:37:38AM +0530, Ramkumar Ramachandra wrote: I don't think that is the right thing to do. The point of SHELL is to point at a bourne-compatible shell. On some systems, the main reason to set it is that /bin/sh is _broken_, and we are

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Andreas Schwab
Ramkumar Ramachandra artag...@gmail.com writes: My SHELL_PATH is not set, and I can see SHELL_PATH ?= $(SHELL) in the Makefile. Which shell is it supposed to point to? Inside a makefile the variable SHELL is special in that it is never imported from the environment. If not set it defaults to

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Hi Peff, Jeff King wrote: On Sat, Sep 22, 2012 at 02:37:38AM +0530, Ramkumar Ramachandra wrote: I don't think that is the right thing to do. The point of SHELL is to point at a bourne-compatible shell. On some systems, the main reason to

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Jeff King
On Sat, Sep 22, 2012 at 03:04:50AM +0530, Ramkumar Ramachandra wrote: No, the #!-header is only information. When you run make test we actually invoke the shell ourselves using $SHELL_PATH. My SHELL_PATH is not set, and I can see SHELL_PATH ?= $(SHELL) in the Makefile. Which shell is it

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Ramkumar Ramachandra
Hi Junio, Junio C Hamano wrote: The reference to ${SHELL-/bin/sh} in the test need to be updated to SHELL_PATH as Peff suggested in the other subthread. For that, the entire block needs to be moved down to come after `. GIT_BUILD_DIR=$TEST_DIRECTORY/..`. Is this okay? diff --git

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Junio C Hamano
Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: The reference to ${SHELL-/bin/sh} in the test need to be updated to SHELL_PATH as Peff suggested in the other subthread. For that, the entire block needs to be moved down to come after `.

Re: [PATCH] t/test-lib.sh: do not trust $SHELL

2012-09-21 Thread Ramkumar Ramachandra
Hi again, On Sat, Sep 22, 2012 at 10:22 AM, Junio C Hamano gits...@pobox.com wrote: Ramkumar Ramachandra artag...@gmail.com writes: Junio C Hamano wrote: The reference to ${SHELL-/bin/sh} in the test need to be updated to SHELL_PATH as Peff suggested in the other subthread. For that, the