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

[One .NET] select defaults for App Bundles #6087

Merged
merged 4 commits into from
Aug 28, 2021
Merged

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented Jul 14, 2021

Fixes #6059

Users will probably want to target more than one App Store. Google is now requiring the aab format for
Google Play Store uploads. Unfortunately this package format is not compatible with other stores.
Users can build their app twice producing an aab for one build and apk for another, but we should
try to make this a bit easier.

bundle-tool has the ability to create a universal apk from the aab file. So lets make use of that to
generate one along side the aab. We are introducing a new property AndroidPackageFormats.
Under .net 6 this will have a value of aab;apk by default for Release builds, for Legacy it will be
empty.

If AndroidPackageFormats is specified and AndroidPackageFormat is empty we will use the
values in AndroidPackageFormats to populate AndroidPackageFormat. If none of these values
are provided the AndroidPackageFormat will still default to apk as it has always done.

The following setting will produce both an aab and an apk.

<PropertyGroup>
  <AndroidPackageFormats>aab;apk</AndroidPackageFormats>
</PropertyGroup>

This will produce just an aab

<PropertyGroup>
  <AndroidPackageFormats>aab</AndroidPackageFormats>
</PropertyGroup>

and this will produce just an apk

<PropertyGroup>
  <AndroidPackageFormats>apk</AndroidPackageFormats>
</PropertyGroup>

For .net 6 users this will be enabled by default for Release builds. For legacy users they will need
to define the property manually.

@dellis1972
Copy link
Contributor Author

Ok , huge number of test failures because we are producing an aab for release under .net 6.

@dellis1972 dellis1972 added the do-not-merge PR should not be merged. label Jul 15, 2021
@jonpryor
Copy link
Member

Conceptually, $(AndroidPackageFormat) + $(AndroidAdditionalPackageFormats) feels "weird": it's inconsistent with other existing properties, such as $(TargetFrameworks), $(RuntimeIdentifiers).

Is there a reason to not go with $(AndroidPackageFormats) (plural)? Is it easier for the IDE to have the two separate properties?

@jonpryor
Copy link
Member

@jonpryor asked:

Is there a reason to not go with $(AndroidPackageFormats) (plural)?

@dellis1972 addressed this on #6059:

Changing over completely to AndroidPackageFormats would have resulted in a rather large PR.

I wonder how much larger it would be, though…. Could we make it work with an "inner build"-like system?

@jonpryor
Copy link
Member

Additionally, the way these properties are worded implies that they're commutative; these should be the same:

<!-- 1 -->
<AndroidPackageFormat>apk</AndroidPackageFormat>
<AndroidAdditionalPackageFormats>aab</AndroidAdditionalPackageFormats>

<!-- 2 -->
<AndroidPackageFormat>aab</AndroidPackageFormat>
<AndroidAdditionalPackageFormats>apk</AndroidAdditionalPackageFormats>

I don't think they are commutative, though, at least not based on the current _CreateUniversalApkFromBundle target:

<Target Name="_CreateUniversalApkFromBundle"
    Condition=" '$(AndroidPackageFormat)' == 'aab' And $(AndroidAdditionalPackageFormats.Contains('apk')) "

@dellis1972
Copy link
Contributor Author

Conceptually, $(AndroidPackageFormat) + $(AndroidAdditionalPackageFormats) feels "weird": it's inconsistent with other existing properties, such as $(TargetFrameworks), $(RuntimeIdentifiers).
Is there a reason to not go with $(AndroidPackageFormats) (plural)? Is it easier for the IDE to have the two separate properties?

We have many MSBuild Properties which rely on an it being one or the other for example (and this is just a small example).

<AndroidUseAapt2            Condition=" '$(AndroidPackageFormat)' == 'aab' ">True</AndroidUseAapt2>
<AndroidUseApkSigner        Condition=" '$(AndroidPackageFormat)' == 'aab' ">False</AndroidUseApkSigner>
<AndroidCreatePackagePerAbi Condition=" '$(AndroidCreatePackagePerAbi)' == 'aab' ">False</AndroidCreatePackagePerAbi>
...

<_BuildApkEmbedOutputs Condition=" '$(AndroidPackageFormat)' == 'aab' ">
<_BuildApkEmbedOutputs Condition=" '$(AndroidPackageFormat)' != 'aab' ">

Also take a look at our current signing system. https://github.com/xamarin/xamarin-android/blob/main/src/Xamarin.Android.Build.Tasks/Xamarin.Android.Common.targets#L2287

Its pretty messy. and depends on if we are signing with ApkSigner or not. aab files MUST use ApkSigner, apk MUST not.

