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.Build.Download] remove AndroidAarFixups #1368

Merged
merged 1 commit into from
May 5, 2022

Conversation

jonathanpeppers
Copy link
Member

Context: dotnet/runtime#68734

A breaking chang in in .NET 7 has uncovered an issue when using
Xamarin.Build.Download:

Renaming: AndroidManifest.xml to AndroidManifest.xml
...
(_XamarinBuildDownloadCore target) ->
    /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-basement/17.6.0/play-services-basement-17.6.0.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesbasement-17.6.0.aar.
    /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-tasks/17.2.1/play-services-tasks-17.2.1.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicestasks-17.2.1.aar.
    /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-base/17.6.0/play-services-base-17.6.0.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesbase-17.6.0.aar.
    /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-maps/17.0.1/play-services-maps-17.0.1.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesmaps-17.0.1.aar.
1 Warning(s)
4 Error(s)

The exception is somewhat swallowed here, the underlying error is
something like:

System.InvalidOperationException : An entry named 'AndroidManifest.xml' already exists in the archive.

This class had various "fixups" to workaround issues in
Xamarin.Android. Many of these issues have long since been fixed.

  1. Removal of aapt/AndroidManifest.xml. Xamarin.Android has been
    handling this since ~Sept 2018:

dotnet/android@f6c3288

  1. Removal of internal_impl-*.jar. Xamarin.Android has been handling
    this since ~Feb 2018:

dotnet/android@6a8ea2b

  1. Replacement of android:name=".SomeService" as shorthand for
    androidx.foo.SomeService in the androidx.foo package. The use
    of manifest-merger solves this issue completely. This setting is
    the default for .NET 6, and has been the default for AndroidX since
    ~July 2020:

xamarin/AndroidX@c6c0e50

Context: dotnet/runtime#68734

A breaking chang in in .NET 7 has uncovered an issue when using
Xamarin.Build.Download:

    Renaming: AndroidManifest.xml to AndroidManifest.xml
    ...
    (_XamarinBuildDownloadCore target) ->
        /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-basement/17.6.0/play-services-basement-17.6.0.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesbasement-17.6.0.aar.
        /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-tasks/17.2.1/play-services-tasks-17.2.1.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicestasks-17.2.1.aar.
        /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-base/17.6.0/play-services-base-17.6.0.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesbase-17.6.0.aar.
        /Users/runner/.nuget/packages/xamarin.build.download/0.11.0/buildTransitive/Xamarin.Build.Download.targets(52,3): error XBD001: Download failed. Please download https://dl.google.com/dl/android/maven2/com/google/android/gms/play-services-maps/17.0.1/play-services-maps-17.0.1.aar to a file called /Users/runner/Library/Caches/XamarinBuildDownload/playservicesmaps-17.0.1.aar.
    1 Warning(s)
    4 Error(s)

The exception is somewhat swallowed here, the underlying error is
something like:

    System.InvalidOperationException : An entry named 'AndroidManifest.xml' already exists in the archive.

This class had various "fixups" to workaround issues in
Xamarin.Android. Many of these issues have long since been fixed.

1. Removal of `aapt/AndroidManifest.xml`. Xamarin.Android has been
   handling this since ~Sept 2018:

dotnet/android@f6c3288

2. Removal of `internal_impl-*.jar`. Xamarin.Android has been handling
   this since ~Feb 2018:

dotnet/android@6a8ea2b

3. Replacement of `android:name=".SomeService"` as shorthand for
   `androidx.foo.SomeService` in the `androidx.foo` package. The use
   of `manifest-merger` solves this issue completely. This setting is
   the default for .NET 6, and has been the default for AndroidX since
   ~July 2020:

xamarin/AndroidX@c6c0e50
@dellis1972
Copy link

Does this have the potential to break any existing customers ? If so I guess the work around would be to downgrade this package to an older version. I'm mainly thinking about people who turn manifest merger off since it is an overridable property.

@jonathanpeppers
Copy link
Member Author

@dellis1972 I think we could probably release an update of Xamarin.Build.Download and not update the dependencies yet.

So keep this on the old version, for example:

https://github.com/xamarin/GooglePlayServicesComponents/blob/b97bb6bb9bfaaea9fa81210b1ef7d548b256a49f/source/GooglePlayServicesProject.cshtml#L297-L299

Then if something goes wrong, we can bring this file back -- and rewrite it so it works in .NET 7.

@jonathanpeppers
Copy link
Member Author

The main thing is if Xamarin.Build.Download is fixing something -- we actually should fix Xamarin.Android so any library not using Xamarin.Build.Download would be able to work...

Copy link
Member

@moljac moljac left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathanpeppers jonathanpeppers merged commit ccee49b into xamarin:main May 5, 2022
@jonathanpeppers jonathanpeppers deleted the AndroidAarFixups branch May 5, 2022 17:55
jonpryor pushed a commit to dotnet/android that referenced this pull request May 5, 2022
Context: dotnet/runtime#56989
Context: dotnet/runtime#68734
Context: dotnet/runtime#68914
Context: dotnet/runtime#68701

Changes: dotnet/installer@04e40fa...c7afae6

	% git diff --shortstat 04e40fa9...c7afae69
	 98 files changed, 1788 insertions(+), 1191 deletions(-)

Changes: dotnet/runtime@a21b9a2...c5d40c9

	% git diff --shortstat a21b9a2d...c5d40c9e
	 28347 files changed, 1609359 insertions(+), 1066473 deletions(-)

Changes: dotnet/linker@01c4f59...04c49c9

	% git diff --shortstat 01c4f590...04c49c9d
	 577 files changed, 28039 insertions(+), 10835 deletions(-)

Updates to build with the .NET 7 SDK and use the runtime specified by
the SDK.  We no longer use different SDK & runtime versions.

This produces a 7.0.100 Android workload.

After this is merged we should be able to enable Maestro to consume
future .NET 7 builds from dotnet/installer/main.

~~ Known Issues ~~

AOT+LLVM crashes on startup:

  * dotnet/runtime#68914

Xamarin.Build.Download hits a breaking change with `ZipEntry`:

  * dotnet/runtime#68734
  * xamarin/XamarinComponents#1368

illink outputs different MVIDs per architecture:

  * dotnet/linker#2203
  * dotnet/runtime#67660

Size of `libmonosgen-2.0.so` regressed:

  * dotnet/runtime#68330
  * dotnet/runtime#68354

Newer .NET 7 builds crash on startup:

  * dotnet/runtime#68701
  * This is worked around by *disabling* lazy loading of AOT'd
    assemblies 6dc426f.
  * TODO: re-enable once we get a fixed .NET 7 runtime.

TODO: We can't yet push to the `dotnet7` feed. Working on this.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Co-authored-by: Peter Collins <pecolli@microsoft.com>
jonathanpeppers added a commit to jonathanpeppers/xamarin-android that referenced this pull request May 6, 2022
Context: dotnet/runtime#68734
Context: xamarin/XamarinComponents#1368

We have a newer Xamarin.Build.Download package that shouldn't suffer
from the duplicate `ZipArchive.CreateEntry()` issue were are hitting
in .NET 7.

Let's use it, and enable previously failing tests.
dellis1972 pushed a commit to dotnet/android that referenced this pull request May 6, 2022
Context: dotnet/runtime#68734
Context: xamarin/XamarinComponents#1368

We have a newer Xamarin.Build.Download package that shouldn't suffer
from the duplicate `ZipArchive.CreateEntry()` issue were are hitting
in .NET 7.

Let's use it, and enable previously failing tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants