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

Use consistent name for OS on GNU/Linux. #159

Merged
6 commits merged into from Oct 2, 2011
Merged

Use consistent name for OS on GNU/Linux. #159

6 commits merged into from Oct 2, 2011

Conversation

hobophobe
Copy link
Contributor

Somehow the platform directories and the platform.rb seem to be desynched for GNU/Linux.

I assume this will fix issue #137, though the submitter of that issue didn't indicate which platform.

@terceiro
Copy link
Contributor

The OS is said to be GNU because other OSes with GNU userland (but not Linux) are also supported.

Also, I don't see where that OS constant is used for anything concrete. Can you ellaborate on why this is a problem? Do you have a concrete use case (or unit test) that fails because of this?

@hobophobe
Copy link
Contributor Author

On my system rbconfig.rb has CONFIG["host_os"] = "linux-gnu". That means platform.rb's OS = case gets set gnu. platform.rb uses that to set CONF_DIR. That means when a consumer tries to resolve a type, the FFI::TypeDefs/types.conf is sought in ffi/platform/x86_64-gnu instead of ffi/platform/x86_64-linux.

The resulting error is the same as that issue linked before, or also Debian Bug #643788.

I see your point about GNU meaning more than Linux, but I don't see type definitions for GNU here. Also, in platform.rb IS_LINUX is defined as equal to IS_GNU = is_os("gnu").

At least the OS as used in the CONF_DIR should depend on the kernel in use rather than GNU?

For example, Debian Sid: libruby1.8: downloads includes versions for Debian using a FreeBSD kernel. The rbconfig.rb distributed with those have host_os of kfreebsd-gnu, which would also look in, say, x86_64-gnu (it would otherwise have correctly matched freebsd). (Note that under the logic mentioned above, it would also mean a FreeBSD system would have true for IS_LINUX.)

Hopefully my updated commit will be clean enough to resolve the issues. Does anything rely on OS being gnu?

@terceiro
Copy link
Contributor

terceiro commented Oct 1, 2011

That makes sense - now we only need a concrete failing spec. The reason why this was not caught before is that there is not a single spec for this.

(BTW I am the Debian maintainer)

Test for GNU systems by checking whether the GNU libc is available.
Also added a spec that reproduces ffi#137.
@terceiro
Copy link
Contributor

terceiro commented Oct 1, 2011

done, I wrote a failing spec. @hobophobe, please merge my pull request on your master branch so that this pull request gets updated with the extra commit I did.

@terceiro
Copy link
Contributor

terceiro commented Oct 1, 2011

Just noticed that this changes make 5 specs fails on debian kfreebsd-amd64, so you are not quite there yet

@terceiro
Copy link
Contributor

terceiro commented Oct 1, 2011

fixed that. @hobophobe, please merge my other pull request

@terceiro
Copy link
Contributor

terceiro commented Oct 2, 2011

@wmeissner, could you please consider merging this branch into the official master branch?

@ghost
Copy link

ghost commented Oct 2, 2011

I think it looks ok - I haven't run a linux system in ages, so as long as you're happy it works on all debian platforms (and I assume all other linuxen work the same), then I'm ok with merging it.

ghost pushed a commit that referenced this pull request Oct 2, 2011
Use consistent name for OS on GNU/Linux.
@ghost ghost merged commit f30661a into ffi:master Oct 2, 2011
@ghost
Copy link

ghost commented Oct 2, 2011

btw, can you check the equivalent files in JRuby to see if they need changes?

It will have much the same platform detection logic.

@terceiro
Copy link
Contributor

terceiro commented Oct 4, 2011

Yes, I guess JRuby needs similar changes:

  • it should use LIBC_SO defined in gnu/lib-names.h instead of hardcoding libc.so.6 (not sure how you would do that in Java, though)
  • it should actually test for GNU instead of Linux when deciding the file extension for dynamic libraries

This pull request was closed.
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.

unable to resolve type 'size_t' (TypeError)
2 participants