ja_JP locale: Fix the offset in era-string for Taisho gan-nen [BZ #24162]

  The offset in era-string format for Taisho gan-nen (1912) is currently
defined as 2, but it should be 1.  So fix it.  "gan-nen" means the 1st
(origin) year, Taisho started on Jul 30, 1912.

ChangeLog:

	[BZ #24162]
	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format
	for Taisho gan-nen is currently defined as 2, but it should be 1.  So
	fix it.
---
 localedata/locales/ja_JP | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Hello,

This patch fixes an obvious typo that is hidden for a long time since
ja_JP localedata was created.  Is it OK for commit?

This issue is reported by "junji morimitsu" via Bugzilla [1].

[1] https://sourceware.org/bugzilla/show_bug.cgi?id=24162

So, I would like to use the Reported-by: tag for him with his
permission.  Is that all right?

From: TAMUKI Shoichi <tamuki@linet.gr.jp>
Subject: [PATCH] ja_JP locale: Fix the offset in era-string for Taisho gan-nen [BZ #24162]
Date: Fri, 08 Feb 2019 17:41:14 +0900

> The offset in era-string format for Taisho gan-nen (1912) is currently
> defined as 2, but it should be 1.  So fix it.  "gan-nen" means the 1st
> (origin) year, Taisho started on Jul 30, 1912.
> 
> ChangeLog:
> 
> 	[BZ #24162]
> 	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format
> 	for Taisho gan-nen is currently defined as 2, but it should be 1.  So
> 	fix it.
> ---
>  localedata/locales/ja_JP | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP
> index 1fd2fee..9bfbb2b 100644
> --- a/localedata/locales/ja_JP
> +++ b/localedata/locales/ja_JP
> @@ -14951,7 +14951,7 @@ era	"+:2:1990//01//01:+*:<U5E73><U6210>:%EC%Ey<U5E74>";/
>  	"+:2:1927//01//01:1989//01//07:<U662D><U548C>:%EC%Ey<U5E74>";/
>  	"+:1:1926//12//25:1926//12//31:<U662D><U548C>:%EC<U5143><U5E74>";/
>  	"+:2:1913//01//01:1926//12//24:<U5927><U6B63>:%EC%Ey<U5E74>";/
> -	"+:2:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
> +	"+:1:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>  	"+:6:1873//01//01:1912//07//29:<U660E><U6CBB>:%EC%Ey<U5E74>";/
>  	"+:1:0001//01//01:1872//12//31:<U897F><U66A6>:%EC%Ey<U5E74>";/
>  	"+:1:-0001//12//31:-*:<U7D00><U5143><U524D>:%EC%Ey<U5E74>"
> -- 
> 2.2.1
  

Hello TAMUKI-san,

I am sorry for this long silence.  I have tested your patch and here is
what it does:

1. Non-patched version:

$ LC_ALL=ja_JP.utf8 date -d "1913-04-05" +"%d %m %Ey"
05 04 2
$ LC_ALL=ja_JP.utf8 date -d "1912-11-22" +"%d %m %Ey"
22 11 2

2. Patched version:

$ LC_ALL=ja_JP.UTF-8 ./testrun_loc.sh /usr/bin/date -d "1913-04-05" +"%d %m
%Ey"
05 04 02
$ LC_ALL=ja_JP.UTF-8 ./testrun_loc.sh /usr/bin/date -d "1912-11-22" +"%d %m
%Ey"
22 11 01

So the issue is that the year 1912 (or at least part of it) is
displayed as the year 2 while it should be 1 and year 2 should
be only 1913, and this patch fixes it, is that correct?

Your patch is correct, I have only few nitpicks which you may
agree with or ignore, they don't change much in your patch.

Could you please shorten the first line of your patch, which is also
the subject line of your email?  If it was only 1 character shorter
it would fit in one line when listing with "git log --oneline" on
a traditional 80-columns terminal.  I think that if you remove the
word "locale" it is OK.  No problem if you don't want to change it,
some people do not keep the 72 columns limit and nobody complains.

8.02.2019 09:41 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> 
> The offset in era-string format for Taisho gan-nen (1912) is currently
> defined as 2, but it should be 1.  So fix it.  "gan-nen" means the 1st

Is is obligatory to write "gan-nen" in lowercase characters even if
it is in the beginning of a sentence, like Unix command names which are
case sensitive and therefore always written in lowercase?  If it does
not break some rules which I am not aware of then please change to
"Gan-nen".

> (origin) year, Taisho started on Jul 30, 1912.

I think it is OK to write unabbreviated "July" here.

> ChangeLog:
> 
> 	[BZ #24162]
> 	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format

Somebody fix me if I'm wrong but I usually write here:

	* localedata/locales/ja_JP (era):

> 	for Taisho gan-nen is currently defined as 2, but it should be 1.  So
> 	fix it.

I can't find the proper reference now but AFAIR the changes should be
described
in an imperative mode, something like:

    Change the offset for Taisho gan-nen from 2 to 1.

No need to mention the era-string because we have already narrowed our
change
down to the era.

You don't have to take this literally, just take it as an inspiration to
modify
according to your actual needs.

Also, the ChangeLog entries do not have to be exact copies of the commit
message.
It's even better when they are shorter and the commit message contains the
full
explanation.

> diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP
> index 1fd2fee..9bfbb2b 100644
> --- a/localedata/locales/ja_JP
> +++ b/localedata/locales/ja_JP
> @@ -14951,7 +14951,7 @@ era
> "+:2:1990//01//01:+*:<U5E73><U6210>:%EC%Ey<U5E74>";/
>  	"+:2:1927//01//01:1989//01//07:<U662D><U548C>:%EC%Ey<U5E74>";/
>  	"+:1:1926//12//25:1926//12//31:<U662D><U548C>:%EC<U5143><U5E74>";/
>  	"+:2:1913//01//01:1926//12//24:<U5927><U6B63>:%EC%Ey<U5E74>";/
> -	"+:2:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
> +	"+:1:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>  	"+:6:1873//01//01:1912//07//29:<U660E><U6CBB>:%EC%Ey<U5E74>";/
>  	"+:1:0001//01//01:1872//12//31:<U897F><U66A6>:%EC%Ey<U5E74>";/
>  	"+:1:-0001//12//31:-*:<U7D00><U5143><U524D>:%EC%Ey<U5E74>"

OK with the remarks mentioned above.

Reviewed-by: Rafal Luzynski <digitalfreak@lingonborough.com>

Also I think that the issue is important and this patch does not depend
on other patches so it is also OK to backport it to old stable branches.
However, I'd like to hear an opinion of other maintainers about it.

15.02.2019 08:41 TAMUKI Shoichi <tamuki@linet.gr.jp> wrote:
> [...]
> 
> This issue is reported by "junji morimitsu" via Bugzilla [1].
> 
> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=24162
> 
> So, I would like to use the Reported-by: tag for him with his
> permission.  Is that all right?

I think we are not using "Reported-by:" tag officially.  So if you want
to credit the bug reporter you may just mention him/her in the free text
part of the commit message.  Finally, it's not hurting if you use the
"Reported-by:" tag, it will just do nothing.

Thank you for your patch, please commit, optionally after applying
my remarks.

Regards,

Rafal
  

On 2/18/19 6:27 PM, Rafal Luzynski wrote:
>> diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP
>> index 1fd2fee..9bfbb2b 100644
>> --- a/localedata/locales/ja_JP
>> +++ b/localedata/locales/ja_JP
>> @@ -14951,7 +14951,7 @@ era
>> "+:2:1990//01//01:+*:<U5E73><U6210>:%EC%Ey<U5E74>";/
>>  	"+:2:1927//01//01:1989//01//07:<U662D><U548C>:%EC%Ey<U5E74>";/
>>  	"+:1:1926//12//25:1926//12//31:<U662D><U548C>:%EC<U5143><U5E74>";/
>>  	"+:2:1913//01//01:1926//12//24:<U5927><U6B63>:%EC%Ey<U5E74>";/
>> -	"+:2:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>> +	"+:1:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>>  	"+:6:1873//01//01:1912//07//29:<U660E><U6CBB>:%EC%Ey<U5E74>";/
>>  	"+:1:0001//01//01:1872//12//31:<U897F><U66A6>:%EC%Ey<U5E74>";/
>>  	"+:1:-0001//12//31:-*:<U7D00><U5143><U524D>:%EC%Ey<U5E74>"
> 
> OK with the remarks mentioned above.
> 
> Reviewed-by: Rafal Luzynski <digitalfreak@lingonborough.com>
> 
> Also I think that the issue is important and this patch does not depend
> on other patches so it is also OK to backport it to old stable branches.
> However, I'd like to hear an opinion of other maintainers about it.

This is hard to judge. Sometimes backporting date changes like this can
cause sorted dates to change their order, and that's not expected in a
stable release. I would be cautious and not backport this.
  

20.02.2019 08:54 Carlos O'Donell <carlos@redhat.com> wrote:
> 
> On 2/18/19 6:27 PM, Rafal Luzynski wrote:
> >> diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP
> >> index 1fd2fee..9bfbb2b 100644
> >> --- a/localedata/locales/ja_JP
> >> +++ b/localedata/locales/ja_JP
> >> @@ -14951,7 +14951,7 @@ era
> >> "+:2:1990//01//01:+*:<U5E73><U6210>:%EC%Ey<U5E74>";/
> >>  	"+:2:1927//01//01:1989//01//07:<U662D><U548C>:%EC%Ey<U5E74>";/
> >>  	"+:1:1926//12//25:1926//12//31:<U662D><U548C>:%EC<U5143><U5E74>";/
> >>  	"+:2:1913//01//01:1926//12//24:<U5927><U6B63>:%EC%Ey<U5E74>";/
> >> -	"+:2:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
> >> +	"+:1:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
> >>  	"+:6:1873//01//01:1912//07//29:<U660E><U6CBB>:%EC%Ey<U5E74>";/
> >>  	"+:1:0001//01//01:1872//12//31:<U897F><U66A6>:%EC%Ey<U5E74>";/
> >>  	"+:1:-0001//12//31:-*:<U7D00><U5143><U524D>:%EC%Ey<U5E74>"
> > 
> > OK with the remarks mentioned above.
> > 
> > Reviewed-by: Rafal Luzynski <digitalfreak@lingonborough.com>
> > 
> > Also I think that the issue is important and this patch does not depend
> > on other patches so it is also OK to backport it to old stable branches.
> > However, I'd like to hear an opinion of other maintainers about it.
> 
> This is hard to judge. Sometimes backporting date changes like this can
> cause sorted dates to change their order, and that's not expected in a
> stable release. I would be cautious and not backport this.

If I understand correctly, a new Japanese era began on 1912-07-30 and
from that day til the end of the year the year number should be 1.
Currently that year number is 2 in glibc which is incorrect.  Also whole
year 1913 has the same number 2 in glibc which is correct.  This already
breaks sorting.  I think that backporting this may only help and will
not break anything.

Do you sustain your objection against backporting?  If not, do you have
any suggestion how many stable versions should be fixed?

Regards,

Rafal
  

On 2/20/19 5:29 PM, Rafal Luzynski wrote:
> 20.02.2019 08:54 Carlos O'Donell <carlos@redhat.com> wrote:
>>
>> On 2/18/19 6:27 PM, Rafal Luzynski wrote:
>>>> diff --git a/localedata/locales/ja_JP b/localedata/locales/ja_JP
>>>> index 1fd2fee..9bfbb2b 100644
>>>> --- a/localedata/locales/ja_JP
>>>> +++ b/localedata/locales/ja_JP
>>>> @@ -14951,7 +14951,7 @@ era
>>>> "+:2:1990//01//01:+*:<U5E73><U6210>:%EC%Ey<U5E74>";/
>>>>  	"+:2:1927//01//01:1989//01//07:<U662D><U548C>:%EC%Ey<U5E74>";/
>>>>  	"+:1:1926//12//25:1926//12//31:<U662D><U548C>:%EC<U5143><U5E74>";/
>>>>  	"+:2:1913//01//01:1926//12//24:<U5927><U6B63>:%EC%Ey<U5E74>";/
>>>> -	"+:2:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>>>> +	"+:1:1912//07//30:1912//12//31:<U5927><U6B63>:%EC<U5143><U5E74>";/
>>>>  	"+:6:1873//01//01:1912//07//29:<U660E><U6CBB>:%EC%Ey<U5E74>";/
>>>>  	"+:1:0001//01//01:1872//12//31:<U897F><U66A6>:%EC%Ey<U5E74>";/
>>>>  	"+:1:-0001//12//31:-*:<U7D00><U5143><U524D>:%EC%Ey<U5E74>"
>>>
>>> OK with the remarks mentioned above.
>>>
>>> Reviewed-by: Rafal Luzynski <digitalfreak@lingonborough.com>
>>>
>>> Also I think that the issue is important and this patch does not depend
>>> on other patches so it is also OK to backport it to old stable branches.
>>> However, I'd like to hear an opinion of other maintainers about it.
>>
>> This is hard to judge. Sometimes backporting date changes like this can
>> cause sorted dates to change their order, and that's not expected in a
>> stable release. I would be cautious and not backport this.
> 
> If I understand correctly, a new Japanese era began on 1912-07-30 and
> from that day til the end of the year the year number should be 1.
> Currently that year number is 2 in glibc which is incorrect.  Also whole
> year 1913 has the same number 2 in glibc which is correct.  This already
> breaks sorting.  I think that backporting this may only help and will
> not break anything.
> 
> Do you sustain your objection against backporting?  If not, do you have
> any suggestion how many stable versions should be fixed?

My objection is not sustained. I only wanted to point out that in a stable
release customers expect sortings to be stable and correctness is not always
an important factor. This is what I have seen in my experience working on
RHEL as an enterprise distribution. A bug is something you can document, but
once a list order changes it can have a chain of consequences that are harder
to fix. So for enterprise: change is the enemy.

This is upstream glibc, and so we can argue that the value of the fix is higher
than the value of a stable sorting. So I can support backporting the fix to
a stable branch if required, but unless someone is asking for it, I would not
backport it.
  

21.02.2019 03:11 Carlos O'Donell <carlos@redhat.com> wrote:
> On 2/20/19 5:29 PM, Rafal Luzynski wrote:
> > [...]
> > If I understand correctly, a new Japanese era began on 1912-07-30 and
> > from that day til the end of the year the year number should be 1.
> > Currently that year number is 2 in glibc which is incorrect.  Also whole
> > year 1913 has the same number 2 in glibc which is correct.  This already
> > breaks sorting.  I think that backporting this may only help and will
> > not break anything.
> > 
> > Do you sustain your objection against backporting?  If not, do you have
> > any suggestion how many stable versions should be fixed?
> 
> My objection is not sustained. I only wanted to point out that in a stable
> release customers expect sortings to be stable and correctness is not
> always
> an important factor. This is what I have seen in my experience working on
> RHEL as an enterprise distribution. A bug is something you can document,
> but
> once a list order changes it can have a chain of consequences that are
> harder
> to fix. So for enterprise: change is the enemy.
> 
> This is upstream glibc, and so we can argue that the value of the fix is
> higher
> than the value of a stable sorting. So I can support backporting the fix
> to
> a stable branch if required, but unless someone is asking for it, I would
> not
> backport it.

OK, I understand this and I will not insist on backporting to multiple
stable branches.  However, I would like to suggest backporting to 2.29
because:

* it has been released only recently and hopefully has not yet reached many
  production systems,
* if any sorting using the Japanese calendar exists it is broken because
  of this bug already,
* sorting dates by strftime() to Japanese calendar does not look to me
  as a good idea at all.

TAMUKI-san, while we are discussing here backporting we agree that you
can push this patch to master already, optionally applying suggestions
from my review.  Also, your opinion whether to backport or not will be
valuable here.

Regards,

Rafal
  

On 2/21/19 6:49 PM, Rafal Luzynski wrote:
> OK, I understand this and I will not insist on backporting to multiple
> stable branches.  However, I would like to suggest backporting to 2.29
> because:
> 
> * it has been released only recently and hopefully has not yet reached many
>   production systems,
> * if any sorting using the Japanese calendar exists it is broken because
>   of this bug already,
> * sorting dates by strftime() to Japanese calendar does not look to me
>   as a good idea at all.

All good reasons. I think backporting to 2.29 is reasonable, conservative,
and serves downstream distributions which are only just adopting 2.29.
  

Hello Rafal-san,

Sorry to be late replying.  Thank you for the detailed review on this
patch.

From: Rafal Luzynski <digitalfreak@lingonborough.com>
Subject: Re: [PATCH] ja_JP locale: Fix the offset in era-string for Taisho gan-nen [BZ #24162]
Date: Tue, 19 Feb 2019 00:27:36 +0100 (CET)

> 1. Non-patched version:
> 
> $ LC_ALL=ja_JP.utf8 date -d "1913-04-05" +"%d %m %Ey"
> 05 04 2
> $ LC_ALL=ja_JP.utf8 date -d "1912-11-22" +"%d %m %Ey"
> 22 11 2
> 
> 2. Patched version:
> 
> $ LC_ALL=ja_JP.UTF-8 ./testrun_loc.sh /usr/bin/date -d "1913-04-05" +"%d %m %Ey"
> 05 04 02
> $ LC_ALL=ja_JP.UTF-8 ./testrun_loc.sh /usr/bin/date -d "1912-11-22" +"%d %m %Ey"
> 22 11 01
> 
> So the issue is that the year 1912 (or at least part of it) is
> displayed as the year 2 while it should be 1 and year 2 should
> be only 1913, and this patch fixes it, is that correct?

Yes, that is correct.

> Could you please shorten the first line of your patch, which is also
> the subject line of your email?  If it was only 1 character shorter
> it would fit in one line when listing with "git log --oneline" on
> a traditional 80-columns terminal.  I think that if you remove the
> word "locale" it is OK.  No problem if you don't want to change it,
> some people do not keep the 72 columns limit and nobody complains.

OK.  I will remove the word "locale" with 's/ locale//'.  When listing
with "git log --oneline" on a traditional 80-columns terminal, the
commit message summary should be within 72-colmuns with the first 7
characters of the commit hash (and 1 space).  If there is a bugzilla
associated with the patch ([BZ #XXXXX]), we need to deduct 12 more
columns.  So, we need to describe the commit message summary within
60-columns.

> > The offset in era-string format for Taisho gan-nen (1912) is currently
> > defined as 2, but it should be 1.  So fix it.  "gan-nen" means the 1st
> 
> Is is obligatory to write "gan-nen" in lowercase characters even if
> it is in the beginning of a sentence, like Unix command names which are
> case sensitive and therefore always written in lowercase?  If it does
> not break some rules which I am not aware of then please change to
> "Gan-nen".

Absolutely agree.  I will fix it.

> > (origin) year, Taisho started on Jul 30, 1912.
> 
> I think it is OK to write unabbreviated "July" here.

OK.  I will fix it.

> > ChangeLog:
> > 
> > 	[BZ #24162]
> > 	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format
> 
> Somebody fix me if I'm wrong but I usually write here:
> 
> 	* localedata/locales/ja_JP (era):

I checked with the following command and confirmed that both are
commonly used.

$ git log | grep "^    "$'\t'"\* localedata/locales/"

> > 	for Taisho gan-nen is currently defined as 2, but it should be 1.  So
> > 	fix it.
> 
> I can't find the proper reference now but AFAIR the changes should be described
> in an imperative mode, something like:
> 
>     Change the offset for Taisho gan-nen from 2 to 1.

OK.

> Also I think that the issue is important and this patch does not depend
> on other patches so it is also OK to backport it to old stable branches.
> However, I'd like to hear an opinion of other maintainers about it.

As you are discussing here backporting, I think it is better to
backport to 2.29 only.

Regards,
TAMUKI Shoichi
  

23.02.2019 o 08:29 TAMUKI Shoichi <tamuki@linet.gr.jp> napisał(a):
> 
> 
> Hello Rafal-san,
> 
> Sorry to be late replying.  Thank you for the detailed review on this
> patch.
> 
> From: Rafal Luzynski <digitalfreak@lingonborough.com>
> Subject: Re: [PATCH] ja_JP locale: Fix the offset in era-string for Taisho
> gan-nen [BZ #24162]
> Date: Tue, 19 Feb 2019 00:27:36 +0100 (CET)
> 
> > [...]
> > Could you please shorten the first line of your patch, which is also
> > the subject line of your email?  If it was only 1 character shorter
> > it would fit in one line when listing with "git log --oneline" on
> > a traditional 80-columns terminal.  I think that if you remove the
> > word "locale" it is OK.  No problem if you don't want to change it,
> > some people do not keep the 72 columns limit and nobody complains.
> 
> OK.  I will remove the word "locale" with 's/ locale//'.  When listing
> with "git log --oneline" on a traditional 80-columns terminal, the
> commit message summary should be within 72-colmuns with the first 7
> characters of the commit hash (and 1 space).  If there is a bugzilla
> associated with the patch ([BZ #XXXXX]), we need to deduct 12 more
> columns.  So, we need to describe the commit message summary within
> 60-columns.

Thank you.

> [...]
> > > ChangeLog:
> > > 
> > > 	[BZ #24162]
> > > 	* localedata/locales/ja_JP (LC_TIME): The offset in era-string format
> > 
> > Somebody fix me if I'm wrong but I usually write here:
> > 
> > 	* localedata/locales/ja_JP (era):
> 
> I checked with the following command and confirmed that both are
> commonly used.
> 
> $ git log | grep "^    "$'\t'"\* localedata/locales/"

Sure.  ChangeLog guidelines are ambiguous so you may consider both
as correct.  What I write here is my personal opinion, my may agree
with it or disagree.  I do not discuss the body of your patch because
it is correct, nothing I can add.

Also, I forgot to say, I think you wanted to mention the reporter
in your ChangeLog.  Feel free to mention.  That we don't officially
use "Reported-by:" tag does not mean you can't just mention who found
the bug.

> [...]
> > Also I think that the issue is important and this patch does not depend
> > on other patches so it is also OK to backport it to old stable branches.
> > However, I'd like to hear an opinion of other maintainers about it.
> 
> As you are discussing here backporting, I think it is better to
> backport to 2.29 only.

Yes, we have all agreed on this.

Please see how other backported patches look and format yours the same.
Hint 1: use "git cherry-pick -x" to copy your commit from master to your
local copy of 2.29 branch.  Hint 2: You should update NEWS file in the
branch manually.  Yes, lots of people forget it but please do it.
Hint 3: Most likely you will have a conflict in the ChangeLog, you
will have to resolve it manually.

Thank you for your work on this patch.

Regards,

Rafal