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] improve native library ABI detection at build time #5432

Closed
jonathanpeppers opened this issue Dec 16, 2020 · 5 comments · Fixed by dotnet/android-tools#121
Closed
Assignees
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Milestone

Comments

@jonathanpeppers
Copy link
Member

Context: #5386

To determine the Android ABI of native libraries, we currently:

  • Look for %(Abi) metadata
  • Look for a directory named: armeabi-v7a, arm64-v8a, x86, x86_64

This will not work in .NET 6, if the native library is coming from a NuGet package with a path such as:

packages\sqlitepclraw.lib.e_sqlite3.android\1.1.11\runtimes\android-arm64\native\libe_sqlite3.so

Instead of doing more inference with directory names, we should actually look at the contents of the .so file and:

  • Give a warning if it is not an Android library and ignore it.
  • Determine the ABI. If none found, emit a warning and ignore it.

We should still use the %(Abi) metadata if specified, for backwards compatibility.

We could try using the ElfSharp library to do this. We would need to make sure the code path is fast, or we have some mechanism for making incremental builds fast.

Useful links:

@jonathanpeppers jonathanpeppers added the Area: App+Library Build Issues when building Library projects or Application projects. label Dec 16, 2020
@jonathanpeppers jonathanpeppers added this to the One .NET milestone Dec 16, 2020
@jonathanpeppers jonathanpeppers self-assigned this Dec 16, 2020
@jonathanpeppers
Copy link
Member Author

jonathanpeppers commented Dec 18, 2020

I wrote a sample program using ElfSharp: ElfSharpTest.zip

It looks for the equivalent of readelf -d | grep NEEDED | grep ld-linux in about 47ms for 10 native libraries.

However one native library breaks the pattern:

$ readelf -d linux-x86/libe_sqlite3.so | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libc.so.6]

We may need to just check the file path and not use ElfSharp at all.

@mattleibow
Copy link
Member

We could just add another rule and see if it came from a nuget and was in the runtimes, then look at the value after the android-.

@jonathanpeppers
Copy link
Member Author

I just haven't gotten around to doing this yet, but I think the plan will be:

  • If the directory name is an Android RuntimeIdentifier, so you could put android-arm64/libfoo.so in an app
  • If the directory name two levels above is an Android RuntimeIdentifier, so NuGet packages will work.

android-arm64/foo/libbar.so would also work in an app, but I think that would be fine.

@mattleibow
Copy link
Member

mattleibow commented May 24, 2021

I am looking at the targets here now and I think this code already works. When the packages restore, there is metadata on the .so files RuntimeIdentifier=android-arm64. There is also the code that detects and converts from this RID to the ABI in the ProcessNativeLibraries task in the _IncludeNativeSystemLibraries target.

However, the files that this task uses is obtained via the _ResolveAssemblies target, which calls msbuild with the _ComputeFilesToPublishForRuntimeIdentifiers target. The result of this operation does not include the RID metadata.

I got a bit of a hack to add it back...

    <Target Name="_IncludeNativeSystemLibrariesFixes" BeforeTargets="_IncludeNativeSystemLibraries">
        <ItemGroup>
            <_FixedResolvedFileToPublish
                Include="@(ResolvedFileToPublish)"
                Condition=" 
                    '%(ResolvedFileToPublish.Extension)' == '.so' and 
                    '%(ResolvedFileToPublish.NuGetPackageId)' != '' and 
                    '%(ResolvedFileToPublish.RuntimeIdentifier)' == '' and 
                    '%(ResolvedFileToPublish.PathInPackage)' != '' " />
            <ResolvedFileToPublish Remove="@(_FixedResolvedFileToPublish)" />
            <ResolvedFileToPublish Include="@(_FixedResolvedFileToPublish)">
                <RuntimeIdentifier>$([System.Text.RegularExpressions.Regex]::Match('%(_FixedResolvedFileToPublish.PathInPackage)', 'runtimes/([^/]+)/native/.*').Groups[1].Value)</RuntimeIdentifier>
            </ResolvedFileToPublish>
        </ItemGroup>
    </Target>

@jonathanpeppers
Copy link
Member Author

