Skip to content

Commit c2aadba

Browse files
jonathanpeppersjonpryor
authored andcommitted
[Xamarin.Android.Build.Tasks] remove Distinct() from <ResolveLibraryProjectImports/> (#3296)
The `<ResolveLibraryProjectImports/>` task has some logic that needs to look at the item metadata of each `@(Reference)`: * `%(AndroidSkipResourceExtraction)` to skip unzipping anything * `%(AndroidSkipResourceProcessing)` to skip `<ConvertResourcesCases/>` To make matters weird, we had some LINQ: foreach (var assemblyPath in Assemblies .Select (a => a.ItemSpec) .Distinct ()) { And we would basically lose the metadata information... To handle this problem, we had two `HashSet`s we used to lookup the values: assembliesToSkipCaseFixup = new HashSet<string> (StringComparer.OrdinalIgnoreCase); assembliestoSkipExtraction = new HashSet<string> (StringComparer.OrdinalIgnoreCase); bool metaDataValue; foreach (var asm in Assemblies) { if (bool.TryParse (asm.GetMetadata (AndroidSkipResourceProcessing), out metaDataValue) && metaDataValue) assembliesToSkipCaseFixup.Add (asm.ItemSpec); if (bool.TryParse (asm.GetMetadata (GetAdditionalResourcesFromAssemblies.AndroidSkipResourceExtraction), out metaDataValue) && metaDataValue) assembliestoSkipExtraction.Add (asm.ItemSpec); } I added a test to see what happens when we use multiple `@(Reference)` such as: * `<PackageReference/>` * `packages.config` and a regular `<Reference/>` * `<Reference/>` with a full path It turns out there are MSBuild targets that disambiguate duplicate assembly references: https://github.com/NuGet/NuGet.BuildTasks/blob/640c8e13a9b7ab6e86264a296638fbf3cc016ad1/src/Microsoft.NuGet.Build.Tasks/Microsoft.NuGet.targets#L186-L207 The `ResolveNuGetPackageAssets` does a `Remove` against duplicate `@(Reference)`, even if there are no NuGet packages present in the project. It seems safe for us to just drop the `Distinct()` and the `HashSet`s. The code is a bit simpler after doing this. I'm also seeing a small ~25ms performance improvement with this change.
1 parent 6ac1aab commit c2aadba

File tree

2 files changed

+29
-18
lines changed

2 files changed

+29
-18
lines changed

src/Xamarin.Android.Build.Tasks/Tasks/ResolveLibraryProjectImports.cs

Lines changed: 6 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,6 @@ public class ResolveLibraryProjectImports : Task
6767
};
6868

6969
AssemblyIdentityMap assemblyMap = new AssemblyIdentityMap();
70-
HashSet<string> assembliesToSkipCaseFixup, assembliestoSkipExtraction;
7170

7271
public ResolveLibraryProjectImports ()
7372
{
@@ -83,16 +82,6 @@ public override bool Execute ()
8382
var resolvedEnvironmentFiles = new List<ITaskItem> ();
8483

8584
assemblyMap.Load (AssemblyIdentityMapFile);
86-
assembliesToSkipCaseFixup = new HashSet<string> (StringComparer.OrdinalIgnoreCase);
87-
assembliestoSkipExtraction = new HashSet<string> (StringComparer.OrdinalIgnoreCase);
88-
bool metaDataValue;
89-
foreach (var asm in Assemblies) {
90-
if (bool.TryParse (asm.GetMetadata (AndroidSkipResourceProcessing), out metaDataValue) && metaDataValue)
91-
assembliesToSkipCaseFixup.Add (asm.ItemSpec);
92-
if (bool.TryParse (asm.GetMetadata (GetAdditionalResourcesFromAssemblies.AndroidSkipResourceExtraction), out metaDataValue) && metaDataValue)
93-
assembliestoSkipExtraction.Add (asm.ItemSpec);
94-
}
95-
9685
using (var resolver = new DirectoryAssemblyResolver (this.CreateTaskLogger (), loadDebugSymbols: false)) {
9786
try {
9887
Extract (resolver, jars, resolvedResourceDirectories, resolvedAssetDirectories, resolvedEnvironmentFiles);
@@ -170,10 +159,9 @@ void Extract (
170159
foreach (var assembly in Assemblies)
171160
res.Load (assembly.ItemSpec);
172161

173-
// FIXME: reorder references by import priority (not sure how to do that yet)
174-
foreach (var assemblyPath in Assemblies
175-
.Select (a => a.ItemSpec)
176-
.Distinct ()) {
162+
bool skip;
163+
foreach (var assemblyItem in Assemblies) {
164+
var assemblyPath = assemblyItem.ItemSpec;
177165
var fileName = Path.GetFileName (assemblyPath);
178166
if (MonoAndroidHelper.IsFrameworkAssembly (fileName) &&
179167
!MonoAndroidHelper.FrameworkEmbeddedJarLookupTargets.Contains (fileName) &&
@@ -185,7 +173,7 @@ void Extract (
185173
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");
186174
continue;
187175
}
188-
if (assembliestoSkipExtraction.Contains (assemblyPath)) {
176+
if (bool.TryParse (assemblyItem.GetMetadata (GetAdditionalResourcesFromAssemblies.AndroidSkipResourceExtraction), out skip) && skip) {
189177
Log.LogDebugMessage ("Skipping resource extraction for '{0}' .", assemblyPath);
190178
continue;
191179
}
@@ -216,7 +204,7 @@ void Extract (
216204
var taskItem = new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
217205
{ OriginalFile, assemblyPath },
218206
});
219-
if (assembliesToSkipCaseFixup.Contains (assemblyPath))
207+
if (bool.TryParse (assemblyItem.GetMetadata (AndroidSkipResourceProcessing), out skip) && skip)
220208
taskItem.SetMetadata (AndroidSkipResourceProcessing, "True");
221209
resolvedResourceDirectories.Add (taskItem);
222210
}
@@ -320,7 +308,7 @@ void Extract (
320308
var taskItem = new TaskItem (Path.GetFullPath (resDir), new Dictionary<string, string> {
321309
{ OriginalFile, assemblyPath }
322310
});
323-
if (assembliesToSkipCaseFixup.Contains (assemblyPath))
311+
if (bool.TryParse (assemblyItem.GetMetadata (AndroidSkipResourceProcessing), out skip) && skip)
324312
taskItem.SetMetadata (AndroidSkipResourceProcessing, "True");
325313
resolvedResourceDirectories.Add (taskItem);
326314
}

src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/BuildTest.cs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,29 @@ public void BuildBasicApplicationAppCompat ([Values (true, false)] bool usePacka
134134
}
135135
}
136136

137+
[Test]
138+
public void DuplicateReferences ()
139+
{
140+
var proj = new XamarinAndroidApplicationProject ();
141+
proj.MainActivity = proj.DefaultMainActivity.Replace ("public class MainActivity : Activity", "public class MainActivity : Android.Support.V7.App.AppCompatActivity");
142+
var package = KnownPackages.SupportV7AppCompat_27_0_2_1;
143+
var fullPath = Path.GetFullPath (Path.Combine (Root, "temp", "packages", $"{package.Id}.{package.Version}", "lib", package.TargetFramework, $"{package.Id}.dll"));
144+
proj.PackageReferences.Add (package);
145+
proj.Packages.Add (package);
146+
proj.References.Add (new BuildItem.Reference (package.Id) {
147+
MetadataValues = "HintPath=" + fullPath,
148+
});
149+
using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
150+
Assert.IsTrue (b.Build (proj), "first build should have succeeded.");
151+
152+
// Remove NuGet packages, but leave References
153+
proj.PackageReferences.Clear ();
154+
proj.Packages.Clear ();
155+
156+
Assert.IsTrue (b.Build (proj), "second build should have succeeded.");
157+
}
158+
}
159+
137160
[Test]
138161
public void BuildXamarinFormsMapsApplication ()
139162
{

0 commit comments

Comments
 (0)