DAY_TO_STR() localization

[alejandro-colomar]

Cc: @eggert Cc: @timparenti Cc: @kenion

---

Revisions:

v2

- Print localized dates and times by default. - Do not use custom formats. - Print timezone information together with date strings, as an RFC 9557 suffix. - Fix STRFTIME() macro ``` $ git range-diff shadow/master gh/time time 1: 2686a033 < -: -------- src/chage.c: Always use ISO 8601 format (YYYY-MM-DD) for printing dates -: -------- > 1: 68a97728 lib/, src/: Use %F instead of %Y-%m-%d with strftime(3) -: -------- > 2: 56067591 src/: Use %c for printing localized date× with strftime(3) -: -------- > 3: 928172c4 src/chage.c: print_day_as_date(): Use %x for printing localized dates with strftime(3) 2: de6f93bf = 4: e78f9782 src/chage.c: print_day_as_date(): Simplify error handling -: -------- > 5: fc46d1f3 lib/time/, src/: day_to_str(): Add support for formatting localized dates 3: bd7701fa ! 6: b2c33c58 src/chage.c: print_day_as_date(): Call DAY_TO_STR() to de-duplicate code @@ src/chage.c: static int new_fields (void) - return; - } - -- STRFTIME(buf, "%Y-%m-%d", &tm); +- STRFTIME(buf, iflg ? "%F" : "%x", &tm); - (void) puts (buf); -+ DAY_TO_STR(buf, day); ++ DAY_TO_STR(buf, day, iflg); + puts(buf); } -: -------- > 7: 9c0ee33c lib/time/day_to_str.h: day_to_str(): Print numeric timezone together with local dates -: -------- > 8: 41a12dfe lib/string/strftime.h: STRFTIME(): Tighten macro definition ```

v3

- Use the time zone name/abbreviation rather than the numeric offset. It's strongly preferred by RFC 9557. [@eggert] ``` $ git range-diff shadow/master gh/time time 1: 68a97728 = 1: 68a97728 lib/, src/: Use %F instead of %Y-%m-%d with strftime(3) 2: 56067591 = 2: 56067591 src/: Use %c for printing localized date× with strftime(3) 3: 928172c4 = 3: 928172c4 src/chage.c: print_day_as_date(): Use %x for printing localized dates with strftime(3) 4: e78f9782 = 4: e78f9782 src/chage.c: print_day_as_date(): Simplify error handling 5: fc46d1f3 = 5: fc46d1f3 lib/time/, src/: day_to_str(): Add support for formatting localized dates 6: b2c33c58 = 6: b2c33c58 src/chage.c: print_day_as_date(): Call DAY_TO_STR() to de-duplicate code 7: 9c0ee33c ! 7: db4f966b lib/time/day_to_str.h: day_to_str(): Print numeric timezone together with local dates @@ Metadata Author: Alejandro Colomar ## Commit message ## - lib/time/day_to_str.h: day_to_str(): Print numeric timezone together with local dates + lib/time/day_to_str.h: day_to_str(): Print timezone together with local dates Print it as a RFC 9557 suffix, which doesn't add any extra whitespace. This reduces the damage to expectations of scripts that might parse @@ Commit message Link: Link: Link: + Link: Suggested-by: Alejandro Colomar Suggested-by: Paul Eggert Cc: "Serge E. Hallyn" Cc: Tim Parenti Cc: Gus Kenion Cc: Iker Pedrosa + Cc: Bruno Haible Signed-off-by: Alejandro Colomar ## lib/time/day_to_str.h ## @@ lib/time/day_to_str.h: day_to_str(size_t size, char buf[size], long day, bool is } - if (strftime(buf, size, iso ? "%F" : "%x", &tm) == 0) -+ if (strftime(buf, size, iso ? "%F[%z]" : "%x[%z]", &tm) == 0) ++ if (strftime(buf, size, iso ? "%F[%Z]" : "%x[%Z]", &tm) == 0) strtcpy(buf, "future", size); } 8: 41a12dfe = 8: bf3481df lib/string/strftime.h: STRFTIME(): Tighten macro definition ```

v4

- Don't use a RFC 9557 suffix; that is, don't print timezone info. date(1) doesn't yet support them, so let's wait until that happens. ``` $ git range-diff shadow/master gh/time time 1: 68a97728 = 1: 68a97728 lib/, src/: Use %F instead of %Y-%m-%d with strftime(3) 2: 56067591 = 2: 56067591 src/: Use %c for printing localized date× with strftime(3) 3: 928172c4 = 3: 928172c4 src/chage.c: print_day_as_date(): Use %x for printing localized dates with strftime(3) 4: e78f9782 = 4: e78f9782 src/chage.c: print_day_as_date(): Simplify error handling 5: fc46d1f3 = 5: fc46d1f3 lib/time/, src/: day_to_str(): Add support for formatting localized dates 6: b2c33c58 = 6: b2c33c58 src/chage.c: print_day_as_date(): Call DAY_TO_STR() to de-duplicate code 7: db4f966b < -: -------- lib/time/day_to_str.h: day_to_str(): Print timezone together with local dates 8: bf3481df = 7: 62b553e3 lib/string/strftime.h: STRFTIME(): Tighten macro definition ```

v5

