Merge pull request #920 from dotnet/fix919

Fix libgit2 native module load failure
dotnet · Apr 16, 2023 · 6795b35 · 6795b35
2 parents 83aa956 + 8b28a71
commit 6795b35
Show file tree

Hide file tree

Showing 10 changed files with 143 additions and 63 deletions.
diff --git a/src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs b/src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs
@@ -6,6 +6,7 @@
 using System.Text;
 using LibGit2Sharp;
 using Validation;
+using Windows.Win32;
 using Version = System.Version;
 
 #nullable enable
@@ -41,6 +42,10 @@ public static class LibGit2GitExtensions
         ContextLines = 0,
     };
 
+#if !NETCOREAPP
+    private static FreeLibrarySafeHandle? nativeLibrary;
+#endif
+
     /// <summary>
     /// Gets the number of commits in the longest single path between
     /// the specified commit and the most distant ancestor (inclusive).
@@ -111,24 +116,51 @@ public static IEnumerable<Commit> GetCommitsFromVersion(LibGit2Context context,
     /// <summary>
     /// Finds the directory that contains the appropriate native libgit2 module.
     /// </summary>
-    /// <param name="basePath">The path to the directory that contains the lib folder.</param>
+    /// <param name="basePath">The path to the directory that contains the runtimes folder.</param>
     /// <returns>Receives the directory that native binaries are expected.</returns>
     public static string? FindLibGit2NativeBinaries(string basePath)
     {
-        if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
-        {
-            return Path.Combine(basePath, "lib", "win32", IntPtr.Size == 4 ? "x86" : "x64");
-        }
-        else if (RuntimeInformation.IsOSPlatform(OSPlatform.Linux))
+        string arch = RuntimeInformation.ProcessArchitecture.ToString().ToLowerInvariant();
+
+        // TODO: learn how to detect when to use "linux-musl".
+        string? os =
+            RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? "win" :
+            RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? "linux" :
+            RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? "osx" :
+            null;
+
+        if (os is null)
         {
-            return Path.Combine(basePath, "lib", "linux", IntPtr.Size == 4 ? "x86" : "x86_64");
+            return null;
         }
-        else if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX))
+
+        string candidatePath = Path.Combine(basePath, "runtimes", $"{os}-{arch}", "native");
+        return Directory.Exists(candidatePath) ? candidatePath : null;
+    }
+
+    /// <summary>
+    /// Loads the git2 native library from the expected path so that it's available later when needed.
+    /// </summary>
+    /// <param name="basePath"><inheritdoc cref="FindLibGit2NativeBinaries(string)" path="/param" /></param>
+    /// <remarks>
+    /// This method should only be called on .NET Framework, and only loads the module when on Windows.
+    /// </remarks>
+    public static void LoadNativeBinary(string basePath)
+    {
+#if NETCOREAPP
+        throw new PlatformNotSupportedException();
+#else
+        if (nativeLibrary is null && RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
         {
-            return Path.Combine(basePath, "lib", "osx", RuntimeInformation.OSArchitecture == Architecture.Arm64 ? "arm_64" : "x86_64");
+            if (FindLibGit2NativeBinaries(basePath) is string directoryPath)
+            {
+                if (Directory.EnumerateFiles(directoryPath).FirstOrDefault() is string filePath)
+                {
+                    nativeLibrary = PInvoke.LoadLibrary(filePath);
+                }
+            }
         }
-
-        return null;
+#endif
     }
 
     /// <summary>

diff --git a/src/NerdBank.GitVersioning/ManagedGit/FileHelpers.cs b/src/NerdBank.GitVersioning/ManagedGit/FileHelpers.cs
@@ -8,8 +8,8 @@
 using System.Runtime.Versioning;
 using Microsoft.Win32.SafeHandles;
 using Windows.Win32;
+using Windows.Win32.Foundation;
 using Windows.Win32.Storage.FileSystem;
-using Windows.Win32.System.SystemServices;
 
 namespace Nerdbank.GitVersioning.ManagedGit;
 
@@ -31,7 +31,7 @@ internal static bool TryOpen(string path, out FileStream? stream)
         if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
 #endif
         {
-            SafeFileHandle? handle = PInvoke.CreateFile(path, FILE_ACCESS_FLAGS.FILE_GENERIC_READ, FILE_SHARE_MODE.FILE_SHARE_READ, lpSecurityAttributes: null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL, null);
+            SafeFileHandle? handle = PInvoke.CreateFile(path, (uint)FILE_ACCESS_RIGHTS.FILE_GENERIC_READ, FILE_SHARE_MODE.FILE_SHARE_READ, lpSecurityAttributes: null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL, null);
 
             if (!handle.IsInvalid)
             {
@@ -76,10 +76,10 @@ internal static unsafe bool TryOpen(ReadOnlySpan<char> path, [NotNullWhen(true)]
             HANDLE handle;
             fixed (char* pPath = &path[0])
             {
-                handle = PInvoke.CreateFile(pPath, FILE_ACCESS_FLAGS.FILE_GENERIC_READ, FILE_SHARE_MODE.FILE_SHARE_READ, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL, default);
+                handle = PInvoke.CreateFile(pPath, (uint)FILE_ACCESS_RIGHTS.FILE_GENERIC_READ, FILE_SHARE_MODE.FILE_SHARE_READ, null, FILE_CREATION_DISPOSITION.OPEN_EXISTING, FILE_FLAGS_AND_ATTRIBUTES.FILE_ATTRIBUTE_NORMAL, default);
             }
 
-            if (!handle.Equals(Constants.INVALID_HANDLE_VALUE))
+            if (!handle.Equals(HANDLE.INVALID_HANDLE_VALUE))
             {
                 var fileHandle = new SafeFileHandle(handle, ownsHandle: true);
                 stream = new FileStream(fileHandle, System.IO.FileAccess.Read);

diff --git a/src/NerdBank.GitVersioning/NativeMethods.txt b/src/NerdBank.GitVersioning/NativeMethods.txt
@@ -1,2 +1,4 @@
 CreateFile
+FILE_ACCESS_RIGHTS
 INVALID_HANDLE_VALUE
+LoadLibrary
diff --git a/src/NerdBank.GitVersioning/Nerdbank.GitVersioning.csproj b/src/NerdBank.GitVersioning/Nerdbank.GitVersioning.csproj
@@ -12,7 +12,7 @@
     <PackageReference Include="DotNetMDDocs" Version="0.112.39" PrivateAssets="all" Condition=" '$(GenerateMarkdownApiDocs)' == 'true' " />
     <PackageReference Include="LibGit2Sharp" Version="0.27.0-preview-0182" PrivateAssets="none" />
     <PackageReference Include="Microsoft.DotNet.PlatformAbstractions" Version="3.1.6" />
-    <PackageReference Include="Microsoft.Windows.CsWin32" Version="0.1.478-beta" PrivateAssets="all" />
+    <PackageReference Include="Microsoft.Windows.CsWin32" Version="0.2.206-beta" PrivateAssets="all" />
     <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
     <PackageReference Include="Validation" Version="2.5.51" />
     <PackageReference Include="Nerdbank.GitVersioning.LKG" Version="3.4.173-alpha" />

diff --git a/src/Nerdbank.GitVersioning.Tasks/ContextAwareTask.cs b/src/Nerdbank.GitVersioning.Tasks/ContextAwareTask.cs
@@ -11,6 +11,7 @@
 #endif
 using Microsoft.Build.Framework;
 using Microsoft.Build.Utilities;
+using Nerdbank.GitVersioning.LibGit2;
 
 namespace MSBuildExtensionTask
 {
@@ -23,10 +24,11 @@ public abstract class ContextAwareTask : Microsoft.Build.Utilities.Task
         /// <inheritdoc/>
         public override bool Execute()
         {
-#if NETCOREAPP
             string taskAssemblyPath = this.GetType().GetTypeInfo().Assembly.Location;
-
-            Assembly inContextAssembly = GitLoaderContext.Instance.LoadFromAssemblyPath(taskAssemblyPath);
+            string unmanagedBaseDirectory = Path.GetDirectoryName(Path.GetDirectoryName(taskAssemblyPath));
+#if NETCOREAPP
+            GitLoaderContext loaderContext = new(unmanagedBaseDirectory);
+            Assembly inContextAssembly = loaderContext.LoadFromAssemblyPath(taskAssemblyPath);
             Type innerTaskType = inContextAssembly.GetType(this.GetType().FullName);
             object innerTask = Activator.CreateInstance(innerTaskType);
 
@@ -55,18 +57,7 @@ public override bool Execute()
 
             return result;
 #else
-            // On .NET Framework (on Windows), we find native binaries by adding them to our PATH.
-            if (this.UnmanagedDllDirectory is not null)
-            {
-                string pathEnvVar = Environment.GetEnvironmentVariable("PATH") ?? string.Empty;
-                string[] searchPaths = pathEnvVar.Split(Path.PathSeparator);
-                if (!searchPaths.Contains(this.UnmanagedDllDirectory, StringComparer.OrdinalIgnoreCase))
-                {
-                    pathEnvVar += Path.PathSeparator + this.UnmanagedDllDirectory;
-                    Environment.SetEnvironmentVariable("PATH", pathEnvVar);
-                }
-            }
-
+            LibGit2GitExtensions.LoadNativeBinary(unmanagedBaseDirectory);
             return this.ExecuteInner();
 #endif
         }

diff --git a/src/Nerdbank.GitVersioning.Tasks/GitLoaderContext.cs b/src/Nerdbank.GitVersioning.Tasks/GitLoaderContext.cs
@@ -3,31 +3,77 @@
 
 // This code originally copied from https://github.com/dotnet/sourcelink/tree/c092238370e0437eb95722f28c79273244dc7f1a/src/Microsoft.Build.Tasks.Git
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See license information at https://github.com/dotnet/sourcelink/blob/c092238370e0437eb95722f28c79273244dc7f1a/License.txt.
+#nullable enable
+
 #if NETCOREAPP
 
 using System;
 using System.IO;
 using System.Reflection;
 using System.Runtime.InteropServices;
 using System.Runtime.Loader;
+using Nerdbank.GitVersioning.LibGit2;
 using RuntimeEnvironment = Microsoft.DotNet.PlatformAbstractions.RuntimeEnvironment;
 
 namespace Nerdbank.GitVersioning
 {
     public class GitLoaderContext : AssemblyLoadContext
     {
         public const string RuntimePath = "./runtimes";
+        private readonly string nativeDependencyBasePath;
+
+        private (string?, IntPtr) lastLoadedLibrary;
 
-        public static readonly GitLoaderContext Instance = new GitLoaderContext();
+        /// <summary>
+        /// Initializes a new instance of the <see cref="GitLoaderContext"/> class.
+        /// </summary>
+        /// <param name="nativeDependencyBasePath">The path to the directory that contains the "runtimes" folder.</param>
+        public GitLoaderContext(string nativeDependencyBasePath)
+        {
+            this.nativeDependencyBasePath = nativeDependencyBasePath;
+        }
 
         /// <inheritdoc/>
         protected override Assembly Load(AssemblyName assemblyName)
         {
-            string path = Path.Combine(Path.GetDirectoryName(typeof(GitLoaderContext).Assembly.Location), assemblyName.Name + ".dll");
+            string path = Path.Combine(Path.GetDirectoryName(typeof(GitLoaderContext).Assembly.Location)!, assemblyName.Name + ".dll");
             return File.Exists(path)
                 ? this.LoadFromAssemblyPath(path)
                 : Default.LoadFromAssemblyName(assemblyName);
         }
+
+        protected override IntPtr LoadUnmanagedDll(string unmanagedDllName)
+        {
+            IntPtr p = base.LoadUnmanagedDll(unmanagedDllName);
+
+            if (p == IntPtr.Zero)
+            {
+                if (unmanagedDllName == this.lastLoadedLibrary.Item1)
+                {
+                    return this.lastLoadedLibrary.Item2;
+                }
+
+                string prefix =
+                    RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? string.Empty :
+                    "lib";
+
+                string? extension =
+                    RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? ".dll" :
+                    RuntimeInformation.IsOSPlatform(OSPlatform.Linux) ? ".so" :
+                    RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? ".dylib" :
+                    null;
+
+                string fileName = $"{prefix}{unmanagedDllName}{extension}";
+                string? directoryPath = LibGit2GitExtensions.FindLibGit2NativeBinaries(this.nativeDependencyBasePath);
+                if (directoryPath is not null && NativeLibrary.TryLoad(Path.Combine(directoryPath, fileName), out p))
+                {
+                    // Cache this to make us a little faster next time.
+                    this.lastLoadedLibrary = (unmanagedDllName, p);
+                }
+            }
+
+            return p;
+        }
     }
 }
 #endif
diff --git a/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.Tasks.csproj b/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.Tasks.csproj
@@ -16,31 +16,6 @@
     <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
   </PropertyGroup>
 
-  <Target Name="SetNuSpecProperties" BeforeTargets="GenerateNuspec" DependsOnTargets="GetBuildVersion">
-    <PropertyGroup>
-      <LibGit2SharpNativeBinaries>$(NuGetPackageRoot)libgit2sharp.nativebinaries\$(LibGit2SharpNativeVersion)\</LibGit2SharpNativeBinaries>
-      <NuspecProperties>$(NuspecProperties);Version=$(Version);commit=$(GitCommitId);BaseOutputPath=$(OutputPath);LibGit2SharpNativeBinaries=$(LibGit2SharpNativeBinaries)</NuspecProperties>
-      <NuspecProperties Condition=" '$(PackLKG)' == 'true' ">$(NuspecProperties);LKGSuffix=.LKG</NuspecProperties>
-    </PropertyGroup>
-  </Target>
-
-  <Target Name="PackBuildOutputs" DependsOnTargets="SatelliteDllsProjectOutputGroup;DebugSymbolsProjectOutputGroup">
-    <PropertyGroup>
-      <BuildSubDir Condition=" '$(TargetFramework)' == 'net6.0' ">MSBuildCore\</BuildSubDir>
-      <BuildSubDir Condition=" '$(TargetFramework)' == 'net462' ">MSBuildFull\</BuildSubDir>
-    </PropertyGroup>
-    <Error Text="Unrecognized TargetFramework" Condition=" '$(BuildSubDir)' == '' " />
-    <ItemGroup>
-      <TfmSpecificPackageFile Include="&#xD;&#xA;                        $(OutputPath)LibGit2Sharp.dll*;&#xD;&#xA;                        $(OutputPath)MSBuildExtensionTask.dll;&#xD;&#xA;                        $(OutputPath)Microsoft.DotNet.PlatformAbstractions.dll;&#xD;&#xA;                        $(OutputPath)Nerdbank.GitVersioning.*dll;&#xD;&#xA;                        $(OutputPath)Newtonsoft.Json.dll;&#xD;&#xA;                        $(OutputPath)System.Text.Json.dll;&#xD;&#xA;                        $(OutputPath)System.Runtime.CompilerServices.Unsafe.dll;&#xD;&#xA;                        $(OutputPath)Validation.dll;&#xD;&#xA;               ">
-        <PackagePath>build\$(BuildSubDir)</PackagePath>
-      </TfmSpecificPackageFile>
-      <!-- Package up the libgit2 native binaries -->
-      <TfmSpecificPackageFile Include="@(ContentWithTargetPath)" Condition=" '%(ContentWithTargetPath.CopyToOutputDirectory)' == 'PreserveNewest' ">
-        <PackagePath>build\$(BuildSubDir)%(ContentWithTargetPath.TargetPath)</PackagePath>
-      </TfmSpecificPackageFile>
-    </ItemGroup>
-  </Target>
-
   <ItemGroup>
     <None Include="build\**">
       <Pack>true</Pack>
@@ -81,4 +56,6 @@
   <ItemGroup>
     <Compile Include="..\Shared\**\*.cs" LinkBase="Shared" />
   </ItemGroup>
+
+  <Import Project="$(MSBuildProjectName).targets"/>
 </Project>
diff --git a/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.Tasks.targets b/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.Tasks.targets
@@ -0,0 +1,35 @@
+<Project>
+  <Target Name="SetNuSpecProperties" BeforeTargets="GenerateNuspec" DependsOnTargets="GetBuildVersion">
+    <PropertyGroup>
+      <LibGit2SharpNativeBinaries>$(NuGetPackageRoot)libgit2sharp.nativebinaries\$(LibGit2SharpNativeVersion)\</LibGit2SharpNativeBinaries>
+      <NuspecProperties>$(NuspecProperties);Version=$(Version);commit=$(GitCommitId);BaseOutputPath=$(OutputPath);LibGit2SharpNativeBinaries=$(LibGit2SharpNativeBinaries)</NuspecProperties>
+      <NuspecProperties Condition=" '$(PackLKG)' == 'true' ">$(NuspecProperties);LKGSuffix=.LKG</NuspecProperties>
+    </PropertyGroup>
+  </Target>
+
+  <Target Name="PackBuildOutputs" DependsOnTargets="SatelliteDllsProjectOutputGroup;DebugSymbolsProjectOutputGroup">
+    <PropertyGroup>
+      <BuildSubDir Condition=" '$(TargetFramework)' == 'net6.0' ">MSBuildCore\</BuildSubDir>
+      <BuildSubDir Condition=" '$(TargetFramework)' == 'net462' ">MSBuildFull\</BuildSubDir>
+    </PropertyGroup>
+    <Error Text="Unrecognized TargetFramework" Condition=" '$(BuildSubDir)' == '' " />
+    <ItemGroup>
+      <TfmSpecificPackageFile Include="
+                        $(OutputPath)LibGit2Sharp.dll*;
+                        $(OutputPath)MSBuildExtensionTask.dll;
+                        $(OutputPath)Microsoft.DotNet.PlatformAbstractions.dll;
+                        $(OutputPath)Nerdbank.GitVersioning.*dll;
+                        $(OutputPath)Newtonsoft.Json.dll;
+                        $(OutputPath)System.Text.Json.dll;
+                        $(OutputPath)System.Runtime.CompilerServices.Unsafe.dll;
+                        $(OutputPath)Validation.dll;
+               ">
+        <PackagePath>build\$(BuildSubDir)</PackagePath>
+      </TfmSpecificPackageFile>
+      <!-- Package up the libgit2 native binaries -->
+      <TfmSpecificPackageFile Include="@(ContentWithTargetPath)" Condition=" '%(ContentWithTargetPath.CopyToOutputDirectory)' == 'PreserveNewest' ">
+        <PackagePath>build\$(BuildSubDir)%(ContentWithTargetPath.TargetPath)</PackagePath>
+      </TfmSpecificPackageFile>
+    </ItemGroup>
+  </Target>
+</Project>
diff --git a/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.nuspec b/src/Nerdbank.GitVersioning.Tasks/Nerdbank.GitVersioning.nuspec
@@ -36,10 +36,6 @@ IMPORTANT: The 3.x release may produce a different version height than prior maj
     <file src="$LibGit2SharpNativeBinaries$runtimes\osx-arm64\native\libgit2-b7bad55.dylib" target="build\MSBuildFull\lib\osx-arm64\libgit2-b7bad55.dylib" />
     <file src="$LibGit2SharpNativeBinaries$runtimes\linux-x64\native\libgit2-b7bad55.so" target="build\MSBuildFull\lib\linux-x64\libgit2-b7bad55.so" />
 
-    <!-- Additional copies to work with our own ps1 scripts (https://github.com/dotnet/Nerdbank.GitVersioning/issues/248) -->
-    <file src="$LibGit2SharpNativeBinaries$runtimes\win-x64\native\git2-b7bad55.dll" target="build\MSBuildFull\lib\win32\x64\git2-b7bad55.dll" />
-    <file src="$LibGit2SharpNativeBinaries$runtimes\win-x86\native\git2-b7bad55.dll" target="build\MSBuildFull\lib\win32\x86\git2-b7bad55.dll" />
-
     <file src="$LibGit2SharpNativeBinaries$libgit2\LibGit2Sharp.dll.config" target="build\MSBuildCore\LibGit2Sharp.dll.config" />
     <file src="$BaseOutputPath$net6.0\LibGit2Sharp.dll" target="build\MSBuildCore\LibGit2Sharp.dll" />
     <file src="$BaseOutputPath$net6.0\Microsoft.DotNet.PlatformAbstractions.dll" target="build\MSBuildCore\Microsoft.DotNet.PlatformAbstractions.dll" />

diff --git a/src/nbgv/Program.cs b/src/nbgv/Program.cs
@@ -79,7 +79,8 @@ public static int Main(string[] args)
         {
             string thisAssemblyPath = typeof(Program).GetTypeInfo().Assembly.Location;
 
-            Assembly inContextAssembly = GitLoaderContext.Instance.LoadFromAssemblyPath(thisAssemblyPath);
+            GitLoaderContext loaderContext = new(Path.GetDirectoryName(thisAssemblyPath));
+            Assembly inContextAssembly = loaderContext.LoadFromAssemblyPath(thisAssemblyPath);
             Type innerProgramType = inContextAssembly.GetType(typeof(Program).FullName);
             object innerProgram = Activator.CreateInstance(innerProgramType);