Skip to content

Using cdecl for calling native functions and providing callbacks. #205

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

Closed
wants to merge 1 commit into from
Closed

Conversation

igor-gorbunov
Copy link

This is intended to solve problem using functions from libgit2 and providing callbacks to libgit2. Should work independently of the compiler for libgit2 (msvc or mingw) and the platform of libgit2sharp (.net or mono).
Can someone run tests?

@travisbot
Copy link

This pull request passes (merged 5913c2c into 64c052e).

@yorah
Copy link
Contributor

yorah commented Aug 21, 2012

The tests are not passing as is on my workstation (win7, x86), I have a System.EntryPointNotFoundException.
This is actually the expected behavior, as the libgit2 bins used atm are compiled with the /Gz switch (the libgit2/libgit2#888 has not been applied yet).

When I apply your libgit2/libgit2#888 PR, the tests are passing on my workstation.

The tests are passing for Travis because it compiles with gcc (thus using cdecl as default).

An interesting thing I learned while looking at this, and which can be found in the Mono doc: The default CallingConvention is platform-specific. Under Windows, Winapi is the default, as this is used for most Win32 API functions. (Winapi is equivalent to Stdcall for Windows 9x and Windows NT.) Under Unix platforms, Cdecl is the default.

@igor-gorbunov
Copy link
Author

@yorah Thank you for the testing.
I think, this would be the solution for both libgit2 and libgit2sharp on the issue of using stdcall convention.

@nulltoken
Copy link
Member

@igor-gorbunov Wow, native AND managed PRs! What an awesome commitment! 👍

However, before merging this one, I'd just like to wait for the dust to settle and make sure Cdecl is indeed the recommended choice for libgit2 binaries on Windows.

I'm sure @vmg and @xpaulbettsx will soon come to a conclusion regarding this subject.

@yorah Great job as always with the tests and the explanations ❤️

@igor-gorbunov
Copy link
Author

...make sure Cdecl is indeed the ...

Sure, that must be mutual decision.

@nulltoken
Copy link
Member

@igor-gorbunov I'm afraid this PR won't be merged. Indeed, libgit2/libgit2#888 has been closed and it looks like switching to cdecl won't happen anytime soon.

Thanks for the very documented proposal, though. It was a very nice job. Looking forward for more contributions from you.

Cheers!

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