[LTP] [PATCH v3] tst_test.sh: Normalize the locale

[LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Akihiko Odaki]

network/tcp_cmds/tracepath/tracepath01.sh fails with LANG=ja_JP.UTF-8
because it parses localized output. The below is an example of such
output:
$ tracepath localhost
 1?: [LOCALHOST]                        0.040ミリ秒 pmtu 65536
 1:  localhost                                             0.274ミリ秒 到達しました
 1:  localhost                                             0.261ミリ秒 到達しました
     要約: pmtu 65536 ホップ数 1 戻りホップ数 1

It is necessary to normalize the locale to avoid such a problem.
There are some tests do the normalization, but that is not
comprehensive. Add code to normalize the locale to tst_test.sh so
that it can cover more tests.

The added code does the normalization by setting LC_ALL, which
takes precedence to the other locale-related environment variables
and does not require that "locale" command exists.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 testcases/lib/tst_test.sh | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
index 28b7d12ba..5ebbe1d25 100644
--- a/testcases/lib/tst_test.sh
+++ b/testcases/lib/tst_test.sh
@@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
 		fi
 	fi
 fi
+
+export LC_ALL=C
-- 
2.37.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Petr Vorel]

Hi all,


> network/tcp_cmds/tracepath/tracepath01.sh fails with LANG=ja_JP.UTF-8
> because it parses localized output. The below is an example of such
> output:
> $ tracepath localhost
>  1?: [LOCALHOST]                        0.040ミリ秒 pmtu 65536
>  1:  localhost                                             0.274ミリ秒 到達しました
>  1:  localhost                                             0.261ミリ秒 到達しました
>      要約: pmtu 65536 ホップ数 1 戻りホップ数 1

> It is necessary to normalize the locale to avoid such a problem.
> There are some tests do the normalization, but that is not
> comprehensive. Add code to normalize the locale to tst_test.sh so
> that it can cover more tests.

> The added code does the normalization by setting LC_ALL, which
> takes precedence to the other locale-related environment variables
> and does not require that "locale" command exists.

> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  testcases/lib/tst_test.sh | 2 ++
>  1 file changed, 2 insertions(+)

> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> index 28b7d12ba..5ebbe1d25 100644
> --- a/testcases/lib/tst_test.sh
> +++ b/testcases/lib/tst_test.sh
> @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
>  		fi
>  	fi
>  fi
> +
> +export LC_ALL=C

Thinking about it twice this might have impact on other tests.
Li, Cyril, any idea about it?

Other option would be to put this change just to tracepath01.sh.
And if we want to test just C locale (I'm really not sure, shouldn't
it be also in C API?

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Akihiko Odaki]

On 2022/10/21 2:30, Petr Vorel wrote:
> Hi all,
> 
> 
>> network/tcp_cmds/tracepath/tracepath01.sh fails with LANG=ja_JP.UTF-8
>> because it parses localized output. The below is an example of such
>> output:
>> $ tracepath localhost
>>   1?: [LOCALHOST]                        0.040ミリ秒 pmtu 65536
>>   1:  localhost                                             0.274ミリ秒 到達しました
>>   1:  localhost                                             0.261ミリ秒 到達しました
>>       要約: pmtu 65536 ホップ数 1 戻りホップ数 1
> 
>> It is necessary to normalize the locale to avoid such a problem.
>> There are some tests do the normalization, but that is not
>> comprehensive. Add code to normalize the locale to tst_test.sh so
>> that it can cover more tests.
> 
>> The added code does the normalization by setting LC_ALL, which
>> takes precedence to the other locale-related environment variables
>> and does not require that "locale" command exists.
> 
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   testcases/lib/tst_test.sh | 2 ++
>>   1 file changed, 2 insertions(+)
> 
>> diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
>> index 28b7d12ba..5ebbe1d25 100644
>> --- a/testcases/lib/tst_test.sh
>> +++ b/testcases/lib/tst_test.sh
>> @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
>>   		fi
>>   	fi
>>   fi
>> +
>> +export LC_ALL=C
> 
> Thinking about it twice this might have impact on other tests.
> Li, Cyril, any idea about it?
> 
> Other option would be to put this change just to tracepath01.sh.
> And if we want to test just C locale (I'm really not sure, shouldn't
> it be also in C API?
> 
> Kind regards,
> Petr
If the change of scope needs to be narrowed, we can prefix tracepath 
invocation with LC_ALL=C and limit the impact completely to tracepath.

However, I opt to add this to tst_test.sh because, in my experience with 
Japanese locale, this kind of problem has been so common that even some 
Japanese people set their locale to C for their command line. It can 
happen anywhere using sed, awk, etc for command outputs and often remain 
unnoticed because of its locale-specific nature.

Regarding C API, the scope of setlocale(), available in C, is limited to 
the program, and is not applicable when invoking an external command 
like tracepath. It could be an option if tracepath also has C API, but 
it doesn't, unfortunately.

Regards,
Akihiko Odaki

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Cyril Hrubis]

