Skip to content

Commit

Permalink
[Xamarin.Android.Build.Tasks] merge two targets to fix AndroidX.Migra…
Browse files Browse the repository at this point in the history
…tion

Context: https://github.com/xamarin/XamarinAndroidXMigration
Fixes: #4068

Developers have been reporting an error when using AndroidX.Migration:

    The "CopyGeneratedJavaResourceClasses" task was not given a value for the required parameter "SourceTopDirectory".

It is reported another build fixes the issue, and it "randomly"
happens on incremental builds.

The error seems to indicate either `$(AaptTemporaryDirectory)` or
`$(ResgenTemporaryDirectory)` are blank? But it only happens when
using the AndroidX.Migration NuGet package.

From a user's build log:

    Skipping target "_PrepareCreateBaseApk" because all output files are up-to-date with respect to the input files.

Which would indicate in the case of AndroidX.Migration:

* `_PrepareCreateBaseApk` was skipped.
* `_AndroidXJetifyManifest` was injected, it came along and modified
  `$(IntermediateOutputPath)android\AndroidManifest.xml`.
* `_CreateBaseApk` ran (with updated inputs), and
  `$(AaptTemporaryDirectory)` is blank!

I could reproduce the problem in a test:

* Build a project with AndroidX.Migration
* Change C# code (a non-Java.Lang.Object class)
* Build the project again

Reviewing the MSBuild targets: `_PrepareCreateBaseApk` and
`_CreateBaseApk`. They define a property that is passed between two
targets. The following sets of targets have nearly identical
`Input`/`Output`:

* `_PrepareCreateBaseApk` and `_CreateBaseApk`
* `_PrepareUpdateAndroidResgen` and `_UpdateAndroidResgen`

This was done to workaround a long-lived bug in MSBuild:

dotnet/msbuild#1006

A fix that doesn't seem break anything is to just run
`_PrepareCreateBaseApk` *after* `$(AfterGenerateAndroidManifest)`. So
if something triggers `_CreateBaseApk`, `_PrepareCreateBaseApk` will
always run as well.

I wrote a test for this scenario, and updated the AndroidX NuGets in
our tests to use the RC now.
  • Loading branch information
jonathanpeppers committed Jan 21, 2020
1 parent 0153c2c commit 9fe854e
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ public void AndroidXMigration ([Values (true, false)] bool isRelease)
proj.PackageReferences.Add (KnownPackages.AndroidXBrowser);
proj.PackageReferences.Add (KnownPackages.AndroidXMediaRouter);
proj.PackageReferences.Add (KnownPackages.AndroidXLegacySupportV4);
proj.PackageReferences.Add (KnownPackages.AndroidXLifecycleLiveData);
proj.PackageReferences.Add (KnownPackages.XamarinGoogleAndroidMaterial);

using (var b = CreateApkBuilder (Path.Combine ("temp", TestName))) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1078,5 +1078,29 @@ public void AndroidResourceChange ()
}
}
}

