Make PackageBuilder.CalcPsmdcpName deterministic#7410
Conversation
|
|
Alternatively, we can try and fix the nuspec glob-resolving code at to return in a deterministic order. |
| using (var hashFunc = new Sha512HashFunction()) | ||
| { | ||
| foreach (var file in Files) | ||
| foreach (var file in Files.OrderBy(f => f, new NormalizedPathComparer())) |
There was a problem hiding this comment.
Any chance you can add a test for this?
An idea could be to have a nuspec with 2 different globs pointing to 2 different folders and files in them, run pack once, and then move the files around but keep the names so that the same net package is generated.
There was a problem hiding this comment.
If I am understanding it right, you are suggesting we run dotnet pack against a foo.nuspec where foo.nuspec contains globs in files. I am trying this out and it seems like this usage of dotnet pack ... doesn't have support for deterministic mode at all (and this hash computation is guarded by deterministic mode). Do you think I should add -deterministc + -deterministicTimestamp args to dotnet pack or is the PackageBuilderTest.cs test in this PR sufficient?
There was a problem hiding this comment.
Yeah, it's fine to pass the property explicitly.
I am trying to make it possible to test net 11 and net10 at the smae time (.NET 11 has deterministic enabled) in #7423, but dotnet/msbuild#13928 is making it complicated.
There was a problem hiding this comment.
I mean, those options don't exist for dotnet pack. I would have to implement them 😄 Seems a bit out of scope for this PR.
There was a problem hiding this comment.
pack is msbuild driven, so it'll accept the property.
If the nuspec only driven one doesn't support it, that's a bug we can fix separately, but for the purposes of your scenario, I think csproj + nuspec would do the job + support the deterministic prop
There was a problem hiding this comment.
Ah, okay. I was using nuspec-driven only. Let me try to use msbuild + nuspec.
There was a problem hiding this comment.
I can't get the integration test to fail.
Claude thinks it's because the test is only run on Windows, and Windows (NTFS) forces a stable order anyway, so no matter what mutation the test does, the order is already stable. We would have to run the test in an environment that doesn't force a stable order (like Linux).
Various other sources on the internet agree:
https://devblogs.microsoft.com/oldnewthing/20050617-10/?p=35293
NTFS automatically sorts filenames
https://wiki.sleuthkit.org/NTFS/:
Directories in NTFS are indexed to make finding a specific entry in them faster. By default, they are stored in a B-Tree sorted in alphabetical order
https://flatcap.github.io/linux-ntfs/ntfs/concepts/directory.html
When an application reads a directory, NTFS returns a list of file names which is already sorted.
Is the unit test sufficient? Or should I look at finding a way to run the integration test on Linux?
There was a problem hiding this comment.
Just use a fact for the integration test and that should be good enough.
it'll automatically run on all 3 platforms.
d031db4 to
88533da
Compare
04d0658 to
4c5d050
Compare
| } | ||
| } | ||
|
|
||
| [PlatformFact(Platform.Windows)] |
There was a problem hiding this comment.
Just use [Fact] here and it'll run on all platforms and that should be enough.
The order of input files can vary between nuget pack runs, specially if nuspec files use the `**` glob when specifying `<file>` elements. A simple workaround is to sort files before computing the hash of the file contents in nuget package for the psmdcp file name. This keeps the hash stable across nuget pack runs. Fixes: NuGet/Home#14916
4c5d050 to
630e3cd
Compare
Bug
Fixes: NuGet/Home#14916
Description
The order of input files can vary between nuget pack runs, specially if nuspec files use the
**glob when specifying<file>elements.A simple workaround is to sort files before computing the hash of the file contents in nuget package for the psmdcp file name. This keeps the hash stable across nuget pack runs.
PR Checklist