runtime: FileInfo.CreationTime should throw PlatformNotSupported on platforms where it isn't supported
On most non-Windows platforms, the stat family of functions returns a struct that is missing st_birthtime. We observe this at build time, and only try to get the value in our PAL if we know it is supported. If we know we don’t have st_birthtime, we return default(DateTimeOffset).
Here’s some output from Ubuntu 16.04:
CreationTime: Monday, January 1, 0001 12:00:00 AM
CreationTimeUtc: Monday, January 1, 0001 12:00:00 AM
LastAccessTime: Saturday, March 4, 2017 3:27:29 PM
LastAccessTimeUtc: Saturday, March 4, 2017 11:27:29 PM
LastWriteTime: Sunday, February 19, 2017 12:21:30 AM
LastWriteTimeUtc: Sunday, February 19, 2017 8:21:30 AM
This should throw PlatformNotSupportedException, as the platform does not support the concept being expressed. It is in fact dangerous behavior the way it is. Imagine a disk cleanup program designed to delete everything older than a week after it backs up everything created this week.
cc: @ianhays @JeremyKuhne
About this issue
- Original URL
- State: closed
- Created 7 years ago
- Reactions: 1
- Comments: 33 (30 by maintainers)
Another thought: A breaking change for 2.0 is arguably the correct thing to do. If someone is using this in a program on a platform where it is not supported, their program is arguably not doing what they intend and they likely have a bug. An exception in this case will make it unambiguous.
At one point, binary compatibility with the full framework surface area was a goal for .NET Core 2.0. Is that still the case? If so, I think we should bias towards maintaining the behavior full framework had (or throwing where we can’t make a reasonable approximation).
I totally get (and am sympathetic to) Steve’s argument that apps that just want to display this data and not take action on it would be busted. I could totally see, PowerShell, for example, being horked if we make the change because maybe they can end up surfacing the value of CreationTime. I knpw we’ve followed this: “hand back dummy data” pattern elsewhere (IIRC we do it in Diagnostics when you want to get information about processes that we just don’t have) and I think it a bunch of places it actually makes sense. In this case, it really feels like the common case is you’re getting this data because you want to use it.
I’d rather us look at our app compat lab (and maybe also packages on NuGet.org) or source on GitHub to try to get a sense of how often the data is obtained for display vs action is taking on it. If the majority of uses are actually expecting meaningful data to be returned, I think we should strongly consider throwing here.
As Bill and Mark point out, there are real world apps that would do the wrong thing here. Is the set of Unix applications that we would end up breaking actually large? I would have expected Unix applications to not touch this property since in a majority of cases it doesn’t give you any reasonable data.
Interestingly enough, this scenario would affect me as well in my home app. I’m cleaning up files older than x date… I have since switched to using the LastWriteTime, but it could have been unfortunate for sure.
Can you link where the discussion was captured? I searched a bit but didn’t stumble on it.
I realize it would be a breaking change at this point. I worry that this (and other issues like this, should they exist) will be a common source of bugs as people port things to other platforms, and those bugs can be very dangerous.
Returning
default(DateTime)rather than throwing PNSE was an explicit design choice. Whether it was the right or wrong choice, changing it would be a very noticeable breaking change from .NET Core 1.x. (Not trying to shut down discussion, just highlighting that the value of this change would need to be significant enough to overcome that significant negative.)