threads / patch / 11792

patch, 7 partsUse 'printf %s $x' notation in t5401

Subject: [PATCH 1/7] Use 'printf %s $x' notation in t5401

## tl;dr

5 messages between Jan 30, 2008 and Jan 31, 2008. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Shawn O. Pearce· Jan 30, 2008, 06:21 UTC · lore

We only care about getting what should be an empty string and sending it to a file, without a trailing LF, so the empty string translates into a 0 byte file. Earlier when I originally wrote these lines Mac OS X allowed the format string of printf to be the empty string, but more recent versions appear to have been 'improved' with error messages if the format is not given.

This may cause problems if we ever wind up with changes to the hook tests. A minor cleanup makes the test more safe on all systems, by conforming to accepted printf conventions.

Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 t/t5401-update-hooks.sh |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)
Show changes to t/t5401-update-hooks.sh +4 −4
diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
index 3eea306..9734fc5 100755
--- a/t/t5401-update-hooks.sh
+++ b/t/t5401-update-hooks.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 
 cat >victim/.git/hooks/pre-receive <<'EOF'
 #!/bin/sh
-printf "$@" >>$GIT_DIR/pre-receive.args
+printf %s "$@" >>$GIT_DIR/pre-receive.args
 cat - >$GIT_DIR/pre-receive.stdin
 echo STDOUT pre-receive
 echo STDERR pre-receive >&2
@@ -35,7 +35,7 @@ chmod u+x victim/.git/hooks/pre-receive
 cat >victim/.git/hooks/update <<'EOF'
 #!/bin/sh
 echo "$@" >>$GIT_DIR/update.args
-read x; printf "$x" >$GIT_DIR/update.stdin
+read x; printf %s "$x" >$GIT_DIR/update.stdin
 echo STDOUT update $1
 echo STDERR update $1 >&2
 test "$1" = refs/heads/master || exit
@@ -44,7 +44,7 @@ chmod u+x victim/.git/hooks/update
 
 cat >victim/.git/hooks/post-receive <<'EOF'
 #!/bin/sh
-printf "$@" >>$GIT_DIR/post-receive.args
+printf %s "$@" >>$GIT_DIR/post-receive.args
 cat - >$GIT_DIR/post-receive.stdin
 echo STDOUT post-receive
 echo STDERR post-receive >&2
@@ -54,7 +54,7 @@ chmod u+x victim/.git/hooks/post-receive
 cat >victim/.git/hooks/post-update <<'EOF'
 #!/bin/sh
 echo "$@" >>$GIT_DIR/post-update.args
-read x; printf "$x" >$GIT_DIR/post-update.stdin
+read x; printf %s "$x" >$GIT_DIR/post-update.stdin
 echo STDOUT post-update
 echo STDERR post-update >&2
 EOF

-- 
1.5.4.rc5.1126.g6ba14
Junio C Hamano· Jan 30, 2008, 06:40 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 1/7] Use 'printf %s $x' notation in t5401

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 6 quoted lines
> We only care about getting what should be an empty string and
> sending it to a file, without a trailing LF, so the empty string
> translates into a 0 byte file.  Earlier when I originally wrote
> these lines Mac OS X allowed the format string of printf to be
> the empty string, but more recent versions appear to have been
> 'improved' with error messages if the format is not given.
That sounds like a bug to me.
Show 10 quoted lines
> diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
> index 3eea306..9734fc5 100755
> --- a/t/t5401-update-hooks.sh
> +++ b/t/t5401-update-hooks.sh
> @@ -25,7 +25,7 @@ test_expect_success setup '
>  
>  cat >victim/.git/hooks/pre-receive <<'EOF'
>  #!/bin/sh
> -printf "$@" >>$GIT_DIR/pre-receive.args
> +printf %s "$@" >>$GIT_DIR/pre-receive.args

Do you really mean this? "printf %s 1 2 3" writes "123" without terminating LF. You seem to check only for size but to be reusable you might want to use something like:

	printf '%s\n' "$@"
