Skip to content

[RAR] Don't do I/O on SDK-provided references #8688

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

Merged
merged 7 commits into from
May 5, 2023
Merged

[RAR] Don't do I/O on SDK-provided references #8688

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
24a177b
Don't do I/O on SDK-provided references
ladipro Apr 19, 2023
3d003a9
Use changewave 17.8, delete changewave 17.2
ladipro May 2, 2023
de1a659
PR feedback: Use GetMetadata(FileName)
ladipro May 3, 2023
5e59038
Use AssemblyName metadata if available
ladipro May 3, 2023
be583ab
Remove extraneous SetGetLastWriteTime call
ladipro May 3, 2023
1623885
Update SDKReferencesAreResolvedWithoutIO to test assembly name
ladipro May 3, 2023
aee5113
Intercept GetAssemblyRuntimeVersion per @rokonec's feedback
ladipro May 4, 2023
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
3 changes: 2 additions & 1 deletion src/Framework/ChangeWaves.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ internal class ChangeWaves
internal static readonly Version Wave17_2 = new Version(17, 2);
internal static readonly Version Wave17_4 = new Version(17, 4);
internal static readonly Version Wave17_6 = new Version(17, 6);
internal static readonly Version[] AllWaves = { Wave17_2, Wave17_4, Wave17_6 };
internal static readonly Version Wave17_7 = new Version(17, 7);
internal static readonly Version[] AllWaves = { Wave17_2, Wave17_4, Wave17_6, Wave17_7 };

/// <summary>
/// Special value indicating that all features behind all Change Waves should be enabled.
Expand Down
3 changes: 3 additions & 0 deletions src/Shared/Constants.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ internal static class ItemMetadataNames
internal const string subType = "SubType";
internal const string executableExtension = "ExecutableExtension";
internal const string embedInteropTypes = "EmbedInteropTypes";
internal const string frameworkReferenceName = "FrameworkReferenceName";
internal const string assemblyVersion = "AssemblyVersion";
internal const string publicKeyToken = "PublicKeyToken";

/// <summary>
/// The output path for a given item.
Expand Down
55 changes: 54 additions & 1 deletion src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.IO;
using System.Linq;
Expand All @@ -11,13 +12,14 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using Microsoft.Build.Tasks;
using Microsoft.Build.UnitTests.Shared;
using Microsoft.Build.Tasks.AssemblyDependency;
using Microsoft.Build.Utilities;
using Microsoft.Win32;
using Shouldly;
using Xunit;
using Xunit.Abstractions;
using Xunit.NetCore.Extensions;
using FrameworkNameVersioning = System.Runtime.Versioning.FrameworkName;
using SystemProcessorArchitecture = System.Reflection.ProcessorArchitecture;

#nullable disable
Expand Down Expand Up @@ -8564,5 +8566,56 @@ private static void GenerateRedistAndProfileXmlLocations(string fullRedistConten

File.WriteAllText(profileRedistList, profileListContents);
}

[Fact]
public void SDKReferencesAreResolvedWithoutIO()
{
InitializeRARwithMockEngine(_output, out MockEngine mockEngine, out ResolveAssemblyReference rar);

string refPath = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());

TaskItem item = new TaskItem(refPath);
item.SetMetadata("ExternallyResolved", "true");

item.SetMetadata("FrameworkReferenceName", "Microsoft.NETCore.App");
item.SetMetadata("FrameworkReferenceVersion", "8.0.0");

item.SetMetadata("AssemblyVersion", "8.1.2.3");
item.SetMetadata("PublicKeyToken", "b03f5f7f11d50a3a");

rar.Assemblies = new ITaskItem[] { item };
rar.SearchPaths = new string[]
{
"{CandidateAssemblyFiles}",
"{HintPathFromItem}",
"{TargetFrameworkDirectory}",
"{RawFileName}",
};
rar.WarnOrErrorOnTargetArchitectureMismatch = "Warning";

// Execute RAR and assert that we receive no I/O callbacks because the task gets what it needs from item metadata.
rar.Execute(
_ => throw new ShouldAssertException("Unexpected FileExists callback"),
directoryExists,
getDirectories,
_ => throw new ShouldAssertException("Unexpected GetAssemblyName callback"),
(string path, ConcurrentDictionary<string, AssemblyMetadata> assemblyMetadataCache, out AssemblyNameExtension[] dependencies, out string[] scatterFiles, out FrameworkNameVersioning frameworkName)
=> throw new ShouldAssertException("Unexpected GetAssemblyMetadata callback"),
#if FEATURE_WIN32_REGISTRY
getRegistrySubKeyNames,
getRegistrySubKeyDefaultValue,
#endif
_ => throw new ShouldAssertException("Unexpected GetLastWriteTime callback"),
getRuntimeVersion,
#if FEATURE_WIN32_REGISTRY
openBaseKey,
#endif
checkIfAssemblyIsInGac,
isWinMDFile,
readMachineTypeFromPEHeader).ShouldBeTrue();

