Please consider adding AOT support for .Diff.Compare<T>()

[darkthread]

When utilizing libgit2sharp in combination with the .NET 8 PublishAot feature, all repository creation, file staging and committing operations are successful, however, the git diff function throws a NullReferenceException.

Is there a way to get this working under AOT?

Steps To Reproduce:

1. Create a .NET 8 console project

   dotnet new console -o aot-test
   cd aot-test

2. Adjust aot-test.csproj to include libgit2sharp reference and activate PublishAot feature

   <Project Sdk="Microsoft.NET.Sdk">
   
     <PropertyGroup>
   	<OutputType>Exe</OutputType>
   	<TargetFramework>net8.0</TargetFramework>
   	<RootNamespace>aot_test</RootNamespace>
   	<ImplicitUsings>enable</ImplicitUsings>
   	<Nullable>enable</Nullable>
   	<PublishAot>true</PublishAot>
     </PropertyGroup>
   
     <ItemGroup>
   	<PackageReference Include="libgit2sharp" Version="0.29.0" />
     </ItemGroup>
   
   </Project>

3. Insert the following code into Progmram.cs:

using LibGit2Sharp;

var path = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
Directory.CreateDirectory(path);
Repository.Init(path, false);
var userName = "Tester";
var email = "tester@mail.net";
Func<Signature> getSignature = () => new Signature(userName, email, DateTimeOffset.Now);
using (var repo = new Repository(path))
{
    var fileName = "test.txt";
    var filePath = Path.Combine(path, fileName);
    File.WriteAllText(filePath, "Hello, World!");
    Commands.Stage(repo, fileName);
    var commit1 = repo.Commit("Initial commit", getSignature(), getSignature());
    File.AppendAllText(filePath, "\nAppended Line");
    Commands.Stage(repo, fileName);
    var commit2 = repo.Commit("Second commit", getSignature(), getSignature());
    Console.WriteLine($"Commits Count {repo.Commits.Count()}");
    var changes = repo.Diff.Compare<Patch>(commit1.Tree, commit2.Tree);
    Console.WriteLine($"Diff Changes Count = {changes.Count()}");
}

4. Use dotnet run for testing, it will display:

Commits Count 2
Diff Changes Count = 1

5. Use dotnet publish, .\bin\Release\net8.0\win-x64\publish\aot-test.exe to test the native AOT version, it throws an exception on repo.Diff.Compare<Patch>():

Commits Count 2
Unhandled Exception: System.NullReferenceException: Object reference not set to an instance of an object.
   at aot-test!<BaseAddress>+0x19af4c
   at aot-test!<BaseAddress>+0x199437

Current Behavior:

The native AOT functions for repository creation, file staging, and committing operations are functional, but the git diff function is not.

Anticipated Behavior:

Please consider adding AOT support for the git diff API.

Version of LibGit2Sharp (release number or SHA1)

0.29.0

Operating system(s) tested; .NET runtime tested

• OS: Windows 11
• .NET: 8.0.100

[igiona]

Interesting that you're looking into AOT these days, since I'm on it as well 😄

Unfortunately, not all operations are successful and it's not only the git-diff that doesn't work.
Also the simple Discover functionality is broken:

            Console.WriteLine("Hello, World!");

            var repo = Repository.Discover(@"C:\git\a-repo");

            if (repo == null)
            {
                Console.WriteLine("Doh! Repo not found");
            }
            else
            {
                Console.WriteLine("Found repo at {0}", repo);
            }

I tried to make the library AOTable igiona#1 but it doesn't seem to help solving my problem.
Later I will try to see if it does solves yours.

The Discover issue boils down to a marshalling problem with the native library in:

        [DllImport(libgit2, CallingConvention = CallingConvention.Cdecl)]
        internal static extern int git_repository_discover(
            GitBuf buf,
            [MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictFilePathMarshaler))] FilePath start_path,
            [MarshalAs(UnmanagedType.Bool)] bool across_fs,
            [MarshalAs(UnmanagedType.CustomMarshaler, MarshalCookie = UniqueId.UniqueIdentifier, MarshalTypeRef = typeof(StrictFilePathMarshaler))] FilePath ceiling_dirs);

basically buf returns "null" at some point...

[ethomson]

Repository.Discover returns null when there was no repository at that path. It works when compiled to a normal assembly, but not when compiled to AOT?

Does AOT do something funny with the filesystem?

[darkthread]

@ethomson It's interesting to me that add, commit and log can work in AOT. I have no idea why diff throws an exception.

[igiona]

