Skip to content

Ensure stable order in .deps.json file #118185

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Ensure stable order in .deps.json file #118185

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 19 additions & 10 deletions src/libraries/Microsoft.Extensions.DependencyModel/src/DependencyContextWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,10 +59,10 @@ private static void WriteRuntimeTargetInfo(DependencyContext context, Utf8JsonWr
private static void WriteRuntimeGraph(DependencyContext context, Utf8JsonWriter jsonWriter)
{
jsonWriter.WriteStartObject(DependencyContextStrings.RuntimesPropertyName);
foreach (RuntimeFallbacks runtimeFallback in context.RuntimeGraph)
foreach (RuntimeFallbacks runtimeFallback in context.RuntimeGraph.OrderBy(r => r.Runtime, StringComparer.Ordinal))
Copy link
Member Author

@omajid omajid Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a risk of this breaking how the runtime fallback is computed/used?

Copy link
Member

@jeffhandley jeffhandley Sep 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ericstj / @ViktorHofer Can one of you weigh in on if this can affect behavior adversely?

Copy link
Member

@ericstj ericstj Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host implements runtime lookup. @elinor-fung I would expect that nothing here is order dependent since the host should be probing in the order defined in the runtimes section of the shared-framework's deps file.

Copy link
Member

@elinor-fung elinor-fung Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The order does matter for RID fallbacks, as the host uses the order of the fallbacks as defined in the .deps.json. This can be a shared framework's deps file or that for the app itself (for example, self-contained) - both should be created by this writer.

The order of the RuntimeFallbacks objects shouldn't matter (this line), but the order of the fallbacks array (line 65) definitely does. That is, a fallback array of [ "linux", "unix-x64", "unix", "any", "base" ] needs to stay in that order and should not become [ "any", "base", "linux", "unix", "unix-x64" ].

{
jsonWriter.WriteStartArray(runtimeFallback.Runtime);
foreach (string? fallback in runtimeFallback.Fallbacks)
foreach (string? fallback in runtimeFallback.Fallbacks.OrderBy(s => s, StringComparer.Ordinal))
{
jsonWriter.WriteStringValue(fallback);
}
Expand Down Expand Up @@ -148,7 +148,7 @@ private static void WritePortableTarget(string key, IReadOnlyList<RuntimeLibrary

jsonWriter.WriteStartObject(key);

foreach (string packageName in runtimeLookup.Keys.Concat(compileLookup.Keys).Distinct())
foreach (string packageName in runtimeLookup.Keys.Concat(compileLookup.Keys).Distinct().OrderBy(s => s, StringComparer.Ordinal))
{
runtimeLookup.TryGetValue(packageName, out RuntimeLibrary? runtimeLibrary);

Expand Down Expand Up @@ -201,7 +201,7 @@ private static void AddDependencies(IReadOnlyCollection<Dependency> dependencies
}

jsonWriter.WriteStartObject(DependencyContextStrings.DependenciesPropertyName);
foreach (Dependency dependency in dependencies)
foreach (Dependency dependency in dependencies.OrderBy(d => d.Name, StringComparer.Ordinal).ThenBy(d => d.Version, StringComparer.Ordinal))
{
jsonWriter.WriteString(dependency.Name, dependency.Version);
}
Expand All @@ -210,6 +210,7 @@ private static void AddDependencies(IReadOnlyCollection<Dependency> dependencies

private static void AddResourceAssemblies(IReadOnlyList<ResourceAssembly> resourceAssemblies, Utf8JsonWriter jsonWriter)
{
resourceAssemblies = resourceAssemblies.OrderBy(r => r.Path, StringComparer.Ordinal).ToList();
Copy link
Member Author

@omajid omajid Jul 29, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was one of the biggest culprits, leading to unpredictable order of resource dlls in deps.json files.

Copy link
Member

@ericstj ericstj Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What made the earlier part of the build non-deterministic in how it passed these in? Typically the order of things in the file, the order in which the filesystem enumerates, etc is deterministic. It's when folks do things like shove them into a dictionary/hashtable then dump that back out that the order becomes non-deterministic.

I'm not too familiar with how much we resign to sorting to solve determinism problems vs rooting out unintentional disorder. Sorting is certainly a more stable solution, but it does burn cycles. I would be interested in hearing from @dsplaisted about this.

Copy link
Member

@akoeplinger akoeplinger Sep 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the order in which the filesystem enumerates

this is not guaranteed though

from Linux readdir

The order in which filenames are read by successive calls to readdir() depends on the filesystem implementation; it is unlikely that the names will be sorted in any fashion.

and on Windows FindNextFileA:

The order in which this function returns the file names is dependent on the file system type. With the NTFS file system and CDFS file systems, the names are usually returned in alphabetical order. With FAT file systems, the names are usually returned in the order the files were written to the disk, which may or may not be in alphabetical order. However, as stated previously, these behaviors are not guaranteed.

Copy link
Member

@ericstj ericstj Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And MSBuild doesn't ensure it either? Is everything that consumes items in the build meant to sort them in order to be deterministic? @rainersigwald

I'm just imagining that we all do less work if the inputs are stable and no one needs to sort than if we all assume inputs are not stable and everyone must sort. I'd prefer a world where every component just preserves input derived order and doesn't apply its own sorting.

Copy link
Member

@rainersigwald rainersigwald Sep 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer a world where every component just preserves input derived order and doesn't apply its own sorting.

This is mostly what MSBuild does--for globs we just preserve filesystem order, and other item operations are order-preserving.

Copy link
Member

@ericstj ericstj Sep 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So presumably that can be done here instead of adding all this sorting. @omajid - do you have a couple example binlogs that show where the ordering gets mixed up? (Bonus if we find and fix a determinism bug that is impacting multiple components instead of just this one).

int count = resourceAssemblies.Count;
if (count == 0)
{
Expand Down Expand Up @@ -264,7 +265,12 @@ private static void WritePortableTargetLibrary(string key, RuntimeLibrary? runti
{
jsonWriter.WriteStartObject(key);

var dependencies = new HashSet<Dependency>();
var dependencyComparer = Comparer<Dependency>.Create((d1, d2) =>
{
int nameComparison = string.Compare(d1.Name, d2.Name, StringComparison.Ordinal);
return (nameComparison != 0) ? nameComparison : string.Compare(d1.Version, d2.Version, StringComparison.Ordinal);
});
var dependencies = new SortedSet<Dependency>(dependencyComparer);
if (runtimeLibrary != null)
{
dependencies.UnionWith(runtimeLibrary.Dependencies);
Expand Down Expand Up @@ -310,7 +316,9 @@ private static void WritePortableTargetLibrary(string key, RuntimeLibrary? runti

private static bool AddRuntimeSpecificAssetGroups(string assetType, IEnumerable<RuntimeAssetGroup> assetGroups, bool wroteObjectStart, Utf8JsonWriter jsonWriter)
{
using IEnumerator<RuntimeAssetGroup> groups = assetGroups.Where(g => !string.IsNullOrEmpty(g.Runtime)).GetEnumerator();
using IEnumerator<RuntimeAssetGroup> groups = assetGroups.Where(g => !string.IsNullOrEmpty(g.Runtime))
.OrderBy(g => g.Runtime, StringComparer.Ordinal)
.GetEnumerator();

if (groups.MoveNext())
{
Expand Down Expand Up @@ -351,7 +359,7 @@ private static bool AddRuntimeSpecificAssetGroups(string assetType, IEnumerable<

private static void AddRuntimeSpecificAssets(IEnumerable<RuntimeFile> assets, string? runtime, string? assetType, Utf8JsonWriter jsonWriter)
{
foreach (RuntimeFile asset in assets)
foreach (RuntimeFile asset in assets.OrderBy(f => f.Path, StringComparer.Ordinal))
{
jsonWriter.WriteStartObject(NormalizePath(asset.Path));

Expand Down Expand Up @@ -380,7 +388,7 @@ private static void AddRuntimeSpecificAssets(IEnumerable<RuntimeFile> assets, st
private static void WriteAssetList(string key, IEnumerable<string> assetPaths, Utf8JsonWriter jsonWriter)
{
jsonWriter.WriteStartObject(key);
foreach (string assembly in assetPaths)
foreach (string assembly in assetPaths.OrderBy(s => s, StringComparer.Ordinal))
{
jsonWriter.WriteStartObject(NormalizePath(assembly));
jsonWriter.WriteEndObject();
Expand All @@ -392,7 +400,7 @@ private static void WriteAssetList(string key, IEnumerable<RuntimeFile> runtimeF
{
jsonWriter.WriteStartObject(key);

foreach (RuntimeFile runtimeFile in runtimeFiles)
foreach (RuntimeFile runtimeFile in runtimeFiles.OrderBy(r => r.Path, StringComparer.Ordinal))
{
jsonWriter.WriteStartObject(NormalizePath(runtimeFile.Path));

Expand Down Expand Up @@ -421,7 +429,8 @@ private static void WriteLibraries(DependencyContext context, Utf8JsonWriter jso
{
IEnumerable<IGrouping<string, Library>> allLibraries =
context.RuntimeLibraries.Cast<Library>().Concat(context.CompileLibraries)
.GroupBy(library => library.Name + DependencyContextStrings.VersionSeparator + library.Version);
.GroupBy(library => library.Name + DependencyContextStrings.VersionSeparator + library.Version)
.OrderBy(grouping => grouping.Key, StringComparer.Ordinal);

jsonWriter.WriteStartObject(DependencyContextStrings.LibrariesPropertyName);
foreach (IGrouping<string, Library> libraryGroup in allLibraries)
Expand Down