[Xamarin.Android.Build.Tasks] trailing / causes missing classes.dex

[jonathanpeppers]

Fixes: #4478
Fixes: #4486

We with the release of VS for Mac 8.5 going stable have been getting
some reports of:

error ADB0010:  Deployment failed
Mono.AndroidTools.InstallFailedException: Failure [INSTALL_FAILED_INVALID_APK: Package couldn't be installed in /data/app/com.contoso.androidapp1-RTZZFPyLkRI7Bk7VDgGkDg==: Package /data/app/com.contoso.androidapp1-RTZZFPyLkRI7Bk7VDgGkDg==/base.apk code is missing]
    at Mono.AndroidTools.Internal.AdbOutputParsing.CheckInstallSuccess (System.String output, System.String packageName) [0x00159] in <65710797f19f43cc9d8f9e05353b9615>:0
    at Mono.AndroidTools.AndroidDevice+<>c__DisplayClass95_0.<InstallPackage>b__0 (System.Threading.Tasks.Task`1[TResult] t) [0x0001c] in <65710797f19f43cc9d8f9e05353b9615>:0
    at System.Threading.Tasks.ContinuationTaskFromResultTask`1[TAntecedentResult].InnerInvoke () [0x00024] in <f9d1b832704f410aa8ec771f4fe80552>:0
    at System.Threading.Tasks.Task.Execute () [0x00000] in <f9d1b832704f410aa8ec771f4fe80552>:0
    --- End of stack trace from previous location where exception was thrown ---
    at AndroidDeviceExtensions.PushAndInstallPackage (Mono.AndroidTools.AndroidDevice device, System.String apkFile, System.String packageName, System.Boolean reinstall, Mono.AndroidTools.Adb.AdbProgressReporter notifyProgress, System.Threading.CancellationToken token) [0x001d3] in <81f66a65e9434daf9d06602cf6ac0bf9>:0
    at AndroidDeviceExtensions.PushAndInstallPackage (Mono.AndroidTools.AndroidDevice device, System.String apkFile, System.String packageName, System.Boolean reinstall, Mono.AndroidTools.Adb.AdbProgressReporter notifyProgress, System.Threading.CancellationToken token) [0x00402] in <81f66a65e9434daf9d06602cf6ac0bf9>:0
    at Xamarin.AndroidTools.AndroidDeploySession.InstallPackage () [0x003be] in <81f66a65e9434daf9d06602cf6ac0bf9>:0
    at Xamarin.AndroidTools.AndroidDeploySession.RunAsync (System.Threading.CancellationToken token) [0x003f8] in <81f66a65e9434daf9d06602cf6ac0bf9>:0
    at Xamarin.AndroidTools.AndroidDeploySession.RunLoggedAsync (System.Threading.CancellationToken token) [0x00077] in <81f66a65e9434daf9d06602cf6ac0bf9>:0  [/Users/macuser/Projects/AndroidApp1/AndroidApp1/AndroidApp1.csproj]

The error means the .apk is missing classes.dex.

It seems to affect apps where:

• AndroidDexTool=dx, this would need to be set explicitly or it
  would default to d8.
• The issue appears to occur on macOS-only.

Brendan was able to reproduce the issue:

#4478 (comment)

Using the zip file, there is an obj directory sitting next to the
.sln file:

• obj
• AndroidApp1
• AndroidApp1/AndroidApp1.csproj
• AndroidApp1/obj
• AndroidApp1.sln

Note that the actual $(BaseIntermediateOutputPath) would be
AndroidApp1/obj. Brendan just moved an obj directory next to the
.sln file while trying to reproduce the issue.

However, it appears the presence of this phantom obj triggers the
issue! You can rename it to obj2 to solve the issue. It is a
directory with sub-directories, but no files.

If you look at the build log, the main difference is:

CompileToDalvik
    Parameters
-       ClassesOutputDirectory = obj/Release/android/bin/classes/
+       ClassesOutputDirectory = obj/Release/android/bin/classes

The presence of the trailing / breaks this code:

cmd.AppendSwitchIfNotNull ("--output ", Path.GetDirectoryName (ClassesOutputDirectory));

classes.dex is output into obj/Release/android/bin/classes instead
of obj/Release/android/bin and doesn't make it into the final
.apk.

So tracking back to what creates the input value of
ClassesOutputDirectory, it is the MSBuild property
$(_AndroidIntermediateJavaClassDirectory), it is merely:

<_AndroidIntermediateJavaClassDirectory>$(IntermediateOutputPath)android\bin\classes\</_AndroidIntermediateJavaClassDirectory>

But then comparing broken vs working logs:

Broken:
    _InitialBaseIntermediateOutputPath = obj/
    BaseIntermediateOutputPath = obj/
    IntermediateOutputPath = obj/Release/
    _AndroidIntermediateJavaClassDirectory = obj/Release/android/bin/classes/
Working:
    _InitialBaseIntermediateOutputPath = obj\
    BaseIntermediateOutputPath = obj\
    IntermediateOutputPath = obj\Release\
    _AndroidIntermediateJavaClassDirectory = obj\Release\android\bin\classes\

It appears the existence of this random obj directory influences
MSBuild's path combining behavior?

The trailing \ works, but / does not, due to TrimEnd:

<CompileToDalvik
    ...
    ClassesOutputDirectory="$(_AndroidIntermediateJavaClassDirectory.TrimEnd('\'))"

The breakage was introduced in 185c529, which added a trailing \ to
the directory passed to <CompileToDalvik/>. We are unsure what is
causing the strange MSBuild behavior.

However, after reviewing <CompileToDalvik/>, I think we should just
remove the Path.GetDirectoryName call. We should just pass in the
directory name we want to use from MSBuild targets.

I could add a test for this scenario by setting a global property:

_AndroidIntermediateJavaClassDirectory=obj/Debug/android/bin/classes/

Setting a global property could test the trailing / character
regardless of MSBuild behavior.

I also cleaned things up a bit by create a new
$(_AndroidIntermediateDexOutputDirectory) property.

[brendanzagaeski]

Looks nice both on the removal of the Path.GetDirectoryName() and on the cleanup with the new property.

Draft release notes

Here's a candidate release note for this change. Feel free to edit as desired and add it to this pull request.

#### Application and library build and deployment

  * [GitHub 4478](https://github.com/xamarin/xamarin-android/issues/4478),
    [GitHub 4486](https://github.com/xamarin/xamarin-android/issues/4486):
    On macOS, starting in Xamarin.Android 10.2, *\[INSTALL_FAILED_INVALID_APK:
    ...  base.apk code is missing\]* and *Module 'base' has no dex files*
    prevented deploying successfully for certain projects configured to use the
    DX DEX compiler.

[dellis1972]

This looks ok. I can't remember why we use the GetDirectoryName call... I wish I could.

[brendanzagaeski]

In case it might be a helpful extra reassurance, it looks like GetDirectoryName came in with: https://github.com/xamarin/monodroid/commit/30c22c7f0affc11fc9bdc96bca8716d452869893#diff-974b9024f51680c37819ca34a137b935R108

Based on that, it looks like it was added because originally the value passed to the ClassesOutputDirectory parameter on <CompileToDalvik> was set to match exactly with the value passed to ClassesOutputDirectory on the <Javac> task, and the <CompileToDalvik> task was once upon a time modifying that directory name to be a file name by appending .dex.

But then in 30c22c7f, the <CompileToDalvik> task started needing to adjust that directory name to be the name of the parent directory.

Another complicating factor back at that time was that the values passed to the ClassesOutputDirectory parameters were not set to include the conventional trailing slash used in MSBuild for properties containing directory names. That led to a potentially slightly funny technique of using Path.GetDirectoryName() instead of Directory.GetParent() to get the parent directory.

[jonathanpeppers]

Somehow this check showed up twice:

This appears to be green, but the Github checks are messed up.