This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 2/2] ldd: Don't use Bash-only $"msgid" quoting


On Fri, Nov 23, 2012 at 5:34 PM, Dmitry V. Levin <ldv@altlinux.org> wrote:
> On Fri, Nov 23, 2012 at 04:47:07PM -0500, P. J. McDermott wrote:
>> On 2012-11-23 15:02, Dmitry V. Levin wrote:
>> > On Fri, Nov 23, 2012 at 01:23:14PM -0500, P. J. McDermott wrote:
>> > [...]
>> >> 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?
>> > [...]
>> >> -TEXTDOMAIN=libc
>> >> -TEXTDOMAINDIR=@TEXTDOMAINDIR@
>> >> +export TEXTDOMAIN=libc
>> >> +export TEXTDOMAINDIR=@TEXTDOMAINDIR@
>> >
>> > One may note that this syntax is not quite portable.  Another issue
>> > with exported variables is that they leak to processes being traced.
>>
>> The exporting is necessary for the gettext utility.
>>
>> An alternative would be a wrapper function that calls gettext like:
>>
>>     TEXTDOMAIN=libc TEXTDOMAINDIR=@TEXTDOMAINDIR@ gettext "$1"
>>
>> Should we do that instead?
>
> Yes, I suppose that would be better.
>
>> Would something like this be better?
>>
>>     if ! gettext 2>/dev/null; then
>>       gettext ()
>>       {
>>         #...
>>       }
>>     fi
>
> Not exactly that way because gettext without arguments exits with a
> non-zero status.  I'd rather use a lazy evaluation, e.g.
>
> a_wrapper_function_that_calls_gettext ()
> {
>         if TEXTDOMAIN=libc TEXTDOMAINDIR=@TEXTDOMAINDIR@ \
>            gettext '' >/dev/null 2>&1; then
>                 a_wrapper_function_that_calls_gettext ()
>                 {
>                         TEXTDOMAIN=libc TEXTDOMAINDIR=@TEXTDOMAINDIR@ \
>                         gettext "$1"
>                 }
>         else
>                 a_wrapper_function_that_calls_gettext ()
>                 {
>                         printf %s "$1"
>                 }
>         fi
>         a_wrapper_function_that_calls_gettext "$1"
> }
>
>> In fact, gettext.sh is only needed for the definition of eval_gettext.
>
> In that case we can happily avoid gettext.sh
>
>> >> -    echo >&2 $"ldd: option \`$1' is ambiguous"
>> >> +    echo >&2 "$(eval_gettext "ldd: option \`\$1' is ambiguous")"
>> >
>> > Let's transform original
>> > echo >&2 $"ldd: option \`$1' is ambiguous"
>> > to
>> > printf >&2 $"ldd: option \`%s' is ambiguous\n" "$1"
>> > so that it could be later changed e.g. to
>> > printf >&2 "$(gettext "ldd: option \`%s' is ambiguous\n")" "$1"
>>
>> I thought the convention with gettext in shell command language was to
>> use variable expansions with echo instead of format strings with printf
>> (also I tried to not modify translatable strings).  I'm probably wrong.
>
> There are only four shell scripts in the tree that set TEXTDOMAIN=libc, and
> these scripts use both echo and printf without any obvious conventions.
>
> With regards to modifications of translatable strings, I suppose it's OK
> unless the branch is frozen and libc.pot is already sent for translations.
>
>> Either way, I do prefer using printf (for this reason and others).
>
> Let's use printf then.  That could be implemented in two commits, the
> first one that changes strings with variable expansions to format strings,
> and the second that introduces gettext.

That sounds like a good way forward.

It would be nice if this made it in for all 3 scripts that have this problem.

That way we've fixed bug #13853 and cleaned up the build errors.

Cheers,
Carlos.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]