Skip to content

Allow custom DLL names #17

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

Merged
merged 4 commits into from
Jul 17, 2015
Merged

Allow custom DLL names #17

merged 4 commits into from
Jul 17, 2015

Conversation

ethomson
Copy link
Member

Like in the pre-NuGet native binary days, we require a way to have a custom libgit2 filename, for example git2-visualstudio.dll. This updates the build and update scripts to include support for this, as well as the ability to then output a custom NuGet package suffix so that we do not confuse our binary packages with the official ones.

This also adds a README.md describing how to work with this system.

ethomson added 2 commits July 15, 2015 18:30
Allow custom shared library names (eg, `git2-mycompany.dll` instead
of `git2-hash.dll`) to allow a single process to potentially load
multiple instances of libgit2.
Not everybody has `.` in their path, and we include `Nuget.exe`
in the build package.  Use that explicitly.
@ethomson
Copy link
Member Author

Oh, doh, @nulltoken - I didn't realize that you had a README in another PR. Let me know if you want me to drop that commit and we can figure something else out.

@bording
Copy link
Member

bording commented Jul 16, 2015

Since the changes in libgit2/libgit2sharp#1157 require libgit2_filename.txt to exist in the package, this file also needs to be commited with the standard naming scheme.

Other than that, I think this looks good. I don't see anything in this that would break on the mono side of things when using a custom name. Did you by any chance test that to confirm? If not, I can double check that.

@Therzok
Copy link
Member

Therzok commented Jul 16, 2015

@bording as long as the dllimport's name is regenerated, nothing will break on mono side.

@bording
Copy link
Member

bording commented Jul 16, 2015

@Therzok My comment was more about making sure the custom name was actually propagated out to the appropriate places for mono to see it, for example the LibGit2Sharp.dll.config file, and it looks like that is all being handled correctly.

LibGit2Sharp must know the custom DLL name to be able to load it.
@ethomson
Copy link
Member Author

Since the changes in libgit2/libgit2sharp#1157 require libgit2_filename.txt to exist in the package, this file also needs to be commited with the standard naming scheme.

Makes sense. Fixed that up. Thanks.

If you need to build your own native binaries for some reason, you can
do so easily with the scripts in this repository:

1. Clone the LibGit2Sharp repository. Do so recursively to ensure that
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clone the LibGit2Sharp.NativeBinaries repository

@nulltoken
Copy link
Member

Oh, doh, @nulltoken - I didn't realize that you had a README in another PR. Let me know if you want me to drop that commit and we can figure something else out.

Are you kidding me? This is an amazing README! Of course, you keep it.

However, I wouldn't mind some 👀 on #5 😉

@@ -38,4 +38,4 @@ if ( -Not (Test-Path $linuxDirectory\*.so) )
Set-Content $linuxDirectory\addbinaries.here $null
}

Nuget.exe Pack nuget.package\NativeBinaries.nuspec -Version $version$versionSuffix -NoPackageAnalysis
.\Nuget.exe Pack nuget.package\NativeBinaries.nuspec -Version $version$versionSuffix -NoPackageAnalysis
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Disclaimer: I'm not a PowerShell wizard. But doesn't the dot-slash notation notation means "here"? Would that be the case, how can we ensure that there's always a NuGet next to this file?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this. I forgot that we actually embed NuGet.exe.

@ethomson
Copy link
Member Author

@nulltoken I made some changes, but I didn't broadly include #5 yet. There's some very good stuff in there and we should merge the two documents for sure, but I didn't do that as part of this PR.

nulltoken added a commit that referenced this pull request Jul 17, 2015
@nulltoken nulltoken merged commit 5f5e53b into master Jul 17, 2015
@nulltoken nulltoken deleted the custom_dllname branch July 17, 2015 17:14
@nulltoken
Copy link
Member

@ethomson Neat!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants