Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Xamarin.Android.Build.Tasks] error for Android.Support #8629

Merged
merged 11 commits into from
Jan 18, 2024

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Jan 11, 2024

Context: #8478

Some hardcoded paths/properties in Xamarin.AndroidX.Migration cause tests failures in #8478 such as:

(_AndroidXCecilfy target) ->
/Users/runner/.nuget/packages/xamarin.androidx.migration/1.0.8/buildTransitive/monoandroid90/Xamarin.AndroidX.Migration.targets(227,9): error : Source assembly does not exist: 'obj/Debug/android/assets/UnnamedProject.dll'.

Where $(MonoAndroidIntermediateAssetsDir) does not account for a new architecture in the path:

<ItemGroup>
  <_AndroidXFileToCecilfy Include="@(ResolvedUserAssemblies->'$(MonoAndroidIntermediateAssetsDir)%(Filename)%(Extension)')"
    Condition="('%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'true') and ('%(ResolvedUserAssemblies.AndroidXSkipAndroidXMigration)' != 'true')" />
</ItemGroup>

We don't really support the old Android.Support libraries or AndroidX.Migration, but we don't have any build warnings or errors in place for this.

Introduce a XA1039 error in .NET 9 if any packages are found matching:

  • Xamarin.Android.Support.*
  • Xamarin.Android.Arch.*

The biggest impact here is going to be many of our old tests, which use the support libraries in various forms.

Improvements & general cleanup:

  • Removed all old/unused packages in KnownPackages.cs

  • Updated KnownPackages.cs to latest versions, including Xamarin.Forms

  • Android Wear tests are now migrated from support to AndroidX:

https://android-developers.googleblog.com/2016/04/build-beautifully-for-android-wear.html

  • CheckEmbeddedSupportLibraryResources -> moved to AndroidX

  • BuildHasNoWarnings was not appropriately applying IsRelease.

  • Android.Support.v8.RenderScript removed in favor of inline .so file.

  • A few tests that used support libraries to create a project large numbers of dependencies, moved to XamarinFormsAndroidApplicationProject.

  • ResolveLibraryProjectImports -> sorted comparisions

  • XamarinFormsAndroidApplicationProject has a workaround for Guava:

xamarin/AndroidX#535

Removed tests:

  • AndroidXMigration, AndroidXMigrationBug

  • ResolveLibraryImportsWithReadonlyFiles, seemed duplicate of other Android Wear tests, and used support libraries.

  • ExtraAaptManifest as it depends on Xamarin.Android.Fabric and Xamarin.Android.Crashlytics. These are deprecated and depend on support libraries.

  • BuildProguardEnabledProjectSource now only tests Release mode. Since updating to AndroidX, Debug mode was triggering multi-dex. Making it difficult to assert contents of *.dex files.

In a future PR, we will backport these changes to .NET 8, but downgrade the error to a warning:

--<AndroidError Code="XA1039"
++<AndroidWarning Code="XA1039"
    ResourceName="XA1039"
    FormatArguments="9"
    Condition=" '@(_AndroidUnsupportedPackages->Count())' != '0' "
/>

@jpobst
Copy link
Contributor

jpobst commented Jan 16, 2024

If we have a place where we document .NET 9 breaking changes, we should add this one to it.

Comment on lines +1545 to +1547
<ItemGroup Condition=" '$(_InstantRunEnabled)' == 'True' and '@(_AndroidTypeMapping->Count())' == '0' ">
<_AndroidTypeMapping Include="$(_NativeAssemblySourceDir)typemaps\*" />
</ItemGroup>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See d374370 for what's going on here.

Context: #8478

Some hardcoded paths/properties in Xamarin.AndroidX.Migration cause
tests failures in #8478 such as:

    (_AndroidXCecilfy target) ->
    /Users/runner/.nuget/packages/xamarin.androidx.migration/1.0.8/buildTransitive/monoandroid90/Xamarin.AndroidX.Migration.targets(227,9): error : Source assembly does not exist: 'obj/Debug/android/assets/UnnamedProject.dll'.

