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

.NET 6 Android AOT fails on certain folder names #56163

Closed
jonathanpeppers opened this issue Jul 22, 2021 · 18 comments · Fixed by #58523 or #62725
Closed

.NET 6 Android AOT fails on certain folder names #56163

jonathanpeppers opened this issue Jul 22, 2021 · 18 comments · Fixed by #58523 or #62725

Comments

@jonathanpeppers
Copy link
Member

Description

Create a dotnet new android project in a folder named such as BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True.

Build with -c Release -p:RunAOTCompilation=true, and you get the error:

Precompiling failed for C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle And├£ml├ñ├╝ts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle And▄mlΣⁿts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location. [C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True\UnnamedProject.csproj]

A screenshot of the .binlog viewer shows odd characters as well:

image

Logs: msbuild.zip

Configuration

> dotnet --version
6.0.100-rc.1.21369.3

I believe this would also happen in Preview 7.

Regression?

No, Android AOT is a new feature.

/cc @akoeplinger @steveisok

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jul 22, 2021
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label Jul 26, 2021
@SamMonoRT SamMonoRT added this to the 6.0.0 milestone Jul 26, 2021
jonathanpeppers added a commit to dotnet/android that referenced this issue Jul 27, 2021
Fixes: #6052

Helpful reading:

* https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/tasks/AotCompilerTask/MonoAOTCompiler.cs
* https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/mono/netcore/nuget/Microsoft.NET.Runtime.MonoAOTCompiler.Task/README.md

To make this work, I moved the existing `<Aot/>` MSBuild task calls to
a new `_AndroidAot` MSBuild target in `Xamarin.Android.Legacy.targets`.

In the .NET 6 targets, there is a *different* `_AndroidAot` MSBuild
target that runs the new `<MonoAOTCompiler/>` MSBuild task. The
`_AndroidAot` target runs per `$(RuntimeIdentifier)` after the linker
completes. Native libraries are added to the
`@(ResolvedFileToPublish)` item group to be included in the final
`.apk`.

To follow convention with Blazor WASM:

https://devblogs.microsoft.com/aspnet/asp-net-core-updates-in-net-6-preview-4/#blazor-webassembly-ahead-of-time-aot-compilation

The `$(RunAOTCompilation)` MSBuild property enables AOT. To help with
backwards compatibility with "legacy" Xamarin.Android, I kept the
`$(AotAssemblies)` MSBuild property intact.

Unfortunately, we have to manually import things to support
`$(AotAssemblies)`:

    <ImportGroup Condition=" '$(MonoAOTCompilerTasksAssemblyPath)' == '' and '$(AotAssemblies)' == 'true' ">
      <Import Project="Sdk.props" Sdk="Microsoft.NET.Runtime.MonoAOTCompiler.Task" />
      <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x86" />
      <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x64" />
      <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm" />
      <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm64" />
    </ImportGroup>

Since, the default Mono workload does not support `$(AotAssemblies)`:

https://github.com/dotnet/runtime/blob/69711860262e44458bbe276393ea3eb9f7a2192a/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in#L20-L25

I think this is reasonable for now.

~~ Limitations ~~

* Profiled AOT is not implemented yet:

#6053

* `$(EnableLLVM)` fails when locating `opt`:

dotnet/runtime#56386

* `AndroidClientHandler` usage causes runtime crash:

dotnet/runtime#56315

* Use of folder names like `Build AndÜmläüts` fails:

dotnet/runtime#56163

~~ Results ~~

All tests were running on a Pixel 5.

Using the HelloAndroid app:

https://github.com/dotnet/maui-samples/tree/main/HelloAndroid

Defaults to two architectures: arm64 and x86

Average of 10 runs with `-c Release` and no AOT:

    Activity: Displayed     00:00:00.308

Apk size: 8367969

Average of 10 runs with `-c Release -p:RunAOTCompilation=true`:

    Activity: Displayed     00:00:00.209

Apk size: 12082123

Using the HelloMaui app:

https://github.com/dotnet/maui-samples/tree/main/HelloMaui

Defaults to two architectures: arm64 and x86

Average of 10 runs with `-c Release` and no AOT:

    Activity: Displayed     00:00:01.117