Now I'm not saying that we don't need to rework this because its clear that we do. However, the approach in this PR seemed like the most straight forward given the time constraints for this feature. Tacking on the additional apk generation for this specific case i.e when the user is in release mode and are generating an aab.

As we talked about on the meeting, we could just make AndroidAdditionalPackageFormats internal. So not something users would use, but it would be automatic if they are using .net 6 AND aab And Release.

I wonder how much larger it would be, though…. Could we make it work with an "inner build"-like system?

my concern with inner builds are performance, the fact that we (in .net 6) have to do an inner build for each abi already, adding yet another one for packaging/signing, then in the future more for Dynamic Features. I'd rather avoid those if possible. But it might well be our only option.

So I guess the question really is, how quickly do we want this? Should we do it fast or right?

@dellis1972
Copy link
Contributor Author

Oh and all the unit tests are going to have to change as well. Most are hardcoded to look for {proj.PackageName}.apk most of the time.

@jonpryor
Copy link
Member

@dellis1972: what I think we should do is "split the difference", somewhat:

  1. Document and "add" a new $(AndroidPackageFormats) property, which is a ;-delimited list of values to use among apk or aab. Order isn't important, and thus the following values are supported:

    • apk
    • aab
    • apk;aab
    • aab;apk
  2. Instead of "properly" supporting $(AndroidPackageFormats), we process it and convert it into $(AndroidPackageFormat) + $(_AndroidAdditionalPackageFormats).

    • apk => $(AndroidPackageFormat)=apk, $(_AndroidAdditionalPackageFormats)=''
    • aab => $(AndroidPackageFormat)=aab, $(_AndroidAdditionalPackageFormats)=''
    • apk;aab => $(AndroidPackageFormat)=aab, $(_AndroidAdditionalPackageFormats)=apk
    • aab;apk => $(AndroidPackageFormat)=aab, $(_AndroidAdditionalPackageFormats)=apk

This should allow a more "future-proof" "public API" in the form of $(AndroidPackageFormats), while allowing us to begin supporting $(AndroidPackageFormats) in a faster manner.

@dellis1972 dellis1972 force-pushed the Issue6059 branch 2 times, most recently from 4f4872f to 03c0fbf Compare July 21, 2021 08:51
@dellis1972
Copy link
Contributor Author

doh.

So .. we have a slight problem here. bundle-tool does NOT support the env:password option, only pass: and file:.
But jarsigner does support both. So with this new system we can build the aab but then when creating a new apk from that with an env: style password we get this from bundletool

[BT : 1.4.0] error : Passwords must be prefixed with "pass:" or "file:".

So we effectively can't use env: anymore.

@dellis1972 dellis1972 force-pushed the Issue6059 branch 2 times, most recently from 27efea7 to 1c18e28 Compare August 5, 2021 12:53
Fixes dotnet#6059

Users will probably want to target more than one App Store. Google is now requiring the `aab` format for
Google Play Store uploads. Unfortunately this package format is not compatible with other stores.
Users can build their app twice producing an `aab` for one build and `apk` for another, but we should
try to make this a bit easier.

`bundle-tool` has the ability to create a universal apk from the `aab` file. So lets make use of that to
generate one along side the `aab`. We are introducing a new property `AndroidPackageFormats`.
Under .net 6 this will have a value of `aab;apk` by default for Release builds, for Legacy it will be
empty.

If `AndroidPackageFormats` is specified and `AndroidPackageFormat` is empty we will use the
values in `AndroidPackageFormats` to populate `AndroidPackageFormat`. If none of these values
are provided the `AndroidPackageFormat` will still default to `apk` as it has always done.

The following setting will produce both an `aab` and an `apk`.
```
<PropertyGroup>
  <AndroidPackageFormats>aab;apk</AndroidPackageFormats>
</PropertyGroup>
```

This will produce just an `aab`

```
<PropertyGroup>
  <AndroidPackageFormats>aab</AndroidPackageFormats>
</PropertyGroup>
```

and this will produce just an `apk`

```
<PropertyGroup>
  <AndroidPackageFormats>apk</AndroidPackageFormats>
</PropertyGroup>
```

For .net 6 users this will be enabled by default for Release builds. For legacy users they will need
to define the property manually.
@jonpryor
Copy link
Member

Commit message for review:

Fixes: https://github.com/xamarin/xamarin-android/issues/6059

Context: https://android-developers.googleblog.com/2020/11/new-android-app-bundle-and-target-api.html
Context: https://blogs.windows.com/windowsexperience/2021/06/24/building-a-new-open-microsoft-store-on-windows-11/
Context: https://developer.amazon.com/blogs/appstore/post/50b1ca0f-bbec-48ec-9eea-3c395efb8f9f/amazon-appstore-to-support-android-app-bundle

