Skip to content
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

Add ability to disable or force system libffi and fix stdcall on Win32 #669

Merged
merged 6 commits into from Feb 18, 2019

Conversation

larskanis
Copy link
Member

This makes sure that we run tests with system libffi and with bunded libffi on Appveyor (Windows build). The specs fail on current libffi master branch due to broken stdcall to closures. This can be seen here.

The failing stdcall issue is fixed by this PR. This is why the PR enables a libffi fork temporary.

Fixes #649

larskanis and others added 6 commits January 23, 2019 22:10
This lets the user select if system or builtin libffi version shall be used.
Default is still "try system libffi and fallback to builtin".

The option can be used like so:
    gem install ffi -- --disable-system-libffi

Primary use is to run specs on builtin libffi on Windows in addition to
system libffi.
rake-10.5.0/lib/rake/application.rb:381: warning: deprecated Object#=~ is called on Proc; it always returns nil
This temporary switches to a forked libffi, until the fixing PR is merged:
  libffi/libffi#465

Use the following command to update the submodule:
  git submodule update --init --remote
PipeHelperWindows.c:15:5: warning: implicit declaration of function
'sprintf' [-Wimplicit-function-declaration]
     sprintf( name, "\\\\.\\Pipe\\pipeHelper-%u-%i",
          ^~~~~~~
@tduehr
Copy link
Member

tduehr commented Jan 25, 2019

Can you split this into multiple PRs?

@larskanis
Copy link
Member Author

@tduehr I already separated the changes into different commits but they belong semantically together. Which objections do you have to merge this?

@tduehr
Copy link
Member

tduehr commented Jan 25, 2019

The only real issue I have is using a libffi fork. Does FFI not work on windows at all without it?

@larskanis
Copy link
Member Author

larskanis commented Jan 25, 2019

The only real issue I have is using a libffi fork.

Uh, that's the simplest way to patch the upstream library. This is also common practice, that core members provide forked repositories in order to apply patches, until the particular issue has been fixed upstream. See here in rails for example. But I can move it to https://github.com/ffi/libffi/ if you like.

Does FFI not work on windows at all without it?

As mentioned above, it fails at this one test case I wrote years ago. But this test case wasn't triggered on Appveyor with system libffi in the past.

@larskanis larskanis merged commit da7b3c2 into ffi:master Feb 18, 2019
@eregon
Copy link
Collaborator

eregon commented Feb 19, 2019

FWIW, I had this in my .git/config:

[submodule "ext/ffi_c/libffi"]
	active = true
	url = https://github.com/libffi/libffi.git

I tried removing it, and rm -rf ext/ffi_c/libffi, but even after that git would still not clone from the correct repository with git submodule update --init --remote:

$ git submodule update --init  --remote
fatal: Needed a single revision
Unable to find current origin/fix-stdcall revision in submodule path 'ext/ffi_c/libffi'

$ git submodule update --init  
error: Server does not allow request for unadvertised object 024800a1331d38794c24cf191cd74b70af1078a1
Fetched in submodule path 'ext/ffi_c/libffi', but it did not contain 024800a1331d38794c24cf191cd74b70af1078a1. Direct fetching of that commit failed.

The only way I found was to:

cd ext/ffi_c/libffi
git remote set-url origin https://github.com/larskanis/libffi.git
git fetch
cd ../../..
git submodule update

Might be a git bug (I use git 2.13.6).
Submodules still looks like a half-implemented feature in git since I last used them, unfortunate.

@larskanis
Copy link
Member Author

@eregon Usually I'm the one who's called in the company when there any git issues, but git submodules are very special and don't fit well to the rest of the git commands. So I don't like them.

Nevertheless I updated two ruby-ffi repositories by git pull && git submodule update without issues. Did you try git submodule update without arguments?

@eregon
Copy link
Collaborator

eregon commented Feb 20, 2019

@larskanis Yes, I think I tried that too. Maybe it's a bad interaction with git worktree which I use on this repository to have two copies but share all git data locally.

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.

EnumDesktopWindows example crashing in ffi 1.9.25
3 participants