Occasionally I come across a point in my code where it'd be useful to have a DateTime.UnixEpoch field as opposed to specifying my own with new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc). Usually this is when I'm calculating expiration dates for web tokens where the expiration property is the number of seconds since the Unix epoch (see https://en.wikipedia.org/wiki/Unix_time for definition).
namespace System {
public struct DateTime {
+ public static readonly DateTime UnixEpoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc);
}
public struct DateTimeOffset {
+ public static readonly DateTimeOffset UnixEpoch = new DateTimeOffset(1970, 1, 1, 0, 0, 0, TimeSpan.Zero);
}
}
```c#
public class TokenIntrospectionResponse
{
public int ExpirationInSeconds
{
get => (int)((ExpirationUtc - DateTime.UnixEpoch).TotalSeconds);
set => ExpirationUtc = DateTime.UnixEpoch.AddSeconds(value);
}
public DateTime ExpirationUtc { get; set; }
}
```
:winces:
I get where you're coming from... but I feel like this is almost certainly going to cause programmers who don't know any better to reinforce the assumptions that DateTime represents an absolute point, or that doing the subtraction is going to work in all cases. That is, for the same reason people are surprised that:
var u = DateTime.UtcNow;
var l = DateTime.Now;
// Prints "07:00:00" for me, is the current offset from UTC
Console.WriteLine(u - l);
@Clockwork-Muse I think we already have this problem with DateTime and that is why we are encouraging people to be careful when using it or just use DateTimeOffset. I am not seeing the proposal will make the situation worse. whoever going to use UnixEpoch are the people who really are doing new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc) today.
Do you think having UTC in the name can help a little with the issue you are mentioned? or renaming the property to something more descriptive can help?
Do you think having UTC in the name
It would be redundant because there's no such thing as a local time Unix epoch. That would fit better in intellisense docs, wouldn't it?
@jnm2 I agree with you. I am thinking how we can mitigate @Clockwork-Muse concern or at least help people not to run into such problem.
@jnm2 - I agree with you, the Unix epoch is an absolute stamp, so adding "Utc" would be redundant.
The problem is that DateTime is a "local" value, even with DateTimeKind.Utc; it's a local stamp in the UTC zone. So the property would be returning something claiming to be absolute, but the rest of the behavior of the type acts like a local/relative value (for example, consider how comparisons work).
@tarekgh - my preferred solution would be getting an all-up new date/time library, as has been previously discussed. Barring that, I'd put it on DateTimeOffset, and let people wanting a local stamp version do the extra work themselves.
The problem is that DateTime is a "local" value, even with DateTimeKind.Utc
This is not true. the ticks inside the DateTime still corresponds to UTC. The problem you raised is only happening if you have an operation that mixes different DateTimeKind.
I'd put it on DateTimeOffset, and let people wanting a local stamp version do the extra work themselves.
I am not seeing hurting adding the property to DateTime too. people who want to use that can still run to the exact same problem you mentioned. my preference here is to have the property on DateTime too.
The docs and the intellisense text can clarify this is UTC date and not absolute value.
This is not true. the ticks inside the DateTime still corresponds to UTC. The problem you raised is only happening if you have an operation that mixes different DateTimeKind.
Sorry, when I said "local", I didn't mean DateTimeKind.Local. I meant "the value is a local timestamp to an assumed zone". And that's why there're problems that result when mixing kinds;
var u = new DateTime(2017, 10, 5, 0, 0, 0, DateTimeKind.Utc);
var l = new DateTime(2017, 10, 5, 0, 0, 0, DateTimeKind.Local);
// prints "636427584000000000|636427584000000000"
Console.WriteLine(u.Ticks + "|" + l.Ticks);
// prints "True"
Console.WriteLine(u == l);
....because the ticks in DateTime are offsets from the same _calendar_ point in the local zone, not the same global/universal _instant_, as they would be normally.
And there's another thing: Ticks have nothing to do with UTC. Ticks represent an offset from an epoch (what is unimportant), and represent an absolute stamp... or are supposed to. The common credo is that they're offsets from UTC, but that doesn't actually matter: If I chose 1969-12-31 16:00:00 PST as my epoch start, so long as we talked in ticks it doesn't matter that we used different (seeming) reference points. For that matter, you could use tick offsets to converse with aliens having no concept of our calendar or timezones, or us theirs (assuming we could agree on tick length, which is an interesting physics problem, but w/e)
Thanks @Clockwork-Muse for the clarification.
For that matter, you could use tick offsets to converse with aliens
Tell them a tick is 919.263177 periods of the radiation corresponding to the transition between the two hyperfine levels of the ground state of the cesium 133 atom. Though presumably the aliens would be so far away that time dilation comes into play and tracking time would become more complex yet. (@jskeet better get Noda Time ready for intergalactic calendars? 馃槂)
I am perfectly happy with this proposal.
Me too. I dread to think about how many places I've written that code myself :(
Maybe naive question, but why is it called UnixEpoch?
Is there some RFC specifying that? Industry standard? Or is it Unix OS specific?
Thanks! Updated top post with the link.
Looks good as proposed. We may want to add the ToUnixXxx and FromUnixXxx to DateTime as well though.
FYI: The API review discussion was recorded - see https://youtu.be/BEBq3__WfDc?t=2343 (13 min duration)
I can take this and work 10/21-10/22. I'll yield to any first time contributor.
@terrajobst the addition of the ToUnixXxx and FromUnixXxx methods to DateTime were discussed in dotnet/runtime#17213 but the issue was closed as it was deemed that adding them would encourage users to continue using DateTime instead of DateTimeOffset. Additionally there were some questions about how ToUnixXxx would work with a DateTime with a DateTimeKind.Unspecified, i.e. should it throw an exception, or assume local or Utc.
@adriangodong thanks for willing doing this. Let me know if there is anything I can help you with.
I spoke with @tarekgh and the problem of To/FromUnixXxx is more about consistency with all other DateTime behavior and therefore expectations the API would set.
Adding these 2 methods would be one way or another inconsistent with some other behaviors of DateTime: Addition/substraction always results in DateTimeKind.Unspecified, while TimeZoneInfo.ConvertTimeToUtc always treats the source as DateTimeKind.Local. Moreover DateTime itself never converts timezones (this would be the first one). We would have even harder time explaining why it behaves the way we implement it (incl. that people should not use DateTime because of these exact reasons).
From that reason I agree that leaving the substraction as manual operation is probably best. We should not add the To/FromUnixXxx APIs.
@karelz
Addition/subtraction always results in
DateTimeKind.Unspecified
That is incorrect.
DateTime 卤 TimeSpan = DateTime has the same Kind as the input DateTime operand.DateTime - DateTime = TimeSpan ignores the Kind of both operands.
TimeZoneInfo.ConvertTimeToUtcalways treats the source asDateTimeKind.Local
That is also incorrect. The documentation describes this correctly, which is as follows:
For ConvertTimeToUtc(DateTime):
DateTimeKind.Local converts from local time to UTCDateTimeKind.Unspecified also converts from local time to UTCDateTimeKind.Utc is passed through without any conversionFor ConvertTimeToUtc(DateTime, TimeZoneInfo)
DateTimeKind.Local converts from local time to UTC only if the time zone is TimeZoneInfo.Local, otherwise it throws an exception.DateTimeKind.Utc is passed through without conversion only if the time zone is TimeZoneInfo.Utc, otherwise it throws an exception.DateTimeKind.Unspecified is converted from the time zone given to UTC.Moreover DateTime itself never converts timezones (this would be the first one)
Not exactly true. DateTime.Now converts from UTC to local time, as does DateTime.ToUniversalTime, and DateTime.ToLocalTime converts from local time to UTC. Since the proposed APIs do not deal with other time zones, I see no difference between them and these.
Some clarifications as it looks there were some confusions:
When said add/subtract result Unspecified kind, was trying to point to any operation that involves 2 DateTime objects tends to ignore the Kind property. e.g comparing 2 DateTime objects.
For TimeZoneInfo.ConvertTimeToUtc, was trying to say if the Unspecified kind DateTime was treated as Local.
for DateTime itself never converts timezones (this would be the first one), meant that when doing any operation on the DateTime (e.g. adding days, ticks...etc.) the Time zone is not considered in such calculations. in other words, DateTime internal calculations mostly ignoring the time zone.
To summarize, what Karel listed still hold as reasons we need to avoid exposing ToUnix/FromUnix functionality on DateTime.
How is making a built-in of new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc) making those things any worse? At least this is better in many ways than new DateTime(1970, 1, 1) - which is how it is often done in the wild. Consider:
DateTime dt = unixepoch.AddSeconds(myUnixTimestamp);
DateTime eastern = TimeZoneInfo.ConvertTimeBySystemTimeZoneId(dt, "Eastern Standard Time");
This works as expected when unixepoch has DateTimeKind.Utc. But if it's implemented without the kind, then it has DateTimeKind.Unspecified, and is treated as local time by the conversion function, resulting in the wrong answer.
Having a built-in DateTime.UnixEpoch can't hurt anything, it can only help.
As I pointed out in dotnet/runtime#17213, the behavior of DateTime is already problematic in the ways you described. But people create DateTime values from UTC-based sources all the time. In this case, it's from a Unix timestamp, but the behaviors are not different than if I just parsed an ISO8601 string with a Z at the end, or if I just constructed a new DateTime directly with UTC-based values.
How is making a built-in of new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc) making those things any worse?
Nobody said that. everyone agree it is useful to have it. This is why it is approved by the design committee.
Karel and myself's comments were regarding the request exposing ToUnixXxx and FromUnixXxx on DateTime.
Anyway, I think we all agree on:
Ok. Then lets move the rest of the discussion over to dotnet/runtime#17213, or open a new one if you think it's appropriate. Thanks for clarification.
I've done an initial scan, so changes needed are:
Are these changes correct?
And tests. Otherwise it sounds reasonable.
Note that after you get the change to coreclr repo, it should get merged to corert repo too https://github.com/dotnet/corert/tree/master/src/System.Private.CoreLib/shared/System. so the changes need to be done in corefx repo should wait for the packages of coreclr and corert be built and merged to corefx first.
If it's alright with @adriangodong I'd like to take this on, though I'm unsure of how to get the coreclr changes merged to the corert repo as @tarekgh has mentioned.
@TylerBrinkley when you change anything in coreclr repo under the shared folder (e.g. https://github.com/dotnet/coreclr/tree/master/src/mscorlib/shared/System) it'll get automatically open a PR in corert repo to mirror the changes. so basically no other work need to be done more than ensuring the changes are merged and the packages (of coreclr and corert) are built and referenced in corefx. we can help you with that as needed.
CC @safern who monitor the change monitoring between coreclr and corert
You can unassign me as @adriangodong already submitted a PR for the coreclr changes, he just never linked it back to this proposal.
@adriangodong did you have a chance to enable the added APIs in corefx repo?
@tarekgh Yep, dotnet/corefx#25235
Can we add the netfx-port-consider label to this issue?
Can we add the netfx-port-consider label to this issue?
The added APIs should be included in the next wave of creating the new netsatndard version and support it on the full framework.
CC @weshaggard @AlexGhiondea