Ensure stable order of files in the nuget package

[omajid]

Bug

Fixes: NuGet/Home#14448

Description

We can add files to the nuget package in any order. To provide more reproducibility/deterministicness, add the files in the archive in a well-defined order.

PR Checklist

• Meaningful title, helpful description and a linked NuGet/Home issue
• Added tests
• Link to an issue or pull request to update docs if this PR changes settings, environment variables, new feature, etc.

[omajid]

Anyone know if this provides the same sorting order across platforms? The unit tests have many comments like this:

// Linux sorts the first two in different order than Window

And I would love to make sure this ordering is the same no matter the platform.

[zivkan]

I think it's a culture/locale issue, not an OS issue. I found 3 instances of the comment, and all 3 were doing stringList.OrderBy(s => s), so it didn't specify a string comparison method, and therefore defaults to culture specific sort.

Using this C# program:

string[] values = [ "[Content_Types].xml", "_rel/.rels" ];

List<string> contentTypesFirst = new List<string>();
List<string> relsFirst = new List<string>();

foreach (var culture in System.Globalization.CultureInfo.GetCultures(System.Globalization.CultureTypes.SpecificCultures))
{
    System.Threading.Thread.CurrentThread.CurrentCulture = culture;
    System.Threading.Thread.CurrentThread.CurrentUICulture = culture;
    if (values.OrderBy(v => v).First() == "[Content_Types].xml")
    {
        contentTypesFirst.Add(culture.Name);
    }
    else
    {
        relsFirst.Add(culture.Name);
    }
}

Console.WriteLine("Cultures where [Content_Types].xml comes first: " + string.Join(", ", contentTypesFirst));
Console.WriteLine("Cultures where _rels/.rels comes first: " + string.Join(", ", relsFirst));

Running on both Windows and Ubuntu (on WSL2), there's only 2 CultureInfo's that sort content types first: th-TH and en-US-POSIX

My guess is that the people at the time who wrote that comment probably had a linux machine using the C/POSIX locale and didn't know better.

I didn't test this PR's change, but since it's explicitly using String.CompareOrdinal, I expect it to work. I think new SortedSet<IPackageFile>(Files, StringComparer.Ordinal) is a shorter way to write the equivalent outcome.

[omajid]

Thanks for digging into this. C/POSIX makes a lot of sense.

I can't get new SortedSet<IPackageFile>(Files, StringComparer.Ordinal) to work. I think that tries to compare the the IPackageFile objects directly instead of using the nested Path (a string) property:

error CS1503: Argument 2: cannot convert from 'System.StringComparer' to 'System.Collections.Generic.IComparer<IPackageFile>?'

[zivkan]

Oops, I missed that it was comparing a property, rather than the object itself. Yes, the anonymous function is needed.

[omajid]

I was away on PTO. I plan to pick this up again in a few days.

[zivkan]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[zivkan]

There are two tests that need to be updated, they appear to expect files in a specific order. Otherwise, it looks good to me.

[zivkan]

Oops, I missed that it was comparing a property, rather than the object itself. Yes, the anonymous function is needed.

[aortiz-msft]

@omajid - We'd need this PR merged this month for it to make it in .NET 10.

[omajid]

Thanks for the ping. I will re-prioritize this.

[Frulfump]

Oh no this was abandoned, has the ship sailed on .NET 10 now?

[nkolev92]

yeah, it's too late now.

Strictly stabilization.