Re: [PATCH] localedata: Reorganize Unicode LC_CTYPE inclusion.

[Rafal Luzynski <digitalfreak at lingonborough dot com>]

Hi Carlos,

Sorry for the late review but here it goes.  I've found few minor
nitpicks and one major problem.

13.10.2017 08:49 Carlos O'Donell <carlos@redhat.com> wrote:
>
> [...]
> (i18n-report): Rename to...
> (i18n_ctype-report): ...this.

Was this change intentional?  This is just the name of the report file
generated by make check.  Technically it does not matter what is its
name.  You didn't have to change it but it's OK if you have decided
to do this.

> diff --git a/localedata/locales/i18n_ctype b/localedata/locales/i18n_ctype
> new file mode 100644
> index 0000000..da88efc
> --- /dev/null
> +++ b/localedata/locales/i18n_ctype
> @@ -0,0 +1,2343 @@
> +escape_char /
> +comment_char %
> +
> +% This file is part of the GNU C Library and contains locale data.
> +% The Free Software Foundation does not claim any copyright interest
> +% in the locale data contained in this file.  The foregoing does not
> +% affect the license of the GNU C Library as a whole.  It does not
> +% exempt you from the conditions of the license if your use would
> +% otherwise be governed by that license.
> +
> +%
> +% The only purpose for this file is to provide the basic Unicode
> +% LC_CTYPE information.  This way other locales that need this
> +% information, but with different transliterations, can include it
> +% directly.

This is probably a minor problem but I've tried to redo your work
and in my regenerated file I've received this single line instead:

% Generated automatically by gen_unicode_ctype.py for Unicode 10.0.0.

My guess is that you've first copied localedata/locales/i18n to
i18n_ctype and then run `make all'.  The script tried not to change
the comments and other lines which were already there.  I used
a different technique: created an empty file i18n_ctype and run
`make all' to make it fully generated from scratch.

Also see more here:

> +LC_IDENTIFICATION
> +title                 ""
> +source                ""
> +address               ""
> +contact               ""
> +email                 ""
> +tel                   ""
> +fax                   ""
> +language              ""
> +territory             ""
> +revision              ""
> +date                  "2017-10-11"
> +
> +category "i18n:2012";LC_IDENTIFICATION
> +category "i18n:2012";LC_CTYPE
> +category "i18n:2012";LC_COLLATE
> +category "i18n:2012";LC_TIME
> +category "i18n:2012";LC_NUMERIC
> +category "i18n:2012";LC_MONETARY
> +category "i18n:2012";LC_MESSAGES
> +category "i18n:2012";LC_PAPER
> +category "i18n:2012";LC_NAME
> +category "i18n:2012";LC_ADDRESS
> +category "i18n:2012";LC_TELEPHONE
> +END LC_IDENTIFICATION

My result here is:

LC_IDENTIFICATION
title     "Unicode 10.0.0 FDCC-set"
source    "UnicodeData.txt, DerivedCoreProperties.txt"
address   ""
contact   ""
email     "bug-glibc-locales@gnu.org"
tel       ""
fax       ""
language  ""
territory "Earth"
revision  "10.0.0"
date      "2017-10-22"
category  "unicode:2014";LC_CTYPE
END LC_IDENTIFICATION

I'm not sure if these differences are really relevant so I don't
mind if you refuse accepting my suggestions.

> diff --git a/localedata/unicode-gen/Makefile b/localedata/unicode-gen/Makefile
> index 4564670..32f4a53 100644
> --- a/localedata/unicode-gen/Makefile
> +++ b/localedata/unicode-gen/Makefile
> @@ -20,7 +20,7 @@
>  
>  # This Makefile is NOT used as part of the GNU libc build.  It needs
>  # to be run manually, within the source tree, at Unicode upgrades
> -# (change UNICODE_VERSION below), to update ../locales/i18n ctype
> +# (change UNICODE_VERSION below), to update ../locales/i18n_ctype ctype
>  # information (part of the file is preserved, so don't wipe it all
>  # out), and ../charmaps/UTF-8.
>  
> @@ -41,15 +41,15 @@ PYTHON3 = python3
>  WGET = wget
>  
>  DOWNLOADS = UnicodeData.txt DerivedCoreProperties.txt EastAsianWidth.txt
> PropList.txt
> -GENERATED = i18n tr_TR UTF-8 translit_combining translit_compat
> translit_circle translit_cjk_compat translit_font translit_fraction
> -REPORTS = i18n-report UTF-8-report
> +GENERATED = i18n_ctype tr_TR UTF-8 translit_combining translit_compat
> translit_circle translit_cjk_compat translit_font translit_fraction
> +REPORTS = i18n_ctype-report UTF-8-report

My comments about i18n_ctype-report file apply here as well.

>  
>  all: $(GENERATED)
>  
> -check: check-i18n check-UTF-8
> +check: check-i18n_ctype check-UTF-8

Here is the major problem: either you should consequently change
check-i18n to check-i18n_ctype everywhere or not change it at all.
Look below:

>  check-i18n: i18n-report
>  	@if grep '\(Missing\|Added\) [^0]\|^Number of errors[^=]* = [^0]' \
> -- 
> 2.7.5

here check-i18n is not changed.  As a result, `make check' fails:

$ make check
make: *** No rule to make target 'check-i18n_ctype', needed by 'check'.  Stop.

I will appreciate if you fix these problem on your own, the last problem
at least.

Regards,

Rafal