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.Glide] add multitargeting for .NET 6 #1261

Merged
merged 17 commits into from
Sep 16, 2021

Conversation

jonathanpeppers
Copy link
Member

@jonathanpeppers jonathanpeppers commented Aug 25, 2021

  • Install latest .NET 6 & stable Xamarin.Android on CI
  • Use Xamarin.Legacy.Sdk.
  • Build with dotnet build
  • Use [assembly: AssemblyMetadata("IsTrimmable", "True")]
  • Use the same pattern as AndroidX, putting .aar/.jar files loose inside the NuGet package
  • Set legacy compatibility properties mentioned here:

https://github.com/xamarin/xamarin-android/blob/5f408ebb22df51b598c62f776ee0f82f92661b58/Documentation/guides/OneDotNetBindingProjects.md

This also updates any AndroidX dependencies so that they are using the
new versions that also multitarget for .NET 6.

* Use Xamarin.Legacy.Sdk.
* Build with `dotnet build`
* Use `[assembly: AssemblyMetadata("IsTrimmable", "True")]`
* Set legacy compatibility properties mentioned here:

https://github.com/xamarin/xamarin-android/blob/5f408ebb22df51b598c62f776ee0f82f92661b58/Documentation/guides/OneDotNetBindingProjects.md

This also updates any AndroidX dependencies so that they are using the
new versions that *also* multitarget for .NET 6.
@jonathanpeppers
Copy link
Member Author

I need to manually verify and test these packages.

It is concerning the file size is so different here:

image

@jpobst
Copy link
Collaborator

jpobst commented Aug 26, 2021

The monoandroid10.0 .dll has the .aar embedded in it. The net6.0-android30.0 .dll does not.

image

@jonathanpeppers
Copy link
Member Author

Yeah, I think this needs some custom .targets now. We don't want the .aar file in here twice, so we need to follow the pattern AndroidX has where the .aar file is included in the .nupkg file.

I'll probably come back to this later, some other stuff has come up.

@moljac
Copy link
Member

moljac commented Sep 7, 2021

Glide is new dependency of some of GPS-FB-MLKit artifacts and it has MonoAndroid10.0 TFM.

Adding multitargeting and MonoAndroid9.0 TFM

#1270

#1271

To mirror what xamarin/AndroidX does:

1. Unzip all `.aar` files, so `classes.jar` is available for binding.
2. Use `@(InputJar)` on `classes.jar`
3. Pack `.aar` files (or `disklrucache.jar`) inside the NuGet package
   as a loose file.
4. Add a `.targets` file to consume the `.aar`/`.jar` when the NuGet
   package is added to projects.

I also moved some common logic to `Directory.Build.props`.
Conflicts:
	Android/Glide/source/Xamarin.Android.Glide.DiskLruCache/Xamarin.Android.Glide.DiskLruCache.csproj
	Android/Glide/source/Xamarin.Android.Glide.GifDecoder/Xamarin.Android.Glide.GifDecoder.csproj
	Android/Glide/source/Xamarin.Android.Glide.RecyclerViewIntegration/Xamarin.Android.Glide.RecyclerViewIntegration.csproj
	Android/Glide/source/Xamarin.Android.Glide/Xamarin.Android.Glide.csproj
I also bumped to XA 11.4.0.5
@jonathanpeppers jonathanpeppers marked this pull request as ready for review September 15, 2021 18:22
@jonathanpeppers
Copy link
Member Author

I manually tested the packages here: https://github.com/jonathanpeppers/glidex/compare/test-glide-update

The file layout is now:

> 7z l .\packages\Xamarin.Android.Glide.4.12.0.1.nupkg

7-Zip 19.00 (x64) : Copyright (c) 1999-2018 Igor Pavlov : 2019-02-21

Scanning the drive for archives:
1 file, 1082782 bytes (1058 KiB)

Listing archive: .\packages\Xamarin.Android.Glide.4.12.0.1.nupkg

--
Path = .\packages\Xamarin.Android.Glide.4.12.0.1.nupkg
Type = zip
Physical Size = 1082782

   Date      Time    Attr         Size   Compressed  Name
