dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.18k stars 4.72k forks source link

Consider changing default file time on Unix #26233

Open nguerrera opened 6 years ago

nguerrera commented 6 years ago

From @jmacato on May 17, 2018 1:40

The following exception is thrown on debug/release:

/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018: The "ResolvePackageAssets" task failed unexpectedly. [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018: System.ArgumentOutOfRangeException: Offset must be within plus or minus 14 hours. [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018: Parameter name: offset [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.DateTimeOffset.ValidateOffset(TimeSpan offset) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.DateTimeOffset..ctor(DateTime dateTime) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.DateTimeOffset.FromFileTime(Int64 fileTime) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.IO.FileStatus.GetLastWriteTime(ReadOnlySpan`1 path, Boolean continueOnError) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.IO.FileSystem.GetLastWriteTime(String fullPath) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at System.IO.File.GetLastWriteTimeUtc(String path) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader.CreateReaderFromDisk(ResolvePackageAssets task, Byte[] settingsHash) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.CacheReader..ctor(ResolvePackageAssets task) [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ReadItemGroups() [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.NET.Build.Tasks.ResolvePackageAssets.ExecuteCore() [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.NET.Build.Tasks.TaskBase.Execute() [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskExecutionHost.Microsoft.Build.BackEnd.ITaskExecutionHost.Execute() [<redacted>.csproj]
/usr/share/dotnet/sdk/2.1.300-rc1-008673/Sdks/Microsoft.NET.Sdk/targets/Microsoft.PackageDependencyResolution.targets(197,5): error MSB4018:    at Microsoft.Build.BackEnd.TaskBuilder.ExecuteInstantiatedTask(ITaskExecutionHost taskExecutionHost, TaskLoggingContext taskLoggingContext, TaskHost taskHost, ItemBucket bucket, TaskExecutionMode howToExecuteTask) [<redacted>.csproj]

I was using an older version dotnet-sdk-2.1.200 then purged that and installed 2.1 RC1. i also cleared /usr/share/dotnet & $HOME/.nuget prior to upgrade.

I encountered this bug while compiling an Avalonia app. This also occurs on the console app 'Hello World' template and on dotnet clean.

Update : The build works when i set my timezone to UTC (Had it set to UTC+8 before). it's quite a pain to go back and forth timezones however..

Copied from original issue: dotnet/sdk#2251

nguerrera commented 6 years ago

From @livarcocc on May 17, 2018 16:49

@nguerrera

nguerrera commented 6 years ago

This looks like a bug in corefx. cc @JeremyKuhne @Petermarcu

We're getting ArgumentOutOfRangeException calling File.GetLastWriteTimeUtc(string path).

nguerrera commented 6 years ago

@jmacato Which operating system are you using. Can you provide dotnet --info output?

nguerrera commented 6 years ago

Workaround: disable package assets cache in project file

<PropertyGroup>
  <DisablePackageAssetsCache>true</DisablePackageAssetsCache>
</PropertyGroup>

NOTE: This will slow down incremental builds.

nguerrera commented 6 years ago

@jmacato Can you try this to (hopefully) nail down a simpler repro for corefx?

  1. Switch back to your original timezone
  2. Write a simple console app that uses FIle.GetLastWriteTimeUtc
  3. Build the app using the DisablePackageAssetsCache workaround above.
  4. Run the app
nguerrera commented 6 years ago

From @JeremyKuhne on May 17, 2018 17:56

@tarekgh can you posit how this might happen?

nguerrera commented 6 years ago

From @tarekgh on May 17, 2018 19:4

@jmacato could you please share you exact time zone settings on your machine? you mentioned UTC+8 but we are not able to repro that. what is the time zone name you have?

Also, it would be good if you can try running the code

    DateTimeOffset.FromFileTime(0);

on your machine when setting UTC+8 as you mentioned.

nguerrera commented 6 years ago

@tarekgh Do you agree from the stack that this is not an issue at our call site, but rather in the BCL? Should I move this to dotnet/coreclr or dotnet/corefx?

nguerrera commented 6 years ago

From @tarekgh on May 17, 2018 22:16

@nguerrera yes, this is most likely is an environmental issue or a corefx issue. I don't think this is SDK issue. @JeremyKuhne please advise if you disagree.

nguerrera commented 6 years ago

From @jmacato on May 18, 2018 1:35

@nguerrera Sorry i can't provide the guilty dotnet --info since i changed my sdk but i am using Ubuntu 18.04 LTS and it is newly reformatted machine. I installed RC1 after apt purge'ing dotnet-sdk-2.1.200. then after installation, the bug occurs. but when i set my timezone from UTC+8:00 to GMT and back, the issue no longer occurs.

nguerrera commented 6 years ago

From @jmacato on May 18, 2018 1:36

@tarekgh @nguerrera I will try to repro using live usb of Ubuntu 18.04. and also, i also had this problem with 16.04, in all of my computers ever since 2.1 preview 2. I just got time now in filing this issue.

nguerrera commented 6 years ago

From @jmacato on May 18, 2018 4:28

@tarekgh DateTimeOffset.FromFileTime(0) output is 01/01/1601 6:58:00 AM +06:58 Currently set timezone is UTC+08:00 (Asia/Manila)

nguerrera commented 6 years ago

From @jmacato on May 18, 2018 14:8

Okay the bug returned when i deleted bin and obj folders, then doing dotnet build. I've set the timezone via sudo dpkg-reconfigure tzdata.

nguerrera commented 6 years ago

From @jmacato on May 18, 2018 14:11

@nguerrera i tried that simple repro and it returned this: 18/05/2018 11:04:40 AM is the original timestamp 18/05/2018 3:04:40 AM is the repro's output

nguerrera commented 6 years ago

Okay the bug returned when i deleted bin and obj folders, then doing dotnet build

@JeremyKuhne, @tarekgh This means we are getting this on File.GetLastWriteTimeUtc("any/File/That/Does/Not/Exist").

@jmacato Can you adjust your repro program to do that?

nguerrera commented 6 years ago

From @jmacato on May 19, 2018 1:23

screenshot from 2018-05-19 09-21-40 @nguerrera i've tried that and the exact exception showed up. That function should just tell that the file doesn't exist and not some cryptic exception message but that's just my 2cents.

tarekgh commented 6 years ago

Looking at Asia/Manila zone info and I am seeing:

# Zone  NAME        GMTOFF  RULES   FORMAT  [UNTIL]
Zone    Asia/Manila -15:56:00 - LMT 1844 Dec 31
            8:04:00 -   LMT 1899 May 11
            8:00    Phil    +08/+09 1942 May
            9:00    -   +09 1944 Nov
            8:00    Phil    +08/+09

in early date,1844 is having a -15:56:00 time offset from UTC which is very weird. DateTimeOffset doesn't expect the UTC offsets can exceed 10 hours.

@JeremyKuhne either we need to relax DateTimeOffset to allow more than 14 hours offset. Or we fix the IO code to not get the file time using DateTimeOffset.FromFileTime(0) which will end up to the year 1601.

JeremyKuhne commented 6 years ago

either we need to relax DateTimeOffset to allow more than 14 hours offset. Or we fix the IO code to not get the file time using DateTimeOffset.FromFileTime(0) which will end up to the year 1601.

I'd think that we would want to relax DateTimeOffset. While arguable whether or not we should match time behavior for missing file times across platforms, I would expect that creating old offsets should be supported (even if they are unusual).

JeremyKuhne commented 6 years ago

I'd think that we would want to relax DateTimeOffse

And perhaps we can relax it for old dates only?

karelz commented 6 years ago

@krwq @tarekgh there are 2 more duplicates. Is it worth porting to 2.1.x servicing?

tarekgh commented 6 years ago

there are 2 more duplicates. Is it worth porting to 2.1.x servicing?

There are 2 different workarounds for anyone hitting this issue. This is why we didn't consider it for the servicing. but if you think this is meeting the servicing bar we can consider.

CC @danmosemsft and @Petermarcu

karelz commented 6 years ago

@tarekgh if it was considered, then fine. Although, 2.1 is LTS, and it is not nice to have a specific timezone broken ...

BTW: I saw only 1 workaround - changing timezones. What is the other one? Maybe we should at least add it into "known issues" list? It may be easier for people to find about it.

tarekgh commented 6 years ago

BTW: I saw only 1 workaround - changing timezones. What is the other one?

The second workaround mentioned in the comment: https://github.com/dotnet/corefx/issues/29820#issuecomment-390690353

Maybe we should at least add it into "known issues" list? It may be easier for people to find about it.

@krwq could you please add the needed information to the know issues doc?

tarekgh commented 6 years ago

if it was considered, then fine. Although, 2.1 is LTS, and it is not nice to have a specific timezone broke

To comment on that. The issue happens only if you are using a historical period of the concerned time zone. the bug exposed because System.IO is using file dates to the year 1600 on Linux which doesn't make sense as Linux epoch is 1970. I already communicated this feedback to @JeremyKuhne

karelz commented 6 years ago

OK, so should we change System.IO to behave better in 2.1.x instead?

tarekgh commented 6 years ago

OK, so should we change System.IO to behave better in 2.1.x instead?

No, I am not suggesting that. this will be riskier. we either service the time zone fix to 2.1 if we think this is necessary or we add it to the known issues doc. @JeremyKuhne will need to decide if want to change the System.IO on 3.0.

karelz commented 6 years ago

Thanks for clarification. So far I have seen 3 independent reports:

Given that the entire timezone "Asia/Manila" has broken dotnet command even for HelloWorld app, I think we should at least consider it for 2.1 servicing - @danmosemsft what do you think?

danmoseley commented 6 years ago

The impact may meet the servicing bar. 2.1 will be in support for some time. I don't know the risk. @tarekgh you are welcome to send the usual email if you believe it makes sense.

danmoseley commented 5 years ago

Reopening per discussion in linked issue. Please consider for 3.0, or 5.0, as you believe appropriate.

JeremyKuhne commented 5 years ago

cc: @carlossanlop

I'm hesitant to change IO this late as it is very subtly breaking. If we want to do this we should consider new API that allows you to see the default value and/or allow you to know if we couldn't get the value in question.

@tarekgh Did we relax DateTimeOffset?

tarekgh commented 5 years ago

Did we relax DateTimeOffset?

No, and this will be riskier than the IO change will be. I learned we limited the DateTimeOffset for 2 reasons. the first is because there is no currently offsets more than 14 hours. but the other reason is the limitation in the interop with components like SQL which restrict it too. I wouldn't risk change DateTimeOffset and potentially can cause breaks down the road. I believe the IO change would be more safer than changing the DateTimeOffset.

tarekgh commented 5 years ago

One more point, using File Time before Linux Epoch on Linux looks wrong in general as the OS doesn't support it anyway.

nguerrera commented 5 years ago

It is not clear to me why https://github.com/dotnet/coreclr/pull/18305 didn't resolve the issue, but was expected to do so...

tarekgh commented 5 years ago

@nguerrera this is a good point. I started to believe this could be specific to mono. I am seeing they picked up the TZ files from coreclr but may be there is something there causing the issue. I'll try to follow up with the Mono guys.

CC @krwq

tarekgh commented 5 years ago

I moved this issue to milestone 5.0 for now as it looks we already handled the case inside TimeZoneInfo as @nguerrera pointed out. I am currently following up with Mono guys as the failure we are seeing in https://github.com/dotnet/core/issues/3105 is on their stack.

tarekgh commented 5 years ago

I confirmed with Mono team the fix dotnet/coreclr#18305 was not in the build that failed the issue dotnet/core#3105 and they have a new version with the fix. https://github.com/mono/corefx/pull/316 Mono version 6.0.0.320 or later should have the fix.

https://github.com/dotnet/core/issues/3105#issuecomment-519673387

I'll keep this issue open to have @JeremyKuhne decide if want to change the min datetime on the file system on Linux. but this is not urgent issue now as the experienced msbuild failures will not happen anymore as we already have the fix.

krwq commented 5 years ago

@JeremyKuhne should we close this or do you think we will address file system to use different min datetime on Linux?

tarekgh commented 5 years ago

@krwq I kept this open to decide about that. I believe in 5.0 would be nice if we use Linux Epoch and not Windows min file/time min.

JeremyKuhne commented 4 years ago

Triage: We need to explore the implications of making this change. We notably don't have a good way of discovering that the default time has been used and that might be even more problematic with two different values. Is it going to create a problem for code that might be checking for DateTime.MinValue? Is that an ok risk? (Check GitHub/StackOverflow for possible data.)

krwq commented 4 years ago

Removing my assignment since I don't work on that right now and it's now being considered System.IO issue.

ghost commented 2 years ago

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of our issue cleanup automation.