Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] <FilterAssemblies/> shouldn't throw FNFE (
Browse files Browse the repository at this point in the history
dotnet#3720)

A few places during a Xamarin.Android build, we have the following
pattern:

    if (DesignTimeBuild && !File.Exists (assemblyPath)) {
        Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");
        continue;
    }

In the case of a `<ProjectReference/>` not building, there starts to
be a problem here.

What happens during a regular build, is we end up throwing a
`FileNotFoundException` in `<FilterAssemblies/>`.

If we remove the `if (DesignTimeBuild)` check, we would instead see
the behavior you would get for a .NET Console app referencing a .NET
Framework library project:

    (ResolveProjectReferences target) ->
        C:\Program Files (x86)\Microsoft Visual Studio\2019\Enterprise\MSBuild\Current\Bin\Microsoft.Common.CurrentVersion.targets(1875,5):
        warning : The referenced project '..\MyLibrary\MyLibrary.csproj' does not exist.
    (CoreCompile target) ->
        Foo.cs(1,20,1,23):
        error CS0246: The type or namespace name 'Bar' could not be found (are you missing a using directive or an assembly reference?)
        1 Warning(s)
        1 Error(s)

We get a warning about a missing `<ProjectReference/>` and any C#
compiler errors that would be encountered.

I think this makes more sense and developers will less likely point
fingers at `<FilterAssemblies/>`?

I added a test around this scenario, too.
  • Loading branch information
jonathanpeppers authored Oct 15, 2019
1 parent 6d85759 commit 783ac9e
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/Xamarin.Android.Build.Tasks/Tasks/FilterAssemblies.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ public override bool RunTask ()

var output = new List<ITaskItem> (InputAssemblies.Length);
foreach (var assemblyItem in InputAssemblies) {
if (DesignTimeBuild && !File.Exists (assemblyItem.ItemSpec)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyItem.ItemSpec}' during a design-time build.");
if (!File.Exists (assemblyItem.ItemSpec)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyItem.ItemSpec}'.");
continue;
}
using (var pe = new PEReader (File.OpenRead (assemblyItem.ItemSpec))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ public override bool RunTask ()
Log.LogDebugMessage ($"Skipping framework assembly '{fileName}'.");
continue;
}
if (DesignTimeBuild && !File.Exists (assemblyPath)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");
if (!File.Exists (assemblyPath)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}'.");
continue;
}
assemblies.Add (resolver.Load (assemblyPath));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ void Extract (
Log.LogDebugMessage ($"Skipping framework assembly '{fileName}'.");
continue;
}
if (DesignTimeBuild && !File.Exists (assemblyPath)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}' during a design-time build.");
if (!File.Exists (assemblyPath)) {
Log.LogDebugMessage ($"Skipping non-existent dependency '{assemblyPath}'.");
continue;
}
if (bool.TryParse (assemblyItem.GetMetadata (GetAdditionalResourcesFromAssemblies.AndroidSkipResourceExtraction), out skip) && skip) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -972,5 +972,54 @@ public void ChangePackageNamingPolicy ()
Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, AndroidSdkPath), $"`{dexFile}` should include `{className}`!");
}
}

[Test]
public void MissingProjectReference ()
{
var path = Path.Combine ("temp", TestName);

var bar = "public class Bar { }";
var lib = new XamarinAndroidLibraryProject {
ProjectName = "MyLibrary",
Sources = {
new BuildItem.Source ("Bar.cs") {
TextContent = () => bar
},
}
};
var app = new XamarinAndroidApplicationProject {
ProjectName = "MyApp",
Sources = {
new BuildItem.Source ("Foo.cs") {
TextContent = () => "public class Foo : Bar { }"
},
}
};
var reference = $"..\\{lib.ProjectName}\\{lib.ProjectName}.csproj";
app.References.Add (new BuildItem.ProjectReference (reference, lib.ProjectName, lib.ProjectGuid));

using (var libBuilder = CreateDllBuilder (Path.Combine (path, lib.ProjectName)))
using (var appBuilder = CreateApkBuilder (Path.Combine (path, app.ProjectName))) {
libBuilder.ThrowOnBuildFailure =
appBuilder.ThrowOnBuildFailure = false;

// Build app before library is built
Assert.IsFalse (appBuilder.Build (app), "app build should have failed.");
Assert.IsTrue (StringAssertEx.ContainsText (appBuilder.LastBuildOutput, $"The referenced project '{reference.Replace ('\\', Path.DirectorySeparatorChar)}' does not exist."));
Assert.IsTrue (StringAssertEx.ContainsText (appBuilder.LastBuildOutput, " 1 Warning(s)"), "Should receive 1 Warning");
Assert.IsTrue (StringAssertEx.ContainsText (appBuilder.LastBuildOutput, "error CS0246"), "Should receive CS0246");
Assert.IsTrue (StringAssertEx.ContainsText (appBuilder.LastBuildOutput, " 1 Error(s)"), "Should receive 1 Error");

// Successful build
Assert.IsTrue (libBuilder.Build (lib), "lib build should have succeeded.");
Assert.IsTrue (appBuilder.Build (app), "app build should have succeeded.");

// Create compiler error in library, the app will still be able to build
bar += "}";
lib.Touch ("Bar.cs");
Assert.IsFalse (libBuilder.Build (lib), "lib build should have failed.");
Assert.IsTrue (appBuilder.Build (app), "app build should have succeeded.");
}
}
}
}

0 comments on commit 783ac9e

Please sign in to comment.