On Sun, Apr 10, 2016 at 12:01:30PM -0700, Junio C Hamano wrote: > > 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. Yeah I wondered about that. And also the fact that the shell script itself doesn't end in newline. But I think that is just an accident, and no shell happened to complain (not that I would expect them to, but we come across enough weirdness around final newlines with tools like sed and tr, I wouldn't have been surprised). > -- >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. Looks good to me. I suspect you could actually just use: echo "$GIT_PREFIX" in the helper script. That is also not completely safe against arbitrary bytes in $GIT_PREFIX (due to unportable backslash escapes), though I suspect it would be fine for the purposes of the test script. Using a proper printf isn't that many more bytes, though. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html