rar.ResolvedFiles.Length.ShouldBe(1);
rar.ResolvedFiles[0].ItemSpec.ShouldBe(refPath);
}
}
}
1 change: 1 addition & 0 deletions src/Tasks.UnitTests/HintPathResolver_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ private bool ResolveHintPath(string hintPath)
sdkName: "",
rawFileNameCandidate: "FakeSystem.Net.Http",
isPrimaryProjectReference: true,
isImmutableFrameworkReference: false,
wantSpecificVersion: false,
executableExtensions: new string[] { ".winmd", ".dll", ".exe" },
hintPath: hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/AssemblyFoldersExResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,26 +183,13 @@ private void LazyInitialize()
}
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName">The sdkname of the reference.</param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/AssemblyFoldersFromConfig/AssemblyFoldersFromConfigResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,26 +151,13 @@ private void LazyInitialize()
}
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName">Not used by this type.</param>
/// <param name="rawFileNameCandidate">Not used by this type.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">Not used by this type.</param>
/// <param name="assemblyFolderKey">Not used by this type.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/AssemblyFoldersResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,13 @@ public AssemblyFoldersResolver(string searchPathElement, GetAssemblyName getAsse
{
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName">The sdk name of the reference.</param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
3 changes: 3 additions & 0 deletions src/Tasks/AssemblyDependency/AssemblyResolution.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal static class AssemblyResolution
/// <param name="sdkName"></param>
/// <param name="rawFileNameCandidate">The file name to match if {RawFileName} is seen. (May be null).</param>
/// <param name="isPrimaryProjectReference">True if this is a primary reference directly from the project file.</param>
/// <param name="isImmutableFrameworkReference">True if <paramref name="rawFileNameCandidate"/> is immutable and guaranteed to exist.</param>
/// <param name="wantSpecificVersion"></param>
/// <param name="executableExtensions">The filename extension of the assembly. Must be this or its no match.</param>
/// <param name="hintPath">This reference's hintpath</param>
Expand All @@ -48,6 +49,7 @@ internal static string ResolveReference(
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
Copy link
Contributor

Choose a reason for hiding this comment

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

We're currently saying this is only for immutable framework references, but do you think we might extend that later? If so, maybe we should use a somewhat more generic name?

Copy link
Member

Choose a reason for hiding this comment

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

I'd say what's interesting is not the immutability, but that we can assume its dependency closure is provided by the framework so don't need to explore it--right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair...fullyUnderstoodReferences? Better name with that intent?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd say what's interesting is not the immutability, but that we can assume its dependency closure is provided by the framework so don't need to explore it--right?

Not quite right. The property of "dependency closure is provided" is signaled with the ExternallyResolved metadatum. It is a pre-existing behavior that for such assemblies we skip the dependency walk. It is happening here:

msbuild/src/Tasks/AssemblyDependency/ReferenceTable.cs

Lines 1692 to 1699 in 768cad1

if (!reference.ExternallyResolved || FindDependenciesOfExternallyResolvedReferences)
{
// Look for dependent assemblies.
if (_findDependencies)
{
FindDependenciesAndScatterFiles(reference, newEntries);
}
}

This new flag means "the file is guaranteed to exist and never change" and in the Resolver hierarchy it's used only in RawFilenameResolver to skip the file existence check.

As for the name, I was on the fence between "immutable" and "framework" so I used both 😀

bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down Expand Up @@ -79,6 +81,7 @@ internal static string ResolveReference(
sdkName,
rawFileNameCandidate,
isPrimaryProjectReference,
isImmutableFrameworkReference,
wantSpecificVersion,
executableExtensions,
hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/CandidateAssemblyFilesResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,26 +36,13 @@ public CandidateAssemblyFilesResolver(string[] candidateAssemblyFiles, string se
_candidateAssemblyFiles = candidateAssemblyFiles;
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName"></param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/DirectoryResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,26 +22,13 @@ public DirectoryResolver(string searchPathElement, GetAssemblyName getAssemblyNa
{
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName"></param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/FrameworkPathResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,26 +30,13 @@ public FrameworkPathResolver(string[] frameworkPaths, InstalledAssemblies instal
_installedAssemblies = installedAssemblies;
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assemblyname of the reference.</param>
/// <param name="sdkName"></param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
17 changes: 2 additions & 15 deletions src/Tasks/AssemblyDependency/GacResolver.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,13 @@ public GacResolver(System.Reflection.ProcessorArchitecture targetProcessorArchit
_getAssemblyPathInGac = getAssemblyPathInGac;
}

/// <summary>
/// Resolve a reference to a specific file name.
/// </summary>
/// <param name="assemblyName">The assembly name object of the assembly.</param>
/// <param name="sdkName"></param>
/// <param name="rawFileNameCandidate">The reference's 'include' treated as a raw file name.</param>
/// <param name="isPrimaryProjectReference">Whether or not this reference was directly from the project file (and therefore not a dependency)</param>
/// <param name="wantSpecificVersion">Whether an exact version match is requested.</param>
/// <param name="executableExtensions">Allowed executable extensions.</param>
/// <param name="hintPath">The item's hintpath value.</param>
/// <param name="assemblyFolderKey">Like "hklm\Vendor RegKey" as provided to a reference by the &lt;AssemblyFolderKey&gt; on the reference in the project.</param>
/// <param name="assembliesConsideredAndRejected">Receives the list of locations that this function tried to find the assembly. May be "null".</param>
/// <param name="foundPath">The path where the file was found.</param>
/// <param name="userRequestedSpecificFile">Whether or not the user wanted a specific file (for example, HintPath is a request for a specific file)</param>
/// <returns>True if the file was resolved.</returns>
/// <inheritdoc/>
public override bool Resolve(
AssemblyNameExtension assemblyName,
string sdkName,
string rawFileNameCandidate,
bool isPrimaryProjectReference,
bool isImmutableFrameworkReference,
bool wantSpecificVersion,
string[] executableExtensions,
string hintPath,
Expand Down
Loading