Skip to content

Conversation

@jonpryor
Copy link
Contributor

Context: dotnet/android-tools#223

Does It Build™?

@jonpryor
Copy link
Contributor Author

it doesn't build:

MSBUILD : java error JAVA0000: Error in /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules: [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
MSBUILD : java error JAVA0000: Failed to read file: /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
MSBUILD : java error JAVA0000: Compilation failed [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
MSBUILD : java error JAVA0000: java.lang.RuntimeException: com.android.tools.r8.CompilationFailedException: Compilation failed to complete, origin [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
MSBUILD : java error JAVA0000:  /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]

The problem is that the Android SDK build-tools package r33 removed the file mainDexClasses.rules, which is used by various ProGuard-related tasks within Xamarin.Android.

See also: #6774.

Looks like we can't bump $(AndroidSdkBuildToolsVersion).

jonpryor added a commit to dotnet/android-tools that referenced this pull request Dec 12, 2023
Context: dotnet/android#8580 (comment)

> it doesn't build:
>
> ```
> MSBUILD : java error JAVA0000: Error in /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules: [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
> MSBUILD : java error JAVA0000: Failed to read file: /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
> MSBUILD : java error JAVA0000: Compilation failed [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
> MSBUILD : java error JAVA0000: java.lang.RuntimeException: com.android.tools.r8.CompilationFailedException: Compilation failed to complete, origin [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
> MSBUILD : java error JAVA0000:  /Users/builder/android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/builder/azdo/_work/4/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
> ```
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Dec 12, 2023
Context: dotnet#8580

Does It Build™?

It *should* build, it *used* to build (on 2023-09-02), but dotnet#8580
*doesn't* build, as it's trying to reference
`android-toolchain/sdk/build-tools/34.0.0/mainDexClasses.rules`,
which doesn't exist.

Furthermore, AFAICT `xaprepare` uses `$(XABuildToolsVersion)` to
specify which build-tools package is installed, which has been `34`
since 326ac88, so… how did it build then and why is it failing to
build now?  What am I missing?
This was referenced Dec 12, 2023
@jonpryor jonpryor force-pushed the dev/jonp/jonp-try-xat-223 branch from 907516b to 5b24f6d Compare December 19, 2023 01:16
@jonpryor
Copy link
Contributor Author

Now that d17-5 should build again (see also #8585), let's try this again:

  • $(AndroidCommandLineToolsVersion)=11.0
  • $(AndroidSdkPlatformToolsVersion)=34.0.5

My expectation is that unit tests which use lint will fail, because commandlinetools-11.0 requires JDK-17 (link?).

Let's verify that expectation.

@jonpryor
Copy link
Contributor Author

Let's verify that expectation.

So much for that expectation; it's fully green!

Well, okay then.

Let's "un-bump" xamarin-android-tools to re-contain the $(AndroidSdkBuildToolsVersion)=34.0.0 change. Does that introduce unit test failures? (That's my expectation…)

@jonpryor
Copy link
Contributor Author

As expected, building with $(AndroidSdkBuildToolsVersion)=34.0.0 resulted in unit test build failures, e.g. run Mono.Android_TestsMultiDex:

MSBUILD : java error JAVA0000: Error in /Users/runner/Library/Android/sdk/build-tools/34.0.0/mainDexClasses.rules: [/Users/runner/work/1/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
MSBUILD : java error JAVA0000: Failed to read file: /Users/runner/Library/Android/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/runner/work/1/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
…

`$(AndroidSdkBuildToolsVersion)` is back to 32.0.0.
@jonpryor jonpryor force-pushed the dev/jonp/jonp-try-xat-223 branch from f3a9012 to 2f168a8 Compare December 20, 2023 03:33
As expected, `$(AndroidSdkBuildToolsVersion)`=34.0.0 causes
unit test failures:

    MSBUILD : java error JAVA0000: Error in /Users/runner/Library/Android/sdk/build-tools/34.0.0/mainDexClasses.rules: [/Users/runner/work/1/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]
    MSBUILD : java error JAVA0000: Failed to read file: /Users/runner/Library/Android/sdk/build-tools/34.0.0/mainDexClasses.rules [/Users/runner/work/1/s/xamarin-android/tests/Mono.Android-Tests/Runtime-MultiDex/Mono.Android-TestsMultiDex.csproj]

Bump to a xamarin-android-tools which has `$(AndroidSdkBuildToolsVersion)`=32.0.0.
@jonpryor
Copy link
Contributor Author

One problem with the current dotnet/android-tools#223 is around the InstallAndroidDependencies target: by default the "Xamarin" manifest is used, and Classic Xamarin.Android 13.2.x will be using the manifest at https://aka.ms/xamarin/sdkmanifest/d17-5. My concern is that the "d17-5 Xamarin manifest" hasn't been touched in over a year -- https://aka.ms/xamarin/sdkmanifest/d17-8 is what .NET 8 is using -- and thus the package versions don't match:

  • Only commandlinetools 7.0 is present, not 11.0.
  • Only platform-tools 33.0.3 and 33.0.2 are present, not 34.0.5

which suggests to me that if we took this xamarin-android-tools bump, msbuild -t:InstallAndroidDependencies -p:AndroidSdkDirectory=… would not (fully) work, because it would attempt to install package versions that it doesn't know how to find, e.g. try to install platform-tools 34.0.5. (It would be similar to the error here.)

The only way to fix this issue would be to also bump monodroid+androidtools+android-sdk-installer(+others?), which is a larger risk than I'm comfortable with.

I thus don't think that this change is actually viable.

@pjcollins : thoughts?

@pjcollins
Copy link
Member

The only way to fix this issue would be to also bump monodroid+androidtools+android-sdk-installer(+others?), which is a larger risk than I'm comfortable with.

This assessment seems correct to me, and I agree that the risk there is going to outweigh the benefit of trying to make these changes. I think it would be best to leave the SDK tool versions that classic Xamarin.Android depends on untouched. My intuition is telling me that it is likely not too common for folks who are still using classic to be installing the latest VS on a clean machine. It seems more likely that they would be following an upgrade path or staying on a particular version that would already have the tools versions they need installed. As for classic folks who do need a clean install of VS 17.9, I think the auto SDK installation workflow is a perfectly adequate solution.

@jonpryor
Copy link
Contributor Author

@jonpryor jonpryor closed this Dec 20, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 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