[PATCH 0/2] Improve LC_MONETARY handling.

[PATCH 0/2] Improve LC_MONETARY handling.

[Carlos O'Donell]

In glibc 2.35 we released C.UTF-8, but if you try to compile
the locale without '-c' you get the following error:

localedef -i C -f UTF-8 C.UTF-8
[error] LC_MONETARY: value for field `mon_decimal_point' 
        must not be an empty string
[error] no output file produced because errors were issued

The first patch in the series fixes bug 28845 and cleans up
LC_MONETARY handling in ld-monetary.c (monetary_finish) 
for default values and generated errors. In general we 
downgrade the errors and make them warnings.

Lastly, to prevent this from happening again we remove the
'-c' from glibc localedef uses since we do not want to force
output generation and we should always have locales that
compile cleanly.

Where locales are not clean we should implement specific
warning disabling e.g. --no-warnings=ascii for SHIFT_JIS
and SHIFT_JISX0213 (non-ascii locales). Any future locale
warnings should fail during locale compilation (install
or testing).

Carlos O'Donell (2):
  localedef: Update LC_MONETARY handling (Bug 28845)
  localedata: Do not generate output if warnings were present.

 locale/programs/ld-monetary.c | 172 +++++++++++++++++++++++++++-------
 localedata/Makefile           |   3 +-
 2 files changed, 137 insertions(+), 38 deletions(-)

-- 
2.31.1

[PATCH 1/2] localedef: Update LC_MONETARY handling (Bug 28845)

[Carlos O'Donell]

ISO C17, POSIX Issue 7, and ISO 30112 all allow the char*
types to be empty strings i.e. "", integer or char values to
be -1 or CHAR_MAX respectively, with the exception of
decimal_point which must be non-empty in ISO C.

We include a broad comment talking about harmonizing ISO C,
POSIX, ISO 30112, and the default C/POSIX locale for glibc.

We reorder all setting based on locale/categories.def order.

We soften all missing definitions from errors to warnings when
defaults exist.

Given that ISO C, POSIX and ISO 30112 allow the empty string
we change LC_MONETARY handling of mon_decimal_point to allow
the empty string.  If mon_decimal_point is not defined at all
then we pick the existing legacy glibc default value of
<U002E> i.e. ".".

We also set the default for mon_thousands_sep_wc at the
same time as mon_thousands_sep, but this is not a change in
behaviour, it is always either a matching value or L'\0',
but if in the future we change the default to a non-empty
string we would need to update both at the same time.

Tested on x86_64 and i686 without regressions.
Tested with install-locale-archive target.
Tested with install-locale-files target.
---
 locale/programs/ld-monetary.c | 172 +++++++++++++++++++++++++++-------
 1 file changed, 136 insertions(+), 36 deletions(-)

diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c
index 3b0412b405..665b80f5a6 100644
--- a/locale/programs/ld-monetary.c
+++ b/locale/programs/ld-monetary.c
@@ -196,21 +196,99 @@ No definition for %s category found"), "LC_MONETARY");
 	}
     }
 
+  /* Generally speaking there are 3 standards the define the default,
+     warning, and error behaviour of LC_MONETARY.  They are ISO/IEC TR 30112,
+     ISO/IEC 9899:2018 (ISO C17), and POSIX.1-2017.  Within 30112 we have the
+     definition of a standard i18n FDCC-set, which for LC_MONETARY has the
+     following default values:
+	int_curr_symbol		""
+	currency_symbol		""
+	mon_decimal_point	"<U002C>" i.e. ","
+	mon_thousand_sep	""
+	mon_grouping		-1
+	positive_sign		""
+	negative_sign		"<U002E>" i.e. "."
+	int_frac_digits		-1
+	frac_digits		-1
+	p_cs_precedes		-1
+	p_sep_by_space		-1
+	n_cs_precedes		-1
+	n_sep_by_space		-1
+	p_sign_posn		-1
+	n_sign_posn		-1
+    Under 30112 a keyword that is not provided implies an empty string ""
+    for string values or a -1 for integer values, and indicates the value
+    is unspecified with no default implied.  No errors are considered.
+    For POSIX Issue 7 we have:
+    https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html
+    and again values not provided default to "" or -1, and indicate the value
+    is not available to the locale. For the POSIX locale the values of
+    LC_MONETARY should be:
+	int_curr_symbol		""
+	currency_symbol		""
+	mon_decimal_point	""
+	mon_thousands_sep	""
+	mon_grouping		-1
+	positive_sign		""
+	negative_sign		""
+	int_frac_digits		-1
+	frac_digits		-1
+	p_cs_precedes		-1
+	p_sep_by_space		-1
+	n_cs_precedes		-1
+	n_sep_by_space		-1
+	p_sign_posn		-1
+	n_sign_posn		-1
+	int_p_cs_precedes	-1
+	int_p_sep_by_space	-1
+	int_n_cs_precedes	-1
+	int_n_sep_by_space	-1
+	int_p_sign_posn		-1
+	int_n_sign_posn		-1
+    Like with 30112, POSIX also considers no error if the keywords are
+    missing, only that if the cateory as a whole is missing the referencing
+    of the category results in unspecified behaviour.
+    For ISO C17 there is no default value provided, but the localeconv
+    specification in 7.11.2.1 admits that members of char * type may point
+    to "" to indicate a value is not available or is of length zero.
+    The exception is decimal_point (not mon_decimal_point) which must be a
+    defined non-empty string.  The values of char, which are generally
+    mapped to integer values in 30112 and POSIX, must be non-negative
+    numbers that map to CHAR_MAX when a value is not available in the
+    locale.
+    In ISO C17 for the "C" locale all values are empty strings "", or
+    CHAR_MAX, with the exception of decimal_point which is "." (defined
+    in LC_NUMERIC).
+
+    Lastly, we must consider the legacy C/POSIX locale that implemented
+    as a builtin in glibc and wether a default value mapping to the
+    C/POSIX locale may benefit the user from a compatibility perspective.
+
+    Thus given 30112, POSIX, ISO C, and the builtin C/POSIX locale we
+    need to pick appropriate defaults below.   */
+
+  /* The members of LC_MONETARY are handled in the order of their definition
+     in locale/categories.def.  Please keep them in that order.  */
+
+  /* The purpose of TEST_ELEM is to define a default value for the fields
+     in the category if the field was not defined in the cateory.  If the
+     category was present but we didn't see a definition for the field then
+     we also issue a warning, otherwise the only warning you get is the one
+     earlier when a default category is created (completely missing category).
+     This missing field warning is glibc-specific since no standard requires
+     this warning, but we consider it valuable to print a warning for all
+     missing fields in the category.  */
 #define TEST_ELEM(cat, initval) \
   if (monetary->cat == NULL)						      \
     {									      \
       if (! nothing)							      \
-	record_error (0, 0, _("%s: field `%s' not defined"),		      \
-		      "LC_MONETARY", #cat);				      \
+	record_warning (_("%s: field `%s' not defined"),		      \
+			"LC_MONETARY", #cat);				      \
       monetary->cat = initval;						      \
     }
 
+  /* Keyword: int_curr_symbol.  */
   TEST_ELEM (int_curr_symbol, "");
-  TEST_ELEM (currency_symbol, "");
-  TEST_ELEM (mon_thousands_sep, "");
-  TEST_ELEM (positive_sign, "");
-  TEST_ELEM (negative_sign, "");
-
   /* The international currency symbol must come from ISO 4217.  */
   if (monetary->int_curr_symbol != NULL)
     {
@@ -247,41 +325,59 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
 	}
     }
 