The dotnet/sdk creates @(ResolvedFileToPublish), and I've noticed that %(RuntimeIdentifier) is blank when the file is coming from a regular NuGet package.

For some reason, it's there for "runtime" packs that are defined such as:

https://github.com/xamarin/xamarin-android/blob/02ab8f723b6b8e327a3d8a6fd99192598ca94668/build-tools/create-packs/Microsoft.Android.Sdk.proj#L103-L114

Native libraries from Microsoft.Android.Runtime.android-arm has RuntimeIdentifier=android-arm filled out for us.

jonathanpeppers added a commit to jonathanpeppers/xamarin-android-tools that referenced this issue Jun 3, 2021
Fixes: dotnet/android#5432

Two cases currently do not work in .NET 6:

1) An Android app project includes files such as:

    android-arm/libfoo.so
    android-arm64/libfoo.so
    android-x86/libfoo.so
    android-x64/libfoo.so

It would be nice if users could use `$(RuntimeIdentifier)` names
here. We can simply check if the directory name is a RID.

2) A NuGet package includes a native library from a path such as:

    packages/sqlitepclraw.lib.e_sqlite3.android/1.1.11/runtimes/android-arm64/native/libe_sqlite3.so

In this case, there is no `%(RuntimeIdentifier)` item metadata on this
native library. So we will have to check if the following is a RID:

    Directory.GetParent (lib.ItemSpec).Parent.Name

I implemented these two cases as last resort to the existing logic.

I think this will be fine for the behavior to be in "legacy"
Xamarin.Android as well as .NET 6.

I added tests for `AndroidRidAbiHelper`, since we had none before.
jonpryor pushed a commit to dotnet/android-tools that referenced this issue Jun 4, 2021
Context: dotnet/android#5432

Two cases currently do not work in .NET 6:

 1. When an Android App project includes native libraries which are
    in directory names consisting of .NET runtime identifiers instead
    of Android ABI names, e.g.

        android-arm/libfoo.so
        android-arm64/libfoo.so
        android-x86/libfoo.so
        android-x64/libfoo.so

 2. When a NuGet package places native libraries into a `native`
    directory *between* the `$(RuntimeIdentifier)` directory and the
    native library, a'la [`SQLitePCLRaw.lib.e_sqlite3.linux`][0]:

        runtimes/linux-arm/native/libe_sqlite3.so

Fix case (1) by checking using
`AndroidRidAbiHelper.RuntimeIdentifierToAbi()` on the directory name
to determine the Android ABI of the library.

Fix case (2) by also checking the native library's parent parent
directory name against Android ABI names and Runtime Identifiers.
This allows us to correctly associate

	runtimes/android-arm64/native/libe_sqlite3.so

as an arm64-v8a native library.

I implemented these two cases as fallbacks to the existing logic.

I think this will be fine for the behavior to be in "legacy"
Xamarin.Android as well as .NET 6.

I added tests for `AndroidRidAbiHelper`, since we had none before.

[0]: https://www.nuget.org/packages/SQLitePCLRaw.lib.e_sqlite3.linux/1.1.14
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jun 4, 2021
Fixes: dotnet#5432
Fixes: dotnet#5992

Changes: dotnet/android-tools@683f375...90d7621

  * dotnet/android-tools@90d7621: [BaseTasks] add ABI detection for RIDs (dotnet#121)
  * dotnet/android-tools@79e3b97: [JdkInfo] handle invalid XML from /usr/libexec/java_home (dotnet#120)
  * dotnet/android-tools@81519fe: Add SECURITY.md (dotnet#119)
jonpryor added a commit to jonpryor/xamarin-android that referenced this issue Jun 4, 2021
Fixes: dotnet#5432
Fixes: dotnet#5992

Changes: dotnet/android-tools@683f375...90d7621

  * dotnet/android-tools@90d7621: [BaseTasks] add ABI detection for RIDs (dotnet#121)
  * dotnet/android-tools@79e3b97: [JdkInfo] handle invalid XML from /usr/libexec/java_home (dotnet#120)
  * dotnet/android-tools@81519fe: Add SECURITY.md (dotnet#119)
@ghost ghost locked as resolved and limited conversation to collaborators Jun 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Area: App+Library Build Issues when building Library projects or Application projects.
Projects
None yet
2 participants