Apk size: 16272964

Average of 10 runs with `-c Release -p:RunAOTCompilation=true`:

    Activity: Displayed     00:00:00.568

Apk size: 42869016
jonpryor pushed a commit to dotnet/android that referenced this issue Jul 28, 2021
Fixes: #6052

Helpful reading:

  * https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/tasks/AotCompilerTask/MonoAOTCompiler.cs
  * https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/mono/netcore/nuget/Microsoft.NET.Runtime.MonoAOTCompiler.Task/README.md

To make this work, I moved the existing `<Aot/>` MSBuild task calls to
a new `_AndroidAot` MSBuild target in `Xamarin.Android.Legacy.targets`.

In the .NET 6 targets, there is a *different* `_AndroidAot` MSBuild
target that runs the new `<MonoAOTCompiler/>` MSBuild task. The
`_AndroidAot` target runs once per `$(RuntimeIdentifier)`, after the
linker completes.

Native libraries are added to the `@(ResolvedFileToPublish)` item
group to be included in the final `.apk`.

To follow [convention with Blazor WASM][0], the `$(RunAOTCompilation)`
MSBuild property enables AOT.

To help with backwards compatibility with "legacy" Xamarin.Android, I
kept the `$(AotAssemblies)` MSBuild property intact.

Unfortunately, we have to manually import things to support
`$(AotAssemblies)`:

	<ImportGroup Condition=" '$(MonoAOTCompilerTasksAssemblyPath)' == '' and '$(AotAssemblies)' == 'true' ">
	  <Import Project="Sdk.props" Sdk="Microsoft.NET.Runtime.MonoAOTCompiler.Task" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x86" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x64" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm64" />
	</ImportGroup>

as the [default Mono workload does not support `$(AotAssemblies)`][1].
I think this is reasonable for now.


~~ Limitations ~~

Profiled AOT is not implemented yet:

  * #6053

`$(EnableLLVM)` fails when locating `opt`:

  * dotnet/runtime#56386

`$(AndroidClientHandler)` usage causes runtime crash:

  * dotnet/runtime#56315

Use of folder names like `Build AndÜmläüts` fails:

  * dotnet/runtime#56163


~~ Results ~~

All tests:

 1. Were running on a [Google Pixel 5][2], and
 2. Enabled two architectures, arm64 and x86, and
 3. **JIT time** was average of 10 runs with `-c Release`, with the
    `Activity: Displayed` time, and
 4. **AOT time** was average of 10 runs with
    `-c Release -p:RunAOTCompilation=true` with the
    `Activity: Displayed` time.
 5. Δ values are (AOT / JIT)*100.

| Test                |      JIT time |      AOT time |  Δ time |  JIT apk size |  AOT apk size | Δ size |
| ------------------- | ------------: | ------------: | ------: | ------------: | ------------: | -----: |
| [HelloAndroid][3]   |  00:00:00.308 |  00:00:00.209 |     68% |     8,367,969 |    12,082,123 | 144.4% |
| [HelloMaui][4]      |  00:00:01.117 |  00:00:00.568 |     51% |    16,272,964 |    42,869,016 | 263.4% |

[0]: https://devblogs.microsoft.com/aspnet/asp-net-core-updates-in-net-6-preview-4/#blazor-webassembly-ahead-of-time-aot-compilation
[1]: https://github.com/dotnet/runtime/blob/69711860262e44458bbe276393ea3eb9f7a2192a/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in#L20-L25
[2]: https://store.google.com/us/product/pixel_5_specs?hl=en-US
[3]: https://github.com/dotnet/maui-samples/tree/714460431541f40570e91225e8ba4bc1fe08025f/HelloAndroid
[4]: https://github.com/dotnet/maui-samples/tree/714460431541f40570e91225e8ba4bc1fe08025f/HelloMaui
jonpryor pushed a commit to dotnet/android that referenced this issue Jul 28, 2021
Fixes: #6052

Helpful reading:

  * https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/tasks/AotCompilerTask/MonoAOTCompiler.cs
  * https://github.com/dotnet/runtime/blob/15dec9a2aa5a4236d6ba70de2e9c146867b9d2e0/src/mono/netcore/nuget/Microsoft.NET.Runtime.MonoAOTCompiler.Task/README.md

To make this work, I moved the existing `<Aot/>` MSBuild task calls to
a new `_AndroidAot` MSBuild target in `Xamarin.Android.Legacy.targets`.

In the .NET 6 targets, there is a *different* `_AndroidAot` MSBuild
target that runs the new `<MonoAOTCompiler/>` MSBuild task. The
`_AndroidAot` target runs once per `$(RuntimeIdentifier)`, after the
linker completes.

Native libraries are added to the `@(ResolvedFileToPublish)` item
group to be included in the final `.apk`.

To follow [convention with Blazor WASM][0], the `$(RunAOTCompilation)`
MSBuild property enables AOT.

To help with backwards compatibility with "legacy" Xamarin.Android, I
kept the `$(AotAssemblies)` MSBuild property intact.

Unfortunately, we have to manually import things to support
`$(AotAssemblies)`:

	<ImportGroup Condition=" '$(MonoAOTCompilerTasksAssemblyPath)' == '' and '$(AotAssemblies)' == 'true' ">
	  <Import Project="Sdk.props" Sdk="Microsoft.NET.Runtime.MonoAOTCompiler.Task" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x86" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-x64" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm" />
	  <Import Project="Sdk.props" Sdk="Microsoft.NETCore.App.Runtime.AOT.Cross.android-arm64" />
	</ImportGroup>

as the [default Mono workload does not support `$(AotAssemblies)`][1].
I think this is reasonable for now.


~~ Limitations ~~

Profiled AOT is not implemented yet:

  * #6053

`$(EnableLLVM)` fails when locating `opt`:

  * dotnet/runtime#56386

`$(AndroidClientHandler)` usage causes runtime crash:

  * dotnet/runtime#56315

Use of folder names like `Build AndÜmläüts` fails:

  * dotnet/runtime#56163


~~ Results ~~

All tests:

 1. Were running on a [Google Pixel 5][2], and
 2. Enabled two architectures, arm64 and x86, and
 3. **JIT time** was average of 10 runs with `-c Release`, with the
    `Activity: Displayed` time, and
 4. **AOT time** was average of 10 runs with
    `-c Release -p:RunAOTCompilation=true` with the
    `Activity: Displayed` time.
 5. Δ values are (AOT / JIT)*100.

| Test                |      JIT time |      AOT time |  Δ time |  JIT apk size |  AOT apk size | Δ size |
| ------------------- | ------------: | ------------: | ------: | ------------: | ------------: | -----: |
| [HelloAndroid][3]   |  00:00:00.308 |  00:00:00.209 |     68% |     8,367,969 |    12,082,123 | 144.4% |
| [HelloMaui][4]      |  00:00:01.117 |  00:00:00.568 |     51% |    16,272,964 |    42,869,016 | 263.4% |

[0]: https://devblogs.microsoft.com/aspnet/asp-net-core-updates-in-net-6-preview-4/#blazor-webassembly-ahead-of-time-aot-compilation
[1]: https://github.com/dotnet/runtime/blob/69711860262e44458bbe276393ea3eb9f7a2192a/src/mono/nuget/Microsoft.NET.Workload.Mono.Toolchain.Manifest/WorkloadManifest.targets.in#L20-L25
[2]: https://store.google.com/us/product/pixel_5_specs?hl=en-US
[3]: https://github.com/dotnet/maui-samples/tree/714460431541f40570e91225e8ba4bc1fe08025f/HelloAndroid
[4]: https://github.com/dotnet/maui-samples/tree/714460431541f40570e91225e8ba4bc1fe08025f/HelloMaui
@SamMonoRT
Copy link
Member

@steveisok @vargaz - any update ?

@vargaz
Copy link
Contributor

vargaz commented Aug 9, 2021

Can someone who is on windows look at this ?

@lewing
Copy link
Member

lewing commented Aug 9, 2021

cc @radekdoulik @radical

@lewing
Copy link
Member

lewing commented Aug 9, 2021

I think we've seen some other strange failures with encoding of filenames on windows in the build.

@radekdoulik
Copy link
Member

I think we've seen some other strange failures with encoding of filenames on windows in the build.