-  /* The decimal point must not be empty.  This is not said explicitly
-     in POSIX but ANSI C (ISO/IEC 9899) says in 4.4.2.1 it has to be
-     != "".  */
+  /* Keyword: currency_symbol */
+  TEST_ELEM (currency_symbol, "");
+
+  /* Keyword: mon_decimal_point */
+  /* ISO C17 7.11.2.1.3 explicitly allows mon_decimal_point to be the
+     empty string e.g. "".  This indicates the value is not available in the
+     current locale or is of zero length.  However, if the value was never
+     defined then we issue a warning and use a glibc-specific default.  ISO
+     30112 in the i18n FDCC-Set uses <U002C> ",", and POSIX Issue 7 in the
+     POSIX locale uses "".  It is specific to glibc that the default is <U002E>
+     "."; we retain this existing behaviour for backwards compatibility.  */
   if (monetary->mon_decimal_point == NULL)
     {
       if (! nothing)
-	record_error (0, 0, _("%s: field `%s' not defined"),
-		      "LC_MONETARY", "mon_decimal_point");
+	record_warning (_("%s: field `%s' not defined, using defaults"),
+			"LC_MONETARY", "mon_decimal_point");
       monetary->mon_decimal_point = ".";
       monetary->mon_decimal_point_wc = L'.';
     }