Where `$(MonoAndroidIntermediateAssetsDir)` does not account for a new
architecture in the path:

    <ItemGroup>
      <_AndroidXFileToCecilfy Include="@(ResolvedUserAssemblies->'$(MonoAndroidIntermediateAssetsDir)%(Filename)%(Extension)')"
        Condition="('%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'true') and ('%(ResolvedUserAssemblies.AndroidXSkipAndroidXMigration)' != 'true')" />
    </ItemGroup>

We don't really *support* the old Android.Support libraries or
AndroidX.Migration, but we don't have any build warnings or errors in
place for this.

Introduce a `XA1039` error in .NET 9 if any packages are found matching:

* `Xamarin.Android.Support.*`
* `Xamarin.Android.Arch.*`

The biggest impact here is going to be many of our old tests, which
use the support libraries in various forms.

Improvements & general cleanup:

* Removed all old/unused packages in `KnownPackages.cs`

* Updated `KnownPackages.cs` to latest versions, including Xamarin.Forms

* Android Wear tests are now migrated from support to AndroidX:

https://android-developers.googleblog.com/2016/04/build-beautifully-for-android-wear.html

* `CheckEmbeddedSupportLibraryResources` -> moved to AndroidX

* `BuildHasNoWarnings` was not appropriately applying `IsRelease`.

* `Android.Support.v8.RenderScript` removed in favor of inline `.so` file.

* A few tests that used support libraries to create a project large
  numbers of dependencies, moved to `XamarinFormsAndroidApplicationProject`.

* `ResolveLibraryProjectImports` -> sorted comparisions

* `XamarinFormsAndroidApplicationProject` has a workaround for Guava:

xamarin/AndroidX#535

Removed tests:

* `AndroidXMigration`, `AndroidXMigrationBug`

* `ResolveLibraryImportsWithReadonlyFiles`, seemed duplicate of other
  Android Wear tests, and used support libraries.

* `ExtraAaptManifest` as it depends on `Xamarin.Android.Fabric` and
  `Xamarin.Android.Crashlytics`. These are deprecated and depend on
  support libraries.

* `BuildProguardEnabledProjectSource` now only tests `Release` mode.
  Since updating to AndroidX, `Debug` mode was triggering multi-dex.
  Making it difficult to assert contents of `*.dex` files.

In a future PR, we will backport these changes, but downgrade the error
to a warning:

    --<AndroidError Code="XA1039"
    ++<AndroidWarning Code="XA1039"
        ResourceName="XA1039"
        FormatArguments="9"
        Condition=" '@(_AndroidUnsupportedPackages->Count())' != '0' "
    />
