Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] Desugar works without ProGuard (dotnet#…
Browse files Browse the repository at this point in the history
…1973)

Looking into a customer's binding project, usage of
`$(AndroidEnableDesugar)` was causing a crash at runtime where the
`mono.MonoRuntimeProvider` type was completely missing from the
`app.apk`.  Looking into it further, it seemed that our desugar
support only worked when used with ProGuard or MultiDex.

What was happening:

  - The `<Desugar/>` MSBuild task "desugars" the Xamarin.Android app
    code into a `__app_classes__.jar` file
  - However `__app_classes__.jar` was not added to the `OutputJars`
    output property
  - In `Xamarin.Android.Common.targets`, `@(_AlternativeJarForAppDx)`
    is used for the input to `javac`, which does not contain any of
    the Xamarin.Android app's Java code!

In fact, a workaround for the customer's project, was to add the
following to their Xamarin.Android app project file:

    <ItemGroup>
      <_AlternativeJarForAppDx Include="$(IntermediateOutputPath)android\bin\desugared\__app_classes__.jar" />
    </ItemGroup>

-- which was less than ideal.

So before I fixed the issue, I improved our tests:

  - Added a `DexUtils` class that invokes the `dexdump` command, so
    we can assert that a dex file contains a specific Java class
  - Added `AndroidSdkDirectory` and `AndroidNdkDirectory` properties
    to `Xamarin.ProjectTools`, since I needed the Android SDK path to
    use `dexdump`
  - Fixed a bug where we need to use
    `Environment.SpecialFolder.UserProfile`, or the
    `android-toolchain\sdk` directory is not found on Windows
  - Added an assertion to the desugar tests, checking that
    `Lmono/MonoRuntimeProvider;` exists in the dex file
  - The desugar tests should also `ThrowOnBuildFailure` if desugar is
    enabled

Next, the fixes for desugar support:

  - We no longer need `Desugar.InputClassesDirectory`, and do not
    need to generate `__app_classes__.jar`, since it was not being
    used at all anyway.
  - In `Xamarin.Android.Common.targets` we should include
    `classes.zip` in the `@(_AlternativeJarForAppDx)` item group.
  - I updated the `<Desugar/>` task to our new pattern of only
    logging `[Output]` properties

The new tests are now green!

Note: I tried *including* `__app_classes__.jar` at first, but
proguard commands were failing.  When the app code was "desugared",
required attributes were removed such as `@Keep`.  Since D8/R8 is
the way forward in the future, I thought it simpler to just omit the
creation of `__app_classes__.jar`.
  • Loading branch information
jonathanpeppers authored and jonpryor committed Jul 18, 2018
1 parent ab70b2d commit 5995f0c
Show file tree
Hide file tree
Showing 6 changed files with 110 additions and 50 deletions.
19 changes: 0 additions & 19 deletions src/Xamarin.Android.Build.Tasks/Tasks/Desugar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
using System.Text;
using System.Collections.Generic;
using Xamarin.Android.Tools;
using Xamarin.Android.Tools.Aidl;