-  else if (monetary->mon_decimal_point[0] == '\0' && ! be_quiet && ! nothing)
+
+  /* Keyword: mon_thousands_sep */
+  if (monetary->mon_thousands_sep == NULL)
     {
-      record_error (0, 0, _("\
-%s: value for field `%s' must not be an empty string"),
-		    "LC_MONETARY", "mon_decimal_point");
+      if (! nothing)
+	record_warning (_("%s: field `%s' not defined, using defaults"),
+			"LC_MONETARY", "mon_thousands_sep");
+      monetary->mon_thousands_sep = "";
+      monetary->mon_thousands_sep_wc = L'\0';
     }
 
+  /* Keyword: mon_grouping */
   if (monetary->mon_grouping_len == 0)
     {
       if (! nothing)
-	record_error (0, 0, _("%s: field `%s' not defined"),
-		      "LC_MONETARY", "mon_grouping");
-
+	record_warning (_("%s: field `%s' not defined"),
+			"LC_MONETARY", "mon_grouping");
       monetary->mon_grouping = (char *) "\177";
       monetary->mon_grouping_len = 1;
     }
 
+  /* Keyword: positive_sign */
+  TEST_ELEM (positive_sign, "");
+
+  /* Keyword: negative_sign */
+  TEST_ELEM (negative_sign, "");
+
 #undef TEST_ELEM
 #define TEST_ELEM(cat, min, max, initval) \
   if (monetary->cat == -2)						      \
     {									      \
        if (! nothing)							      \
-	 record_error (0, 0, _("%s: field `%s' not defined"),		      \
-		       "LC_MONETARY", #cat);				      \
+	 record_warning (_("%s: field `%s' not defined"),		      \
+			 "LC_MONETARY", #cat);				      \
        monetary->cat = initval;						      \
     }									      \
   else if ((monetary->cat < min || monetary->cat > max)			      \
@@ -300,16 +396,11 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
   TEST_ELEM (p_sign_posn, -1, 4, -1);
   TEST_ELEM (n_sign_posn, -1, 4, -1);
 
-  /* The non-POSIX.2 extensions are optional.  */
-  if (monetary->duo_int_curr_symbol == NULL)
-    monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
-  if (monetary->duo_currency_symbol == NULL)
-    monetary->duo_currency_symbol = monetary->currency_symbol;
-
-  if (monetary->duo_int_frac_digits == -2)
-    monetary->duo_int_frac_digits = monetary->int_frac_digits;
-  if (monetary->duo_frac_digits == -2)
-    monetary->duo_frac_digits = monetary->frac_digits;
+  /* Keyword: crncystr */
+  monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
+					 + 2);
+  monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
+  strcpy (&monetary->crncystr[1], monetary->currency_symbol);
 
 #undef TEST_ELEM
 #define TEST_ELEM(cat, alt, min, max) \
@@ -327,6 +418,17 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
   TEST_ELEM (int_p_sign_posn, p_sign_posn, -1, 4);
   TEST_ELEM (int_n_sign_posn, n_sign_posn, -1, 4);
 
+  /* The non-POSIX.2 extensions are optional.  */
+  if (monetary->duo_int_curr_symbol == NULL)
+    monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
+  if (monetary->duo_currency_symbol == NULL)
+    monetary->duo_currency_symbol = monetary->currency_symbol;
+
+  if (monetary->duo_int_frac_digits == -2)
+    monetary->duo_int_frac_digits = monetary->int_frac_digits;
+  if (monetary->duo_frac_digits == -2)
+    monetary->duo_frac_digits = monetary->frac_digits;
+
   TEST_ELEM (duo_p_cs_precedes, p_cs_precedes, -1, 1);
   TEST_ELEM (duo_p_sep_by_space, p_sep_by_space, -1, 2);
   TEST_ELEM (duo_n_cs_precedes, n_cs_precedes, -1, 1);
@@ -349,17 +451,15 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
   if (monetary->duo_valid_to == 0)
     monetary->duo_valid_to = 99991231;
 
+  /* Keyword: conversion_rate */
   if (monetary->conversion_rate[0] == 0)
     {
       monetary->conversion_rate[0] = 1;
       monetary->conversion_rate[1] = 1;
     }
 
-  /* Create the crncystr entry.  */
-  monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
-					 + 2);
-  monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
-  strcpy (&monetary->crncystr[1], monetary->currency_symbol);
+  /* A value for monetary-decimal-point-wc was set when
+     monetary_decimal_point was set, likewise for monetary-thousands-sep-wc.  */
 }
 
 
