Re: [PATCH] Improve sed portability

Previous thread: git-daemon on Windows? by Chris Hoffman on Wednesday, June 11, 2008 - 5:51 am. (6 messages)

Next thread: clone git over http fails - Unable to find 1f405709e7341c27e20c0159fb7c17efbf85975c by John Yesberg on Wednesday, June 11, 2008 - 7:58 am. (2 messages)
From: Chris Ridd
Date: Wednesday, June 11, 2008 - 6:09 am

On Solaris /usr/bin/sed apparently fails to process input that doesn't
end in a \n. Consequently constructs like

  re=$(printf '%s' foo | sed -e 's/bar/BAR/g' $)

cause re to be set to the empty string. Such a construct is used in
git-submodule.sh.

Changing the printf to add a \n seems the safest change. The
POSIX-compliant seds shipped with Solaris do not have this problem.

Signed-off-by: Chris Ridd <chris.ridd@isode.com>
---
 git-submodule.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-submodule.sh b/git-submodule.sh
index 1007372..e515bcc 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -73,7 +73,7 @@ resolve_relative_url ()
 module_name()
 {
 	# Do we have "submodule.<something>.path = $1" defined in .gitmodules file?
-	re=$(printf '%s' "$1" | sed -e 's/[].[^$\\*]/\\&/g')
+	re=$(printf "%s\n" "$1" | sed -e 's/[].[^$\\*]/\\&/g')
 	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
 		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
        test -z "$name" &&
-- 
1.5.3.6

--

From: Johannes Sixt
Date: Wednesday, June 11, 2008 - 7:04 am

You change sq into dq. Is this not dangerous? Shouldn't backslash-en be

I trust you have tested this. But I wonder whether this leaves a stray


-- Hannes
--

From: Chris Ridd
Date: Wednesday, June 11, 2008 - 8:29 am

I ought to have mentioned this occurs on Solaris 8, 10, build 90 of 
OpenSolaris, and on HP-UX 11iv1. I stared at that regex for quite a 

It is necessary to use double quotes. This:

     printf '%s\n' foobar

prints a literal \, a literal n, and no newline:

     foobar\n

Not desirable :-(

Of course, using a plain old:

     echo "$1"


Yes, I've tested this as we use submodules heavily. I think the $( .. ) 
notation will remove the trailing \n printed by sed, but to be sure I 
inserted a 'set -x' at the top of the module_name() function and 
double-checked that the re variable didn't get any stray \n 
character(s). Bash versions 2 and 3 were used.

So without the change, on Solaris I get:

     No submodule mapping found in .gitmodules for path 'foobar'

for the first submodule that we use, and the repository clone fails.

With the change, all our repositories clone OK.

Cheers,

Chris
--

From: Jeff King
Date: Wednesday, June 11, 2008 - 9:39 am

Because the original didn't have a newline, and "echo -n" isn't
portable?

-Peff
--

From: Johannes Sixt
Date: Thursday, June 12, 2008 - 12:46 am

On both Linux and AIX 4.3 I see:

$  printf 'x\ny'; echo z
x
yz

The printf turns the \n into LF.

I mentioned this in the first place because I don't know what various
shells do with \n when they see "%s\n". But one way or the other, the \n
will be turned into LF, either by the shell or by printf. So it's not a

Because the "$1" could contain character sequences that some 'echo'
implementations mangle.

-- Hannes

--

From: Chris Ridd
Date: Thursday, June 12, 2008 - 1:29 am

Yes, and I don't know *what* I did yesterday, but Solaris 8, 10, (every 
OS I mentioned before) behave the same as your test.



Indeed. If $1 started with -n that might cause problems on some platforms.

Should I revise my commit to use single quotes again?

Cheers,

Chris
--

From: Jeff King
Date: Thursday, June 12, 2008 - 2:07 am

It's much worse than that. Any backslash sequence can be interpolated.
4b7cc26 (git-am: use printf instead of echo on user-supplied strings).

-Peff
--

From: Jakub Narebski
Date: Sunday, July 13, 2008 - 1:00 pm

Uh, because 'echo -n' is not portable enough, and for some reason it
was though that there shouldn't be final newline?

-- 
Jakub Narebski
Poland
ShadeHawk on #git
--

Previous thread: git-daemon on Windows? by Chris Hoffman on Wednesday, June 11, 2008 - 5:51 am. (6 messages)

Next thread: clone git over http fails - Unable to find 1f405709e7341c27e20c0159fb7c17efbf85975c by John Yesberg on Wednesday, June 11, 2008 - 7:58 am. (2 messages)