From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: "Tom G. Christensen" <tgc@jupiterrise.com>,
Elia Pinto <gitter.spiros@gmail.com>,
git@vger.kernel.org
Subject: Re: Hardcoded #!/bin/sh in t5532 causes problems on Solaris
Date: Sun, 10 Apr 2016 12:01:30 -0700 [thread overview]
Message-ID: <xmqq37qtthit.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <xmqqmvp2ti20.fsf@gitster.mtv.corp.google.com> (Junio C. Hamano's message of "Sat, 09 Apr 2016 17:37:43 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> I looked at
>
> $ git grep -c '#! */bin/sh' t | grep -v ':1$'
>
> and did a few just for fun. Doing it fully may be a good
> microproject for next year ;-)
>
> t/t1020-subdirectory.sh | 6 +++---
> t/t2050-git-dir-relative.sh | 11 ++++++-----
> t/t3404-rebase-interactive.sh | 7 +++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
> index 8e22b03..6dedb1c 100755
> --- a/t/t1020-subdirectory.sh
> +++ b/t/t1020-subdirectory.sh
> @@ -142,9 +142,9 @@ test_expect_success 'GIT_PREFIX for built-ins' '
> # Use GIT_EXTERNAL_DIFF to test that the "diff" built-in
> # receives the GIT_PREFIX variable.
> printf "dir/" >expect &&
> - printf "#!/bin/sh\n" >diff &&
> - printf "printf \"\$GIT_PREFIX\"" >>diff &&
> - chmod +x diff &&
> + write_script diff <<-\EOF &&
> + printf "%s" "$GIT_PREFIX"
> + EOF
> (
> cd dir &&
> printf "change" >two &&
Regarding this one, I notice that "expect" and "actual" (produced
later in this script by executing "diff" script) are eventually
compared by test_cmp, which runs "diff" to show the actual
differences. If we are doing this modernization to use write_script
more, we probably should make "expect" and "actual" text files that
end with a complete line.
I.e.
-- >8 --
Subject: t1020: do not overuse printf and use write_script
The test prepares a sample file "dir/two" with a single incomplete
line in it with "printf", and also prepares a small helper script
"diff" to create a file with a single incomplete line in it, again
with "printf". The output from the latter is compared with an
expected output, again prepared with "printf" hance lacking the
final LF. There is no reason for this test to be using files with
an incomplete line at the end, and these look more like a mistake
of not using
printf "%s\n" "string to be written"
and using
printf "string to be written"
Depending on what would be in $GIT_PREFIX, using the latter form
could be a bug waiting to happen. Correct them.
Also, the test uses hardcoded #!/bin/sh to create a small helper
script. For a small task like what the generated script does, it
does not matter too much in that what appears as /bin/sh would not
be _so_ broken, but while we are at it, use write_script instead,
which happens to make the result easier to read by reducing need
of one level of quoting.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
t/t1020-subdirectory.sh | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/t/t1020-subdirectory.sh b/t/t1020-subdirectory.sh
index 8e22b03..df3183e 100755
--- a/t/t1020-subdirectory.sh
+++ b/t/t1020-subdirectory.sh
@@ -141,13 +141,13 @@ test_expect_success 'GIT_PREFIX for !alias' '
test_expect_success 'GIT_PREFIX for built-ins' '
# Use GIT_EXTERNAL_DIFF to test that the "diff" built-in
# receives the GIT_PREFIX variable.
- printf "dir/" >expect &&
- printf "#!/bin/sh\n" >diff &&
- printf "printf \"\$GIT_PREFIX\"" >>diff &&
- chmod +x diff &&
+ echo "dir/" >expect &&
+ write_script diff <<-\EOF &&
+ printf "%s\n" "$GIT_PREFIX"
+ EOF
(
cd dir &&
- printf "change" >two &&
+ echo "change" >two &&
GIT_EXTERNAL_DIFF=./diff git diff >../actual
git checkout -- two
) &&
next prev parent reply other threads:[~2016-04-10 19:02 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-09 20:27 Hardcoded #!/bin/sh in t5532 causes problems on Solaris Tom G. Christensen
2016-04-09 21:04 ` Jeff King
2016-04-09 22:29 ` Tom G. Christensen
2016-04-09 22:37 ` Jeff King
2016-04-10 0:37 ` Junio C Hamano
2016-04-10 19:01 ` Junio C Hamano [this message]
2016-04-10 21:51 ` Eric Sunshine
2016-04-11 16:42 ` Junio C Hamano
2016-04-11 17:27 ` Jeff King
2016-04-11 17:32 ` Jeff King
2016-04-12 16:58 ` Junio C Hamano
2016-04-12 17:12 ` Junio C Hamano
2016-04-12 17:23 ` Jeff King
2016-04-12 17:22 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq37qtthit.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=gitter.spiros@gmail.com \
--cc=peff@peff.net \
--cc=tgc@jupiterrise.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.