This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH 2/2] ldd: Don't use Bash-only $"msgid" quoting
On 2012-11-23 13:47, Carlos O'Donell wrote:
> On Fri, Nov 23, 2012 at 1:23 PM, P. J. McDermott <pjm@nac.net> wrote:
>> On 2012-11-23 11:03, Carlos O'Donell wrote:
>>> Paul,
>>
>> My first name is Patrick, not Paul. :)
>
> Sorry! I saw P, immediately thought Paul :-)
No problem; in my experience, that's usually a person's first guess. :)
>>> Thank you very much for your patch! :-)
>>>
>>> Unfortunately the script must function correctly without the presence
>>> of gettext.sh.
>>>
>>> Requiring gettext.sh is a non-starter. It would mean glibc (as a
>>> package with it's associated tools) would have a runtime dependency on
>>> the gettext runtimes (to install gettext.sh), and I am strongly
>>> against that. Dependencies are hard things to get right and we must
>>> not add them without very good reasons.
>>
>> Of course. I'm not sure how I added ". gettext.sh" without realizing
>> that it added a new dependency...
>>
>>> I would consider a patch that checked for gettext.sh and used it if
>>> possible to avoid bash-isms, but not outright require it.
>>>
>>> Hopefully that gives you some guidance and a way forward.
>>
>> Well here's a patch that makes ldd use gettext.sh if found and otherwise
>> define simple gettext and eval_gettext functions. Though these
>> functions should probably be defined more globally for use in other
>> scripts in glibc.
>>
>> Comments/improvements?
>
> I like this patch *much* more.
>
> However, and here is the point at which you can say no, I'd like to
> ask you to take it a step further.
>
> As Joseph pointed out we have an existing bug #13853 that is related.
>
> http://sourceware.org/bugzilla/show_bug.cgi?id=13853
>
> In the BZ I point out that we have 3 files, 2 in addition to
> ldd.bash.in, namely:
> * debug/xtrace.sh
> * debug/memusage.sh
>
> Given that you're on the path to fixing this, would you mind fixing
> those two also and thus fixing BZ #13853?
I think you mean malloc/memusage.sh. And elf/sotruss.ksh is also
addressed by the BZ #13853 patch. Searching the source tree, I also see
$"msgind" in nscd/nscd.init.
I'll plan on merging my ldd patch and your BZ #13853 patch, as well as
patching nscd/nscd.init and using printf+gettext instead of
echo+eval_gettext as requested by Dmitry.
Expect to see a revised patch within a day or two. :)
> At the moment I don't care if all three get copies of your fix below.
If I avoid using eval_gettext, this should be even less of a problem.
>> ---
>> elf/ldd.bash.in | 77 ++++++++++++++++++++++++++++++++++++++++---------------
>> 1 file changed, 56 insertions(+), 21 deletions(-)
>>
>> diff --git a/elf/ldd.bash.in b/elf/ldd.bash.in
>> index 86f47e6..5fdbe01 100644
>> --- a/elf/ldd.bash.in
>> +++ b/elf/ldd.bash.in
>> @@ -23,8 +23,41 @@
>> # variable LD_TRACE_LOADED_OBJECTS to a non-empty value.
>>
>> # We should be able to find the translation right at the beginning.
>> -TEXTDOMAIN=libc
>> -TEXTDOMAINDIR=@TEXTDOMAINDIR@
>> +export TEXTDOMAIN=libc
>> +export TEXTDOMAINDIR=@TEXTDOMAINDIR@
>> +
>> +if (. 2>/dev/null gettext.sh); then
>> + . gettext.sh
>> +else
>> + # No internationalization available; just print the original strings.
>> + gettext ()
>> + {
>> + printf '%s' "$1"
>> + }
>
> I'm happy with this, the use of $"" should go away
> (http://www.gnu.org/software/gettext/manual/html_node/bash.html).
Indeed.
> Going from translated text to untranslated text seems like a good
> compromise with a solution that is simply "Install gettext if you want
> translations to work."
Yeah, it's like an implicit --disable-nls.
>> + eval_gettext ()
>> + {
>> + vars=$({
>> + echo "$1" | sed -n '
>> + # First remove ${VARIABLES}.
>> + s/\${[A-Za-z_][A-Za-z0-9_]*}//g;
>> + # Then get $VARIABLES.
>> + s/[^$]*\$\([A-Za-z_][A-Za-z0-9_]*\)[^$]*/\1\n/gp;'
>> + echo "$1" | sed -n '
>> + # First remove $VARIABLES.
>> + s/\$[A-Za-z_][A-Za-z0-9_]*//g;
>> + # Then get ${VARIABLES}.
>> + s/[^$]*\${\([A-Za-z_][A-Za-z0-9_]*\)}[^$]*/\1\n/gp;'
>> + } | sort | uniq)
>> + sed_script=
>> + for var in $vars; do
>> + # Get the value and escape / characters.
>> + value=$(eval echo \"\$$var\" | sed 's|/|\\/|g')
>> + # Build a sed script to perform the variable expansions.
>> + sed_script="${sed_script}s/\$$var/$value/g;s/\${$var}/$value/g;"
>> + done
>> + printf '%s' "$1" | sed "$sed_script"
>> + }
>> +fi
>>
[...]
>
> What kind of testing did you do?
I worked on the eval_gettext code in an interactive Bash shell with some
fairly complex msgid strings (with $VARIABLES, ${VARIABLES}, and
newlines) and variable values with / characters. Once I got it working,
I implemented it in a shell script to test on dash and compared the
results of gettext (with LANG=en_US.UTF-8) with those of my functions.
Then I plugged the code directly into ldd.bash.in.
Thanks,
--
P. J. McDermott
http://www.pehjota.net/
http://www.pehjota.net/contact.html