git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Chris Ridd <chris.ridd@isode.com>
To: unlisted-recipients:; (no To-header on input)
Cc: git@vger.kernel.org
Subject: Re: [PATCH] Improve sed portability
Date: Wed, 11 Jun 2008 16:29:53 +0100	[thread overview]
Message-ID: <484FEF71.2030909@isode.com> (raw)
In-Reply-To: <484FDB5D.7060606@viscovery.net>

Johannes Sixt wrote:
> Chris Ridd schrieb:
>> 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.
> 
> So does /usr/bin/sed of AIX 4.3!

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 
while before realising the problem was with the input :-)

>> @@ -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')
> 
> You change sq into dq. Is this not dangerous? Shouldn't backslash-en be
> hidden from the shell so that printf can interpret it?

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"

should work well too. Why is printf being used here and not echo, anyway?

>>  	name=$( git config -f .gitmodules --get-regexp '^submodule\..*\.path$' |
>>  		sed -n -e 's|^submodule\.\(.*\)\.path '"$re"'$|\1|p' )
> 
> I trust you have tested this. But I wonder whether this leaves a stray
> newline in $re that gets in the way inside the sed expression...

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

  reply	other threads:[~2008-06-11 15:31 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-06-11 13:09 [PATCH] Improve sed portability Chris Ridd
2008-06-11 14:04 ` Johannes Sixt
2008-06-11 15:29   ` Chris Ridd [this message]
2008-06-11 16:39     ` Jeff King
2008-06-12  7:46     ` Johannes Sixt
2008-06-12  8:29       ` Chris Ridd
2008-06-12  9:07         ` Jeff King
2008-07-13 20:00     ` Jakub Narebski
2008-06-12  8:33   ` Junio C Hamano

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=484FEF71.2030909@isode.com \
    --to=chris.ridd@isode.com \
    --cc=git@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).