…ssemblies::Dexes`

The app at runtime in the `ApplicationRunsWithDebuggerAndBreaks` test
was crashing at runtime with:

    01-16 13:11:42.395  4273  4273 E monodroid-assembly: typemap: failed to stat TypeMap index file '/data/user/0/com.xamarin.applicationrunswithdebuggerandbreaks/files/.__override__/typemaps/typemap.index': No such file or directory
    01-16 13:11:42.395  4273  4273 F monodroid-assembly: typemap: unable to load TypeMap data index from '/data/user/0/com.xamarin.applicationrunswithdebuggerandbreaks/files/.__override__/typemaps/typemap.index'

This only happens when `AndroidFastDeploymentType=Assemblies::Dexes` is
used, as it is the case when typemap files like this are fast deployed
and used at runtime.

What was even more odd, was the file seems to exist after a
`-t:Install`, but ends up missing after `-t:Run`:

    > adb shell run-as com.xamarin.applicationrunswithdebuggerandbreaks ls -la files/.__override__/typemaps/typemap.index
    ls: files/.__override__/typemaps/typemap.index: No such file or directory

It appears that `-t:Install` successfully deploys the file:

    Pushed 3969 to /data/local/tmp/.xatools/typemap.index
    DEBUG RunShellCommand emulator-5554 "run-as" "com.xamarin.applicationrunswithdebuggerandbreaks" "--user" "0" "files/.__tools__/xamarin.cp" "/data/local/tmp/.xatools/typemap.index" "files/.__override__/typemaps/typemap.index" "1705432079367" [5ms]
    files/.__tools__/xamarin.cp returned: moved [/data/local/tmp/.xatools/typemap.index] to [files/.__override__/typemaps/typemap.index] modifieddate [1705432079367]
    moved /data/local/tmp/.xatools/typemap.index to files/.__override__/typemaps/typemap.index
    Installed files/.__override__/typemaps/typemap.index. [12ms]
    NotifySync CopyFile obj\Debug\android\typemaps\typemap.index. [0ms]

But then `-t:Run` deletes the file!

    Remove redundant file files/.__override__/typemaps/typemap.index
    DEBUG RunShellCommand 0A041FDD400327 "run-as" "com.xamarin.applicationrunswithdebuggerandbreaks" "rm" "-Rf" "files/.__override__/typemaps/typemap.index" [29ms]

This happens because the `@(_AndroidTypeMapping)` item group is empty
during an incremental build:

* The `<GenerateJavaStubs/>` MSBuild task, during the first build
  outputs `@(_AndroidTypeMapping)` items

* During an incremental build, the `_GenerateJavaStubs` MSBuild *target*
  is skipped, and so the `@(_AndroidTypeMapping)` item group is empty!

* The `<FastDeploy/>` task happily deletes files that it thinks should
  be removed.

For now, let's add logic to the `_GenerateJavaStubs` target to fill in
the `@(_AndroidTypeMapping)` item group during incremental builds:

  <ItemGroup Condition=" '$(_InstantRunEnabled)' == 'True' and '@(_AndroidTypeMapping->Count())' == '0' ">
    <_AndroidTypeMapping Include="$(_NativeAssemblySourceDir)typemaps\*" />
  </ItemGroup>

`<ItemGroup>`s are still evaluated when a target is *skipped*, solving
the problem.

I assume this is working in `main`, because Xamarin.AndroidX.Migration
package was involved. It likely was running the `_GenerateJavaStubs`
target on every build. Will verify.
@@ -445,26 +433,6 @@ public void ApplicationIdPlaceholder ()
}
}

[Test]
[Category ("XamarinBuildDownload")]
public void ExtraAaptManifest ()
Copy link
Member

@jonpryor jonpryor Jan 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this test removed? It doesn't appear to (directly?) use the Support libraries; presumably it's because it starts with Xamarin.Android. Is there not a newer Crashytics? It looks like there's a Xamarin.Firebase.Crashlytics package which exists…

This test is also weird, in that it's the only test that uses Crashlytics.HandleManagedExceptions()

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to put it back, using:

  • Xamarin.Firebase.Crashlytics NuGet
  • Firebase.Crashlytics.FirebaseCrashlytics.Instance.SendUnsentReports()
  • Look for firebaseinitprovider in the AndroidManifest.xml

@jonpryor
Copy link
Member

jonpryor commented Jan 18, 2024

Draft comit message:

[Xamarin.Android.Build.Tasks] XA1039 error for Android.Support (#8629)

Context: c92702619f5fabcff0ed88e09160baf9edd70f41
Context: 68368189d67c46ddbfed4e90e622f635c4aff11e
Context: 929e7012410233e6814af369db582f238ba185ad
Context: https://github.com/xamarin/xamarin-android/pull/8478
Context: https://github.com/xamarin/xamarin-android/issues/8155
Context: https://github.com/xamarin/xamarin-android/issues/8168

Our build process has a bit of a "consistent sanity" problem: large
portions of the build process assume that the output of a `.csproj`
is a single assembly, and that single assembly is (eventually)
embedded into the `.apk`, to be loaded at runtime.

Unfortunately, that wasn't strictly true starting with .NET 5:
there were *multiple* `System.Private.CoreLib.dll` assemblies, which
needed to be treated specially; see also c9270261.

We discovered that this assumption was even *less* true because of
the linker, which would quite happily *replace* `IntPtr.get_Size()`
method invocations with a *constant*, specific for the target ABI;
see also 68368189, 929e7012.  This in turn could be responsible for
all sorts of weirdness if e.g. a 64-bit assembly were used in a
32-bit process, or vice versa.

Which brings us to the original assumption: there is (usually) only
one "source" assembly, which is (optionally) linked into one "linked"
assembly, which is packaged into one assembly in the `.apk`.

With the linker, though, we can have one "source" assembly, which
when linked becomes *N* assemblies, which *should be* separately
packaged as per-ABI assemblies within the `.apk`, but
*probably aren't*, which we *think* is the "root cause" of #8155.

PR #8478 is attempting fix this assumption, imbuing the build system
with knowledge that the linker may produce *multiple outputs* for a
single input assembly.

Unfortunately, in trying to fix things, various intermediate assembly
locations have *changed*, which in turn breaks
[AndroidX Migration in Xamarin.Forms][0], as it makes assumptions
about where various assemblies are located within `obj`:

	(_AndroidXCecilfy target) ->
	/Users/runner/.nuget/packages/xamarin.androidx.migration/1.0.8/buildTransitive/monoandroid90/Xamarin.AndroidX.Migration.targets(227,9): error : Source assembly does not exist: 'obj/Debug/android/assets/UnnamedProject.dll'.

as [`@(_AndroidXFileToCecilfy)`][1] uses
`$(MonoAndroidIntermediateAssetsDir)`, which does not account for the
ABI which is now in the path:

	<ItemGroup>
	  <_AndroidXFileToCecilfy Include="@(ResolvedUserAssemblies->'$(MonoAndroidIntermediateAssetsDir)%(Filename)%(Extension)')"
	    Condition="('%(ResolvedUserAssemblies.TargetFrameworkIdentifier)' == 'MonoAndroid' or '%(ResolvedUserAssemblies.HasMonoAndroidReference)' == 'true') and ('%(ResolvedUserAssemblies.AndroidXSkipAndroidXMigration)' != 'true')" />
	</ItemGroup>

Given that AndroidX Migration is mostly for Xamarin.Forms customers
(and *kinda* buggy, and unmaintained), and MAUI doesn't support the
Android Support libraries, and thus doesn't need AndroidX Migration,
we'd like to just *not worry about this*.

The problem?  The above error message is not actionable, and doesn't
tell anybody how to fix it.

Introduce a new `XA1039` *actionable* error in .NET 9:

	error XA1039: The Android Support libraries are not supported in .NET 9 and later,
	please migrate to AndroidX. See https://aka.ms/xamarin/androidx for more details.

The XA1039 error is generated if any NuGet packages are found matching:

  * `Xamarin.Android.Support.*`
  * `Xamarin.Android.Arch.*`

TODO: "port" XA1039 to .NET 8 *as a warning*, so that customers will
have some time to migrate off of the Android Support libraries before
.NET 9 is released in 2024-Nov.

	--<AndroidError Code="XA1039"
	++<AndroidWarning Code="XA1039"
	    ResourceName="XA1039"
	    Condition=" '@(_AndroidUnsupportedPackages->Count())' != '0' "
	/>

The biggest impact here is going to be many of our old tests, which
use the support libraries in various forms.

Improvements & general cleanup:

  * Removed all old/unused packages in `KnownPackages.cs`

  * Updated `KnownPackages.cs` to latest versions, including Xamarin.Forms

  * [Android Wear tests are now migrated from support to AndroidX][2]

  * `AndroidUpdateResourcesTest.CheckEmbeddedSupportLibraryResources()`
    is renamed to `.CheckEmbeddedAndroidXResources()`.

  * `BuildTest2.BuildHasNoWarnings()` was not appropriately applying
    `IsRelease`.

  * `Android.Support.v8.RenderScript` removed in favor of an inline
    `.so` file.

  * A few tests that used support libraries to create a project large
    numbers of dependencies, moved to
    `XamarinFormsAndroidApplicationProject`.

  * `IncrementalBuildTest.ResolveLibraryProjectImports()` now sorts
    items before comparing them.

  * `XamarinFormsAndroidApplicationProject` has a workaround for Guava:  
    <https://github.com/xamarin/AndroidX/issues/535>

  * Fix a bug in `AndroidFastDeploymentType=Assemblies::Dexes`;
    see "AndroidFastDeploymentType=Assemblies::Dexes" section, below.

Removed tests:

  * `AndroidXMigration`, `AndroidXMigrationBug`

  * `ResolveLibraryImportsWithReadonlyFiles`, seemed duplicate of other
    Android Wear tests, and used support libraries.

  * `ExtraAaptManifest` as it depends on `Xamarin.Android.Fabric` and
    `Xamarin.Android.Crashlytics`. These are deprecated and depend on
    support libraries.

  * `BuildProguardEnabledProjectSource` now only tests `Release` mode.
    Since updating to AndroidX, `Debug` mode was triggering multi-dex.
    Making it difficult to assert contents of `*.dex` files.


~~ AndroidFastDeploymentType=Assemblies::Dexes ~~

The runtime test in `ApplicationRunsWithDebuggerAndBreaks()` was
crashing at runtime with:

	E monodroid-assembly: typemap: failed to stat TypeMap index file '/data/user/0/com.xamarin.applicationrunswithdebuggerandbreaks/files/.__override__/typemaps/typemap.index': No such file or directory
	F monodroid-assembly: typemap: unable to load TypeMap data index from '/data/user/0/com.xamarin.applicationrunswithdebuggerandbreaks/files/.__override__/typemaps/typemap.index'

This only happens when `AndroidFastDeploymentType=Assemblies::Dexes` is
used, as it is the case when typemap files like this are fast deployed
and used at runtime.

What was even more odd, was the file seems to exist after a
`-t:Install`, but ends up missing after `-t:Run`:

	> adb shell run-as com.xamarin.applicationrunswithdebuggerandbreaks ls -la files/.__override__/typemaps/typemap.index
	ls: files/.__override__/typemaps/typemap.index: No such file or directory

It appears that `-t:Install` successfully deploys the file:

	Pushed 3969 to /data/local/tmp/.xatools/typemap.index
	DEBUG RunShellCommand emulator-5554 "run-as" "com.xamarin.applicationrunswithdebuggerandbreaks" "--user" "0" "files/.__tools__/xamarin.cp" "/data/local/tmp/.xatools/typemap.index" "files/.__override__/typemaps/typemap.index" "1705432079367" [5ms]
	files/.__tools__/xamarin.cp returned: moved [/data/local/tmp/.xatools/typemap.index] to [files/.__override__/typemaps/typemap.index] modifieddate [1705432079367]
	moved /data/local/tmp/.xatools/typemap.index to files/.__override__/typemaps/typemap.index
	Installed files/.__override__/typemaps/typemap.index. [12ms]
	NotifySync CopyFile obj\Debug\android\typemaps\typemap.index. [0ms]

But then `-t:Run` deletes the file!

	Remove redundant file files/.__override__/typemaps/typemap.index
	DEBUG RunShellCommand 0A041FDD400327 "run-as" "com.xamarin.applicationrunswithdebuggerandbreaks" "rm" "-Rf" "files/.__override__/typemaps/typemap.index" [29ms]

This happens because the `@(_AndroidTypeMapping)` item group is empty
during an incremental build:

  * The `<GenerateJavaStubs/>` MSBuild task, during the first build
    outputs `@(_AndroidTypeMapping)` items

  * During an incremental build, the `_GenerateJavaStubs` MSBuild
    *target* is skipped, and so the `@(_AndroidTypeMapping)` item
    group is empty!

  * The `<FastDeploy/>` task happily deletes files that it thinks
    should be removed.

For now, let's add logic to the `_GenerateJavaStubs` target to fill
in the `@(_AndroidTypeMapping)` item group during incremental builds:

	<ItemGroup Condition=" '$(_InstantRunEnabled)' == 'True' and '@(_AndroidTypeMapping->Count())' == '0' ">
	  <_AndroidTypeMapping Include="$(_NativeAssemblySourceDir)typemaps\*" />
	</ItemGroup>

`<ItemGroup>`s are still evaluated when a target is *skipped*,
solving the problem.

I assume this is working in `main`, because Xamarin.AndroidX.Migration
package was involved.  It likely was running the `_GenerateJavaStubs`
target on every build.

[0]: https://learn.microsoft.com/xamarin/xamarin-forms/platform/android/androidx-migration
[1]: https://github.com/xamarin/AndroidX/blob/17e596fafe20331d7feb69240c38e0fbdc3ea640/source/migration/BuildTasks/Xamarin.AndroidX.Migration.targets#L205-L206
[2]: https://android-developers.googleblog.com/2016/04/build-beautifully-for-android-wear.html

@jonpryor jonpryor merged commit 2f19238 into main Jan 18, 2024
47 checks passed
@jonpryor jonpryor deleted the dev/peppers/android/support branch January 18, 2024 13:18
@jonathanpeppers
Copy link
Member Author

If we have a place where we document .NET 9 breaking changes, we should add this one to it.

I added information here: https://github.com/xamarin/xamarin-android/wiki/Planned-Future-Deprecations-and-Removals

I'm working on the backport PR for .NET 8 next, where it will be a warning that you can opt out of. When that change ships, we can decide if we need to do more if we get lots of customer feedback.

grendello added a commit that referenced this pull request Jan 18, 2024
* main:
  [Xamarin.Android.Build.Tasks] XA1039 error for Android.Support (#8629)
jonathanpeppers added a commit that referenced this pull request Jan 18, 2024
Context: #8629

This partially backports 2f19238 to .NET 8, but reduces XA1039 to a
warning.

Also introduce `$(_AndroidIgnoreAndroidSupportWarning)` as an escape
hatch if someone needs to completely disable the warning:

    <_AndroidIgnoreAndroidSupportWarning>true</_AndroidIgnoreAndroidSupportWarning>
grendello added a commit that referenced this pull request Jan 25, 2024
* main:
  Localized file check-in by OneLocBuild Task (#8668)
  [Xamarin.Android.build.Tasks] `<CheckDuplicateJavaLibraries/>` ignores `repackaged.jar` (#8664)
  LEGO: Merge pull request 8665
  [Xamarin.Android.Build.Tasks] parse JDK `release` file directly (#8663)
  Bump to dotnet/installer@f91d4ca399 9.0.100-alpha.1.24070.3 (#8635)
  [.github] Re-enable locking issues after 30 days of inactivity (#8655)
  Localized file check-in by OneLocBuild Task (#8657)
  LEGO: Merge pull request 8656
  Localized file check-in by OneLocBuild Task (#8652)
  Bump to xamarin/xamarin-android-tools/main@b175674 (#8644)
  [Xamarin.Android.Build.Tasks] remove checks for `$(UsingAndroidNETSdk)` (#8647)
  [Xamarin.Android.Build.Tasks] XA1039 error for Android.Support (#8629)
@github-actions github-actions bot locked and limited conversation to collaborators Feb 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants