# [PATCH] t0300-credentials: Word around a solaris /bin/sh bug 22 messages from 2012-02-02 to 2012-02-04. Participants: Ben Walton, Frans Klaver, Jeff King, Jonathan Nieder, Matthieu Moy, Junio C Hamano. Thread: https://gitlist.dev/t/29527 ## Ben Walton, 2012-02-02 19:32 Subject: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <1328211135-25217-1-git-send-email-bwalton@artsci.utoronto.ca> URL: https://gitlist.dev/e/1328211135-25217-1-git-send-email-bwalton%40artsci.utoronto.ca ``` Solaris' /bin/sh was making the IFS setting permanent instead of temporary when using it to slurp in credentials in the generated 'dump' script of the 'setup helper scripts' test in t0300-credentials. The stderr file that was being compared to expected-stderr contained the following stray line from the credential helper run: warning: invalid credential line: username foo To avoid this bug, capture the original IFS and force it to be reset after its use is no longer required. For now, this is lighter weight than altering which shell these scripts use as their shebang. Signed-off-by: Ben Walton --- t/t0300-credentials.sh | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index 885af8f..1be3fe2 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -8,10 +8,12 @@ test_expect_success 'setup helper scripts' ' cat >dump <<-\EOF && whoami=`echo $0 | sed s/.*git-credential-//` echo >&2 "$whoami: $*" + OIFS=$IFS while IFS== read key value; do echo >&2 "$whoami: $key=$value" eval "$key=$value" done + IFS=$OIFS EOF cat >git-credential-useless <<-\EOF && -- 1.7.8.3 ``` ## Frans Klaver, 2012-02-02 19:44 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: URL: https://gitlist.dev/e/op.v82g3ura0aolir%40keputer In-Reply-To: <1328211135-25217-1-git-send-email-bwalton@artsci.utoronto.ca> ``` Wor_k_ around ... On Thu, 02 Feb 2012 20:32:15 +0100, Ben Walton wrote: > Solaris' /bin/sh was making the IFS setting permanent instead of > temporary when using it to slurp in credentials in the generated > 'dump' script of the 'setup helper scripts' test in t0300-credentials. > > The stderr file that was being compared to expected-stderr contained the > following stray line from the credential helper run: > > warning: invalid credential line: username foo > > To avoid this bug, capture the original IFS and force it to be reset > after its use is no longer required. For now, this is lighter weight > than altering which shell these scripts use as their shebang. > > Signed-off-by: Ben Walton > --- > t/t0300-credentials.sh | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh > index 885af8f..1be3fe2 100755 > --- a/t/t0300-credentials.sh > +++ b/t/t0300-credentials.sh > @@ -8,10 +8,12 @@ test_expect_success 'setup helper scripts' ' > cat >dump <<-\EOF && > whoami=`echo $0 | sed s/.*git-credential-//` > echo >&2 "$whoami: $*" > + OIFS=$IFS > while IFS== read key value; do > echo >&2 "$whoami: $key=$value" > eval "$key=$value" > done > + IFS=$OIFS > EOF > cat >git-credential-useless <<-\EOF && -- Using Opera's revolutionary e-mail client: http://www.opera.com/mail/ ``` ## Ben Walton, 2012-02-02 19:48 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <1328212038-sup-9896@pinkfloyd.chass.utoronto.ca> URL: https://gitlist.dev/e/1328212038-sup-9896%40pinkfloyd.chass.utoronto.ca In-Reply-To: ``` Excerpts from Frans Klaver's message of Thu Feb 02 14:44:08 -0500 2012: > Wor_k_ around ... *face*palm* Thanks for catching that. Junio, can you make that tweak if the patch is ok or would you prefer a new mail? Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 ``` ## Jeff King, 2012-02-02 20:02 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120202200240.GC9246@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120202200240.GC9246%40sigill.intra.peff.net In-Reply-To: <1328211135-25217-1-git-send-email-bwalton@artsci.utoronto.ca> ``` On Thu, Feb 02, 2012 at 02:32:15PM -0500, Ben Walton wrote: > Solaris' /bin/sh was making the IFS setting permanent instead of > temporary when using it to slurp in credentials in the generated > 'dump' script of the 'setup helper scripts' test in t0300-credentials. Hmm. Presumably you are setting SHELL_PATH, as Solaris /bin/sh would be useless for running the rest of the tests. Usually scripts inside the tests use #!$SHELL_PATH, but I often don't bother if it's a simple "even Solaris /bin/sh could run this" script. But in this case I either underestimated the complexity of my script or overestimated the quality of the Solaris /bin/sh. I wonder if a better solution is to use a known-good shell instead of trying to work around problems in a bogus shell. Does the patch below fix it for you? diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index 885af8f..edf6547 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -14,15 +14,15 @@ test_expect_success 'setup helper scripts' ' done EOF - cat >git-credential-useless <<-\EOF && - #!/bin/sh + cat >git-credential-useless <<-EOF && + #!$SHELL_PATH . ./dump exit 0 EOF chmod +x git-credential-useless && - cat >git-credential-verbatim <<-\EOF && - #!/bin/sh + echo "#!$SHELL_PATH" >git-credential-verbatim && + cat >>git-credential-verbatim <<-\EOF && user=$1; shift pass=$1; shift . ./dump ``` ## Jonathan Nieder, 2012-02-02 20:16 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120202201629.GA20200@burratino> URL: https://gitlist.dev/e/20120202201629.GA20200%40burratino In-Reply-To: <1328211135-25217-1-git-send-email-bwalton@artsci.utoronto.ca> ``` Ben Walton wrote: > --- a/t/t0300-credentials.sh > +++ b/t/t0300-credentials.sh > @@ -8,10 +8,12 @@ test_expect_success 'setup helper scripts' ' > cat >dump <<-\EOF && > whoami=`echo $0 | sed s/.*git-credential-//` > echo >&2 "$whoami: $*" > + OIFS=$IFS > while IFS== read key value; do > echo >&2 "$whoami: $key=$value" > eval "$key=$value" > done > + IFS=$OIFS Oh, good catch. Technically "read" is not a special builtin so POSIX shells are not supposed to do this (and Jeff's patch definitely looks right), but in any case temporary variable settings while running a builtin are close enough to the assignment-during-special-builtin-or-function case to make me shiver a little. ;-) Would something like ( IFS== while read key value do ... done ) make sense? ``` ## Matthieu Moy, 2012-02-02 20:43 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: URL: https://gitlist.dev/e/vpq62fp3r15.fsf%40bauges.imag.fr In-Reply-To: <20120202201629.GA20200@burratino> ``` Jonathan Nieder writes: > Would something like > > ( > IFS== > while read key value > do > ... > done > ) > > make sense? I don't think so since the "..." contains eval "$key=$value" -- Matthieu Moy http://www-verimag.imag.fr/~moy/ ``` ## Jonathan Nieder, 2012-02-02 21:11 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120202211146.GC19520@burratino> URL: https://gitlist.dev/e/20120202211146.GC19520%40burratino In-Reply-To: ``` Matthieu Moy wrote: > Jonathan Nieder writes: >> ( >> IFS== >> while read key value >> do >> ... >> done >> ) [...] > I don't think so since the "..." contains > > eval "$key=$value" Oh, whoops. Thanks for noticing. Here's an updated patch, for amusement value. No functional change intended. I don't think it's actually worth applying unless people actively working on this file find the result easier to work with. -- >8 -- Subject: t0300 (credentials): shell scripting style cleanups As Ben noticed, the helper used by this test script assigns a temporary value to IFS while calling the "read" builtin, which in ancient shells causes the value to leak into the environment and affect later code in the same script. Explicitly save and restore IFS to avoid rekindling old memories. While at it, put the "do" associated to a "while" statement on its own line to match the house style and define helper scripts in the test data section above all test assertions so the "setup" test itself is less cluttered and we can worry a little less about quoting issues. Inspired-by: Ben Walton Signed-off-by: Jonathan Nieder --- t/t0300-credentials.sh | 36 ++++++++++++++++++++---------------- 1 files changed, 20 insertions(+), 16 deletions(-) diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh index edf65478..780d5dcb 100755 --- a/t/t0300-credentials.sh +++ b/t/t0300-credentials.sh @@ -4,33 +4,37 @@ test_description='basic credential helper tests' . ./test-lib.sh . "$TEST_DIRECTORY"/lib-credential.sh -test_expect_success 'setup helper scripts' ' - cat >dump <<-\EOF && +cat >dump <<-\EOF whoami=`echo $0 | sed s/.*git-credential-//` echo >&2 "$whoami: $*" - while IFS== read key value; do + save_IFS=$IFS + IFS== + while read key value + do echo >&2 "$whoami: $key=$value" eval "$key=$value" done - EOF + IFS=$save_IFS +EOF - cat >git-credential-useless <<-EOF && +cat >git-credential-useless <<-EOF #!$SHELL_PATH . ./dump exit 0 - EOF +EOF + +cat >git-credential-verbatim <<-EOF + #!$SHELL_PATH + user=\$1; shift + pass=\$1; shift + . ./dump + test -z "\$user" || echo username=\$user + test -z "\$pass" || echo password=\$pass +EOF + +test_expect_success setup ' chmod +x git-credential-useless && - - echo "#!$SHELL_PATH" >git-credential-verbatim && - cat >>git-credential-verbatim <<-\EOF && - user=$1; shift - pass=$1; shift - . ./dump - test -z "$user" || echo username=$user - test -z "$pass" || echo password=$pass - EOF chmod +x git-credential-verbatim && - PATH="$PWD:$PATH" ' -- 1.7.9 ``` ## Junio C Hamano, 2012-02-03 01:02 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <7vr4ycu3ty.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr4ycu3ty.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120202200240.GC9246@sigill.intra.peff.net> ``` Jeff King writes: > I wonder if a better solution is to use a known-good shell instead of > trying to work around problems in a bogus shell. Yeah, I think that is a better approach. What prevents us from doing 's|^#! */bin/sh|$#$SHELL_PATH|' on everything in t/ directory (I am not suggesting to do this. I just want to know if there is a reason we want hardcoded "#!/bin/sh" for some instances). ``` ## Jeff King, 2012-02-03 12:06 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120203120657.GB31441@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120203120657.GB31441%40sigill.intra.peff.net In-Reply-To: <7vr4ycu3ty.fsf@alter.siamese.dyndns.org> ``` On Thu, Feb 02, 2012 at 05:02:17PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > I wonder if a better solution is to use a known-good shell instead of > > trying to work around problems in a bogus shell. > > Yeah, I think that is a better approach. > > What prevents us from doing 's|^#! */bin/sh|$#$SHELL_PATH|' on everything > in t/ directory (I am not suggesting to do this. I just want to know if > there is a reason we want hardcoded "#!/bin/sh" for some instances). The quoting is more annoying, because you usually don't want interpolation on the rest of the lines of your embedded script. So: cat >foo.sh <<\EOF #!/bin/sh echo my arguments are "$@" EOF cannot have the mechanical replace you mentioned above. It would need: cat >foo.sh <foo.sh When I have hard-coded "#!/bin/sh", my thinking is usually "this is less cumbersome to type and to read, and this script-let is so small that even Solaris will get it right". -Peff ``` ## Ben Walton, 2012-02-03 13:45 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <1328276601-sup-5887@pinkfloyd.chass.utoronto.ca> URL: https://gitlist.dev/e/1328276601-sup-5887%40pinkfloyd.chass.utoronto.ca In-Reply-To: <20120203120657.GB31441@sigill.intra.peff.net> ``` Excerpts from Jeff King's message of Fri Feb 03 07:06:57 -0500 2012: > When I have hard-coded "#!/bin/sh", my thinking is usually "this is > less cumbersome to type and to read, and this script-let is so small > that even Solaris will get it right". This is why I opted to stick with /bin/sh and just avoid the damage. Overall, using a sane shell is a better option...It is harder to read though. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 ``` ## Junio C Hamano, 2012-02-03 20:32 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <7v7h03odyo.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v7h03odyo.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120203120657.GB31441@sigill.intra.peff.net> ``` Jeff King writes: > cat >foo.sh <<\EOF > #!/bin/sh > echo my arguments are "$@" > EOF > > cannot have the mechanical replace you mentioned above. It would need: > > cat >foo.sh < #!$SHELL_PATH > echo my arguments are "\$@" > EOF > > or: > > { > echo "#!$SHELL_PATH" && > cat < echo my arguments are "$@" > EOF > } >foo.sh > > When I have hard-coded "#!/bin/sh", my thinking is usually "this is less > cumbersome to type and to read, and this script-let is so small that > even Solaris will get it right". I am toying with the pros-and-cons of write_script () { echo "#!$1" shift cat } so that the above can become write_script "$SHELL_PATH" >foo.sh <<-EOF echo my arguments are "\$@" EOF without requiring the brain-cycle to waste on the "Is this simple enough for even Solaris to grok?" guess game. This should also be reusable for other stuff like $PERL_PATH, I would think. ``` ## Jeff King, 2012-02-03 21:26 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120203212604.GA1890@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120203212604.GA1890%40sigill.intra.peff.net In-Reply-To: <7v7h03odyo.fsf@alter.siamese.dyndns.org> ``` On Fri, Feb 03, 2012 at 12:32:15PM -0800, Junio C Hamano wrote: > I am toying with the pros-and-cons of > > write_script () { > echo "#!$1" > shift > cat > } > > so that the above can become > > write_script "$SHELL_PATH" >foo.sh <<-EOF > echo my arguments are "\$@" > EOF > > without requiring the brain-cycle to waste on the "Is this simple enough > for even Solaris to grok?" guess game. This should also be reusable for > other stuff like $PERL_PATH, I would think. I like it. Even better would be: write_script() { echo "#!$2" >"$1" && cat >>"$1" && chmod +x "$1" } write_script foo.sh "$SHELL_PATH" <<-\EOF echo my arguments are "$@" EOF -Peff ``` ## Junio C Hamano, 2012-02-03 21:50 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <7vr4ybmvrq.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr4ybmvrq.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120203212604.GA1890@sigill.intra.peff.net> ``` Jeff King writes: >> without requiring the brain-cycle to waste on the "Is this simple enough >> for even Solaris to grok?" guess game. This should also be reusable for >> other stuff like $PERL_PATH, I would think. > > I like it. Even better would be: > > write_script() { > echo "#!$2" >"$1" && > cat >>"$1" && > chmod +x "$1" > } > > write_script foo.sh "$SHELL_PATH" <<-\EOF > echo my arguments are "$@" > EOF I first thought that the order of parameters were unusual, but with that order, you could even go something fancier like: write_script () { case "$#" in 1) case "$1" in *.perl | *.pl) echo "#!$PERL_PATH" ;; *) echo "#!$SHELL_PATH" ;; esac 2) echo "#!$2" ;; *) BUG ;; esac >"$1" && cat >>"$1" && chmod +x "$1" } write_script foo.sh write_script bar.perl write_script pre-receive /no/frobnication/today The tongue-in-cheek comment aside, I think ${2-"$SHELL_PATH"} or some form of fallback would be a good idea in any case, as 99% of the time what we write in the test scripts is a shell script. Also "chmod +x" is a very good idea. ``` ## Jeff King, 2012-02-03 21:55 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120203215507.GB3472@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120203215507.GB3472%40sigill.intra.peff.net In-Reply-To: <7vr4ybmvrq.fsf@alter.siamese.dyndns.org> ``` On Fri, Feb 03, 2012 at 01:50:33PM -0800, Junio C Hamano wrote: > > write_script foo.sh "$SHELL_PATH" <<-\EOF > > echo my arguments are "$@" > > EOF > > I first thought that the order of parameters were unusual, but with that > order, you could even go something fancier like: > > write_script () { > case "$#" in > 1) case "$1" in > *.perl | *.pl) echo "#!$PERL_PATH" ;; > *) echo "#!$SHELL_PATH" ;; > esac > 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). -Peff ``` ## Ben Walton, 2012-02-03 22:00 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <1328306228-sup-8799@pinkfloyd.chass.utoronto.ca> URL: https://gitlist.dev/e/1328306228-sup-8799%40pinkfloyd.chass.utoronto.ca In-Reply-To: <20120203215507.GB3472@sigill.intra.peff.net> ``` Excerpts from Jeff King's message of Fri Feb 03 16:55:07 -0500 2012: > > write_script () { > > case "$#" in > > 1) case "$1" in > > *.perl | *.pl) echo "#!$PERL_PATH" ;; > > *) echo "#!$SHELL_PATH" ;; > > esac > > 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). This looks like a very nice, general purpose, solution to the problem. Thanks -Ben -- Ben Walton Systems Programmer - CHASS University of Toronto C:416.407.5610 | W:416.978.4302 ``` ## Junio C Hamano, 2012-02-03 22:45 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <7vipjnmt8a.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vipjnmt8a.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120203215507.GB3472@sigill.intra.peff.net> ``` Jeff King writes: >> 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: # ``` ## Jeff King, 2012-02-03 23:27 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120203232755.GA11953@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120203232755.GA11953%40sigill.intra.peff.net In-Reply-To: <7vipjnmt8a.fsf@alter.siamese.dyndns.org> ``` 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 ``` ## Jeff King, 2012-02-04 06:27 Subject: Re: [PATCH] t0300-credentials: Word around a solaris /bin/sh bug Message-ID: <20120204062712.GA20076@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120204062712.GA20076%40sigill.intra.peff.net In-Reply-To: <7vipjnmt8a.fsf@alter.siamese.dyndns.org> ``` On Fri, Feb 03, 2012 at 02:45:25PM -0800, Junio C Hamano wrote: > > 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 ``` ## Jeff King, 2012-02-04 06:29 Subject: [PATCH 1/2] tests: add write_script helper function Message-ID: <20120204062901.GA21559@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120204062901.GA21559%40sigill.intra.peff.net In-Reply-To: <20120204062712.GA20076@sigill.intra.peff.net> ``` From: Junio C Hamano 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 --- 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 ``` ## Jeff King, 2012-02-04 06:30 Subject: [PATCH 2/2] t0300: use write_script helper Message-ID: <20120204063018.GB21559@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120204063018.GB21559%40sigill.intra.peff.net In-Reply-To: <20120204062712.GA20076@sigill.intra.peff.net> ``` 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 --- 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 ``` ## Junio C Hamano, 2012-02-04 06:58 Subject: Re: [PATCH 2/2] t0300: use write_script helper Message-ID: <7vd39vjda9.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd39vjda9.fsf%40alter.siamese.dyndns.org In-Reply-To: <20120204063018.GB21559@sigill.intra.peff.net> ``` Jeff King writes: > 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 > --- > 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 An innocuous facepalm I'd be glad to remove myself ;-) > . ./dump > exit 0 > EOF > - chmod +x git-credential-useless && > > - cat >git-credential-verbatim <<-\EOF && > + write_script git-credential-verbatim <<-\EOF && > #!/bin/sh But other than that, looks good. > 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" > ' ``` ## Jeff King, 2012-02-04 07:00 Subject: Re: [PATCH 2/2] t0300: use write_script helper Message-ID: <20120204070009.GA22188@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120204070009.GA22188%40sigill.intra.peff.net In-Reply-To: <7vd39vjda9.fsf@alter.siamese.dyndns.org> ``` On Fri, Feb 03, 2012 at 10:58:06PM -0800, Junio C Hamano wrote: > > 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 > > An innocuous facepalm I'd be glad to remove myself ;-) Heh, it took me a second to notice it, even after you mentioned it. And it's even right there in the context. At least the line written by write_script takes precedence. :) Thanks. -Peff ```