Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug

Jeff King [off-list ref] writes:

quoted

                2)	echo "#!$2" ;;
		*)	BUG ;;
                esac >"$1" &&
                cat >>"$1" &&
                chmod +x "$1"
	}

Nice. I was going to suggest a wrapper like "write_sh_script" so you
didn't have to spell out $SHELL_PATH, but I think the auto-detection
makes sense (and falling back to shell makes even more sense, as that
covers 99% of the cases anyway).

Let's not over-engineer this and stick to the simple-stupid-sufficient.

Something like this?

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

diff --git a/t/test-lib.sh b/t/test-lib.sh
index bdd9513..1b9c461 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -379,6 +379,15 @@ test_config () {
 	git config "$@"
 }
 
+# Prepare a script to be used in the test
+write_script () {
+	{
+		echo "#!${2-"$SHELL_PATH"}"
+		cat
+	} >"$1" &&
+	chmod +x "$1"
+}
+
 # Use test_set_prereq to tell that a particular prerequisite is available.
 # The prerequisite can later be checked for in two ways:
 #

On Fri, Feb 03, 2012 at 02:45:25PM -0800, Junio C Hamano wrote:

Let's not over-engineer this and stick to the simple-stupid-sufficient.

Fair enough.

Something like this?
[...]
+# Prepare a script to be used in the test
+write_script () {
+	{
+		echo "#!${2-"$SHELL_PATH"}"
+		cat
+	} >"$1" &&
+	chmod +x "$1"
+}

Looks good to me (it probably doesn't matter, but you may want to
connect the echo and cat via &&).

-Peff

On Fri, Feb 03, 2012 at 02:45:25PM -0800, Junio C Hamano wrote:

quoted

Nice. I was going to suggest a wrapper like "write_sh_script" so you
didn't have to spell out $SHELL_PATH, but I think the auto-detection
makes sense (and falling back to shell makes even more sense, as that
covers 99% of the cases anyway).

Let's not over-engineer this and stick to the simple-stupid-sufficient.

Something like this?

Here it is as patches with commit messages.  I don't think it's worth
doing a mechanical conversion of the whole test suite to write_script.

  [1/2]: tests: add write_script helper function
  [2/2]: t0300: use write_script helper

-Peff

From: Junio C Hamano <redacted>

Many of the scripts in the test suite write small helper
shell scripts to disk. It's best if these shell scripts
start with "#!$SHELL_PATH" rather than "#!/bin/sh", because
/bin/sh on some platforms is too buggy to be used.

However, it can be cumbersome to expand $SHELL_PATH, because
the usual recipe for writing a script is:

	cat >foo.sh <<-\EOF
	#!/bin/sh
	echo my arguments are "$@"
	EOF

To expand $SHELL_PATH, you have to either interpolate the
here-doc (which would require quoting "\$@"), or split the
creation into two commands (interpolating the $SHELL_PATH
line, but not the rest of the script). Let's provide a
helper function that makes that less syntactically painful.

While we're at it, this helper can also take care of the
"chmod +x" that typically comes after the creation of such a
script, saving the caller a line.

Signed-off-by: Jeff King <redacted>
---
I suspect you already have this in your repo, but maybe the commit
message is useful.

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

diff --git a/t/test-lib.sh b/t/test-lib.sh
index b22bee7..254849e 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh

@@ -400,6 +400,14 @@ test_config_global () {
 	git config --global "$@"
 }
 
+write_script () {
+	{
+		echo "#!${2-"$SHELL_PATH"}" &&
+		cat
+	} >"$1" &&
+	chmod +x "$1"
+}
+
 # Use test_set_prereq to tell that a particular prerequisite is available.
 # The prerequisite can later be checked for in two ways:
 #

-- 
1.7.9.rc1.28.gf4be5

t0300 creates some helper shell scripts, and marks them with
"!/bin/sh". Even though the scripts are fairly simple, they
can fail on broken shells (specifically, Solaris /bin/sh
will persist a temporary assignment to IFS in a "read"
command).

Rather than work around the problem for Solaris /bin/sh,
using write_script will make sure we point to a known-good
shell that the user has given us.

Signed-off-by: Jeff King <redacted>
---
This works fine on my Linux box, but just to sanity check that I didn't
screw anything up in the whopping 5 lines of changes, can you confirm
this fixes the issue for you, Ben?

 t/t0300-credentials.sh |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 885af8f..0b46248 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh

@@ -14,14 +14,13 @@ test_expect_success 'setup helper scripts' '
 	done
 	EOF
 
-	cat >git-credential-useless <<-\EOF &&
+	write_script git-credential-useless <<-\EOF &&
 	#!/bin/sh
 	. ./dump
 	exit 0
 	EOF
-	chmod +x git-credential-useless &&
 
-	cat >git-credential-verbatim <<-\EOF &&
+	write_script git-credential-verbatim <<-\EOF &&
 	#!/bin/sh
 	user=$1; shift
 	pass=$1; shift

@@ -29,7 +28,6 @@ test_expect_success 'setup helper scripts' '
 	test -z "$user" || echo username=$user
 	test -z "$pass" || echo password=$pass
 	EOF
-	chmod +x git-credential-verbatim &&
 
 	PATH="$PWD:$PATH"
 '

-- 
1.7.9.rc1.28.gf4be5