Fix AndroidPackagingOptionsExclude so it can be overridden in the csproj

[dellis1972]

Context #8456

Commit https://github.com/xamarin/monodroid/commit/43583d026ff4663fa8165b7c8085189f6210b13e added the AndroidPackagingOptionsExclude ItemGroup. This is used to exclude certain files from the final apk. It was initially
desiged to filter out kotlin based meta data.

However it turns out the default RegEx we used is too restrictive. It removes some files which are required at runtime by the
various kotlin libraries. In the example given in #8456 , kotlin.reflect.jvm required the kotlin/reflect/reflect.kotlin_builtins file in order to work (and probably others). But when users tried to change the RegEx or remove it they could not.

The problem was that users would try to use the following in the csproj which would not work.

<ItemGroup>
    <AndroidPackagingOptionsExclude Remove="$([MSBuild]::Escape('*.kotlin_*'))" />
  </ItemGroup>

This is because of the order in which the ItemGroups are evaluated by MSBuild. At the time the csproj is evaluated the AndroidPackagingOptionsExclude has not yet been set. So if users want to override the default behaviour they need to do the following

<Target Name="_Foo" AfterTargets="AfterBuild">
    <ItemGroup>
        <AndroidPackagingOptionsExclude Remove="$([MSBuild]::Escape('*.kotlin_*'))" />
        <AndroidPackagingOptionsExclude Include="$([MSBuild]::Escape('*.kotlin_m*'))" />
    </ItemGroup>
  </Target>

This is not very intuative. So lets make this easier by moving the definition of the default AndroidPackagingOptionsExclude setting to AutoImport.props which happens very early in the evaluation step.

The second option was the RegEx is too restictive. It was excluding the *_builtins by default. Now we could change the regex to the following .kotlin_m*' as per the example above. But this will end up including files which are not needed. For example this is a list of files which end up being included by the kotlin.reflect.jvm

     928  Defl:N      561  40% 01-01-1981 01:01 8e787046  kotlin/annotation/annotation.kotlin_builtins
    3685  Defl:N     1521  59% 01-01-1981 01:01 f41c30cf  kotlin/collections/collections.kotlin_builtins
     200  Defl:N      157  22% 01-01-1981 01:01 c69fe143  kotlin/coroutines/coroutines.kotlin_builtins
     646  Defl:N      394  39% 01-01-1981 01:01 d8d2330d  kotlin/internal/internal.kotlin_builtins
   18640  Defl:N     5252  72% 01-01-1981 01:01 f77f89dd  kotlin/kotlin.kotlin_builtins
    3399  Defl:N     1292  62% 01-01-1981 01:01 cb3b8aed  kotlin/ranges/ranges.kotlin_builtins
    2396  Defl:N     1266  47% 01-01-1981 01:01 852a0981  kotlin/reflect/reflect.kotlin_builtins

Now I'm not 100% sure if ALL of these are needed. So we need a way to force the inclusion of certain files.
This is where the partner ItemGroup AndroidPackagingOptionsInclude comes in. This ItemGroup behaves similarly to
the AndroidPackagingOptionsExclude but works in the opposite way. It is evaluated first and any file which matches any of the RegEx patterns in this group will be included in the apk. If a file does not match it at all it will then be evaluated against the AndroidPackagingOptionsExclude patterns.

Because this AndroidPackagingOptionsInclude is public it can be used by our NuGet packages for things like Xamarin.Kotlin.Reflect to make sure certain files are included. This can be done in the NuGet by providing a .targets file which has the ItemGroup in it which defines which files need including.

[jonathanpeppers]

Using MSBuild item groups feels kind of weird:

  <ItemGroup>
    <AndroidPackagingOptionsExclude Include="DebugProbesKt.bin" />
    <AndroidPackagingOptionsExclude Include="$([MSBuild]::Escape('*.kotlin*'))" />
    <AndroidPackagingOptionsInclude Include="$([MSBuild]::Escape('*.kotlin_builtins'))" />
  </ItemGroup>

Item groups already have Include and Exclude, but we are using them for holding lists of Regexes. I'm not sure have a better idea, though.

[jonathanpeppers]

If this fixes an issue, is there any chance we'd take this to .NET 8 servicing?

[dellis1972]

It doesn't fix an issue, it is a new feature. So my guess is that won't qualify for backporting.

[jonathanpeppers]

Ok, I thought it might fix: #8456

If we have a way to include/exclude more or less files.

[dellis1972]

It does fix #8456 in that it moves the the ItemGroup to AutoImport.props, but the new ItemGroup is not required to fix that issue.

[jonpryor]

This paragraph confuses me: it appears to be saying that wildcards such as * and ? "MUST use URL encoding", but the examples here don't use URL encoding, so…???