Likewise for post-receive.args.
Shawn O. Pearce· Jan 30, 2008, 06:48 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/7] Use 'printf %s $x' notation in t5401

Junio C Hamano <gitster@pobox.com> wrote:
Show 15 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> > diff --git a/t/t5401-update-hooks.sh b/t/t5401-update-hooks.sh
> > index 3eea306..9734fc5 100755
> > --- a/t/t5401-update-hooks.sh
> > +++ b/t/t5401-update-hooks.sh
> > @@ -25,7 +25,7 @@ test_expect_success setup '
> >  
> >  cat >victim/.git/hooks/pre-receive <<'EOF'
> >  #!/bin/sh
> > -printf "$@" >>$GIT_DIR/pre-receive.args
> > +printf %s "$@" >>$GIT_DIR/pre-receive.args
> 
> Do you really mean this? "printf %s 1 2 3" writes "123" without
> terminating LF.  You seem to check only for size but to be
> reusable you might want to use something like:

The only thing we care about is was there args or not to the hook. I probably could do that test differently, like say:

	echo $# >$GIT_DIR/pre-receive.args

and then test that the file contains "0" instead of looking for it to be empty. Not sure why I didn't write it that way in the beginning.

> 	printf '%s\n' "$@"

Eh. Since all we care is that the argument count is 0 we probably should be looking at $# and calling it a day.

-- 
Shawn.
Junio C Hamano· Jan 31, 2008, 01:16 UTC · re: Shawn O. Pearce · lore

Re: [PATCH 1/7] Use 'printf %s $x' notation in t5401

"Shawn O. Pearce" <spearce@spearce.org> writes:
Show 18 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>> Do you really mean this? "printf %s 1 2 3" writes "123" without
>> terminating LF.  You seem to check only for size but to be
>> reusable you might want to use something like:
> 
> The only thing we care about is was there args or not to the hook.
> I probably could do that test differently, like say:
>
> 	echo $# >$GIT_DIR/pre-receive.args
>
> and then test that the file contains "0" instead of looking for it to
> be empty.  Not sure why I didn't write it that way in the beginning.
>
>> 	printf '%s\n' "$@"
>
> Eh.  Since all we care is that the argument count is 0 we probably
> should be looking at $# and calling it a day.

I'll apply as-is, but once you break the code that calls pre-receive hook in such a way that it sends unexpected arguments, you'd thank me for suggesting to print the arguments separately while debugging the problem with the test script.

Shawn O. Pearce· Jan 31, 2008, 03:41 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/7] Use 'printf %s $x' notation in t5401

Junio C Hamano <gitster@pobox.com> wrote:
Show 10 quoted lines
> "Shawn O. Pearce" <spearce@spearce.org> writes:
> >> 	printf '%s\n' "$@"
> >
> > Eh.  Since all we care is that the argument count is 0 we probably
> > should be looking at $# and calling it a day.
> 
> I'll apply as-is, but once you break the code that calls
> pre-receive hook in such a way that it sends unexpected
> arguments, you'd thank me for suggesting to print the arguments
> separately while debugging the problem with the test script.

Well, I don't really think it matters. I doubt we'll break that code that invokes the pre-receive or post-receive hooks. And as those hooks are defined to never take arguments if we did suddenly start passing arguments we want the test to break. When tests break sometimes its not obvious why they have broken.

Case in point, the "remote: " prefixing caused by the sideband demultiplexer really confused me for about 15 minutes. I could not figure out why the last test in this script was failing. Until I pulled my head out of the sand and looked at the "actual" file and saw those prefixes. At which point it was very obvious.

:)

We haven't touched that script in almost a year. I poked at it only to fix a stupid error from my printf implementation, and because of the new "remote:" prefix. Which since it is a change in behavior may cause problems for anyone who is trying to screen-scrape the stderr of git-push. Gawd I hope nobody's scripts are that convoluted.

-- 
Shawn.

← back to recent threads