From patchwork Mon Oct 23 22:33:13 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Carlos O'Donell X-Patchwork-Id: 23771 Received: (qmail 64576 invoked by alias); 23 Oct 2017 22:33:22 -0000 Mailing-List: contact libc-alpha-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: libc-alpha-owner@sourceware.org Delivered-To: mailing list libc-alpha@sourceware.org Received: (qmail 64557 invoked by uid 89); 23 Oct 2017 22:33:21 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.4 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_LAZY_DOMAIN_SECURITY, RCVD_IN_DNSWL_NONE, RCVD_IN_SORBS_SPAM autolearn=ham version=3.3.2 spammy= X-HELO: mail-qt0-f177.google.com X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:organization :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=rqJcCM+E8g+KkD5XbER8dMYV5b4KSgzsSB9eG0ZqAQU=; b=dUAkqYsi92jcfEbNHnmnyKMVocHQXTyrSDQdOLGV5L9Nf6keszzgF4kZi36/Bqv5RO w0eDUPUxCJp5oxTtordfrT3wmFMXvp0ZEQ0NXwVip+PTJBWs7NvwA0pvKZ8O1AvlJ4oW RlTafAOwExg/StBKPvCZ128SLkKkSLvwnztcrGY4reppzOOF1kxCQ337z9qDKV2diinf /FFoc0heqBeEysillLgPoOg7jUZ+yZffhiPTStML5KfwgW4+xECyxZZtpLUFEwJnsGTN S0MtIW+fOWrQwbErFRHGcqXe/3AgN6N8BHBLirQbc26IqUOH9dx20MkxR4ALnOHSshaf tD3g== X-Gm-Message-State: AMCzsaXuGhare3jXnvROHEiu882wAbwc9YrV5jWGZHabQlVxijRMNjhi 0TygGaFTnUbNGMcLeH+as1HxatHh8I4= X-Google-Smtp-Source: ABhQp+S3R46kxblcJw5olkNQCV7ie92OplL6M/7yLxlu8hjZtL7bG0pVs6DanrZ3liGUayD0eEgK7w== X-Received: by 10.200.48.37 with SMTP id f34mr22747743qte.228.1508797997490; Mon, 23 Oct 2017 15:33:17 -0700 (PDT) Subject: Re: [PATCH] localedata: Reorganize Unicode LC_CTYPE inclusion. To: Rafal Luzynski , GNU C Library , Mike Fabian References: <393503973.1212392.1508754068809@poczta.nazwa.pl> From: Carlos O'Donell Message-ID: <1a82e6bb-ff89-3fd9-fc60-4aeaf8762181@redhat.com> Date: Mon, 23 Oct 2017 15:33:13 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <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 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? --- 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. 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