Repository.Discover returns null when there was no repository at that path. It works when compiled to a normal assembly, but not when compiled to AOT?

Does AOT do something funny with the filesystem?

Correct, it just doesn't work in AOT.
To narrow down the problem even further, I went on a even simpler call: git_config_find_global .

I exposed it nastily to my Program.cs:

Configuration.cs

public static string TestMe() => Proxy.git_config_find_global()?.ToString() ?? "Was null";

Program.cs

   Console.WriteLine("=> {0}", Configuration.TestMe());

=> Was null in AOT
my-user-home\.gitconfig non-AOT

Also this call accesses the file-system, weather this is the relation though is hard to say for me.
What's interesting is that the native call returns actually a valid result (0=Ok), but a "null" buffer pointer:

I added some debugging information in ConvertPath

        private static FilePath ConvertPath(Func<GitBuf, int> pathRetriever)
        {
            using (var buf = new GitBuf())
            {
                int result = pathRetriever(buf);
                Console.WriteLine($"=> result {result}");
                Console.WriteLine($"=> ptr {buf.ptr:x}");
                if (result == (int)GitErrorCode.NotFound)
                {
                    return null;
                }

                Ensure.ZeroResult(result);
                return LaxFilePathMarshaler.FromNative(buf.ptr);
            }
        }

AOT

=> result 0
=> ptr 0

[igiona]

@ethomson It's interesting to me that add, commit and log can work in AOT. I have no idea why diff throws an exception.

I still get the null exception even in the AOT "friendly" branch.

[darkthread]

I guess I found the reason why AOT version doesn't work. The problem is more complicated than I thought. According to the documentation:

The Native AOT application model addresses issues with dynamic code generation by precompiling all code ahead of time directly into native code.
The P/Invoke source generator, included with the .NET 7 SDK and enabled by default, looks for LibraryImportAttribute on a static and partial method to trigger compile-time source generation of marshalling code, removing the need for the generation of an IL stub at run time and allowing the P/Invoke to be inlined.

In other words, take 'git_config_find_global' as an example, we need to change [DllImport] part to internal static partial int git_config_find_global, and then redefine a set of [LibraryImport] internal static partial int git_config_find_global. Also, we need to define [CustomMarshaller(typeof(GitBuf), MarshalMode.Default, typeof(GitBufMarshaller))] internal static unsafe class GitBufMarshaller to handle parameters marshaing.

• Source generation for platform invokes
• Tutorial: Use custom marshallers in source-generated P/Invokes

I would like to give it a try, but dealing with Unmanaged code is beyond my capabilities.

[bording]

I did take a look at this a bit this weekend, and I'm not sure that the issue is solely a DllImport vs. LibraryImport thing. There is enough evidence around that at least some forms of DllImport should just work as part of AOT. For example, this article exists and doesn't mention needing to use the source-generated P/Invokes.

It also seems a bit strange to me that there are analyzers that that point out other problematic code for AOT, but don't flag DllImport as a problem. Seems like they would if that was required.

That being said, I do think the source of the problem is related to the non-trivial uses of DllImport in this library that you don't see in a lot of other interop scenarios. Moving to LibraryImport might address that, but I do recall that the last time I looked into using them, they didn't support all the things required by the existing DllImport calls in here.

It would be pretty easy to get LibGit2Sharp fully compatible with trimming and single-file deployment, but getting all the way to AOT is likely going to be a lot of tricky work, if it's possible at all currently.

[bording]

Found a couple more issues on the dotnet repo that likely explain a bit more what's going on here. Currently it looks like warnings for AOT-problematic P/Invokes via DllImport are incomplete: dotnet/runtime#74697

It also sounds like one benefit of moving to the source generator imports would be that you would actually get warnings for P/Invokes that aren't compatible with AOT.

However, even if those places were properly identified, it's not clear to me yet if there would even be a way to change LibGit2Sharp to accommodate the AOT limitations. It's quite possible that a change on the libgit2 side would be required.

[ethomson]

What would we need to do on the libgit2 side?

[bording]

What would we need to do on the libgit2 side?

I'm just theorizing here, but if it turns out there was a libgit2 API pattern that there was no way to adjust the P/Invoke for to make it compatible with AOT, then the only way to solve it currently would be to modify the libgit2 API. Otherwise, we'd be waiting for Microsoft to change something in their AOT stack to make it work.

I don't yet know if there is anything like that, though. It may turn out that every DllImport can be changed to work with AOT. The main problem at this point is there is nothing pointing to the problematic ones or warnings to tell me what part of it is not compatible with AOT.

