Skip to content

libexpr: Canonicalize TOML timestamps for toml11 > 4.0 (backport #13741)#13882

Merged
xokdvium merged 5 commits into2.31-maintenancefrom
mergify/bp/2.31-maintenance/pr-13741
Sep 1, 2025
Merged

libexpr: Canonicalize TOML timestamps for toml11 > 4.0 (backport #13741)#13882
xokdvium merged 5 commits into2.31-maintenancefrom
mergify/bp/2.31-maintenance/pr-13741

Conversation

@mergify
Copy link
Contributor

@mergify mergify bot commented Aug 31, 2025

Motivation

This addresses several changes from toml11 4.0 bump in
nixpkgs 1.

  1. Added more regression tests for timestamp formats.
    Special attention needs to be paid to the precision
    of the subsecond range for local-time. Prior versions select the closest
    (upwards) multiple of 3 with a hard cap of 9 digits.

  2. Normalize local datetime and offset datetime to always
    use the uppercase separator T. This is actually the issue
    surfaced in 2. This canonicalization is basically a requirement
    by (a certain reading) of rfc3339 section 5.6 3.

  3. If using toml11 >= 4.0 also keep the old behavior wrt
    to the number of digits used for subsecond part of the local-time.
    Newer versions cap it at 6 digits unconditionally preserve the input.

Context

Fixes #11441


Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.


This is an automatic backport of pull request #13741 done by [Mergify](https://mergify.com).

Current test suite doesn't cover the subsecond formatting at
all and toml11 is quite finicky with that. We should at the very
least test its behavior to avoid silent breakages on updates.

(cherry picked from commit 7ed0229)
This looks really weird after the reformat.

(cherry picked from commit df4e55f)
There's no reason to use a std::function for recursive lambdas
since there are polymorphic lambdas.

(cherry picked from commit a80a5c4)
This addresses several changes from toml11 4.0 bump in
nixpkgs [1].

1. Added more regression tests for timestamp formats.
   Special attention needs to be paid to the precision
   of the subsecond range for local-time. Prior versions select the closest
   (upwards) multiple of 3 with a hard cap of 9 digits.

2. Normalize local datetime and offset datetime to always
   use the uppercase separator `T`. This is actually the issue
   surfaced in [2]. This canonicalization is basically a requirement
   by (a certain reading) of rfc3339 section 5.6 [3].

3. If using toml11 >= 4.0 also keep the old behavior wrt
   to the number of digits used for subsecond part of the local-time.
   Newer versions cap it at 6 digits unconditionally.

[1]: https://www.github.com/NixOS/nixpkgs/pull/331649
[2]: https://www.github.com/NixOS/nix/issues/11441
[3]: https://datatracker.ietf.org/doc/html/rfc3339

(cherry picked from commit dc769d7)
@mergify mergify bot added automatic backport This PR is a backport produced by automation (does not trigger backporting) conflicts merge-queue labels Aug 31, 2025
@mergify mergify bot requested a review from edolstra as a code owner August 31, 2025 22:52
@mergify

This comment was marked as resolved.

@github-actions github-actions bot added the with-tests Issues related to testing. PRs with tests have some priority label Aug 31, 2025
@xokdvium xokdvium force-pushed the mergify/bp/2.31-maintenance/pr-13741 branch from df235f1 to 13d1be0 Compare August 31, 2025 23:01
@mergify
Copy link
Contributor Author

mergify bot commented Aug 31, 2025

This pull request has been removed from the queue for the following reason: pull request dequeued.

Pull request #13882 has been dequeued. The pull request could not be merged. This could be related to an activated branch protection or ruleset rule that prevents us from merging. (details: Required status check "installer test on ubuntu" is in progress.).

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.
If you do update this pull request, it will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue instead, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@xokdvium xokdvium merged commit 4006d0f into 2.31-maintenance Sep 1, 2025
29 of 31 checks passed
@xokdvium xokdvium deleted the mergify/bp/2.31-maintenance/pr-13741 branch September 1, 2025 07:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automatic backport This PR is a backport produced by automation (does not trigger backporting) conflicts merge-queue with-tests Issues related to testing. PRs with tests have some priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant