Skip to content

Conversation

@jonpryor
Copy link
Contributor

Partially reverts commits 8a25534, daddcdf.

A @(ProjectReference) with
%(ProjectReference.ReferenceOutputAssembly) set to False is useful
to control MSBuild project dependencies, to ensure that a particular
project is built before other projects.

Xamarin.Android.Build.Tasks.csproj contains two of these of
questionable value:

  1. Xamarin.Android.Tools.BootstrapTasks.csproj (8a25534) is
    referenced, and I don't recall why it's referenced.
    Additionally, removing this reference doesn't break anything.
  2. class-parse.csproj (daddcdf) is referenced, but the logic in the
    commit message doesn't make sense, given that
    Xamarin.Android.Build.Tasks.dll doesn't invoke
    class-parse.exe, it instead uses
    Xamarin.Android.Tools.Bytecode.dll, which is referenced
    normally.
    I thus don't understand why the class-parse.csproj reference is
    currently required.

Remove these two project references.

Partially reverts commits 8a25534, daddcdf.

A `@(ProjectReference)` with
`%(ProjectReference.ReferenceOutputAssembly)` set to False is useful
to control MSBuild project dependencies, to ensure that a particular
project is built before other projects.

`Xamarin.Android.Build.Tasks.csproj` contains two of these of
questionable value:

1. `Xamarin.Android.Tools.BootstrapTasks.csproj` (8a25534) is
    referenced, and I don't recall *why* it's referenced.
    Additionally, removing this reference doesn't break anything.

2. `class-parse.csproj` (daddcdf) is referenced, but the logic in the
    commit message doesn't make sense, given that
    `Xamarin.Android.Build.Tasks.dll` doesn't invoke
    `class-parse.exe`, it instead uses
    `Xamarin.Android.Tools.Bytecode.dll`, which is referenced
    normally.
    I thus don't understand why the `class-parse.csproj` reference is
    currently required.

Remove these two project references.
@jonpryor jonpryor merged commit ae3b93b into dotnet:master Aug 17, 2016
radekdoulik pushed a commit to radekdoulik/xamarin-android that referenced this pull request Aug 18, 2016
Partially reverts commits 8a25534, daddcdf.

A `@(ProjectReference)` with
`%(ProjectReference.ReferenceOutputAssembly)` set to False is useful
to control MSBuild project dependencies, to ensure that a particular
project is built before other projects.

`Xamarin.Android.Build.Tasks.csproj` contains two of these of
questionable value:

1. `Xamarin.Android.Tools.BootstrapTasks.csproj` (8a25534) is
    referenced, and I don't recall *why* it's referenced.
    Additionally, removing this reference doesn't break anything.

2. `class-parse.csproj` (daddcdf) is referenced, but the logic in the
    commit message doesn't make sense, given that
    `Xamarin.Android.Build.Tasks.dll` doesn't invoke
    `class-parse.exe`, it instead uses
    `Xamarin.Android.Tools.Bytecode.dll`, which is referenced
    normally.
    I thus don't understand why the `class-parse.csproj` reference is
    currently required.

Remove these two project references.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jun 14, 2022
Context: 70272db
Context: 38b1236
Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md
Context: actions/runner-images@2950cbf

Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724

  * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (dotnet#171)
  * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (dotnet#170)

We've had an "inadvertent thinko" for the past couple of months:
in commit 70272db we updated the default `$(AndroidNdkDirectory)`
value to be `$(ANDROID_NDK_LATEST_HOME)`, if set.  *At the time*,
we think this was NDK r23.

In commit 38b1236, we updated `xaprepare` to install NDK r24.

The "thinko" is that commit 38b1236 was *irrelevant*, as commit
70272db was "in control".  The Windows **Prepare Solution** log
would contain e.g.:

	  Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313
	  Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying
	      installed

It also *mis-detected* NDK r23 as r24 (?!).

It "worked" only by coincidence.

Around 2022-Jun-09, GitHub actions updated the default NDK installed
on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was
NDK r23 or earlier (we're not sure); *now*, it is NDK r24.

This subtle change -- invisible!  The build environment changed! --
*introduced* unit test failures, of two patterns.

Pattern 1 is that `readelf` could not be found:

	System.ComponentModel.Win32Exception : An error occurred trying to start process
	'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf'
	with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'.
	The system cannot find the file specified.
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489

	-or-
	System.ComponentModel.Win32Exception : The system cannot find the file specified
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489

This happened because `readelf` was *removed* in NDK r24.

Fix this pattern by updating `$(AndroidNdkDirectory)` to instead
default to `$(ANDROID_NDK_HOME)`, which is now NDK r23.
Additionally, update
`EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also
look for `llvm-readelf`, the replacement for `readelf` in NDK r24+.

Pattern 2 is that the `aarch64-linux-android28-clang.cmd` & related
scripts on Windows don't support being run from a directory
containing spaces:

	CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
	AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
	…
	[CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
	  -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
	  obj\Release\bundles\armeabi-v7a\temp.c
	[cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
	[cc stderr] operable program or batch file.

Fix this pattern by updating `NdkToolsWithClangNoBinutils` to
override the C and C++ compiler toolnames, a'la: 71ae556.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jun 14, 2022
Context: 70272db
Context: 38b1236
Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md
Context: actions/runner-images@2950cbf

Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724

  * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (dotnet#171)
  * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (dotnet#170)

We've had an "inadvertent thinko" for the past couple of months:
in commit 70272db we updated the default `$(AndroidNdkDirectory)`
value to be `$(ANDROID_NDK_LATEST_HOME)`, if set.  *At the time*,
we think this was NDK r23.

In commit 38b1236, we updated `xaprepare` to install NDK r24.

The "thinko" is that commit 38b1236 was *irrelevant*, as commit
70272db was "in control".  The Windows **Prepare Solution** log
would contain e.g.:

	  Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313
	  Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying
	      installed

It also *mis-detected* NDK r23 as r24 (?!).

It "worked" only by coincidence.

Around 2022-Jun-09, GitHub actions updated the default NDK installed
on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was
NDK r23 or earlier (we're not sure); *now*, it is NDK r24.

This subtle change -- invisible!  The build environment changed! --
*introduced* unit test failures, of two patterns.

Pattern 1 is that `readelf` could not be found:

	System.ComponentModel.Win32Exception : An error occurred trying to start process
	'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf'
	with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'.
	The system cannot find the file specified.
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489

	-or-
	System.ComponentModel.Win32Exception : The system cannot find the file specified
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489

This happened because `readelf` was *removed* in NDK r24.

Fix this pattern updating
`EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also
look for `llvm-readelf`, the replacement for `readelf` in NDK r24+.

Pattern 2 is that the `aarch64-linux-android28-clang.cmd` & related
scripts on Windows don't support being run from a directory
containing spaces:

	CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
	AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
	…
	[CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
	  -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
	  obj\Release\bundles\armeabi-v7a\temp.c
	[cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
	[cc stderr] operable program or batch file.

Fix this pattern by updating `NdkToolsWithClangNoBinutils` to
override the C and C++ compiler toolnames, a'la: 71ae556.
jonpryor added a commit to jonpryor/xamarin-android that referenced this pull request Jun 15, 2022
Context: 70272db
Context: 38b1236
Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md
Context: actions/runner-images@2950cbf

Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724

  * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (dotnet#171)
  * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (dotnet#170)

We've had an "inadvertent thinko" for the past couple of months:
in commit 70272db we updated the default `$(AndroidNdkDirectory)`
value to be `$(ANDROID_NDK_LATEST_HOME)`, if set.  *At the time*,
we think this was NDK r23.

In commit 38b1236, we updated `xaprepare` to install NDK r24.

The "thinko" is that commit 38b1236 was *irrelevant*, as commit
70272db was "in control".  The Windows **Prepare Solution** log
would contain e.g.:

	  Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313
	  Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying
	      installed

It also *mis-detected* NDK r23 as r24 (?!).

It "worked" only by coincidence.

Around 2022-Jun-09, GitHub actions updated the default NDK installed
on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was
NDK r23 or earlier (we're not sure); *now*, it is NDK r24.

This subtle change -- invisible!  The build environment changed! --
*introduced* unit test failures, of two patterns.

Pattern 1 is that `readelf` could not be found:

	System.ComponentModel.Win32Exception : An error occurred trying to start process
	'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf'
	with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'.
	The system cannot find the file specified.
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489

	-or-
	System.ComponentModel.Win32Exception : The system cannot find the file specified
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489

This happened because `readelf` was *removed* in NDK r24.

Fix this pattern updating
`EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also
look for `llvm-readelf`, the replacement for `readelf` in NDK r24+.

Pattern 2 is that the `aarch64-linux-android28-clang.cmd` & related
scripts on Windows don't support being run from a directory
containing spaces:

	CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
	AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
	…
	[CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
	  -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
	  obj\Release\bundles\armeabi-v7a\temp.c
	[cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
	[cc stderr] operable program or batch file.

Fix this pattern by updating `NdkToolsWithClangNoBinutils` to
override the C and C++ compiler toolnames, a'la: 71ae556.
jonpryor added a commit that referenced this pull request Jun 15, 2022
Context: 70272db
Context: 38b1236
Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md
Context: actions/runner-images@2950cbf

Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724

  * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (#171)
  * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (#170)

We've had an "inadvertent behavior" for the past couple of months:
in commit 70272db we updated the default `$(AndroidNdkDirectory)`
value to be `$(ANDROID_NDK_LATEST_HOME)`, if set.  *At the time*,
we think this was NDK r23.

In commit 38b1236, we updated `xaprepare` to install NDK r24.

This appears to have caused `xaprepare` to update the *system*
`$(ANDROID_NDK_LATEST_HOME)` location, not a xamarin-android-specific
NDK installation.  The Windows **Prepare Solution** log would
contain e.g.:

	  Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313
	  Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying
	      installed

This appears to imply that `C:\Android\android-sdk\ndk\23.2.8568313`
was updated to have NDK 24.0.8215888.

Meanwhile, *unit tests* were using `$(ANDROID_SDK_PATH)` and
`$(ANDROID_NDK_PATH)`, which had been using NDK *r21*.

Around 2022-Jun-09, GitHub actions updated the default NDK installed
on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was
NDK r23 or earlier (we're not sure); *now*, it is NDK r24.
`$(ANDROID_NDK_PATH)` had been NDK r21; now it is NDK r23.

This subtle change -- invisible!  The build environment changed! --
*introduced* unit test failures, of two patterns.

Scenario 1 is that `readelf` could not be found:

	System.ComponentModel.Win32Exception : An error occurred trying to start process
	'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf'
	with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'.
	The system cannot find the file specified.
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489

	-or-
	System.ComponentModel.Win32Exception : The system cannot find the file specified
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489

This happened because `readelf` was *removed* in NDK r24.

Fix this scenario updating
`EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also
look for `llvm-readelf`, the replacement for `readelf` in NDK r24+.

Scenario 2 is that the `aarch64-linux-android28-clang.cmd` & related
scripts on Windows in NDK r23+ don't support being run from a
directory containing spaces:

	CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
	AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
	…
	[CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
	  -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
	  obj\Release\bundles\armeabi-v7a\temp.c
	[cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
	[cc stderr] operable program or batch file.

Fix this scenario by updating `NdkToolsWithClangNoBinutils` to
override the C and C++ compiler toolnames, a'la commit 71ae556.

To help make investigating
`AndroidDependenciesTests.InstallAndroidDependenciesTest()` tests
failures easier, send the `InstallAndroidDependencies` target
execution output to an `install-deps.log` file.  Previously, the
subsequent `Build` target would *overwrite* the output of the
`InstallAndroidDependencies` output, as they both shared `build.log`.

Finally, update `AndroidSdkResolver.GetAndroidSdkPath()` and
`AndroidSdkResolver.GetAndroidNdkPath()` to *stop* looking at the
`$(ANDROID_SDK_PATH)` and `$(ANDROID_NDK_PATH)` environment variables.
Instead, unit tests should use the `$(TEST_ANDROID_SDK_PATH)`
environment variable, which is managed by the unit test
infrastructure.
jonathanpeppers pushed a commit that referenced this pull request Jun 16, 2022
Context: 70272db
Context: 38b1236
Context: https://github.com/actions/virtual-environments/blob/9cf1ebd754807fba137c8ce9fa2140311609fdd6/images/win/Windows2022-Readme.md
Context: actions/runner-images@2950cbf

Changes: https://github.com/xamarin/xamarin-android-tools/compare/20f611202bef0fc7c1659366dd38865eb119dde5...ec346d07cf3ac7fc74d08eae4f19043b51485724

  * dotnet/android-tools@ec346d0: [Xamarin.Android.Tools.AndroidSdk] Permit NDK r24 (#171)
  * dotnet/android-tools@47832f1: [Xamarin.Android.Tools.AndroidSdk] AndroidSdkInfo validation locator (#170)

We've had an "inadvertent behavior" for the past couple of months:
in commit 70272db we updated the default `$(AndroidNdkDirectory)`
value to be `$(ANDROID_NDK_LATEST_HOME)`, if set.  *At the time*,
we think this was NDK r23.

In commit 38b1236, we updated `xaprepare` to install NDK r24.

This appears to have caused `xaprepare` to update the *system*
`$(ANDROID_NDK_LATEST_HOME)` location, not a xamarin-android-specific
NDK installation.  The Windows **Prepare Solution** log would
contain e.g.:

	  Checking if android-ndk-r24-windows exists in C:\Android\android-sdk\ndk\23.2.8568313
	  Component 'android-ndk-r24-windows' requires Pkg.Revision to be '24.0.8215888', verifying
	      installed

This appears to imply that `C:\Android\android-sdk\ndk\23.2.8568313`
was updated to have NDK 24.0.8215888.

Meanwhile, *unit tests* were using `$(ANDROID_SDK_PATH)` and
`$(ANDROID_NDK_PATH)`, which had been using NDK *r21*.

Around 2022-Jun-09, GitHub actions updated the default NDK installed
on their Windows images; previously, `$(ANDROID_NDK_LATEST_HOME)` was
NDK r23 or earlier (we're not sure); *now*, it is NDK r24.
`$(ANDROID_NDK_PATH)` had been NDK r21; now it is NDK r23.

This subtle change -- invisible!  The build environment changed! --
*introduced* unit test failures, of two patterns.

Scenario 1 is that `readelf` could not be found:

	System.ComponentModel.Win32Exception : An error occurred trying to start process
	'C:\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\readelf'
	with working directory 'C:\a_work\1\s\xamarin-android\bin\TestRelease\net6.0'.
	The system cannot find the file specified.
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 550
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in C:\a\_work\1\s\src\Xamarin.Android.Build.Tasks\Tests\Xamarin.Android.Build.Tests\Utilities\EnvironmentHelper.cs:line 489

	-or-
	System.ComponentModel.Win32Exception : The system cannot find the file specified
	   at System.Diagnostics.Process.StartWithCreateProcess(ProcessStartInfo startInfo)
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.RunCommand(String executablePath, String arguments) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 551
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertSharedLibraryHasRequiredSymbols(String dsoPath, String readElfPath) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 495
	   at Xamarin.Android.Build.Tests.EnvironmentHelper.AssertValidEnvironmentSharedLibrary(String outputDirectoryRoot, String sdkDirectory, String ndkDirectory, String supportedAbis) in /Users/builder/azdo/_work/1/s/xamarin-android/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/Utilities/EnvironmentHelper.cs:line 489

This happened because `readelf` was *removed* in NDK r24.

Fix this scenario updating
`EnvironmentHelper.AssertValidEnvironmentSharedLibrary()` to also
look for `llvm-readelf`, the replacement for `readelf` in NDK r24+.

Scenario 2 is that the `aarch64-linux-android28-clang.cmd` & related
scripts on Windows in NDK r23+ don't support being run from a
directory containing spaces:

	CC="C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD"
	AS="C:\Program Files\Microsoft Visual Studio\2022\Enterprise\MSBuild\Xamarin\Android\binutils\bin\arm-linux-androideabi-as.CMD"
	…
	[CC] "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\bin\armv7a-linux-androideabi19-clang.CMD" ^
	  -c -D__ANDROID_API__=19 -DANDROID -o obj\Release\bundles\armeabi-v7a\temp.o ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include\arm-linux-androideabi" ^
	  -I "C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK Ümläüts\ndk\toolchains\llvm\prebuilt\windows-x86_64\sysroot\usr\include" ^
	  obj\Release\bundles\armeabi-v7a\temp.c
	[cc stderr] 'C:\a\_work\1\a\TestRelease\06-13_18.07.09\temp\SDK' is not recognized as an internal or external command,
	[cc stderr] operable program or batch file.

Fix this scenario by updating `NdkToolsWithClangNoBinutils` to
override the C and C++ compiler toolnames, a'la commit 71ae556.

To help make investigating
`AndroidDependenciesTests.InstallAndroidDependenciesTest()` tests
failures easier, send the `InstallAndroidDependencies` target
execution output to an `install-deps.log` file.  Previously, the
subsequent `Build` target would *overwrite* the output of the
`InstallAndroidDependencies` output, as they both shared `build.log`.

Finally, update `AndroidSdkResolver.GetAndroidSdkPath()` and
`AndroidSdkResolver.GetAndroidNdkPath()` to *stop* looking at the
`$(ANDROID_SDK_PATH)` and `$(ANDROID_NDK_PATH)` environment variables.
Instead, unit tests should use the `$(TEST_ANDROID_SDK_PATH)`
environment variable, which is managed by the unit test
infrastructure.
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 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.

2 participants