-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Test main for failure seen in #7378 for Y2038 bug #7399
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
base: main
Are you sure you want to change the base?
Conversation
71e2c73
to
8efc9ab
Compare
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Those three failures seem like actual failures / y2038 bugs ? Which I'm going to guess ends up in the |
I don't understand why this change is in a separated PR from #7378 |
#7378 test (and fix) compatibility with gnu touch, i.e. that
So really #7378, test the mapping from 2 character years to 4 character years, and could just test This PR tests that dates just before, at, or just after y2038 bug (even when already a 4 character years) get actually written on filesystem with the correct timestamp (they are not on 32 bits systems). So it tests for a potential bug, independently of whether the year is 2 or 4 characters. |
Ok, that makes sense, thanks ! |
GNU testsuite comparison:
|
I guess one question I don't know how to answer is what should touch do on 32 bit systems, and is it a bug in filetime or something else that should tell you (or prevent you) from setting a wrong timestamp. |
Well, that may be an unpopular opinion, but I'd say that's where our dedication to sticking to GNU's behavior should end. |
I tend to agree, but maybe an error (instead of a silent failure) as long as this project is tested on 32 bits is still worth it ? |
tests/by-util/test_touch.rs
Outdated
seconds + deltas | ||
); | ||
let file = format!("Y2038_{}_{}", date, deltas); | ||
ucmd.args(&["-t", &date, &file]).succeeds().no_output(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would be simpler/more concise to just specify the time argument directly? So the three test cases would be
ucmd.args(&["-t", "203801190314.06", "f1"])
ucmd.args(&["-t", "203801190314.07", "f2"])
ucmd.args(&["-t", "203801190314.08", "f3"])
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expanded to 3 tests one just before, one at and one just after.
@Carreau Can you solve the conflict ? |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
gentle nudge |
GNU testsuite comparison:
|
some jobs are failing |
It's unclear to me how the failure seen in uutils#7378 is related to the change in the PR, it just happen that the test need to try touching in 2068. The 2^32 error in the test makes me strongly think of the year 2038 bug, so I added a test for that.
Yes, because the goal of the PR here is to add tests for a behavior that is already failing and was not tested. I don't think I know how you want to fix it (if you want to fix it, or even if fixable), or if you want me to ignore on some platforms/aarch. (rebased) |
GNU testsuite comparison:
|
GNU testsuite comparison:
|
It's unclear to me how the failure seen in #7378
is related to the change in the PR, it just happen that the test need to try touching in 2068.
The 2^32 error in the test makes me strongly think of the year 2038 bug, so I added a test for that.