The Google Play Store requires Android App Bundles (`.aab` files) for
all new apps as of August 2021, and for all apps in November 2021.
Meanwhile, Microsoft has announced that Windows 11 will support
running Android apps and installing them via the Amazon Appstore,
which currently only supports `.apk` file uploads.

While a Xamarin.Android user could build their app twice, changing
the [`$(AndroidPackageFormat)`][0] between each build, we feel it
would be "better" -- faster, more convenient -- if a single
`SignAndroidPackage` target invocation could produce *both* `.aab`
and `.apk` outputs.

[`bundletool.jar`][1] has the ability to create a universal `.apk`
from the `.aab` file.

Introduce support for a new `$(AndroidPackageFormats)` (plural)
MSBuild property, which is a `;`-delimited sequence of
`$(AndroidPackageFormat)` values to produce as build outputs.
For example, if `$(AndroidPackageFormats)`=`aab;apk`, then *both*
`.aab` and `.apk` outputs will be produced.

	<PropertyGroup>
	  <AndroidPackageFormats>aab;apk</AndroidPackageFormats>
	</PropertyGroup>

The `.apk` output will be generated from the `.aab` file, ensuring
consistency.

In .NET 6 Debug configuration builds, `$(AndroidPackageFormats)` will
default to `.apk`, as this behaves better with Fast Deployment and
developer productivity.

In .NET 6 Release configuration builds, `$(AndroidPackageFormats)`
will default to `aab;apk`, i.e. both `.aab` and `.apk` files will be
produced by default.

In Legacy Xamarin.Android, `$(AndroidPackageFormats)` will not be
set, and `$(AndroidPackageFormat)` will continue to default to `.apk`,
meaning that only `.apk` files will be produced (this week…).

[0]: https://docs.microsoft.com/en-us/xamarin/android/deploy-test/building-apps/build-properties#androidpackageformat
[1]: https://developer.android.com/studio/command-line/bundletool

@jonpryor jonpryor merged commit f9f879c into dotnet:main Aug 28, 2021
jonpryor pushed a commit that referenced this pull request Aug 29, 2021
Fixes: #6059

Context: https://android-developers.googleblog.com/2020/11/new-android-app-bundle-and-target-api.html
Context: https://blogs.windows.com/windowsexperience/2021/06/24/building-a-new-open-microsoft-store-on-windows-11/
Context: https://developer.amazon.com/blogs/appstore/post/50b1ca0f-bbec-48ec-9eea-3c395efb8f9f/amazon-appstore-to-support-android-app-bundle

The Google Play Store requires Android App Bundles (`.aab` files) for
all new apps as of August 2021, and for all apps in November 2021.
Meanwhile, Microsoft has announced that Windows 11 will support
running Android apps and installing them via the Amazon Appstore,
which currently only supports `.apk` file uploads.

While a Xamarin.Android user could build their app twice, changing
the [`$(AndroidPackageFormat)`][0] between each build, we feel it
would be "better" -- faster, more convenient -- if a single
`SignAndroidPackage` target invocation could produce *both* `.aab`
and `.apk` outputs.

[`bundletool.jar`][1] has the ability to create a universal `.apk`
from the `.aab` file.

Introduce support for a new `$(AndroidPackageFormats)` (plural)
MSBuild property, which is a `;`-delimited sequence of
`$(AndroidPackageFormat)` values to produce as build outputs.
For example, if `$(AndroidPackageFormats)`=`aab;apk`, then *both*
`.aab` and `.apk` outputs will be produced.

	<PropertyGroup>
	  <AndroidPackageFormats>aab;apk</AndroidPackageFormats>
	</PropertyGroup>

The `.apk` output will be generated from the `.aab` file, ensuring
consistency.

In .NET 6 Debug configuration builds, `$(AndroidPackageFormats)` will
default to `.apk`, as this behaves better with Fast Deployment and
developer productivity.

In .NET 6 Release configuration builds, `$(AndroidPackageFormats)`
will default to `aab;apk`, i.e. both `.aab` and `.apk` files will be
produced by default.

In Legacy Xamarin.Android, `$(AndroidPackageFormats)` will not be
set, and `$(AndroidPackageFormat)` will continue to default to `.apk`,
meaning that only `.apk` files will be produced (this week…).

[0]: https://docs.microsoft.com/en-us/xamarin/android/deploy-test/building-apps/build-properties#androidpackageformat
[1]: https://developer.android.com/studio/command-line/bundletool
@github-actions github-actions bot locked and limited conversation to collaborators Jan 25, 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.

[One .NET] select defaults for App Bundles
4 participants