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

Commit 4ca4987 is unsound #1

Open
jhpratt opened this issue Aug 31, 2021 · 2 comments
Open

Commit 4ca4987 is unsound #1

jhpratt opened this issue Aug 31, 2021 · 2 comments

Comments

@jhpratt
Copy link

jhpratt commented Aug 31, 2021

The time crate has unsoundness, but so does chrono (for the exact same reason). Chrono just ignores this fact, while the time crate confronts it and forces end users to explicitly opt in to the unsoundness. Commit 4ca4987 is based on the false premise that "The chrono crate doesn't suffer from this limitation".

I have no problem with you switching crates, but I want to make sure that you're aware that your reasoning has flaws.

@andreabolognani
Copy link
Owner

@jhpratt thanks for bringing this up and sorry for taking a few days to get back to you!

I was actually aware of the fact that the chrono crate simply ignores the unsoundness rather than addressing it. Reading back the commit message, I see that it gives a different impression and seems perhaps a bit uncharitable of the time crate. That was not my intention, so I apologies for it.

I would love to go back to the much leaner time crate, but I was not able to find a way to get it to address my use case, which is: obtain the current date in YYYY-MM-DD format when running on Linux. To be completely honest, I didn't spend too much effort looking for a way to do that, especially after I realized that the chrono crate would paper over the issue for me.

Can you recommend a way to do the above using the time crate? If so, I'll gladly switch back - and make sure the corresponding commit message does a better job at explaining the situation this time ;)

Thanks again!

@jhpratt
Copy link
Author

jhpratt commented Sep 6, 2021

Not an issue! I didn't take offense, just wanted to make sure that you were fully informed.

I didn't take the time to look through the repository. If it's a library, you can't enable the functionality (this is by design). If it's a binary, you can set RUSTFLAGS="--cfg unsound_local_offset", which will enable the internal cfg (and thus the unsoundness). If you never call setenv, you've got nothing to worry about with regard to soundness.

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

No branches or pull requests

2 participants