[ethomson]

I'm just theorizing here, but if it turns out there was a libgit2 API pattern that there was no way to adjust the P/Invoke for to make it compatible with AOT, then the only way to solve it currently would be to modify the libgit2 API.

Got it. Happy to work on this if we need to. I think that we could also figure something out here where we build a shim over libgit2 to not break API compatibility but mutate it into something AOT can handle. 🤷

[igiona]

Having this compatible with AOT would be a huge benefit for tools using libgit2sharp, like GitVersion.
Finally there would be a tool which can handle semantic versioning without hassle and dependency to the .NET runtime.

I'm happy to help here, but source generators and unmanaged code are a new field for me

[bording]

Finally there would be a tool which can handle semantic versioning without hassle and dependency to the .NET runtime.

That could already be achieved through publishing a trimmed self-contained application. AOT isn't required for that.

[igiona]

Finally there would be a tool which can handle semantic versioning without hassle and dependency to the .NET runtime.

That could already be achieved through publishing a trimmed self-contained application. AOT isn't required for that.

That's an interesting approach indeed, I gave it a quick try on GitVersion targeting linux-x64:

self-contained, master untrimmed : 75.5 MB (224 files)
self-contained, master trimmed: 26.8 MB (86 files)
self-contained, MakeLibTrimFriendly trimmed: 28.1 MB (95files)
AOT, CliAot : 20.0MB (2 files => I removed the .pdb)

I tested it by simply run the binary with default settings on a "normal" repo.

• Both untrimmed (master branch) and the AOT-hacked (https://github.com/igiona/GitVersion/tree/CliAot) solutions give the same expected results ).

• The trimmed version on the master runs but fails due to reflection issues on master.
  That's why I've got a trial version which works (for me) but still produces a ton of trimming warnings while building (https://github.com/igiona/GitVersion/commits/MakeLibTrimFriendly/). I guess that some functionalities are going to be broken.

A part of the startup time which is remarkably faster on AOT, self-contained + trimming seems like a valuable and somewhat easier solution.
Although addressing all the trimming warnings will likely be quite a hassle as well...

[bording]

Although addressing all the trimming warnings will likely bit quite a hassle as well...

I've already fixed them all locally while investigating the AOT stuff this weekend. They were actually nothing too bad. I'll look into pushing them up and getting them out soon.

A part of the startup time which is remarkably faster on AOT

Yes, startup time is one thing that AOT would improve.

[igiona]

Although addressing all the trimming warnings will likely bit quite a hassle as well...

I've already fixed them all locally while investigating the AOT stuff this weekend. They were actually nothing too bad. I'll look into pushing them up and getting them out soon.

A part of the startup time which is remarkably faster on AOT

Yes, startup time is one thing that AOT would improve.

That's nice!
I guess you refer libgit2sharp only? My comment was more on the GitVersion tool side :)

[bording]

Yes, I meant for LibGit2Sharp, which is all that is relevant here. 😄

But yes, all trimming errors would need to be addressed for whatever program you're trying to trim. AOT uses trimming as well, so the first prerequisite is to address all trimming problems.

[bording]

I just released LibGit2Sharp 0.30.0, which includes #2084, so it should be fully compatible with trimming and single file publishing now.

I also started looking into what is involved in converting from DllImport to LibraryImport, and yeah that's going to take some work to get done.

For starters, LibraryImport is only available on .NET 7+, so that's going to mean I'll need to add a net8.0 target to the project to be able to use it. That also means the DllImport version of the code has to stay to keep supporting net472 and net6.0, which isn't ideal.

In some ways, switching to LibraryImport simplifies things, for example how string marshalling is done. There is a now a UTF-8 string marshalling class built in, so I can use that instead of needing a custom marshaller.

However, there are some new requirements around the types involved in the P/Invoke definitions, and I suspect those are what are tripping up using AOT right now.

Effectively, a lot of the existing interop types defined in LibGit2Sharp aren't blittable and require runtime marshalling to actually work. LibraryImport requires everything to either be a blittable type, or a struct made up of those types since the runtime marshalling doesn't exist for them.

Lots of the interop types are defined as classes right now, and those need to be redefined as structs (along with other changes) to be able to work. However, making those changes has impacts on the higher layers of the code that also have to be understood and adjusted.

I'm going to keep working on it, but no promises as to when I'll be able get it all done!

For anyone interested in the details of the changes, I've pushed up my spike branch: https://github.com/libgit2/libgit2sharp/tree/libraryimport