I know about this one #52108. It is happening during runtime though.

@SamMonoRT
Copy link
Member

Removing the area-Codegen-AOT-mono label.

@ghost
Copy link

ghost commented Aug 12, 2021

Tagging subscribers to 'arch-android': @steveisok, @akoeplinger
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Create a dotnet new android project in a folder named such as BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True.

Build with -c Release -p:RunAOTCompilation=true, and you get the error:

Precompiling failed for C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle And├£ml├ñ├╝ts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle And▄mlΣⁿts_arm64-v8a_False_True\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location. [C:\src\xamarin-android\bin\TestDebug\temp\BuildAotApplicationAndBundle AndÜmläüts_arm64-v8a_False_True\UnnamedProject.csproj]

A screenshot of the .binlog viewer shows odd characters as well:

image

Logs: msbuild.zip

Configuration

> dotnet --version
6.0.100-rc.1.21369.3

I believe this would also happen in Preview 7.

Regression?

No, Android AOT is a new feature.

/cc @akoeplinger @steveisok

Author: jonathanpeppers
Assignees: vargaz
Labels:

os-android, area-Build-mono

Milestone: 6.0.0

@SamMonoRT
Copy link
Member

@lewing @steveisok - this is something we can discuss on Monday, not too certain if this is a release blocking or a 7.0 issue at the moment.

@steveisok
Copy link
Member

steveisok commented Aug 17, 2021

I think this is something we should try to fix for 6.0.

This isn't exclusive to Android, right?

@lewing
Copy link
Member

lewing commented Aug 17, 2021

I'm pretty sure this is the AOT compiler assuming something that isn't always true about the character encoding of the filesystem on windows so I doubt this is android specific.

@lewing
Copy link
Member

lewing commented Aug 17, 2021

@jonathanpeppers what locale is your system using?

@jonathanpeppers
Copy link
Member Author

I'm using en-US. We have a unit test from legacy Xamarin.Android testing this scenario:

https://github.com/xamarin/xamarin-android/blob/f7d32102cc3ada67aa12cc0beed2cf97226f41b3/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AotTests.cs#L158-L161

I saw it fail on CI as well as locally.

I think we have this test because usernames could commonly have special characters, and VS puts new projects in %userprofile%\source\repos by default.

@vargaz
Copy link
Contributor

vargaz commented Aug 17, 2021

So this used to work in xamarin.android on windows ?

@jonathanpeppers
Copy link
Member Author

Yes, these tests still pass for legacy Xamarin.Android on Windows.

We setup a symbolic link named SDK Ümläüts to an Android NDK as well:

https://github.com/xamarin/xamarin-android/blob/f7d32102cc3ada67aa12cc0beed2cf97226f41b3/src/Xamarin.Android.Build.Tasks/Tests/Xamarin.Android.Build.Tests/AotTests.cs#L18-L22

Neither were working with .NET 6 yet.

@lewing lewing assigned radekdoulik and unassigned vargaz Aug 17, 2021
@radekdoulik
Copy link
Member

I am able to reproduce it with our samples. Building the bench sample in src\mono\sample\wasm\browser-bench-řeřicha ends with:

Error: Loaded assembly '...\src\mono\sample\wasm\browser-bench-┼Öe┼Öicha\bin\browser-wasm\publish\System.Private.CoreLib.dll' doesn't match original file name '...\src\mono\sample\wasm\browser-bench-°e°icha\bin\browser-wasm\publish\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location.

radekdoulik added a commit to radekdoulik/runtime that referenced this issue Sep 1, 2021
Fix dotnet#56163

The getcwd call is returning the string encoded in system character
encoding instead of utf-8.

Use native GetCurrentDirectoryW call instead and convert to utf-8.
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 1, 2021
@radekdoulik
Copy link
Member

radekdoulik commented Sep 1, 2021

The non-utf8 path is being created in the mono_path_canonicalize which uses the g_get_current_dir to retrieve the current working directory. On windows it is returning the path in the system encoding, like CP-1250 for example. So the above PR adds win32 implementation, which uses native call to retrieve utf-16 string and converts it to utf-8.

I didn't checked the legacy xamarin.android on windows. IIRC it was built with mingw though, so that might have different getcwd implementation and thus returning the string in utf-8?

github-actions bot pushed a commit that referenced this issue Sep 2, 2021
Fix #56163

The getcwd call is returning the string encoded in system character
encoding instead of utf-8.

Use native GetCurrentDirectoryW call instead and convert to utf-8.
radekdoulik added a commit that referenced this issue Sep 2, 2021
Fix #56163

The getcwd call is returning the string encoded in system character
encoding instead of utf-8.

Use native GetCurrentDirectoryW call instead and convert to utf-8.
@ghost ghost locked as resolved and limited conversation to collaborators Oct 2, 2021
jonathanpeppers added a commit to jonathanpeppers/runtime that referenced this issue Dec 13, 2021
Fixes: dotnet#56163

PR dotnet#58523 fixed something on Windows, but it didn't actually address
our issues on Android.

A directory name like `foo Ümläüts` fails:

* `mkdir 'foo Ümläüts' ; cd 'foo Ümläüts'`
* `dotnet new android`
* `dotnet build -c Release -p:RunAOTCompilation=true` (adding
  `-p:EnableLLVM=true` complicates further)

The error:

    Precompiling failed for C:\src\foo Ümläüts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\foo ├£ml├ñ├╝ts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\foo ▄mlΣⁿts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location.

Reviewing the existing AOT implementation in Xamarin.Android, I found
out *why* Xamarin.Android works:

    [AOT] response file obj\Release\120\aot\arm64-v8a\App36.dll\response.txt: --llvm "--aot=temp-path=obj\Release\120\aot\arm64-v8a\App36.dll,llvm-path=C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android,outfile=obj\Release\120\aot\arm64-v8a\libaot-App36.dll.so,msym-dir=obj\Release\120\aot\arm64-v8a,asmwriter,mtriple=aarch64-linux-android,tool-prefix=C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\aarch64-linux-android-,ld-name=ld.EXE,ld-flags=\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\";\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\libgcc.a\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libc.so\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libm.so\"" C:\Users\jopepper\source\repos\App36\App36\obj\Release\120\android\assets\shrunk\App36.dll

1. Xamarin.Android passes *relative* paths. The `foo Ümläüts`
   directory name doesn't even come into play for some arguments.

2. With LLVM, `ld-flags` contains a `;`.

The existing code splits on `;` and joins on `,`:

https://github.com/dotnet/runtime/blob/25c207351c4f57cf2daa98caaf327a8b8d83edb8/src/tasks/AotCompilerTask/MonoAOTCompiler.cs#L505-L509

So we lose any `;` delimiters for the `ld-flags` value, they get
replaced by `,`.

I think the solution here is:

1. Add several missing properties to `<MonoAOTCompiler/>` so we don't
   have to rely on the `%(AotArguments)` item metadata. No splitting
   on `;` would be required, `ld-flags` can be passed in and used as-is.

2. Add a new `WorkingDirectory` property. When this is set, assume
   paths passed in might be relative -- and don't transform paths by
   calling `Path.GetFullPath()`.

Lastly, I fixed a place where the UTF8 encoding wasn't passed when
MSBuild logging the response file.

These changes I tried to make in a way where this shouldn't break
other .NET workloads like wasm. If existing MSBuild targets call this
task (not using the new properties), the behavior should remain
unchanged.

I tested these changes by commiting a modified `MonoAOTCompiler.dll`:

dotnet/android#6562

I'm able to enable several AOT tests related to dotnet#56163.
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 13, 2021
@jonathanpeppers
Copy link
Member Author

This isn't actually fixed in dotnet new android projects, I think this will fix it: #62725

jonathanpeppers added a commit that referenced this issue Dec 15, 2021
Fixes: #56163

PR #58523 fixed something on Windows, but it didn't actually address
our issues on Android.

A directory name like `foo Ümläüts` fails:

* `mkdir 'foo Ümläüts' ; cd 'foo Ümläüts'`
* `dotnet new android`
* `dotnet build -c Release -p:RunAOTCompilation=true` (adding
  `-p:EnableLLVM=true` complicates further)

The error:

    Precompiling failed for C:\src\foo Ümläüts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\foo ├£ml├ñ├╝ts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\foo ▄mlΣⁿts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location.