------------------- ----- ------------ ------------  ------------------------
2021-09-15 18:01:48 .....          515          288  _rels\.rels
2021-09-15 18:01:48 .....         2834          729  Xamarin.Android.Glide.nuspec
2021-09-15 18:01:20 .....      1062912       215522  lib\monoandroid10.0\Xamarin.Android.Glide.dll
2021-09-15 18:01:20 .....          146           94  lib\monoandroid10.0\Xamarin.Android.Glide.xml
2021-09-15 18:01:36 .....      1074688       220208  lib\net6.0-android31.0\Xamarin.Android.Glide.dll
2021-09-15 18:01:36 .....          146           94  lib\net6.0-android31.0\Xamarin.Android.Glide.xml
2021-09-15 18:00:36 .....       640219       640101  aar\glide.aar
2021-09-15 17:46:38 .....          235          144  build\MonoAndroid10.0\Xamarin.Android.Glide.targets
2021-09-15 17:46:38 .....          235          144  buildTransitive\MonoAndroid10.0\Xamarin.Android.Glide.targets
2021-09-15 17:46:38 .....          235          144  build\net6.0-android31.0\Xamarin.Android.Glide.targets
2021-09-15 17:46:38 .....          235          144  buildTransitive\net6.0-android31.0\Xamarin.Android.Glide.targets
2021-09-15 17:46:38 .....         5468         2288  THIRD-PARTY-NOTICES.txt
2021-09-15 18:01:50 .....          721          234  [Content_Types].xml
2021-09-15 18:01:50 .....          645          380  package\services\metadata\core-properties\86abf10194fb49b688a09cae84233cc1.psmdcp
------------------- ----- ------------ ------------  ------------------------
2021-09-15 18:01:50            2789234      1080514  14 files

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

@jpobst
Copy link
Collaborator

jpobst commented Sep 15, 2021

Glide is new dependency of some of GPS-FB-MLKit artifacts and it has MonoAndroid10.0 TFM.

Adding multitargeting and MonoAndroid9.0 TFM

#1270

#1271

Per this comment, I don't think we can remove MonoAndroid9.0.

Also, what do we need to get CI to run on this? Bump the version number? (cc @moljac)

I would like a chance to examine the final NuGet package before we merge since there are a lot of changes to this.

@jonathanpeppers
Copy link
Member Author

CI ran previously... because I downloaded artifacts...

@jonathanpeppers
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@moljac
Copy link
Member

moljac commented Sep 15, 2021

Also, what do we need to get CI to run on this? Bump the version number? (cc @moljac)

for publishing?

tagging. But leave it to me. Tags in XC repo are messy and I never got to clean them up.

@moljac
Copy link
Member

moljac commented Sep 15, 2021

Per this comment, I don't think we can remove MonoAndroid9.0.

Yes. This is why I wrote that.
OTOH it might take a while for google to make some dependencies releases, until then MonoAndroid9.0 might become obsolete.

Directory.Build.props has MonoAndroid9.0, so it should be OK if I'll be necessary to add those dependencies.

I would like a chance to examine the final NuGet package before we merge since there are a lot of changes to this.

Now I see MonoAndroid9.0 is not packaged.

Copy link
Collaborator

@jpobst jpobst left a comment

Choose a reason for hiding this comment

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

Not sure why the CI wasn't showing up in GitHub. 🤷

Packages look good. Will have to bump version numbers if you want to release new packages with these changes: https://github.com/xamarin/XamarinComponents/pull/1271/files.

@jonathanpeppers jonathanpeppers merged commit a245afa into xamarin:main Sep 16, 2021
@jonathanpeppers jonathanpeppers deleted the glide-net6 branch September 16, 2021 14:58
@jonathanpeppers
Copy link
Member Author

@moljac would you be able to release the Glide packages later this week? Thanks!

@moljac
Copy link
Member

moljac commented Sep 17, 2021

@moljac would you be able to release the Glide packages later this week? Thanks!

Tagged. Waiting for CI. By the time you get up I think I will publish it.

@jonathanpeppers
Copy link
Member Author

The latest release is working fine: dotnet/maui#2563

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.

None yet

3 participants