-- 
2.31.1

Re: [PATCH 1/2] localedef: Update LC_MONETARY handling (Bug 28845)

[DJ Delorie]

"Carlos O'Donell via Libc-alpha" <libc-alpha@sourceware.org> writes:
> diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c

> +  /* Generally speaking there are 3 standards the define the default,
> +     warning, and error behaviour of LC_MONETARY.  They are ISO/IEC TR 30112,

spelling: behaviour -> behavior (4 occurrences)

(assuming we're standardizing on American English spellings)

> +	negative_sign		"<U002E>" i.e. "."

I almost said "should be -" but the standard actually says this.
Amusing, since n_sign_posn isn't required.

> +    Like with 30112, POSIX also considers no error if the keywords are
> +    missing, only that if the cateory as a whole is missing the referencing

spelling: cateory -> category (2 occurrences)

> +    In ISO C17 for the "C" locale all values are empty strings "", or
> +    CHAR_MAX, with the exception of decimal_point which is "." (defined
> +    in LC_NUMERIC).

The code actually uses a string with *contains* a CHAR_MAX char, which
confused me.  If this is allowed, this should be documented.  Example:

>        monetary->mon_grouping = (char *) "\177";

> +    Lastly, we must consider the legacy C/POSIX locale that implemented
> +    as a builtin in glibc and wether a default value mapping to the

spelling: wether -> whether

> -	record_error (0, 0, _("%s: field `%s' not defined"),		      \
> -		      "LC_MONETARY", #cat);				      \
> +	record_warning (_("%s: field `%s' not defined"),		      \
> +			"LC_MONETARY", #cat);				      \

Ok.

> +  /* Keyword: int_curr_symbol.  */
>    TEST_ELEM (int_curr_symbol, "");
> -  TEST_ELEM (currency_symbol, "");
> -  TEST_ELEM (mon_thousands_sep, "");
> -  TEST_ELEM (positive_sign, "");
> -  TEST_ELEM (negative_sign, "");
> -

Ok.

>    /* The international currency symbol must come from ISO 4217.  */
>    if (monetary->int_curr_symbol != NULL)
>      {
> @@ -247,41 +325,59 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>  	}
>      }
>  
> -  /* The decimal point must not be empty.  This is not said explicitly
> -     in POSIX but ANSI C (ISO/IEC 9899) says in 4.4.2.1 it has to be
> -     != "".  */

Ok.

> +  /* Keyword: currency_symbol */
> +  TEST_ELEM (currency_symbol, "");

Ok.

> +  /* Keyword: mon_decimal_point */
> +  /* ISO C17 7.11.2.1.3 explicitly allows mon_decimal_point to be the
> +     empty string e.g. "".  This indicates the value is not available in the
> +     current locale or is of zero length.  However, if the value was never
> +     defined then we issue a warning and use a glibc-specific default.  ISO
> +     30112 in the i18n FDCC-Set uses <U002C> ",", and POSIX Issue 7 in the
> +     POSIX locale uses "".  It is specific to glibc that the default is <U002E>
> +     "."; we retain this existing behaviour for backwards compatibility.  */
>    if (monetary->mon_decimal_point == NULL)
>      {
>        if (! nothing)
> -	record_error (0, 0, _("%s: field `%s' not defined"),
> -		      "LC_MONETARY", "mon_decimal_point");
> +	record_warning (_("%s: field `%s' not defined, using defaults"),
> +			"LC_MONETARY", "mon_decimal_point");

Ok.

>        monetary->mon_decimal_point = ".";
>        monetary->mon_decimal_point_wc = L'.';
>      }
> -  else if (monetary->mon_decimal_point[0] == '\0' && ! be_quiet && ! nothing)

Ok.

> +
> +  /* Keyword: mon_thousands_sep */
> +  if (monetary->mon_thousands_sep == NULL)
>      {
> -      record_error (0, 0, _("\
> -%s: value for field `%s' must not be an empty string"),
> -		    "LC_MONETARY", "mon_decimal_point");
> +      if (! nothing)
> +	record_warning (_("%s: field `%s' not defined, using defaults"),
> +			"LC_MONETARY", "mon_thousands_sep");
> +      monetary->mon_thousands_sep = "";
> +      monetary->mon_thousands_sep_wc = L'\0';
>      }

Ok.

