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

Timestamps of file of packed package is shifted by the timezone #7395

Closed
zapzap3 opened this issue Oct 12, 2018 · 16 comments · Fixed by NuGet/NuGet.Client#3793
Closed

Timestamps of file of packed package is shifted by the timezone #7395

zapzap3 opened this issue Oct 12, 2018 · 16 comments · Fixed by NuGet/NuGet.Client#3793

Comments

@zapzap3
Copy link

zapzap3 commented Oct 12, 2018

I'm using nuget 4.7.1 .
When I pack with nuget then extract on included files, the extracted files last modified times are -9 hours. (I'm in a +9:00 timezone.)

The problem of Last Modified time for included files, seems fixed in #5240, But LastWriteTime is set UTC time.

ZipArchiveEntry.LastWriteTime(in .NET) seems not supported extended timestamp of Zip.
Therefore, I think that it is better to set local time to LastWriteTime.
Or is there a way to set local time correctly?

Details about Problem

NuGet.exe version = 4.7.1.5393(also tried 4.6.2.5055 and 4.8.1.5435)
.NET version = 4.7.03062
OS version = Windows 7 Professional (build 7601 : Service Pack 1)

Detailed repro steps so we can see the same problem

nuget.exe pack xxxx.nuspec

@rrelyea
Copy link
Contributor

rrelyea commented Oct 12, 2018

What is the impact of our current behavior?

@zapzap3
Copy link
Author

zapzap3 commented Oct 13, 2018

I include source code etc in package.
When pack a package, unchanged files are taken from the previous package and placed in a new package.
Currently, the Last Modified time of a file which has not changed is also shifted by the time zone every time it is packed, and it is unknown whether it is the latest file or not.
I hope Last Modified time of the file will not be changed.

@rrelyea
Copy link
Contributor

rrelyea commented Oct 15, 2018

We are moving away from .symbols.nupkg files towards .snupkg files in the VS 15.9 timeframe.
As part of that, we are moving toward source inclusion via sourcelink, instead of embedding those files.
Can you take a look at sourcelink to see if that helps your future direction?

@zapzap3
Copy link
Author

zapzap3 commented Oct 17, 2018

I never used the source link, but I will check if it is available in the future.
At the moment, if package contains source code, include files, and other assemblies, Do I have to adjust the time zone of the Last Modified time of the file by myself?

bantolov added a commit to Rhetos/Rhetos that referenced this issue Mar 11, 2020
…ple issues with timestamps in NuGet packages, but they should not be important, because in practice all compared file versions should be extracted from same NuGet package.

Btw, issues are:
1. Old version of NuGet overwrite LastModifiedTime to time when the package is created (still used for Rhetos.CommonConcepts, for backward compatibility with DeployPackages)
2. New NuGet pack/unpack can shift last modified time zone: NuGet/Home#7395
3. Zip entry timestamps in NuGet package are recorded only to 2 second precision.
@andrew-boyarshin
Copy link

andrew-boyarshin commented May 11, 2020

Note, that this behavior causes MSBuild to consider files up-to-date and skip running targets if they are dependent on files within NuGet package. For instance, I'd expect MSBuild task assembly I've just modified+built+packed+restored to be considered an out-of-date input. But instead, I have a newer input being considered older (due to UTC), than actually older files (UTC+6).

Cross-referencing my SharpGen branch.

@tompazourek
Copy link

tompazourek commented Jan 13, 2021

I have the same issue. One additional thing I noticed is that only the timestamps of the DLL/PDB files are shifted.

Other files inside the nupkg zip file (e.g. the "nuspec" file or the "[Content_Types].xml" or the "_rels") all have correct timestamps.

@rrelyea Regarding the impact, other than that I don't like that there's incorrect and misleading information in the package, this is also causing issues with up-to-date checks same as @andrew-boyarshin mentioned. Incorrectly working up-to-date checks on my solution is also how I discovered the issue in the first place...

@erdembayar erdembayar changed the title timestamps of file of packed package is shifted by the timezone Timestamps of file of packed package is shifted by the timezone Jan 15, 2021
@erdembayar
Copy link
Contributor

@zivkan @nkolev92 @JonDouglas

I want to fix this along with my fix for Issue#7001 in PR#3793.
It looks dotnet/runtime really doesn't care about which timezone you're in. So I'm going to remove UTC checking from my PR.

https://github.com/dotnet/runtime/blob/master/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipArchiveEntry.cs#L231-L232

https://github.com/dotnet/runtime/blob/a3b37a289b00c748eb92bd4eb4dc78146bd72176/src/libraries/System.IO.Compression/src/System/IO/Compression/ZipHelper.cs#L15-L18

Just one reminder is if I pack package in PST timezone then unpack it in Samao timezone then they'll get exact same timestamp.
Sometime like this:
image

But caveat is 1/1/1980 12:00:00AM in Samao is not same as 1/1/1980 12:00:00AM in PST, but it's very easy to reason and maybe good for repeatable build scenarios. Is it ok?

@zivkan
Copy link
Member

zivkan commented Jan 15, 2021

@erdembayar In my opnion, we should always convert to UTC before setting the timestamp in the nupkg.

I don't know if zip files store timezone in addition to date and time. If not, it's critical to store times as UTC, always. But, if it is, we should make sure we do the right conversion to local time when saving the file on the local file system (extracting the nupkg), because not all nupkgs are created with official tools, so we can't trust the data in a nupkg (to always use UTC time).

@erdembayar
Copy link
Contributor

@erdembayar In my opnion, we should always convert to UTC before setting the timestamp in the nupkg.

I don't know if zip files store timezone in addition to date and time. If not, it's critical to store times as UTC, always. But, if it is, we should make sure we do the right conversion to local time when saving the file on the local file system (extracting the nupkg), because not all nupkgs are created with official tools, so we can't trust the data in a nupkg (to always use UTC time).

@zivkan
Let me confirm 1 more time.
So if I create package in current PST time zone at "7.50PM 1/15/2021" (5.50 PM 1/16/2021 Samao) then someone in Samao extract same nupkg after few week then he'll still see "5.50 PM 1/16/2021" in Samao timezone on timestamp. Is it correct?
Does Nuget.Packaging.Extraction handle extraction of nupkg?

Can you elaborate more on we can't trust the data in a nupkg (to always use UTC time) part? If zip file doesn't store timezone regardless of UTC or not it's hard to tell exact time, so better to exact as it's. At least it's guaranteed to be 1980::2107 date range.

@tompazourek
Copy link

tompazourek commented Jan 16, 2021

@erdembayar I'll explain the issue I'm experiencing. I'm in the Prague timezone (UTC+1). I build some DLLs, and their LastModified timestamp will be 19:47 in UTC+1 Prague, or, in other words, 18:47 UTC.

Then I pack the NuGet and then restore it, and the DLLs in the NuGet cache have LastModified timestamp of 18:47 in UTC+1 Prague, or in other words, 17:47 UTC.

Just by packing the NuGet, the timestamps shifted by 1 hour. The timestamps are already wrong inside the ZIP package, so it's an issue with the "pack" command, not the "restore" command. And also it only affects DLL or PDB files, other files inside the package have their timestamps working correctly as expected.

@zivkan
Copy link
Member

zivkan commented Jan 20, 2021

I spent entirely too much time on this, but what I found was:

First, Wikipedia's article on the zip file format say

The ZIP format has no notion of time zone, so timestamps are only meaningful if it is known what time zone they were created in.

Therefore, the best we can do is assume that the time stamp in the file is in the UTC (+0) time zone, which also means we need to make sure when we save a nupkg, that the time is adjusted to UTC.

Consider the following code:

using (var memoryStream = new MemoryStream())
{
    using (var zip = new ZipArchive(memoryStream, ZipArchiveMode.Create, true))
    {
        var entry = zip.CreateEntry("file.ext");
        var now = DateTimeOffset.UtcNow;
        entry.LastWriteTime = now;
        var when = entry.LastWriteTime;
        var diff = when - now;
    }

    memoryStream.Position = 0;

    using (var zip = new ZipArchive(memoryStream, ZipArchiveMode.Read))
    {
        var entry = zip.Entries.Single();
        var timestamp = entry.LastWriteTime;
        var expected = DateTimeOffset.Now;
        var diff = expected - timestamp;

        if (diff.TotalMinutes > 1.0 || diff.TotalMinutes < -1.0)
        {
            Console.WriteLine("Did not round trip the time successfully");
        }
        else
        {
            Console.WriteLine("Time stamp is good");
        }
    }
}

We see this results in a message saying the time did not round trip! If you change the first line to an actual file instead of a memory stream, and open the zip in whatever program you like, you see the time written is the value specified in the entry.LastWriteTime = now; statement, completely ignoring the timzone. You can see this by changing the now = ... statement between local and UTC time, and see the time stamp in the zip file changes, despite the fact that the DateTimeOffset instance represented the same time, just different offsets.

However, in the second half of the code, where it reads the last write time, .NET assumes the time is local. So, if we write the UTC time to the zip file, when we read it it's offset by whatever time zone our computer is in. One issue is that the .NET type already thinks it's in local time, meaning that entry.LastWriteTime.ToUniversalTime() maintains the error.

From a 10 second test, it seems that var timestamp = entry.LastWriteTime.Add(entry.LastWriteTime.Offset); will adjust the time.

Therefore, I have two suggestions:

  1. When we pack, when we set entry.LastWriteTime = ..., make sure to use .ToUniversalTime() (do this before checking the min/max zip date). Although it appears that NuGet is already correctly using the UTC time on files it creates, someone might be using the NuGet.Packaging package in their own app, and if they don't pass a UTC timestamp to our API, then their nupkg will contain files with the "wrong" timestamp. We should ensure to enforce our conventions (zip file uses UTC time).
  2. When we read the nupkg/zip file, we need to adjust the LastWriteTime, as the value that the .NET API will give us is "wrong" unless your computer happens to be in the UTC+0 ("zulu") timezime.

@tompazourek
Copy link

@zivkan Fascinating, thanks for sharing your findings. One thing that was strange to me is that the DLLs and PDBs inside the NuGet were the only shifted files. The other files (like the [Content_Types].xml, the nuspec file, the stuff in rels directory, package metadata, etc.) all had their timestamp unaffected by this, and so they were correct. So there must be something that NuGet does specifically to DLLs and PDBs to break their timestamps. If it would just pack them like it packs any of the other files, it would work fine.

@zivkan
Copy link
Member

zivkan commented Jan 20, 2021

@tompazourek, when we pack files off the disk, we use FileInfo.LastWriteTimeUtc, whereas for the generated files we either use DateTime[Offset].Now or we don't set a default, and the .NET zip APIs default to Now (local). Hence in my opinion, the time we write to the zip is wrong for the generated files. If you extracted the nupkgs on a computer with a different time zone than the one you packed on, I think you'd agree the time is not correct. This also means that the time we set when we extract is always incorrect. Since the "wrongness" of the generated files matches the wrongness of extraction, it gives the false impression that it's correct, but only when done on computers with the same timezone.

@tompazourek
Copy link

@zivkan That explains it, thanks. In addition to your suggestions how to fix this that all make sense to me (like if we adjust to UTC during packing, we should adjust from UTC when restoring), I'd also suggest handling all files the same way. I don't see why some files should be treated differently than other.

@erdembayar
Copy link
Contributor

@zivkan
Thank you for valuable input. I have taken them into consideration. Just final concern is for deterministic packing, we do exact same thing right?
Here is deterministic packed package. (1/1/1980 12.00am)

image

After I install package in PST it looks like. Is it correct? (12/31/1979 4.00PM)

image

@erdembayar
Copy link
Contributor

erdembayar commented Jan 29, 2021

@zivkan
I believe this issue is addressed in PR#3793. One caveat is this changes only appear for new packages/new versions, because previously download packages/versions already set this value.

For example in file in Newtonsoft nupkg(zip) package:
image

After extraction on PST timezone. Timestamp difference is 8 hours.
image

Same package after extraction on Samao timezone.

image

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