Skip to content
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

Optionally output unresolved assembly conflicts #5990

Merged
merged 6 commits into from
Jan 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ public ResolveAssemblyReference() { }
public Microsoft.Build.Framework.ITaskItem[] InstalledAssemblySubsetTables { get { throw null; } set { } }
public Microsoft.Build.Framework.ITaskItem[] InstalledAssemblyTables { get { throw null; } set { } }
public string[] LatestTargetFrameworkDirectories { get { throw null; } set { } }
public bool OutputUnresolvedAssemblyConflicts { get { throw null; } set { } }
public string ProfileName { get { throw null; } set { } }
[Microsoft.Build.Framework.OutputAttribute]
public Microsoft.Build.Framework.ITaskItem[] RelatedFiles { get { throw null; } }
Expand Down Expand Up @@ -954,6 +955,8 @@ public ResolveAssemblyReference() { }
public string[] TargetFrameworkSubsets { get { throw null; } set { } }
public string TargetFrameworkVersion { get { throw null; } set { } }
public string TargetProcessorArchitecture { get { throw null; } set { } }
[Microsoft.Build.Framework.OutputAttribute]
public Microsoft.Build.Framework.ITaskItem[] UnresolvedAssemblyConflicts { get { throw null; } }
public bool UnresolveFrameworkAssembliesFromHigherFrameworks { get { throw null; } set { } }
public string WarnOrErrorOnTargetArchitectureMismatch { get { throw null; } set { } }
public override bool Execute() { throw null; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ public ResolveAssemblyReference() { }
public Microsoft.Build.Framework.ITaskItem[] InstalledAssemblySubsetTables { get { throw null; } set { } }
public Microsoft.Build.Framework.ITaskItem[] InstalledAssemblyTables { get { throw null; } set { } }
public string[] LatestTargetFrameworkDirectories { get { throw null; } set { } }
public bool OutputUnresolvedAssemblyConflicts { get { throw null; } set { } }
public string ProfileName { get { throw null; } set { } }
[Microsoft.Build.Framework.OutputAttribute]
public Microsoft.Build.Framework.ITaskItem[] RelatedFiles { get { throw null; } }
Expand Down Expand Up @@ -699,6 +700,8 @@ public ResolveAssemblyReference() { }
public string[] TargetFrameworkSubsets { get { throw null; } set { } }
public string TargetFrameworkVersion { get { throw null; } set { } }
public string TargetProcessorArchitecture { get { throw null; } set { } }
[Microsoft.Build.Framework.OutputAttribute]
public Microsoft.Build.Framework.ITaskItem[] UnresolvedAssemblyConflicts { get { throw null; } }
public bool UnresolveFrameworkAssembliesFromHigherFrameworks { get { throw null; } set { } }
public string WarnOrErrorOnTargetArchitectureMismatch { get { throw null; } set { } }
public override bool Execute() { throw null; }
Expand Down
31 changes: 31 additions & 0 deletions src/Tasks.UnitTests/AssemblyDependency/Miscellaneous.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3686,6 +3686,37 @@ public void ConflictGeneratesMessageReferencingAssemblyName()
warningMessage.ShouldContain(ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ResolveAssemblyReference.FourSpaceIndent", ResourceUtilities.FormatResourceStringIgnoreCodeAndKeyword("ResolveAssemblyReference.ReferenceDependsOn", "D, Version=1.0.0.0, CulTUre=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa", Path.Combine(s_myLibraries_V1Path, "D.dll"))));
}

[Fact]
public void ConflictOutputsExtraInformationOnDemand()
Copy link
Member

Choose a reason for hiding this comment

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

Should we make this a theory and test what happens when the item has the .dll extension? @dsplaisted is there a concrete answer for including .dll in the assembly name? Or did your comment, "possibly with .dll possibly not," refer to the fact that assemblyName.Name may not have the extension along with it?

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 understood him to mean that the SDK can handle the output having the .dll extension or not having it—it's the same information, just presented slightly differently. Since I'm creating the item, and I went with no .dll extension, that shouldn't be an issue.

Copy link
Member

Choose a reason for hiding this comment

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

I wasn't sure whether the assembly name processed by ResolveAssemblyReference would have the .dll extension on it. If it consistently does not have the extension, then I would leave it out. It's also theoretically possible to reference an .exe, I believe.

Copy link
Member

Choose a reason for hiding this comment

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

The summary behind assemblyName.Name:

Gets or sets the simple name of the assembly. This is usually, but not necessarily,
the file name of the manifest file of the assembly, minus its extension.

Looks like the SDK will have to account for both scenarios, unless there's a preference on how we handle it here. I'm okay with either scenario.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like MSBuild shouldn't do anything special here then (ie don't add the .dll extension).