> +  /* Keyword: mon_grouping */
>    if (monetary->mon_grouping_len == 0)
>      {
>        if (! nothing)
> -	record_error (0, 0, _("%s: field `%s' not defined"),
> -		      "LC_MONETARY", "mon_grouping");
> -
> +	record_warning (_("%s: field `%s' not defined"),
> +			"LC_MONETARY", "mon_grouping");

Ok.

> +  /* Keyword: positive_sign */
> +  TEST_ELEM (positive_sign, "");
> +
> +  /* Keyword: negative_sign */
> +  TEST_ELEM (negative_sign, "");
> +

Ok.

>  #undef TEST_ELEM
>  #define TEST_ELEM(cat, min, max, initval) \
>    if (monetary->cat == -2)						      \
>      {									      \
>         if (! nothing)							      \
> -	 record_error (0, 0, _("%s: field `%s' not defined"),		      \
> -		       "LC_MONETARY", #cat);				      \
> +	 record_warning (_("%s: field `%s' not defined"),		      \
> +			 "LC_MONETARY", #cat);				      \

Ok.

> @@ -300,16 +396,11 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>    TEST_ELEM (p_sign_posn, -1, 4, -1);
>    TEST_ELEM (n_sign_posn, -1, 4, -1);
>  
> -  /* The non-POSIX.2 extensions are optional.  */
> -  if (monetary->duo_int_curr_symbol == NULL)
> -    monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
> -  if (monetary->duo_currency_symbol == NULL)
> -    monetary->duo_currency_symbol = monetary->currency_symbol;
> -
> -  if (monetary->duo_int_frac_digits == -2)
> -    monetary->duo_int_frac_digits = monetary->int_frac_digits;
> -  if (monetary->duo_frac_digits == -2)
> -    monetary->duo_frac_digits = monetary->frac_digits;

Ok.

> +  /* Keyword: crncystr */
> +  monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
> +					 + 2);
> +  monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
> +  strcpy (&monetary->crncystr[1], monetary->currency_symbol);

Ok.

> +  /* The non-POSIX.2 extensions are optional.  */
> +  if (monetary->duo_int_curr_symbol == NULL)
> +    monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
> +  if (monetary->duo_currency_symbol == NULL)
> +    monetary->duo_currency_symbol = monetary->currency_symbol;
> +
> +  if (monetary->duo_int_frac_digits == -2)
> +    monetary->duo_int_frac_digits = monetary->int_frac_digits;
> +  if (monetary->duo_frac_digits == -2)
> +    monetary->duo_frac_digits = monetary->frac_digits;

Ok.

> @@ -349,17 +451,15 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>    if (monetary->duo_valid_to == 0)
>      monetary->duo_valid_to = 99991231;
>  
> +  /* Keyword: conversion_rate */
>    if (monetary->conversion_rate[0] == 0)
>      {
>        monetary->conversion_rate[0] = 1;
>        monetary->conversion_rate[1] = 1;
>      }
>  
> -  /* Create the crncystr entry.  */
> -  monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
> -					 + 2);
> -  monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
> -  strcpy (&monetary->crncystr[1], monetary->currency_symbol);
> +  /* A value for monetary-decimal-point-wc was set when
> +     monetary_decimal_point was set, likewise for monetary-thousands-sep-wc.  */
>  }

Ok.

Re: [PATCH 1/2] localedef: Update LC_MONETARY handling (Bug 28845)

[Andreas Schwab]

On Feb 04 2022, Carlos O'Donell via Libc-alpha wrote:

> ISO C17, POSIX Issue 7, and ISO 30112 all allow the char*
> types to be empty strings i.e. "", integer or char values to
> be -1 or CHAR_MAX respectively, with the exception of
> decimal_point which must be non-empty in ISO C.
>
> We include a broad comment talking about harmonizing ISO C,
> POSIX, ISO 30112, and the default C/POSIX locale for glibc.
>
> We reorder all setting based on locale/categories.def order.
>
> We soften all missing definitions from errors to warnings when
> defaults exist.
>
> Given that ISO C, POSIX and ISO 30112 allow the empty string
> we change LC_MONETARY handling of mon_decimal_point to allow
> the empty string.  If mon_decimal_point is not defined at all
> then we pick the existing legacy glibc default value of
> <U002E> i.e. ".".
>
> We also set the default for mon_thousands_sep_wc at the
> same time as mon_thousands_sep, but this is not a change in
> behaviour, it is always either a matching value or L'\0',
> but if in the future we change the default to a non-empty
> string we would need to update both at the same time.
>
> Tested on x86_64 and i686 without regressions.
> Tested with install-locale-archive target.
> Tested with install-locale-files target.

Ok.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

[PATCH 2/2] localedata: Do not generate output if warnings were present.

[Carlos O'Donell]

With LC_MONETARY parsing fixed we can now generate locales
without forcing output with '-c'.

Removing '-c' from localedef invocation is the equivalent of
using -Werror for localedef.  The glibc locale sources should
always be clean and free from warnings.

We remove '-c' from both test locale generation and the targets
used for installing locales e.g. install-locale-archive, and
install-locale-files.

Tested on x86_64 and i686 without regressions.
Tested with install-locale-archive target.
Tested with install-locale-files target.
---
 localedata/Makefile      |  3 +--
 localedata/gen-locale.sh | 10 ++++++++--
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/localedata/Makefile b/localedata/Makefile
index 9ae2e5c161..45acfde237 100644
--- a/localedata/Makefile
+++ b/localedata/Makefile
@@ -468,11 +468,10 @@ define build-one-locale
 endef
 
 $(INSTALL-SUPPORTED-LOCALE-ARCHIVE): install-locales-dir
-	@flags="-c"; \
 	$(build-one-locale)
 
 $(INSTALL-SUPPORTED-LOCALE-FILES): install-locales-dir
-	@flags="-c --no-archive --no-hard-links"; \
+	@flags="--no-archive --no-hard-links"; \
 	$(build-one-locale)
 
 tst-setlocale-ENV = LC_ALL=ja_JP.EUC-JP
diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
index 7fce35f212..8053c816a6 100644
--- a/localedata/gen-locale.sh
+++ b/localedata/gen-locale.sh
@@ -54,8 +54,14 @@ modifier=`echo $locfile|sed 's|[^.]*[.]\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\2|'`
 
 echo "Generating locale $locale.$charmap: this might take a while..."
 
-# Run quietly and force output.
-flags="--quiet -c"
+# Do not force output with '-c', all locales should compile without
+# warning or errors.  There is likewise no need to run quietly with
+# '--quiet' since all locales should compile without additional
+# diagnostics.  If there are messages printed then we want to see
+# them, fix them, and the associated error or warning.  During
+# development it may be beneficialy to put '--quiet -c' here to allow
+# you to develop in-progress locales.
+flags=""
 
 # For SJIS the charmap is SHIFT_JIS. We just want the locale to have
 # a slightly nicer name instead of using "*.SHIFT_SJIS", but that
-- 
2.31.1

Re: [PATCH 2/2] localedata: Do not generate output if warnings were present.

[DJ Delorie]

"Carlos O'Donell via Libc-alpha" <libc-alpha@sourceware.org> writes:
> diff --git a/localedata/Makefile b/localedata/Makefile
>  $(INSTALL-SUPPORTED-LOCALE-ARCHIVE): install-locales-dir
> -	@flags="-c"; \
>  	$(build-one-locale)

Ok.  (or do we set this to "" ?)

>  $(INSTALL-SUPPORTED-LOCALE-FILES): install-locales-dir
> -	@flags="-c --no-archive --no-hard-links"; \
> +	@flags="--no-archive --no-hard-links"; \
>  	$(build-one-locale)

Ok.

> diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
> -# Run quietly and force output.
> -flags="--quiet -c"
> +# Do not force output with '-c', all locales should compile without
> +# warning or errors.  There is likewise no need to run quietly with
> +# '--quiet' since all locales should compile without additional
> +# diagnostics.  If there are messages printed then we want to see
> +# them, fix them, and the associated error or warning.  During
> +# development it may be beneficialy to put '--quiet -c' here to allow
> +# you to develop in-progress locales.
> +flags=""

Ok, but this assumes that all locales are fully defined, with no fields
left to their defaults.

LGTM
Reviewed-by: DJ Delorie <dj@redhat.com>

Re: [PATCH 2/2] localedata: Do not generate output if warnings were present.

[Andreas Schwab]

On Feb 04 2022, Carlos O'Donell via Libc-alpha wrote:

> With LC_MONETARY parsing fixed we can now generate locales
> without forcing output with '-c'.
>
> Removing '-c' from localedef invocation is the equivalent of
> using -Werror for localedef.  The glibc locale sources should
> always be clean and free from warnings.
>
> We remove '-c' from both test locale generation and the targets
> used for installing locales e.g. install-locale-archive, and
> install-locale-files.
>
> Tested on x86_64 and i686 without regressions.
> Tested with install-locale-archive target.
> Tested with install-locale-files target.

Ok.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."