Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

date: fails to parse +N and -N in absolute time string as a time zone #7065

Open
jfinkels opened this issue Jan 3, 2025 · 11 comments
Open
Assignees
Labels

Comments

@jfinkels
Copy link
Collaborator

jfinkels commented Jan 3, 2025

Environment: Ubuntu 20.04, uutils main branch (git commit 88cdf16), GNU coreutils v8.30

Steps to reproduce:

TZ=UTC0 date -d '1997-01-19 08:17:48 +0'

What happens now: uutils date terminates with an error because it doesn't understand the +0 time zone:

date: invalid date '1997-01-19 08:17:48 +0'

If you remove the +0, then uutils date does successfully parse the date string.

What I expected to happen: GNU date successfully interprets the +0 as a time zone:

Sun 19 Jan 1997 08:17:48 AM UTC

Notes: this is causing a test failure in GNU test file tests/date/date.pl.

@Felle33
Copy link
Contributor

Felle33 commented Jan 9, 2025

Making some tests, the code can sucessfully parse the strings TZ=UTC0 date -d '1997-01-19 08:17:48 +0000' and TZ=UTC0 date -d '1997-01-19 08:17:48 +00:00'.
We could try to parse the timezone and then pass the right argument to the function parse_datetime::parse_datetime_at_date(...).
What do you think?

@jfinkels
Copy link
Collaborator Author

jfinkels commented Jan 9, 2025

Maybe the parse_datetime_at_date function in https://github.com/uutils/parse_datetime just needs to be updated to understand the string "+0", etc.?

@Felle33
Copy link
Contributor

Felle33 commented Jan 9, 2025

Inspecting the code, I see that they try to parse also the timezone, but since %z accepts only 4 digits (i.e +0000) or 4 digits separated by : (i.e +00:00), the function returns an error

@jfinkels
Copy link
Collaborator Author

I'm proposing adding support for this to parse_datetime here: uutils/parse_datetime#101

@jfinkels jfinkels self-assigned this Jan 18, 2025
@jfinkels
Copy link
Collaborator Author

Apparently my pull request uutils/parse_datetime#101 was not sufficient, I'll keep investigating.

@jfinkels
Copy link
Collaborator Author

Pull request uutils/parse_datetime#102 should have provided a basic implementation of this feature, and I expected it to become available in the just released version of parse_datetime 0.7.0, but it is not working as I expected. Maybe there was an issue with the release of the crate?

For example, after cargo new mytest && cargo add [email protected], this basic program

use parse_datetime::parse_datetime;

fn main() {
    dbg!(parse_datetime("1997-01-19 08:17:48 +0").unwrap());
}

when executed with cargo run fails with

thread 'main' panicked at src/main.rs:4:51:
called `Result::unwrap()` on an `Err` value: InvalidInput
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

@sylvestre
Copy link
Contributor

i just tagged the release + cargo publish

@jfinkels
Copy link
Collaborator Author

Hmm is it possible the tag is on the wrong commit? This is the log from the parse_datetime repository. The tag v0.7.0 is on a commit that branches away from the main sequence, and it doesn't include my commit "Parse datetimes with single digit timezone offset":

$ git log -9 --all --oneline --graph
*   eda2422 (HEAD -> main, upstream/main) Merge pull request #103 from jfinkels/parse-datetime-with-single-digit-offset-t-sep
|\  
| * c948274 Parse datetime with T separator and digit offset
|/  
*   b209341 Merge pull request #105 from sylvestre/0.7.0
|\  
| * 9cd22af prepare new release 0.7.0
|/  
*   a7061bc Merge pull request #102 from jfinkels/parse-datetime-with-single-digit-offset
|\  
| * ea49dc9 Parse datetimes with single digit timezone offset
|/  
| * ea52a79 (tag: v0.7.0) prepare new release 0.7.0
|/  
*   938aa96 Merge pull request #100 from jfinkels/add-months-overflow-surplus-days
|\  
| * 90d22a2 Overflow surplus days when adding months to a date

@sylvestre
Copy link
Contributor

nope, it just misses
Parse datetime with T separator and digit offset

you can see the code of the lib here:
~/.cargo/registry/src/index.crates.io-*/parse_datetime-0.7.0/

@jfinkels
Copy link
Collaborator Author

jfinkels commented Jan 26, 2025

I checked in ~/.cargo/registry/src/index.crates.io-*/parse_datetime-0.7.0/ and the code there doesn't include my changes from uutils/parse_datetime#103. For example, the line

    pub const YYYYMMDDHHMMSS_T_SEP_HYPHENATED_OFFSET: &str = "%Y-%m-%dT%H:%M:%S%#z";

should be in lib.rs but it isn't. Can you confirm whether that line is present or absent in your ~/.cargo/registry/src/index.crates.io-*/parse_datetime-0.7.0/src/lib.rs file?

Edit: nevermind, that's what you said 🙈 .

@jfinkels
Copy link
Collaborator Author

Ignore my previous comment. What I meant to write was this:

I checked in ~/.cargo/registry/src/index.crates.io-*/parse_datetime-0.7.0/ and the code there doesn't include my changes from uutils/parse_datetime#102. For example, the line

    pub const YYYYMMDDHHMMSS_HYPHENATED_OFFSET: &str = "%Y-%m-%d %H:%M:%S %#z";

should be in lib.rs but it isn't. Can you confirm whether that line is present or absent in your ~/.cargo/registry/src/index.crates.io-*/parse_datetime-0.7.0/src/lib.rs file?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants