Libgit2 dll naming #430

ben · 2013-05-13T01:14:29Z

This adds automation and marshaling junk so that two different versions of Libgit2Sharp won't have conflicting copies of git2.dll in memory at the same time. This should help with #241.

ben · 2013-05-13T01:15:30Z

I originally started with a T4 template, but it appears that the template doesn't re-build its target even on a clean build. I moved the (rather simple) content of the generated file into the powershell script, so it's totally guaranteed to be updated when you build libgit2 with that script.

dahlbyk · 2013-05-13T02:34:10Z

LibGit2Sharp/LibGit2Sharp.csproj

@@ -258,6 +259,9 @@
  <ItemGroup>
    <EmbeddedResource Include="libgit2sharp_hash.txt" />
  </ItemGroup>
+  <ItemGroup>
+    <Service Include="{508349B6-6B84-4DF5-91F0-309BEEBAD82D}" />


What is this?

It's a letfover from the T4 tryout. You're right, it should be removed.

nulltoken · 2013-05-13T09:37:44Z

@ben This looks awesome! Just a final nitpick: Travis is failing because the Mono build also seeks for a git2_b641c00 binary.

How complex would it be to tweak the build.libgit2sharp.sh script so that it also appends the short sha to the generated binaries?

/cc @cmn

nulltoken · 2013-05-13T10:36:53Z

How complex would it be to tweak the build.libgit2sharp.sh script so that it also appends the short sha to the generated binaries and debug symbols?

Hmmm. Thinking a bit about this, I don't think this would actually be easy to tackle (or even makes sense) because of the linking between the [.so|.dylib] files.

Maybe would it be easier, for now, to add a processor directive in NativeDllName.cs

#if NOT_WIN
   private const string libgit2 = "git2";
#else
   private const string libgit2 = "git2_b641c00";
#endif

and pass /property:DefineConstants="NOT_WIN" to the xbuild call from the bash script.

Thoughts?

nulltoken · 2013-05-13T12:16:37Z

An even better proposal may be to rely on the Mono provided <dllmap/> redirection mechanism. This would require to create a LibGit2Sharp.dll.config file with the following content:

<configuration> 
  <dllmap os="!windows,osx" dll="git2_b641c00" target="git2.so"/> 
  <dllmap os="osx" dll="git2_b641c00" target="git2.dylib"/> 
  <dllmap os="windows" dll="git2_b641c00" target="git2_b641c00.dll"/> 
</configuration>

Provided this is the chosen solution, UpdateLibgit2ToSha.ps1 would have to update this file as part of its job.

References:

nulltoken · 2013-05-13T12:42:40Z

Another counter proposal: Tweak the libgit2 CMakeList to make it accept an optional suffix. This would make CMake generate git2-{suffix}.[dll|so|dylib] binary, depending on the target platform.

/cc @carlosmn

nulltoken · 2013-05-13T21:03:12Z

Ok. I've pushed a counter proposal in nulltoken/vNext which relies on the NOT_WIN preprocessor constants.
This should fix the Windows side of the problem.

Can anyone please give a test at this and confirm that this fixes the issue?

/cc @xpaulbettsx @martinwoodward

anaisbetts · 2013-05-13T21:06:23Z

✨

nulltoken · 2013-05-13T21:43:01Z

So proposed plan is:

If nulltoken/libgit2sharp@7dda24368bbcf9a016179dedb5dab14fc3c77917 passes @xpaulbettsx test -> Tag it as v0.11.1 and release a new version of the NuGet package. Otherwise... Try again.
Once CMake accepts an optional suffix (@carlosmn 😉), remove the NOT_WIN hack so that Mono and .Net assemblies are the same again.

anaisbetts · 2013-05-13T22:28:19Z

Working on this now, will report back soonish

anaisbetts · 2013-05-13T22:52:24Z

No dice, here's what I did:

Fetched to latest vNext on nulltoken/libgit2sharp
Artificially bumped the version to 0.11.0.5
Built a NuGet package
Added the local dir as a source, ran Upgrade-Package in SaveAllTheTime
Reinstalled the TFS Git provider
Rigged IsTFSGitPluginInstalled to return false

Here's what shows up in the logs:

2013-05-13 15:49:17.9926 - WARN: Couldn't read status for repo: C:\Users\Paul\GitHub\SaveAllTheTime
EXCEPTION: System.Runtime.InteropServices.MarshalDirectiveException: FilePathMarshaler must be used on a FilePath.
   at LibGit2Sharp.Core.FilePathMarshaler.MarshalManagedToNative(Object managedObj)
   at System.StubHelpers.MngdRefCustomMarshaler.ConvertContentsToNative(IntPtr pMarshalState, Object& pManagedHome, IntPtr pNativeHome)
   at LibGit2Sharp.Core.NativeMethods.git_repository_open(RepositorySafeHandle& repository, FilePath path)
   at LibGit2Sharp.Core.Proxy.git_repository_open(String path)
   at LibGit2Sharp.Repository..ctor(String path, RepositoryOptions options)
   at SaveAllTheTime.Models.GitRepoOps.<>c__DisplayClass9.<GetStatus>b__8()

We're definitely loading both versions of libgit2 though:

0:039> lmvm git2_b641c00
start    end        module name
61420000 614b0000   git2_b641c00   (deferred)             
    Image path: C:\USERS\PAUL\APPDATA\LOCAL\MICROSOFT\VISUALSTUDIO\11.0\EXTENSIONS\0TX2F0B4.XSO\git2_b641c00.DLL
    Image name: git2_b641c00.DLL
    Timestamp:        Fri May 03 11:12:08 2013 (5183FDF8)
    CheckSum:         0009A76A
    ImageSize:        00090000
    File version:     0.18.0.0
    Product version:  0.18.0.0
    File flags:       0 (Mask 3F)
    File OS:          40004 NT Win32
    File type:        2.0 Dll
    File date:        00000000.00000000
    Translations:     0409.04e4
    ProductName:      libgit2
    InternalName:     git2.dll
    OriginalFilename: git2.dll
    ProductVersion:   0.18.0
    FileVersion:      0.18.0
    FileDescription:  libgit2 - the Git linkable library
    LegalCopyright:   Copyright (C) the libgit2 contributors. All rights reserved.
    Comments:         For more information visit http://libgit2.github.com/

0:039> lmvm git2
start    end        module name
66450000 664cb000   git2       (deferred)             
    Image path: C:\Users\Paul\AppData\Local\Microsoft\VisualStudio\11.0\Extensions\x1tplafe.sp2\git2.DLL
    Image name: git2.DLL
    Timestamp:        Fri Apr 19 14:05:43 2013 (5171B1A7)
    CheckSum:         000851FE
    ImageSize:        0007B000
    File version:     0.17.0.0
    Product version:  0.17.0.0
    File flags:       0 (Mask 3F)
    File OS:          40004 NT Win32
    File type:        2.0 Dll
    File date:        00000000.00000000
    Translations:     0409.04e4
    ProductName:      libgit2
    InternalName:     git2.dll
    OriginalFilename: git2.dll
    ProductVersion:   0.17.0
    FileVersion:      0.17.0
    FileDescription:  libgit2 - the Git linkable library
    LegalCopyright:   Copyright (C) the libgit2 contributors. All rights reserved.
    Comments:         For more information visit http://libgit2.github.com/

dahlbyk · 2013-05-14T00:14:29Z

Let's supplement the FilePathMarshaler error message with the expected type/assembly/location and found type/assembly/location. It feels like wires are getting crossed somehow between FilePath types from different assemblies.

anaisbetts · 2013-05-14T00:16:22Z

If anyone wants to give this a try, follow my steps on SaveAllTheTime, make sure to follow step 6 above. You also need to install the VSIX (i.e. rather than just hitting F5) because Git for VS won't be in the Experimental instance

phkelley · 2013-05-14T12:09:22Z

Paul,

Would you mind reproing that MarshalDirectiveException again and having the debugger break when that exception is thrown (sxe clr)? I would love to take a look at a full minidump.

Thanks,
Philip

anaisbetts · 2013-05-14T17:23:03Z

@phkelley Sure, though tbh it's really easy to hit, just follow the steps above and you'll have a live repro

nulltoken · 2013-05-14T22:01:22Z

UpdateLibgit2ToSha.ps1

+"@
+
+	sc -Encoding UTF8 .\Libgit2sharp\Core\NativeDllName.cs $dllNameClass
+	sc -Encoding UTF8 .\Lib\Libgit2sharp.dll.config $dllMap
 	sc -Encoding UTF8 libgit2sharp\libgit2_hash.txt $sha


Could we make this one ASCII?

I made them all ASCII. BOMs suck, man.

phkelley · 2013-05-23T15:06:03Z

Thanks Paul for providing the dump. I took a look at the issue and my initial appraisal is that it is probably a CLR bug with the construction of custom marshalers. The dump I got has the custom marshaler already created (since we are already to the point of throwing the MarshalDirectiveException), and it has the wrong MDs on it. We might entertain workarounds such as: Not using custom marshalers, or using custom marshalers that bind by string type names rather than Types. We could also see if we can get CLR to fix the issue for their next release or even for a GDR.

I will be taking a deeper look on Monday -- I am traveling this week.

phkelley · 2013-05-23T22:25:05Z

This is confirmed as a bug in the CLR and it has been filed with them.

dahlbyk · 2013-05-24T01:12:13Z

If managedObj as FilePath fails, we could fallback to use reflection to grab a Posix property and fail if there isn't one.

nulltoken · 2013-05-26T17:31:57Z

@ben Manually merged. Thanks for all your help! ❤️

anaisbetts · 2013-05-26T18:40:08Z

As I mentioned somewhere else (or maybe just in my head), we still need to fix this in libgit2sharp. Waiting on a CLR Hotfix is a non-starter, especially since non-security updates are marked as Optional in Windows Update (i.e. nobody installs them).

nulltoken · 2013-05-26T19:02:44Z

@xpaulbettsx I agree. This one has been closed because it was scoped to making LibGit2Sharp run against a non generically named git2.dll.

Main marshaling issue is still dealt with as part of #241.

ben added 4 commits May 12, 2013 15:35

Libgit2 build script now generates hash-named binaries

0fd7929

Rename binaries to new naming convention

8e2b912

Add template to update hash-name for libgit2 binaries

223d6c7

Replace T4 template with script-generated content

111a141

dahlbyk reviewed May 13, 2013
View reviewed changes

nulltoken mentioned this pull request May 13, 2013

FilePathMarshaler must be used on a FilePath #241

Closed

nulltoken mentioned this pull request May 13, 2013

Remove Tag from TreeEntryTargetType enum #433

Merged

martinwoodward mentioned this pull request May 13, 2013

Disable when Microsoft Git Plugin is installed anaisbetts/SaveAllTheTime#15

Merged

ben added 7 commits May 14, 2013 13:34

Remove T4 turd

2a38ace

Use libgit2's output-renaming

bff5baf

Generate dll config file for name mapping

50f381b

Copy dll config file to output dir (dev build)

53eae23

Share custom DLL name with [Unsafe]NativeMethods

ce98f39

Clean stale binaries

ccc4028

Use new cmake flag

22cd4f6

ben added 2 commits May 14, 2013 14:28

Upgrade libgit2 to 686c24a

d0ae056

TEMP: use cmn's fork

990ef1e

nulltoken reviewed May 14, 2013
View reviewed changes

ben added 8 commits May 14, 2013 15:08

Stop ignoring libgit2 PDBs

f165bcc

libgit2 binaries have dashes

0f80a8c

No more UTF-8 BOMs!

1966142

Quit with the dllmap file

6a7a2a4

Update libgit2 to 686c24a

eef528f

TRAAAAVIIIIISS!!

f633cec

Travis bash string indexes are 0-based

c877dd5

Turns out ALL bash string indices are 0-based

91c4ad4

nulltoken mentioned this pull request May 14, 2013

CMake: vendor strings libgit2/libgit2#1580

Merged

nulltoken closed this May 26, 2013

dahlbyk mentioned this pull request May 27, 2013

Less-strongly-typed FilePathMarshaler #437

Closed

nulltoken referenced this pull request in GitTools/GitVersion Nov 12, 2013

remove fody dependency

d980913

bording mentioned this pull request Apr 17, 2018

Avoid reading and writing global state when loading native library #1563

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Libgit2 dll naming #430

Libgit2 dll naming #430

ben commented May 13, 2013

ben commented May 13, 2013

dahlbyk May 13, 2013

nulltoken May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

anaisbetts commented May 13, 2013

nulltoken commented May 13, 2013

anaisbetts commented May 13, 2013

anaisbetts commented May 13, 2013

dahlbyk commented May 14, 2013

anaisbetts commented May 14, 2013

phkelley commented May 14, 2013

anaisbetts commented May 14, 2013

nulltoken May 14, 2013

ben May 14, 2013

phkelley commented May 23, 2013

phkelley commented May 23, 2013

dahlbyk commented May 24, 2013

nulltoken commented May 26, 2013

anaisbetts commented May 26, 2013

nulltoken commented May 26, 2013

Libgit2 dll naming #430

Libgit2 dll naming #430

Conversation

ben commented May 13, 2013

ben commented May 13, 2013

dahlbyk May 13, 2013

Choose a reason for hiding this comment

nulltoken May 13, 2013

Choose a reason for hiding this comment

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

nulltoken commented May 13, 2013

anaisbetts commented May 13, 2013

nulltoken commented May 13, 2013

anaisbetts commented May 13, 2013

anaisbetts commented May 13, 2013

dahlbyk commented May 14, 2013

anaisbetts commented May 14, 2013

phkelley commented May 14, 2013

anaisbetts commented May 14, 2013

nulltoken May 14, 2013

Choose a reason for hiding this comment

ben May 14, 2013

Choose a reason for hiding this comment

phkelley commented May 23, 2013

phkelley commented May 23, 2013

dahlbyk commented May 24, 2013

nulltoken commented May 26, 2013

anaisbetts commented May 26, 2013

nulltoken commented May 26, 2013