Hi!
> > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > index 28b7d12ba..5ebbe1d25 100644
> > --- a/testcases/lib/tst_test.sh
> > +++ b/testcases/lib/tst_test.sh
> > @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
> >  		fi
> >  	fi
> >  fi
> > +
> > +export LC_ALL=C
> 
> Thinking about it twice this might have impact on other tests.
> Li, Cyril, any idea about it?

Actually I think that we should have put this into the tst_test.sh from
the start. We do have this as a workaround in du01.sh and telnet01.sh
already.

The plus side is that all possible bugreports from users will have all
messages in english which would make debugging easier.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Petr Vorel]

Hi Cyril,

> Hi!
> > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > index 28b7d12ba..5ebbe1d25 100644
> > > --- a/testcases/lib/tst_test.sh
> > > +++ b/testcases/lib/tst_test.sh
> > > @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
> > >  		fi
> > >  	fi
> > >  fi
> > > +
> > > +export LC_ALL=C

> > Thinking about it twice this might have impact on other tests.
> > Li, Cyril, any idea about it?

> Actually I think that we should have put this into the tst_test.sh from
> the start. We do have this as a workaround in du01.sh and telnet01.sh
> already.

> The plus side is that all possible bugreports from users will have all
> messages in english which would make debugging easier.

Thanks for acking this.

I suppose Akihiko is correct that we don't have to put it via setlocale()
in C API, right?

Also I guess you prefer this version (LC_ALL=C). I suggested unset LC_ALL;
export LC_COLLATE=C; export LC_NUMERIC=C as it's used in kernel's Makefile (they
have more there, but IMHO only this is relevant for us).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Cyril Hrubis]

Hi!
> > > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > > index 28b7d12ba..5ebbe1d25 100644
> > > > --- a/testcases/lib/tst_test.sh
> > > > +++ b/testcases/lib/tst_test.sh
> > > > @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
> > > >  		fi
> > > >  	fi
> > > >  fi
> > > > +
> > > > +export LC_ALL=C
> 
> > > Thinking about it twice this might have impact on other tests.
> > > Li, Cyril, any idea about it?
> 
> > Actually I think that we should have put this into the tst_test.sh from
> > the start. We do have this as a workaround in du01.sh and telnet01.sh
> > already.
> 
> > The plus side is that all possible bugreports from users will have all
> > messages in english which would make debugging easier.
> 
> Thanks for acking this.
> 
> I suppose Akihiko is correct that we don't have to put it via setlocale()
> in C API, right?

I do not think so. It's not entirely clear from documentation how this
is supposed to work. However it seems that in bash the effect is
immediate, if you export new locale and then type nonexisting command
name to bash prompt to get a message from bash it's in the new locale.
So either setlocale() is not needed or bash calls it if it detects that
one of the LC_ variables has been changed and calls it.

For dash it does not seem that it has been translated at all, it
produces messages in english either way, so we do not have to care.

And all localized programs that we start from the shell after the export
do call setlocale() at the start of the main(). That is actually
required to enable locale support to begin with.

> Also I guess you prefer this version (LC_ALL=C). I suggested unset LC_ALL;
> export LC_COLLATE=C; export LC_NUMERIC=C as it's used in kernel's Makefile (they
> have more there, but IMHO only this is relevant for us).