Reviewing the existing AOT implementation in Xamarin.Android, I found
out *why* Xamarin.Android works:

    [AOT] response file obj\Release\120\aot\arm64-v8a\App36.dll\response.txt: --llvm "--aot=temp-path=obj\Release\120\aot\arm64-v8a\App36.dll,llvm-path=C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android,outfile=obj\Release\120\aot\arm64-v8a\libaot-App36.dll.so,msym-dir=obj\Release\120\aot\arm64-v8a,asmwriter,mtriple=aarch64-linux-android,tool-prefix=C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\aarch64-linux-android-,ld-name=ld.EXE,ld-flags=\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\";\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\libgcc.a\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libc.so\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libm.so\"" C:\Users\jopepper\source\repos\App36\App36\obj\Release\120\android\assets\shrunk\App36.dll

1. Xamarin.Android passes *relative* paths. The `foo Ümläüts`
   directory name doesn't even come into play for some arguments.

2. With LLVM, `ld-flags` contains a `;`.

The existing code splits on `;` and joins on `,`:

https://github.com/dotnet/runtime/blob/25c207351c4f57cf2daa98caaf327a8b8d83edb8/src/tasks/AotCompilerTask/MonoAOTCompiler.cs#L505-L509

So we lose any `;` delimiters for the `ld-flags` value, they get
replaced by `,`.

I think the solution here is:

1. Add several missing properties to `<MonoAOTCompiler/>` so we don't
   have to rely on the `%(AotArguments)` item metadata. No splitting
   on `;` would be required, `ld-flags` can be passed in and used as-is.

2. Add a new `WorkingDirectory` property. When this is set, assume
   paths passed in might be relative -- and don't transform paths by
   calling `Path.GetFullPath()`.

Lastly, I fixed a place where the UTF8 encoding wasn't passed when
MSBuild logging the response file.

These changes I tried to make in a way where this shouldn't break
other .NET workloads like wasm. If existing MSBuild targets call this
task (not using the new properties), the behavior should remain
unchanged.

I tested these changes by commiting a modified `MonoAOTCompiler.dll`:

dotnet/android#6562

I'm able to enable several AOT tests related to #56163.
github-actions bot pushed a commit that referenced this issue Dec 15, 2021
Fixes: #56163

PR #58523 fixed something on Windows, but it didn't actually address
our issues on Android.

A directory name like `foo Ümläüts` fails:

* `mkdir 'foo Ümläüts' ; cd 'foo Ümläüts'`
* `dotnet new android`
* `dotnet build -c Release -p:RunAOTCompilation=true` (adding
  `-p:EnableLLVM=true` complicates further)

The error:

    Precompiling failed for C:\src\foo Ümläüts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\foo ├£ml├ñ├╝ts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\foo ▄mlΣⁿts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location.

Reviewing the existing AOT implementation in Xamarin.Android, I found
out *why* Xamarin.Android works:

    [AOT] response file obj\Release\120\aot\arm64-v8a\App36.dll\response.txt: --llvm "--aot=temp-path=obj\Release\120\aot\arm64-v8a\App36.dll,llvm-path=C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android,outfile=obj\Release\120\aot\arm64-v8a\libaot-App36.dll.so,msym-dir=obj\Release\120\aot\arm64-v8a,asmwriter,mtriple=aarch64-linux-android,tool-prefix=C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\aarch64-linux-android-,ld-name=ld.EXE,ld-flags=\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\";\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\libgcc.a\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libc.so\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libm.so\"" C:\Users\jopepper\source\repos\App36\App36\obj\Release\120\android\assets\shrunk\App36.dll

1. Xamarin.Android passes *relative* paths. The `foo Ümläüts`
   directory name doesn't even come into play for some arguments.

2. With LLVM, `ld-flags` contains a `;`.

The existing code splits on `;` and joins on `,`:

https://github.com/dotnet/runtime/blob/25c207351c4f57cf2daa98caaf327a8b8d83edb8/src/tasks/AotCompilerTask/MonoAOTCompiler.cs#L505-L509

So we lose any `;` delimiters for the `ld-flags` value, they get
replaced by `,`.

I think the solution here is:

1. Add several missing properties to `<MonoAOTCompiler/>` so we don't
   have to rely on the `%(AotArguments)` item metadata. No splitting
   on `;` would be required, `ld-flags` can be passed in and used as-is.

2. Add a new `WorkingDirectory` property. When this is set, assume
   paths passed in might be relative -- and don't transform paths by
   calling `Path.GetFullPath()`.

Lastly, I fixed a place where the UTF8 encoding wasn't passed when
MSBuild logging the response file.

These changes I tried to make in a way where this shouldn't break
other .NET workloads like wasm. If existing MSBuild targets call this
task (not using the new properties), the behavior should remain
unchanged.

I tested these changes by commiting a modified `MonoAOTCompiler.dll`:

dotnet/android#6562

I'm able to enable several AOT tests related to #56163.
akoeplinger pushed a commit that referenced this issue Dec 16, 2021
Fixes: #56163

PR #58523 fixed something on Windows, but it didn't actually address
our issues on Android.

A directory name like `foo Ümläüts` fails:

* `mkdir 'foo Ümläüts' ; cd 'foo Ümläüts'`
* `dotnet new android`
* `dotnet build -c Release -p:RunAOTCompilation=true` (adding
  `-p:EnableLLVM=true` complicates further)

The error:

    Precompiling failed for C:\src\foo Ümläüts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll: Error: Loaded assembly 'C:\src\foo ├£ml├ñ├╝ts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll' doesn't match original file name 'C:\foo ▄mlΣⁿts\obj\Release\android-arm64\linked\System.Private.CoreLib.dll'. Set MONO_PATH to the assembly's location.

Reviewing the existing AOT implementation in Xamarin.Android, I found
out *why* Xamarin.Android works:

    [AOT] response file obj\Release\120\aot\arm64-v8a\App36.dll\response.txt: --llvm "--aot=temp-path=obj\Release\120\aot\arm64-v8a\App36.dll,llvm-path=C:\Program Files\Microsoft Visual Studio\2022\Preview\MSBuild\Xamarin\Android,outfile=obj\Release\120\aot\arm64-v8a\libaot-App36.dll.so,msym-dir=obj\Release\120\aot\arm64-v8a,asmwriter,mtriple=aarch64-linux-android,tool-prefix=C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\bin\aarch64-linux-android-,ld-name=ld.EXE,ld-flags=\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\";\"-LC:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\toolchains\llvm\prebuilt\windows-x86_64\lib\gcc\aarch64-linux-android\4.9.x\libgcc.a\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libc.so\";\"C:\Program Files (x86)\Android\android-sdk\ndk-bundle\platforms\android-21\arch-arm64\usr\lib\libm.so\"" C:\Users\jopepper\source\repos\App36\App36\obj\Release\120\android\assets\shrunk\App36.dll

1. Xamarin.Android passes *relative* paths. The `foo Ümläüts`
   directory name doesn't even come into play for some arguments.

2. With LLVM, `ld-flags` contains a `;`.

The existing code splits on `;` and joins on `,`:

https://github.com/dotnet/runtime/blob/25c207351c4f57cf2daa98caaf327a8b8d83edb8/src/tasks/AotCompilerTask/MonoAOTCompiler.cs#L505-L509

So we lose any `;` delimiters for the `ld-flags` value, they get
replaced by `,`.

I think the solution here is:

1. Add several missing properties to `<MonoAOTCompiler/>` so we don't
   have to rely on the `%(AotArguments)` item metadata. No splitting
   on `;` would be required, `ld-flags` can be passed in and used as-is.

2. Add a new `WorkingDirectory` property. When this is set, assume
   paths passed in might be relative -- and don't transform paths by
   calling `Path.GetFullPath()`.

Lastly, I fixed a place where the UTF8 encoding wasn't passed when
MSBuild logging the response file.

These changes I tried to make in a way where this shouldn't break
other .NET workloads like wasm. If existing MSBuild targets call this
task (not using the new properties), the behavior should remain
unchanged.

I tested these changes by commiting a modified `MonoAOTCompiler.dll`:

dotnet/android#6562

I'm able to enable several AOT tests related to #56163.

Co-authored-by: Jonathan Peppers <jonathan.peppers@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.