Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @Mark-Simulacrum (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

• @rustbot author: the review is finished, PR author should check the comments and take action accordingly
• @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

My plan is to:

1. Refactor Timespec and add regression tests both pre- and post-fix.
2. Use Timespec in all targets' SystemTime except for windows, hence not refactoring this into crate::sys::common per its README.md

This initial draft PR is because tests are passing on aarch64-apple-darwin but I'm having trouble getting them to build properly for wasm32-unknown-unknown. I'm hoping the PR checks will do this, though it' not looking like they do from what I'm seeing queued so far.

Ah, you desire our magic powers. BEHOLD:
@bors try

⌛ Trying commit ee86a2c with merge 4b05334...

A job failed! Check out the build log: (web) (plain)

wasm32-unknown-unknown
x86_64-pc-windows-msvc
x86_64-unknown-linux-gnu
##[endgroup]
INFO:root:Job type: TryRunType(custom_jobs=['wasm32-unknown-unknown', 'x86_64-pc-windows-msvc', 'x86_64-unknown-linux-gnu'])
  File "/home/runner/work/rust/rust/src/ci/github-actions/calculate-job-matrix.py", line 195, in <module>
  File "/home/runner/work/rust/rust/src/ci/github-actions/calculate-job-matrix.py", line 195, in <module>
    jobs = calculate_jobs(run_type, data)
  File "/home/runner/work/rust/rust/src/ci/github-actions/calculate-job-matrix.py", line 138, in calculate_jobs
    raise Exception(f"Custom job(s) `{unknown_jobs}` not found in auto jobs")
Exception: Custom job(s) `['wasm32-unknown-unknown', 'x86_64-pc-windows-msvc', 'x86_64-unknown-linux-gnu']` not found in auto jobs
##[error]Process completed with exit code 1.

You need to use the name of CI jobs, rather than the names of targets. For a list see https://github.com/rust-lang/rust/blob/master/src/ci/github-actions/jobs.yml. Note that Linux is tested in the PR CI already (by x86_64-gnu-llvm-17).

So for msvc it'd be x86_64-msvc. I'm not sure if we test wasm-unknown-unknown in CI but that's probably under test-various if it is. And that's a useful job to run in any case because it includes a lot of the more interesting targets.

So, observe that error. That means you're going to want to use, for try-jobs, our build/test job names instead of the target tuple names. We don't have a CI job per target tuple because we often want to have one builder build the distribution binaries for many related tuples.

Thus, we translate:

...obviously I lost initiative there because I spent time on formatting out a nice table.

... You may also be interested in (if you think these can be affected):

• 32-bit windows gnu i686-mingw
• cross-compile armhf-gnu
• aarch64 linux gnu aarch64-gnu

... also I realize three of us are providing suggestions but none of us are actually helping to try... so we should actually help try again
@bors try

⌛ Trying commit ee86a2c with merge b0486eb...

☀️ Try build successful - checks-actions
Build commit: b0486eb (b0486ebdeadb1bd71b16e06b686fc8b799042ea0)

... You may also be interested in (if you think these can be affected):

• 32-bit windows gnu i686-mingw
• cross-compile armhf-gnu
• aarch64 linux gnu aarch64-gnu

Eventually I will. The way I plan to refactor, I really only need one currently "unsupported" target platform and, given it was wasm32-unknown-unknown that started this whole endeavor, I figured I'd use that one.

@heaths does this still need to be a draft? if you are ready for a review, you can mark it as »ready for review« and also do a @rustbot ready so it adjusts the labels accordingly. Thanks

@Dylan-DPC I apologize. I think it's still a WIP because my intent was to effectively copy the design of all the working platform implementation except for windows because it's implementation is specific to its FILETIME, which is why I found this bug in the first place (FILETIME has an epoch of 1600-01-01 and I was parsing a Windows format that uses FILETIME timestamps). But there are strict(?) rules for this repo that we shouldn't put things in common unless they are common to all platform-specific implementation. Given I'd need to exclude Windows, it seems I'm not allowed to put things in common (not even if I exclude it from compilation for windows?!).

Thus, this was a test to make sure my solution - which duplicates the code (unnecessarily sans the above rule) - actually works before I start replicating it across all the platform-specific implementations that naively use a Duration (unsigned integer) to represent system time, thus not allowing times before the Unix epoch of 1970-01-01.

Given this passed, I can go ahead and start replicating this to all the other platforms but it'd be nice if I was given an exception for putting this in common so everything but windows could just use this and we not end up with lots of duplicate code.

Please just make a new time folder in https://github.com/rust-lang/rust/tree/ee86a2ca4ceadea60531b0c0a5a751fc5fe17686/library/std/src/sys

Please just make a new time folder in https://github.com/rust-lang/rust/tree/ee86a2ca4ceadea60531b0c0a5a751fc5fe17686/library/std/src/sys

@workingjubilee and then just import that into all the broken platform implementations at least (those using Duration)? Or even replace the ones that are working just to avoid duplication? That's what I'd normally do in my own projects.

@heaths I don't understand what you mean? Please examine the structure of modules like thread_local or sync and how they handle differing platforms.