{
ResolveAssemblyReference t = new ResolveAssemblyReference();

MockEngine e = new MockEngine(_output);
t.BuildEngine = e;

t.Assemblies = new ITaskItem[]
{
new TaskItem("B"),
new TaskItem("D, Version=1.0.0.0, Culture=neutral, PublicKeyToken=aaaaaaaaaaaaaaaa")
};

t.SearchPaths = new string[]
{
s_myLibrariesRootPath, s_myLibraries_V2Path, s_myLibraries_V1Path
};

t.TargetFrameworkDirectories = new string[] { s_myVersion20Path };
t.OutputUnresolvedAssemblyConflicts = true;

Execute(t);

ITaskItem[] conflicts = t.UnresolvedAssemblyConflicts;
conflicts.Length.ShouldBe(1);
conflicts[0].ItemSpec.ShouldBe("D");
conflicts[0].GetMetadata("victorVersionNumber").ShouldBe("1.0.0.0");
conflicts[0].GetMetadata("victimVersionNumber").ShouldBe("2.0.0.0");
}

/// <summary>
/// Consider this dependency chain:
///
Expand Down
33 changes: 30 additions & 3 deletions src/Tasks/AssemblyDependency/ResolveAssemblyReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public ResolveAssemblyReference()
private ITaskItem[] _scatterFiles = Array.Empty<TaskItem>();
private ITaskItem[] _copyLocalFiles = Array.Empty<TaskItem>();
private ITaskItem[] _suggestedRedirects = Array.Empty<TaskItem>();
private List<ITaskItem> _unresolvedConflicts = new List<ITaskItem>();
private string[] _targetFrameworkSubsets = Array.Empty<string>();
private string[] _fullTargetFrameworkSubsetNames = Array.Empty<string>();
private string _targetedFrameworkMoniker = String.Empty;
Expand Down Expand Up @@ -214,6 +215,11 @@ public bool IgnoreTargetFrameworkAttributeVersionMismatch
/// </remarks>
public bool FindDependenciesOfExternallyResolvedReferences { get; set; }

/// <summary>
/// If true, outputs any unresolved assembly conflicts (MSB3277) in UnresolvedAssemblyConflicts.
/// </summary>
public bool OutputUnresolvedAssemblyConflicts { get; set; }

/// <summary>
/// List of target framework subset names which will be searched for in the target framework directories
/// </summary>
Expand Down Expand Up @@ -915,6 +921,13 @@ public String DependsOnNETStandard
private set;
}

/// <summary>
/// If OutputUnresolvedAssemblyConflicts then a list of information about unresolved conflicts that normally would have
/// been outputted in MSB3277. Otherwise empty.
/// </summary>
[Output]
public ITaskItem[] UnresolvedAssemblyConflicts => _unresolvedConflicts.ToArray();

#endregion
#region Logging

Expand Down Expand Up @@ -990,16 +1003,30 @@ quiet at the engine level.
// Log the reference which lost the conflict and the dependencies and source items which caused it.
LogReferenceDependenciesAndSourceItemsToStringBuilder(fusionName, conflictCandidate, logDependencies.AppendLine());