I do not think that LC_COLLATE and LC_NUMERIC is enough, we want actuall
messages in english. If we do not want to use LC_ALL, which is probably
the easiest we should use at least LC_MESSAGES and LC_NUMERIC but
possibly LC_TIME too since date and time can be part of the messages we
do parse as well.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Petr Vorel]

> Hi!
> > > > > diff --git a/testcases/lib/tst_test.sh b/testcases/lib/tst_test.sh
> > > > > index 28b7d12ba..5ebbe1d25 100644
> > > > > --- a/testcases/lib/tst_test.sh
> > > > > +++ b/testcases/lib/tst_test.sh
> > > > > @@ -831,3 +831,5 @@ if [ -z "$TST_NO_DEFAULT_RUN" ]; then
> > > > >  		fi
> > > > >  	fi
> > > > >  fi
> > > > > +
> > > > > +export LC_ALL=C

> > > > Thinking about it twice this might have impact on other tests.
> > > > Li, Cyril, any idea about it?

> > > Actually I think that we should have put this into the tst_test.sh from
> > > the start. We do have this as a workaround in du01.sh and telnet01.sh
> > > already.

> > > The plus side is that all possible bugreports from users will have all
> > > messages in english which would make debugging easier.

> > Thanks for acking this.

> > I suppose Akihiko is correct that we don't have to put it via setlocale()
> > in C API, right?

> I do not think so. It's not entirely clear from documentation how this
> is supposed to work. However it seems that in bash the effect is
> immediate, if you export new locale and then type nonexisting command
> name to bash prompt to get a message from bash it's in the new locale.
> So either setlocale() is not needed or bash calls it if it detects that
> one of the LC_ variables has been changed and calls it.

> For dash it does not seem that it has been translated at all, it
> produces messages in english either way, so we do not have to care.

> And all localized programs that we start from the shell after the export
> do call setlocale() at the start of the main(). That is actually
> required to enable locale support to begin with.

Yep. I thought more about pure C API programs. Mostly they don't have any locale
related code, but sometimes they call other programs via shell (e.g.
tst_system(), but there are more).

I also aimed for consistency between C and shell API.
That's why I thought we should consider setlocale() on LC_ALL and LANG in
lib/tst_test.c. But maybe I'm wrong.

> > Also I guess you prefer this version (LC_ALL=C). I suggested unset LC_ALL;
> > export LC_COLLATE=C; export LC_NUMERIC=C as it's used in kernel's Makefile (they
> > have more there, but IMHO only this is relevant for us).

> I do not think that LC_COLLATE and LC_NUMERIC is enough, we want actuall
> messages in english. If we do not want to use LC_ALL, which is probably
> the easiest we should use at least LC_MESSAGES and LC_NUMERIC but
> possibly LC_TIME too since date and time can be part of the messages we
> do parse as well.
After some searching and thinking I guess LC_ALL=C and LANG=C should be enough.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Cyril Hrubis]

Hi!
> Yep. I thought more about pure C API programs. Mostly they don't have any locale
> related code, but sometimes they call other programs via shell (e.g.
> tst_system(), but there are more).
>
> I also aimed for consistency between C and shell API.
> That's why I thought we should consider setlocale() on LC_ALL and LANG in
> lib/tst_test.c. But maybe I'm wrong.

I do not think that we need to call setlocale() in tst_test.c we do not
have any support for locales there anyways.

As for any programs executed from these as long as environment is
propagated correctly the locale would be set to C once the corresponding
program calls setlocale() in main().

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] tst_test.sh: Normalize the locale

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> Yep. I thought more about pure C API programs. Mostly they don't have any locale
>> related code, but sometimes they call other programs via shell (e.g.
>> tst_system(), but there are more).
>>
>> I also aimed for consistency between C and shell API.
>> That's why I thought we should consider setlocale() on LC_ALL and LANG in
>> lib/tst_test.c. But maybe I'm wrong.
>
> I do not think that we need to call setlocale() in tst_test.c we do not
> have any support for locales there anyways.
>
> As for any programs executed from these as long as environment is
> propagated correctly the locale would be set to C once the corresponding
> program calls setlocale() in main().
>
> -- 
> Cyril Hrubis
> chrubis@suse.cz

Well I see no argument against setting the locale to C. So I have merged
it.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp