Message ID | 20221020133715.256521-1-akihiko.odaki@daynix.com |
---|---|
State | Accepted |
Headers | show |
Series | [v3] tst_test.sh: Normalize the locale | expand |
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
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
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.
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
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.
> 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
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().
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.
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
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(+)