[Test]
[NonParallelizable]
public void AndroidXMigrationBug ()
{
var proj = new XamarinFormsAndroidApplicationProject ();
proj.PackageReferences.Add (KnownPackages.AndroidXMigration);
proj.PackageReferences.Add (KnownPackages.AndroidXAppCompat);
proj.PackageReferences.Add (KnownPackages.AndroidXBrowser);
proj.PackageReferences.Add (KnownPackages.AndroidXMediaRouter);
proj.PackageReferences.Add (KnownPackages.AndroidXLegacySupportV4);
proj.PackageReferences.Add (KnownPackages.AndroidXLifecycleLiveData);
proj.PackageReferences.Add (KnownPackages.XamarinGoogleAndroidMaterial);

string source = "class Foo { }";
proj.Sources.Add (new BuildItem.Source ("Foo.cs") { TextContent = () => source });

using (var b = CreateApkBuilder ()) {
Assert.IsTrue (b.Build (proj), "first build should have succeeded.");
source = source.Replace ("Foo", "Bar");
proj.Touch ("Foo.cs");
Assert.IsTrue (b.Build (proj, doNotCleanupOnUpdate: true), "second build should have succeeded.");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,17 +223,17 @@ public static class KnownPackages
};
public static Package AndroidXMigration = new Package {
Id = "Xamarin.AndroidX.Migration",
Version = "1.0.0-preview04",
Version = "1.0.0-rc1",
TargetFramework = "MonoAndroid10",
};
public static Package AndroidXAppCompat = new Package {
Id = "Xamarin.AndroidX.AppCompat",
Version = "1.0.2-preview02",
Version = "1.1.0-rc1",
TargetFramework = "MonoAndroid10",
};
public static Package AndroidXBrowser = new Package {
Id = "Xamarin.AndroidX.Browser",
Version = "1.0.0-preview02",
Version = "1.0.0-rc1",
TargetFramework = "MonoAndroid90",
References = {
new BuildItem.Reference ("Xamarin.AndroidX.Browser") {
Expand All @@ -243,7 +243,7 @@ public static class KnownPackages
};
public static Package AndroidXMediaRouter = new Package {
Id = "Xamarin.AndroidX.MediaRouter",
Version = "1.0.0-preview02",
Version = "1.1.0-rc1",
TargetFramework = "MonoAndroid90",
References = {
new BuildItem.Reference ("Xamarin.AndroidX.MediaRouter") {
Expand All @@ -253,17 +253,27 @@ public static class KnownPackages
};
public static Package AndroidXLegacySupportV4 = new Package {
Id = "Xamarin.AndroidX.Legacy.Support.V4",
Version = "1.0.0-preview02",
Version = "1.0.0-rc1",
TargetFramework = "MonoAndroid90",
References = {
new BuildItem.Reference ("Xamarin.AndroidX.Legacy.Support.V4") {
MetadataValues = "HintPath=..\\packages\\Xamarin.AndroidX.Legacy.Support.V4.1.0.0-preview02\\lib\\MonoAndroid90\\Xamarin.AndroidX.Legacy.Support.V4.dll"
},
}
};
public static Package AndroidXLifecycleLiveData = new Package {
Id = "Xamarin.AndroidX.Lifecycle.LiveData",
Version = "2.1.0-rc1",
TargetFramework = "MonoAndroid90",
References = {
new BuildItem.Reference ("Xamarin.AndroidX.Lifecycle.LiveData") {
MetadataValues = "HintPath=..\\packages\\Xamarin.AndroidX.Lifecycle.LiveData.2.1.0-rc1\\lib\\MonoAndroid90\\Xamarin.AndroidX.Lifecycle.LiveData.dll"
},
}
};
public static Package XamarinGoogleAndroidMaterial = new Package {
Id = "Xamarin.Google.Android.Material",
Version = "1.0.0-preview02",
Version = "1.0.0-rc1",
TargetFramework = "MonoAndroid90",
References = {
new BuildItem.Reference ("Xamarin.Google.Android.Material") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2265,7 +2265,6 @@ because xbuild doesn't support framework reference assemblies.
_CheckDuplicateJavaLibraries;
UpdateAndroidAssets;
$(_AfterCreateBaseApkDependsOnTargets);
_PrepareCreateBaseApk;
</_CreateBaseApkDependsOnTargets>
<_CreateBaseApkInputs>
$(MSBuildAllProjects)
Expand All @@ -2279,7 +2278,7 @@ because xbuild doesn't support framework reference assemblies.
</PropertyGroup>

<Target Name="_CreateBaseApk"
DependsOnTargets="$(_CreateBaseApkDependsOnTargets);$(AfterGenerateAndroidManifest)"
DependsOnTargets="$(_CreateBaseApkDependsOnTargets);$(AfterGenerateAndroidManifest);_PrepareCreateBaseApk"
Inputs="$(_CreateBaseApkInputs)"
Outputs="$(_PackagedResources)">

Expand Down

0 comments on commit 9fe854e

Please sign in to comment.