-
Notifications
You must be signed in to change notification settings - Fork 257
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
Remove forced override of package contents #1156
Changes from 3 commits
b2d155f
9524cd7
d3b9acc
914c051
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -460,14 +460,51 @@ await PackageExtractor.ExtractPackageAsync( | |
} | ||
} | ||
|
||
// Prefer assemblies from the reference assembly package to ones otherwise provided | ||
// Prefer newer assemblies when more than one have the same name | ||
if (ReferenceAssemblyPackage is not null) | ||
{ | ||
var referenceAssemblies = new HashSet<string>(resolvedAssemblies.Where(resolved => resolved.StartsWith(referenceAssemblyInstalledPath!))); | ||
var comparer = new FrameworkPrecedenceSorter(DefaultFrameworkNameProvider.Instance, allEquivalent: false); | ||
var assembliesByName = resolvedAssemblies.GroupBy(Path.GetFileNameWithoutExtension, StringComparer.OrdinalIgnoreCase); | ||
var assembliesToRemove = new List<string>(); | ||
foreach (var assemblyNameGroup in assembliesByName) | ||
{ | ||
var assembliesByPrecedence = assemblyNameGroup.OrderBy(GetFrameworkNameFromPath, comparer).ThenByDescending(GetFrameworkNameFromPath, new NuGetFrameworkSorter()).ToArray(); | ||
for (var i = 1; i < assembliesByPrecedence.Length; i++) | ||
{ | ||
// We want to keep the last reference listed for the most recent supported target framework. | ||
// Typically, if more than one item has the most recent supported target framework, it will | ||
// be a case where the reference assembly package provides the assembly and a newer version | ||
// is provided explicitly. For example: | ||
// | ||
// Microsoft.NETCore.App.Ref 6.0.0 provides System.Collections.Immutable in the net6.0 folder | ||
// System.Collections.Immutable 8.0.0 provides System.Collections.Immutable in the net6.0 folder | ||
// | ||
// In this example, the Microsoft.NETCore.App.Ref package is resolved first, so by taking | ||
// the last net6.0 assembly, we ensure the assembly from System.Collections.Immutable 8.0.0 | ||
// is resolved. | ||
if (Equals(GetFrameworkNameFromPath(assembliesByPrecedence[0]), GetFrameworkNameFromPath(assembliesByPrecedence[i]))) | ||
{ | ||
assembliesToRemove.Add(assembliesByPrecedence[i - 1]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I assume it's the GroupBy usage, but if that were the case, I would have thought just realizing that result would have been easier) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ➡️ I'm going to keep the current form for ease of debugging. Will add a comment in the code explaining that. |
||
} | ||
else | ||
{ | ||
assembliesToRemove.Add(assembliesByPrecedence[i]); | ||
} | ||
} | ||
|
||
static NuGetFramework GetFrameworkNameFromPath(string path) | ||
{ | ||
var frameworkFolder = Path.GetFileName(Path.GetDirectoryName(path)); | ||
if (frameworkFolder is null) | ||
{ | ||
return NuGetFramework.UnsupportedFramework; | ||
} | ||
|
||
return NuGetFramework.ParseFolder(frameworkFolder); | ||
} | ||
} | ||
|
||
// Suppression due to https://github.com/dotnet/roslyn/issues/44735 | ||
var referenceAssemblyNames = new HashSet<string>(referenceAssemblies.Select((Func<string, string>)Path.GetFileNameWithoutExtension!)); | ||
resolvedAssemblies.RemoveWhere(resolved => referenceAssemblyNames.Contains(Path.GetFileNameWithoutExtension(resolved)) && !referenceAssemblies.Contains(resolved)); | ||
resolvedAssemblies.ExceptWith(assembliesToRemove); | ||
} | ||
|
||
// Add the facade assemblies | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as this is invariant, consider moving it outside the loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
➡️ Moved to single evaluation of
GetFrameworkNameFromPath