- Rebase ``` $ git range-diff master..gh/time shadow/master..time 1: 68a97728 < -: -------- lib/, src/: Use %F instead of %Y-%m-%d with strftime(3) 2: 56067591 = 1: b5d8ff0e src/: Use %c for printing localized date× with strftime(3) 3: 928172c4 < -: -------- src/chage.c: print_day_as_date(): Use %x for printing localized dates with strftime(3) 4: e78f9782 < -: -------- src/chage.c: print_day_as_date(): Simplify error handling 5: fc46d1f3 = 2: 53554dc6 lib/time/, src/: day_to_str(): Add support for formatting localized dates 6: b2c33c58 < -: -------- src/chage.c: print_day_as_date(): Call DAY_TO_STR() to de-duplicate code 7: 62b553e3 < -: -------- lib/string/strftime.h: STRFTIME(): Tighten macro definition ```

[alejandro-colomar]

Queued after https://github.com/shadow-maint/shadow/pull/1058.

[alejandro-colomar]

v1b:

• Rebase

$ git range-diff gh/localtime..gh/time shadow/master..time 
1:  6d62fef0 = 1:  2686a033 src/chage.c: Always use ISO 8601 format (YYYY-MM-DD) for printing dates
2:  b48d0342 = 2:  de6f93bf src/chage.c: print_day_as_date(): Simplify error handling
3:  03e91f78 = 3:  bd7701fa src/chage.c: print_day_as_date(): Call DAY_TO_STR() to de-duplicate code

[alejandro-colomar]

Well, this changed a bit since my last review.

I am not completely convinced about the changes, but if we make them we should add in the release notes that from now on all the dates printed follow the ISO standard.

Nah, I reverted that change. That was only present in v1. :)

Since v2 of this patch set, I changed custom(American)-formatted dates and date-times by locale-formatted dates (%x) and date-times (%c). And kept printing ISO dates only where we already printed them as ISO.

[hallyn]

So this unfortunately begs the usual question - how many dark server rooms are running 20 year old scripts depending on the current date format that are going to break from a perfectly reasonable change?

I think none. I think this is fine.

[alejandro-colomar]

So this unfortunately begs the usual question - how many dark server rooms are running 20 year old scripts depending on the current date format that are going to break from a perfectly reasonable change?

The two breaking changes are:

https://github.com/shadow-maint/shadow/pull/1059/commits/56067591034f51c083a347d374ccd0ed6aa3c715 https://github.com/shadow-maint/shadow/pull/1059/commits/928172c43495c08b8eb7502cd7890f78e3689ad8

I think none. I think this is fine.

Agree.

[hallyn]

I'm worried about that second one (https://github.com/shadow-maint/shadow/commit/928172c43495c08b8eb7502cd7890f78e3689ad8).

[alejandro-colomar]

I'm worried about that second one (928172c).

That's the one I'm less worried about. If they wanted to parse the output, they had the option to pass -i for a standard format.

I was more worried about the first one, since it was unconditionally non-standard.

(Or you might be worried about a script that would be written before chage(1) ever got a -i flag, which might be a possibility. Let's hope such a script doesn't exist.)

---

Edit: Oh, now I see that -i was only added in 2019. Hmmm.

[hallyn]

@alejandro-colomar Ok, for all of the commits (including the already recently merged ones), let's please add an /etc/login.defs switch to dictate whether to use the classic format, or the standards-compliant formats.

I'm 100% convinced there's some site out there parsing the date on '-' (%F) or on the %b %d %Y, and https://github.com/shadow-maint/shadow/commit/928172c43495c08b8eb7502cd7890f78e3689ad8 will mess them up.

Generally I'd prefer having fewer ways of dictating the format as it's going to make it harder to spot an accidental format printing bug, but I think it's worth it here.

[alejandro-colomar]

@alejandro-colomar Ok, for all of the commits (including the already recently merged ones), let's please add an /etc/login.defs switch to dictate whether to use the classic format, or the standards-compliant formats.

I'm 100% convinced there's some site out there parsing the date on '-' (%F) or on the %b %d %Y, and 928172c will mess them up.

Generally I'd prefer having fewer ways of dictating the format as it's going to make it harder to spot an accidental format printing bug, but I think it's worth it here.

Hmm, I don't like adding configurability either. How about the following instead?

Deprecate running chage(1) to print a date without -i. Print a deprecation warning to stderr when a date is printed in the non-standard format. Keep the nonstandard format untouched, for compatibility reasons.

Then we can come back in some years (4 or 5?) and unconditionally enable %F.

[kenion]

Just for clarification, has a consensus been reached on how to move forward? Since change #1058 was reverted, issue #1057 is still present. For now, any user who updates to 4.14.6 from an earlier version will see the change in date input/output behavior described in that issue.

[alejandro-colomar]

Hi Gus,

On Mon, Aug 26, 2024 at 03:18:44AM GMT, Gus Kenion wrote:

Just for clarification, has a consensus been reached on how to move forward?

Not yet.

Since change #1058 was reverted,

No, it wasn't reverted.

issue #1057 is still present.

And no, issue 1057 isn't present.

We have a different one now: we print a different date than the one that is stored, since we print a local date, but store a UTC date. This might be problematic too.

The solution would be to append a Z to the ????-??-?? date and revert

1058 to print a UTC date again. However, that might break scripts, so

I would just leave it imprecise. One can always use TZ=UTC to print the actual date stored.

Have a lovely day! Alex

For now, any user who updates to 4.14.6 from an earlier version will see the change in date input/output behavior described in that issue.

-- Reply to this email directly or view it on GitHub: https://github.com/shadow-maint/shadow/pull/1059#issuecomment-2309862694 You are receiving this because you were mentioned.

Message ID: @.***>

-- https://www.alejandro-colomar.es/

[kenion]

@alejandro-colomar sorry for my misunderstanding. I've gone over the changes and comments again and I see the root cause of #1057 is addressed - thanks again to you and @hallyn.

[alejandro-colomar]

No problem. Thanks! :)