This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] localedata: Reorganize Unicode LC_CTYPE inclusion.
- From: Carlos O'Donell <carlos at redhat dot com>
- To: Rafal Luzynski <digitalfreak at lingonborough dot com>, GNU C Library <libc-alpha at sourceware dot org>, Mike Fabian <mfabian at redhat dot com>
- Date: Mon, 23 Oct 2017 15:33:13 -0700
- Subject: Re: [PATCH] localedata: Reorganize Unicode LC_CTYPE inclusion.
- Authentication-results: sourceware.org; auth=none
- References: <be8121c4-f5f3-9cd4-4d66-01fc318189cb@redhat.com> <393503973.1212392.1508754068809@poczta.nazwa.pl>
On 10/23/2017 03:21 AM, Rafal Luzynski wrote:
> Sorry for the late review but here it goes. I've found few minor
> nitpicks and one major problem.
Thank you!
> 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.
It was intentional.
The file is now `i18n_ctype` so I wanted the report to reflect that it
was producing a report for that file.
>> 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.
Correct.
> 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.
Not at all. Your changes look good!
Please feel free to adjust the file accordingly.
Something like this?
diff --git a/localedata/locales/i18n_ctype b/localedata/locales/i18n_ctype
index da88efc..a4ecc65 100644
--- a/localedata/locales/i18n_ctype
+++ b/localedata/locales/i18n_ctype
@@ -8,37 +8,26 @@ comment_char %
% 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.
-%
-LC_IDENTIFICATION
-title ""
-source ""
-address ""
-contact ""
-email ""
-tel ""
-fax ""
-language ""
-territory ""
-revision ""
-date "2017-10-11"
+% Generated automatically by gen_unicode_ctype.py for Unicode 10.0.0.
-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
+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-23"
+category "unicode:2014";LC_CTYPE
END LC_IDENTIFICATION
LC_CTYPE
---
Looks good to me.
>> 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.
This was on purpose. The report relates to the i18n_ctype file data only.
>>
>> 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:
Thank you for spotting this!
>> 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.
I will fix this immediately and commit the changes. Thank you again for the review.
--
Cheers,
Carlos.