namespace Xamarin.Android.Tasks
{
Expand All @@ -29,7 +28,6 @@ public class Desugar : JavaToolTask
[Required]
public string OutputDirectory { get; set; }

public string InputClassesDirectory { get; set; }

public string [] InputJars { get; set; }

Expand All @@ -38,15 +36,6 @@ public class Desugar : JavaToolTask

public override bool Execute ()
{
Log.LogDebugMessage ("Desugar Task");
Log.LogDebugMessage (" JavaPlatformJarPath: ", JavaPlatformJarPath);
Log.LogDebugMessage (" DesugarJarPath: ", DesugarJarPath);
Log.LogDebugMessage (" DesugarExtraArguments: ", DesugarExtraArguments);
Log.LogDebugMessage (" ManifestFile: ", ManifestFile);
Log.LogDebugMessage (" OutputDirectory: ", OutputDirectory);
Log.LogDebugMessage (" InputClassesDirectory: ", InputClassesDirectory);
Log.LogDebugTaskItems (" InputJars: ", InputJars);

if (!Directory.Exists (OutputDirectory))
Directory.CreateDirectory (OutputDirectory);

Expand Down Expand Up @@ -91,13 +80,6 @@ protected override string GenerateCommandLineCommands ()
if (!string.IsNullOrEmpty (DesugarExtraArguments))
cmd.AppendSwitch (DesugarExtraArguments); // it should contain "--dex".

if (!string.IsNullOrEmpty (InputClassesDirectory)) {
cmd.AppendSwitch ("--input ");
cmd.AppendFileNameIfNotNull (InputClassesDirectory);
cmd.AppendSwitch ("--output ");
cmd.AppendFileNameIfNotNull (Path.Combine (OutputDirectory, "__app_classes__.jar"));
}

var outputs = new List<string> ();
var md5 = System.Security.Cryptography.MD5.Create ();
foreach (var jar in InputJars) {
Expand All @@ -110,7 +92,6 @@ protected override string GenerateCommandLineCommands ()
}

OutputJars = outputs.ToArray ();
Log.LogDebugTaskItems (" OutputJars: ", OutputJars);

return cmd.ToString ();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2831,9 +2831,15 @@ public void foo()
0QMAAAwAAAAAAAAAAAAAAAAAwwAAAExhbWJkYS5jbGFzc1BLBQYAAAAAAwADALcAAAD7AgAAAAA=
") });
using (var builder = CreateApkBuilder (Path.Combine ("temp", TestContext.CurrentContext.Test.Name))) {
builder.ThrowOnBuildFailure = false;
builder.ThrowOnBuildFailure = enableDesugar;
Assert.AreEqual (enableDesugar, builder.Build (proj), "Unexpected build result");
Assert.IsFalse (builder.LastBuildOutput.ContainsText ("Duplicate zip entry"), "Should not get warning about [META-INF/MANIFEST.MF]");

if (enableDesugar) {
var className = "Lmono/MonoRuntimeProvider;";
var dexFile = builder.Output.GetIntermediaryPath (Path.Combine ("android", "bin", "classes.dex"));
Assert.IsTrue (DexUtils.ContainsClass (className, dexFile, builder.AndroidSdkDirectory), $"`{dexFile}` should include `{className}`!");
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,39 @@ public void Dispose ()
GC.SuppressFinalize (this);
}

public string AndroidSdkDirectory { get; private set; }

public string AndroidNdkDirectory { get; private set; }

/// <summary>
/// Locates and sets AndroidSdkDirectory and AndroidNdkDirectory
/// </summary>
public void ResolveSdks ()
{
var homeDirectory = Environment.GetFolderPath (Environment.SpecialFolder.UserProfile);
var androidSdkToolPath = Path.Combine (homeDirectory, "android-toolchain");
if (string.IsNullOrEmpty (AndroidSdkDirectory)) {
var sdkPath = Environment.GetEnvironmentVariable ("ANDROID_SDK_PATH");
if (String.IsNullOrEmpty (sdkPath))
sdkPath = GetPathFromRegistry ("AndroidSdkDirectory");
if (String.IsNullOrEmpty (sdkPath))
sdkPath = Path.GetFullPath (Path.Combine (androidSdkToolPath, "sdk"));
if (Directory.Exists (sdkPath)) {
AndroidSdkDirectory = sdkPath;
}
}
if (string.IsNullOrEmpty (AndroidNdkDirectory)) {
var ndkPath = Environment.GetEnvironmentVariable ("ANDROID_NDK_PATH");
if (String.IsNullOrEmpty (ndkPath))
ndkPath = GetPathFromRegistry ("AndroidNdkDirectory");
if (String.IsNullOrEmpty (ndkPath))
ndkPath = Path.GetFullPath (Path.Combine (androidSdkToolPath, "ndk"));
if (Directory.Exists (ndkPath)) {
AndroidNdkDirectory = ndkPath;
}
}
}

protected virtual void Dispose (bool disposing)
{
}
Expand Down Expand Up @@ -252,20 +285,9 @@ protected bool BuildInternal (string projectOrSolution, string target, string []
: string.Format ("/noconsolelogger \"/flp1:LogFile={0};Encoding=UTF-8;Verbosity={1}\"",
buildLogFullPath, Verbosity.ToString ().ToLower ());

var start = DateTime.UtcNow;
var homeDirectory = Environment.GetFolderPath (Environment.SpecialFolder.Personal);
var androidSdkToolPath = Path.Combine (homeDirectory, "android-toolchain");
var sdkPath = Environment.GetEnvironmentVariable ("ANDROID_SDK_PATH");
if (String.IsNullOrEmpty (sdkPath))
sdkPath = GetPathFromRegistry ("AndroidSdkDirectory");
if (String.IsNullOrEmpty (sdkPath))
sdkPath = Path.GetFullPath (Path.Combine (androidSdkToolPath, "sdk"));
var ndkPath = Environment.GetEnvironmentVariable ("ANDROID_NDK_PATH");
if (String.IsNullOrEmpty (ndkPath))
ndkPath = GetPathFromRegistry ("AndroidNdkDirectory");
if (String.IsNullOrEmpty (ndkPath))
ndkPath = Path.GetFullPath (Path.Combine (androidSdkToolPath, "ndk"));
ResolveSdks ();

var start = DateTime.UtcNow;
var args = new StringBuilder ();
var psi = new ProcessStartInfo (XABuildExe);
args.AppendFormat ("{0} /t:{1} {2}",
Expand All @@ -274,11 +296,11 @@ protected bool BuildInternal (string projectOrSolution, string target, string []
args.Append (" /p:BuildingOutOfProcess=true");
else
args.Append (" /p:UseHostCompilerIfAvailable=false /p:BuildingInsideVisualStudio=true");
if (Directory.Exists (sdkPath)) {
args.AppendFormat (" /p:AndroidSdkDirectory=\"{0}\" ", sdkPath);
if (!string.IsNullOrEmpty (AndroidSdkDirectory)) {
args.AppendFormat (" /p:AndroidSdkDirectory=\"{0}\" ", AndroidSdkDirectory);
}
if (Directory.Exists (ndkPath)) {
args.AppendFormat (" /p:AndroidNdkDirectory=\"{0}\" ", ndkPath);
if (!string.IsNullOrEmpty (AndroidNdkDirectory)) {
args.AppendFormat (" /p:AndroidNdkDirectory=\"{0}\" ", AndroidNdkDirectory);
}
if (parameters != null) {
foreach (var param in parameters) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
using System;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Text;
using Xamarin.Android.Tools;

namespace Xamarin.ProjectTools
{
public static class DexUtils
{
/// <summary>
/// Runs the dexdump command to see if a class exists in a dex file
/// dexdump returns data of the form:
/// Class descriptor : 'Landroid/app/ActivityTracker;'
/// </summary>
/// <param name="className">A Java class name of the form 'Landroid/app/ActivityTracker;'</param>
public static bool ContainsClass (string className, string dexFile, string androidSdkDirectory)
{
bool containsClass = false;
DataReceivedEventHandler handler = (s, e) => {
if (e.Data != null && e.Data.Contains ("Class descriptor") && e.Data.Contains (className))
containsClass = true;
};

var androidSdk = new AndroidSdkInfo ((l, m) => {
Console.WriteLine ($"{l}: {m}");
if (l == TraceLevel.Error) {
throw new Exception (m);
}
}, androidSdkDirectory);
var buildToolsPath = androidSdk.GetBuildToolsPaths ().FirstOrDefault ();
if (string.IsNullOrEmpty (buildToolsPath)) {
throw new Exception ($"Unable to find build-tools in `{androidSdkDirectory}`!");
}

var psi = new ProcessStartInfo {
FileName = Path.Combine (buildToolsPath, "dexdump"),
Arguments = $"\"{dexFile}\"",
CreateNoWindow = true,
WindowStyle = ProcessWindowStyle.Hidden,
UseShellExecute = false,
RedirectStandardError = true,
RedirectStandardOutput = true,
};
var builder = new StringBuilder ();
using (var p = new Process { StartInfo = psi }) {
p.ErrorDataReceived += handler;
p.OutputDataReceived += handler;

p.Start ();
p.BeginErrorReadLine ();
p.BeginOutputReadLine ();
p.WaitForExit ();
}

return containsClass;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
<Compile Include="Android\KnownProperties.cs" />
<Compile Include="Android\AndroidLinkMode.cs" />
<Compile Include="Common\FailedBuildException.cs" />
<Compile Include="Utilities\DexUtils.cs" />
<Compile Include="Utilities\PNGChecker.cs" />
<Compile Include="Utilities\ZipHelper.cs" />
<Compile Include="Common\XamarinPCLProject.cs" />
Expand All @@ -79,17 +80,9 @@
<Compile Include="Common\DotNetStandard.cs" />
<Compile Include="Common\DotNetXamarinProject.cs" />
<Compile Include="..\..\..\..\bin\Test$(CONFIGURATION)\XABuildPaths.cs">
<Link>XABuildPaths.cs</Link>
<Link>XABuildPaths.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup>
<Folder Include="Common\" />
<Folder Include="Android\" />
<Folder Include="Resources\" />
<Folder Include="Resources\Wear\" />
<Folder Include="Resources\Base\" />
<Folder Include="Utilities\" />
</ItemGroup>
<ItemGroup>
<EmbeddedResource Include="Resources\Wear\LayoutMain.axml">
<LogicalName>Xamarin.ProjectTools.Resources.Wear.LayoutMain.axml</LogicalName>
Expand Down Expand Up @@ -139,8 +132,7 @@
</ItemGroup>
<ItemGroup>
<None Include="packages.config" />
<Content
Include="..\..\..\..\.nuget\NuGet.exe">
<Content Include="..\..\..\..\.nuget\NuGet.exe">
<CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
</Content>
</ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2483,7 +2483,6 @@ because xbuild doesn't support framework reference assemblies.
ToolPath="$(JavaToolPath)"
JavaMaximumHeapSize="$(JavaMaximumHeapSize)"
JavaOptions="$(JavaOptions)"
InputClassesDirectory="$(IntermediateOutputPath)android\bin\classes"
InputJars="@(_JavaLibrariesToCompileForAppDx)"
ManifestFile="$(IntermediateOutputPath)android\AndroidManifest.xml"
OutputDirectory="$(IntermediateOutputPath)android\bin\desugared">
Expand Down Expand Up @@ -2534,7 +2533,7 @@ because xbuild doesn't support framework reference assemblies.
<Output TaskParameter="Include" ItemName="_AlternativeJarForAppDx" />
</CreateItem>
<CreateItem
Include="@(_DesugaredJars)"
Include="$(IntermediateOutputPath)android\bin\classes.zip;@(_DesugaredJars)"
Condition="('$(AndroidEnableProguard)' != 'True' or '$(_ProguardProjectConfiguration)' == '') and '$(AndroidEnableDesugar)' == 'True'">
<Output TaskParameter="Include" ItemName="_AlternativeJarForAppDx" />
</CreateItem>
Expand Down

0 comments on commit 5995f0c

Please sign in to comment.