locale: Fix localedef exit code [BZ #22292]

Message ID 55f31feb-fc4e-b73e-34eb-2c713fc0219e@redhat.com
State Committed
Headers

Commit Message

Carlos O'Donell Oct. 15, 2017, 10:06 p.m. UTC
  On 10/15/2017 02:34 PM, Andreas Schwab wrote:
> On Okt 15 2017, Carlos O'Donell <carlos@redhat.com> wrote:
> 
>> Which of (a) or (b) do you prefer?
> 
> I don't really have a preference.

Given that a locale can be used for much more than just ISO C, I'm
going to downgrade this to a verbose message that appears if you
compile the locale with `-v' to verify the locale compilation
in detail. The locale will compile with `-v' but will produce an
exit code of 1 because of the extra warnings enabled by verbose
e.g. field `country_post' not defined.

With the patch the locale compiles without warnings and localedef
exits with exit code 0 and the OpenSUSE build should succeed when
building the combination of ja_JP and SHIFT_JIS or SHIFT_JISX0213.

Tested on x86_64 with no regressions.
Tested by installing and compiling all SUPPORTED locales.
Tested by compiling ja_JP.SHIFT_JISX0213 and verifying exit code was 0.
Tested by compiling ja_JP.SHIFT_JIS and verifying exit code was 0.

OK to checkin?

2017-10-15  Carlos O'Donell  <carlos@redhat.com>

	* locale/programs/charmap.c (charmap_read): ASCII compatibility
	should only be a verbose message.
	* localedata/gen-locale.sh: All locales must compile cleanly
	with exit code 0.

---
  

Comments

Carlos O'Donell Oct. 16, 2017, 3:44 p.m. UTC | #1
On 10/15/2017 03:06 PM, Carlos O'Donell wrote:
> On 10/15/2017 02:34 PM, Andreas Schwab wrote:
>> On Okt 15 2017, Carlos O'Donell <carlos@redhat.com> wrote:
>>
>>> Which of (a) or (b) do you prefer?
>>
>> I don't really have a preference.
> 
> Given that a locale can be used for much more than just ISO C, I'm
> going to downgrade this to a verbose message that appears if you
> compile the locale with `-v' to verify the locale compilation
> in detail. The locale will compile with `-v' but will produce an
> exit code of 1 because of the extra warnings enabled by verbose
> e.g. field `country_post' not defined.
> 
> With the patch the locale compiles without warnings and localedef
> exits with exit code 0 and the OpenSUSE build should succeed when
> building the combination of ja_JP and SHIFT_JIS or SHIFT_JISX0213.
> 
> Tested on x86_64 with no regressions.
> Tested by installing and compiling all SUPPORTED locales.
> Tested by compiling ja_JP.SHIFT_JISX0213 and verifying exit code was 0.
> Tested by compiling ja_JP.SHIFT_JIS and verifying exit code was 0.
> 
> OK to checkin?

Joseph,

Do you have any opinion here about the severity of being a non-ASCII
compatible character map?

The comment in question is here:

  /* Test of ASCII compatibility of locale encoding.

     Verify that the encoding to be used in a locale is ASCII compatible,
     at least for the graphic characters, excluding the control characters,
     '$' and '@'.  This constraint comes from an ISO C 99 restriction.

     ISO C 99 section 7.17.(2) (about wchar_t):
       the null character shall have the code value zero and each member of
       the basic character set shall have a code value equal to its value
       when used as the lone character in an integer character constant.
     ISO C 99 section 5.2.1.(3):
       Both the basic source and basic execution character sets shall have
       the following members: the 26 uppercase letters of the Latin alphabet
            A B C D E F G H I J K L M N O P Q R S T U V W X Y Z
       the 26 lowercase letters of the Latin alphabet
            a b c d e f g h i j k l m n o p q r s t u v w x y z
       the 10 decimal digits
            0 1 2 3 4 5 6 7 8 9
       the following 29 graphic characters
            ! " # % & ' ( ) * + , - . / : ; < = > ? [ \ ] ^ _ { | } ~
       the space character, and control characters representing horizontal
       tab, vertical tab, and form feed.

     Therefore, for all members of the "basic character set", the 'char' code
     must have the same value as the 'wchar_t' code, which in glibc is the
     same as the Unicode code, which for all of the enumerated characters
     is identical to the ASCII code. */

Is it OK to make this just a verbose message?

Are there more serious implications?

The problem is that SHIFT_JIS and SHIFT_JISX0213 have the Yen sign as
<U005c> when it should be reverse solidus '\', and the overline as
<U007e> when it should be tilde '~'.

> 2017-10-15  Carlos O'Donell  <carlos@redhat.com>
> 
> 	* locale/programs/charmap.c (charmap_read): ASCII compatibility
> 	should only be a verbose message.
> 	* localedata/gen-locale.sh: All locales must compile cleanly
> 	with exit code 0.
> 
> diff --git a/locale/programs/charmap.c b/locale/programs/charmap.c
> index a670db9..72a37c6 100644
> --- a/locale/programs/charmap.c
> +++ b/locale/programs/charmap.c
> @@ -256,8 +256,8 @@ charmap_read (const char *filename, int verbose, int error_not_found,
>  
>        if (failed)
>         {
> -         record_warning (_("\
> -character map `%s' is not ASCII compatible, locale not ISO C compliant\n"),
> +         record_verbose (stderr, _("\
> +character map `%s' is not ASCII compatible, locale not ISO C compliant"),
>                           result->code_set_name);
>           enc_not_ascii_compatible = true;
>         }
> diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
> index b4ec68c..deb09de 100644
> --- a/localedata/gen-locale.sh
> +++ b/localedata/gen-locale.sh
> @@ -34,12 +34,7 @@ generate_locale ()
>    ${localedef_before_env} ${run_program_env} I18NPATH=../localedata \
>         ${localedef_after_env} --quiet -c -f $charmap -i $input \
>         ${common_objpfx}localedata/$out || ret=$?
> -  # All locales compile fine, except those with SHIFT_JIS charmap
> -  # and those fail with exit code 1 because SHIFT_JIS issues a
> -  # warning (it is not ASCII compatible).
> -  if [ $ret -eq 0 ] \
> -     || ( [ $ret -eq 1 ] \
> -          && [ "$charmap" = "SHIFT_JIS" ] ); then
> +  if [ $ret -eq 0 ]; then
>      # The makefile checks the timestamp of the LC_CTYPE file,
>      # but localedef won't have touched it if it was able to
>      # hard-link it to an existing file.
> ---
>
  
Joseph Myers Oct. 16, 2017, 5:32 p.m. UTC | #2
On Mon, 16 Oct 2017, Carlos O'Donell wrote:

>   /* Test of ASCII compatibility of locale encoding.
> 
>      Verify that the encoding to be used in a locale is ASCII compatible,
>      at least for the graphic characters, excluding the control characters,
>      '$' and '@'.  This constraint comes from an ISO C 99 restriction.

The character '`' is also not part of the ISO C basic character sets.  (It 
looks like that's a bug in the comment, not mentioning ` as excluded 
alongwide $ and @.)

I agree with Florian that a command-line option to ignore this warning 
(treat it as a verbose message not affecting exit status) is appropriate.
  

Patch

diff --git a/locale/programs/charmap.c b/locale/programs/charmap.c
index a670db9..72a37c6 100644
--- a/locale/programs/charmap.c
+++ b/locale/programs/charmap.c
@@ -256,8 +256,8 @@  charmap_read (const char *filename, int verbose, int error_not_found,
 
       if (failed)
        {
-         record_warning (_("\
-character map `%s' is not ASCII compatible, locale not ISO C compliant\n"),
+         record_verbose (stderr, _("\
+character map `%s' is not ASCII compatible, locale not ISO C compliant"),
                          result->code_set_name);
          enc_not_ascii_compatible = true;
        }
diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
index b4ec68c..deb09de 100644
--- a/localedata/gen-locale.sh
+++ b/localedata/gen-locale.sh
@@ -34,12 +34,7 @@  generate_locale ()
   ${localedef_before_env} ${run_program_env} I18NPATH=../localedata \
        ${localedef_after_env} --quiet -c -f $charmap -i $input \
        ${common_objpfx}localedata/$out || ret=$?
-  # All locales compile fine, except those with SHIFT_JIS charmap
-  # and those fail with exit code 1 because SHIFT_JIS issues a
-  # warning (it is not ASCII compatible).
-  if [ $ret -eq 0 ] \
-     || ( [ $ret -eq 1 ] \
-          && [ "$charmap" = "SHIFT_JIS" ] ); then
+  if [ $ret -eq 0 ]; then
     # The makefile checks the timestamp of the LC_CTYPE file,
     # but localedef won't have touched it if it was able to
     # hard-link it to an existing file.