Or are you trying to say that if you want to specify a regex instead of a glob, you need to URL encode?

This doesn't make sense either, because we always treat the value as a fileglob: https://github.com/xamarin/xamarin-android/blob/57337eda1f0d6b8533b3e3a98eb015b037f8ac5c/src/Xamarin.Android.Build.Tasks/Tasks/BuildApk.cs#L347-L363

[dellis1972]

The URL encoding or MSBuild::Escape is needed because if you just did

<AndroidPackagingOptionsExclude Include="*.kotlin_*" />

The MSBuild Parsers would try to find files on disk called '.kotlin_' put those in the ItemGroup. So the values passed into BuildApk would be empty. Its the line below that paragraph that gives the explanation as to why it needs to be URL encoded.

[jonpryor]

Given that we now know that *.kotlin* is problematic, are we sure we want to continue using it by default? Or is it that *.kotlin*, in combination with @(AndroidPackagingOptionsInclude) of *.kotlin_builtins, means that we're good now?

[dellis1972]

Its the combination which means we are ok now.
I'm not sure changing to *.kotlin_m* would end up including more files than we needed.

[jonpryor]

draft commit message:

Context: https://github.com/xamarin/xamarin-android/issues/8456
Context: 2726a386ab92fb7ae86ac6299f3e25b5ccb968c7
Context: https://github.com/xamarin/monodroid/commit/43583d026ff4663fa8165b7c8085189f6210b13e

Commit 2726a386added the `@(AndroidPackagingOptionsExclude)` ItemGroup,
which is used to exclude certain files from the final apk.
xamarin/monodroid@43583d02 updated the `_BuildApkFastDev` target so
that `@(AndroidPackagingOptionsExclude)` was provided to `<BuildApk/>`.
`@(AndroidPackagingOptionsExclude)` was initially designed to filter
out Kotlin based meta data:

	<AndroidPackagingOptionsExclude Include="$([MSBuild]::Escape('*.kotlin_*'))" />

However, it turns out the default file glob of `*.kotlin_*` is too
restrictive, removing e.g. `kotlin/reflect/reflect.kotlin_builtins`,
which can result in runtime errors (see xamarin/xamarin-android#8456):

	java.lang.AssertionError: Built-in class kotlin.Any is not found
	   at kotlin.reflect.jvm.internal.impl.builtins.KotlinBuiltIns$3.invoke(KotlinBuiltIns.java:93)

The "obvious" workaround is to `%(Remove)` the offending item:

	<ItemGroup>
	  <AndroidPackagingOptionsExclude Remove="$([MSBuild]::Escape('*.kotlin_*'))" />
	</ItemGroup>

Unfortunately this doesn't work, because of the order in which
`<ItemGroups/>`s are evaluated by MSBuild.  At the time the `.csproj`
is evaluated, `Xamarin.Android.Common.targets` has not yet been loaded,
and thus `@(AndroidPackagingOptionsExclude)` has not yet been set.

The "less obvious but works" workaround is to change the item group
within a target:

	<Target Name="_UpdateAndroidPackagingOptionsExclude" AfterTargets="AfterBuild">
	  <ItemGroup>
	    <AndroidPackagingOptionsExclude Remove="$([MSBuild]::Escape('*.kotlin_*'))" />
	    <AndroidPackagingOptionsExclude Include="$([MSBuild]::Escape('*.kotlin_m*'))" />
	  </ItemGroup>
	</Target>

This is not very intuitive.

Make this easier by moving the definition of the default
`@(AndroidPackagingOptionsExclude)` values into `AutoImport.props`,
which happens before the `.csproj` is read.  This allows the more
"obvious" `%(Remove)` usage to work as expected.

The next problem is that the default file glob of `*.kotlin_*` was
too expansive, excluding the `*.kotlin_builtins` files which we now
know are important.  We could try changing the default glob to
something else, such as the `*.kotlin_m*` value used by
`_UpdateAndroidPackagingOptionsExclude`, but this can mean that files
we didn't want to include are now included.

Partially rethink things by introducing a new
`@(AndroidPackagingOptionsInclude)` item group, which contains file
globs.  Any packaging artifact which matches the
`@(AndroidPackagingOptionsInclude)` file glob will be included, before
`@(AndroidPackagingOptionsInclude)` has a chance to exclude it.

`@(AndroidPackagingOptionsInclude)` will be public, and can be used
by NuGet packages such as [Xamarin.Kotlin.Reflect][0] to ensure that
specific files are included.  This can be done in the NuGet by
providing a `.targets` file which adds items to
`@(AndroidPackagingOptionsInclude)`.

[0]: https://www.nuget.org/packages/Xamarin.Kotlin.Reflect

[dellis1972]

@jonpryor all green