string output = StringBuilderCache.GetStringAndRelease(logConflict);
string details = string.Empty;
if (logWarning)
{
// This warning is logged regardless of AutoUnify since it means a conflict existed where the reference
// chosen was not the conflict victor in a version comparison. In other words, the victor was older.
Log.LogWarningWithCodeFromResources("ResolveAssemblyReference.FoundConflicts", assemblyName.Name, StringBuilderCache.GetStringAndRelease(logConflict));
Log.LogWarningWithCodeFromResources("ResolveAssemblyReference.FoundConflicts", assemblyName.Name, output);
}
else
{
Log.LogMessage(ChooseReferenceLoggingImportance(conflictCandidate), StringBuilderCache.GetStringAndRelease(logConflict));
Log.LogMessage(MessageImportance.Low, StringBuilderCache.GetStringAndRelease(logDependencies));
details = StringBuilderCache.GetStringAndRelease(logDependencies);
Log.LogMessage(ChooseReferenceLoggingImportance(conflictCandidate), output);
Log.LogMessage(MessageImportance.Low, details);
}

if (OutputUnresolvedAssemblyConflicts)
{
_unresolvedConflicts.Add(new TaskItem(assemblyName.Name, new Dictionary<string, string>()
{
{ "logMessage", output },
{ "logMessageDetails", details },
{ "victorVersionNumber", victor.ReferenceVersion.ToString() },
{ "victimVersionNumber", conflictCandidate.ReferenceVersion.ToString() }
}));
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/Tasks/Microsoft.Common.CurrentVersion.targets
Original file line number Diff line number Diff line change
Expand Up @@ -2156,6 +2156,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<ResolveAssemblyReferencesFindRelatedSatellites Condition="'$(ResolveAssemblyReferencesFindRelatedSatellites)' == ''">$(BuildingProject)</ResolveAssemblyReferencesFindRelatedSatellites>
<ResolveAssemblyReferencesFindSerializationAssemblies Condition="'$(ResolveAssemblyReferencesFindSerializationAssemblies)' == ''">$(BuildingProject)</ResolveAssemblyReferencesFindSerializationAssemblies>
<ResolveAssemblyReferencesFindRelatedFiles Condition="'$(ResolveAssemblyReferencesFindRelatedFiles)' == ''">$(BuildingProject)</ResolveAssemblyReferencesFindRelatedFiles>
<ResolveAssemblyReferenceOutputUnresolvedAssemblyConflicts Condition="'$(ResolveAssemblyReferenceOutputUnresolvedAssemblyConflicts)' == ''">false</ResolveAssemblyReferenceOutputUnresolvedAssemblyConflicts>
</PropertyGroup>

<ItemGroup>
Expand Down Expand Up @@ -2218,6 +2219,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
IgnoreTargetFrameworkAttributeVersionMismatch ="$(ResolveAssemblyReferenceIgnoreTargetFrameworkAttributeVersionMismatch)"
FindDependenciesOfExternallyResolvedReferences="$(FindDependenciesOfExternallyResolvedReferences)"
ContinueOnError="$(ContinueOnError)"
OutputUnresolvedAssemblyConflicts="$(ResolveAssemblyReferenceOutputUnresolvedAssemblyConflicts)"
Condition="'@(Reference)'!='' or '@(_ResolvedProjectReferencePaths)'!='' or '@(_ExplicitReference)' != ''"
>

Expand All @@ -2233,6 +2235,7 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<Output TaskParameter="FilesWritten" ItemName="FileWrites"/>
<Output TaskParameter="DependsOnSystemRuntime" PropertyName="DependsOnSystemRuntime"/>
<Output TaskParameter="DependsOnNETStandard" PropertyName="_DependsOnNETStandard"/>
<Output TaskParameter="UnresolvedAssemblyConflicts" ItemName="ResolveAssemblyReferenceUnresolvedAssemblyConflicts"/>
</ResolveAssemblyReference>
</Target>

Expand Down