-
Notifications
You must be signed in to change notification settings - Fork 8
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
Remove attempt to link xml_ruby in extconf.rb. #11
base: master
Are you sure you want to change the base?
Conversation
@jc00ke I'm sorry, but what makes you think we could be of any help? AFAIK, project collaborators are @cfis and possibly @dreamcat4 (see here and here). |
My bad. I went to the collaborators page and your names were listed as the top contributors. My apologies for misunderstanding the Github ui. |
No worries. I find it irritating myself that there doesn't seem to be a place where the actual project collaborators, i.e. those with commit access, are listed. Good luck, though. |
So libxslt-ruby does link to libxml-ruby...except on OSX. So I think that check is necessary. |
Could you show me where/why it needs that link? I've installed libxslt-ruby successfully on a non-OSX system (debian wheezy) without this check -- it seems to only need |
The library uses the libxml-ruby shared library for access to the document object. So it needs access to the header files to compile, and the library to run. |
But it doesn't need the shared library to compile. libxslt-ruby does also run successfully (tests pass, and works within a rails project) on debian wheezy + squeeze without the library linking step in extconf. |
@sshao I think I have an idea with what's happening, but I can't confirm it at the moment. I have a potential compromise though: would it work to add to the conditional on lines 133/134 to skip that section of code for Rbx like is done for OS X? |
Mostly passing Travis build of this branch, for several rubies. (The rbx-2 build fails right now because of a bug in its implementation of Travis CI VMs are "based on Ubuntu 12.04 LTS Server Edition 64 bit". |
@tekwiz That compromise would definitely work for us! I'm just still uncertain that this linking is needed during compilation at all, given that the installs/tests I've tried have been working without it. |
Ok, let's do something along the lines of what Travis is saying. But it should be to include that step on platforms that don't allow unfufilled references when compiling - ie, Windows. |
This reverts commit 47407d0.
Conflicts: .travis.yml Conflict was that master .travis.yml only contained 1.8.7 in the rvm list, and remove-xml-ruby-link .travis.yml added more rubies in.
should not assume that date has already been loaded
@cfis @tekwiz I implemented Travis' suggestion in ba15f86, with a fully passing Travis build. |
Proposing that extconf.rb not link xml_ruby at all, as it doesn't seem to use any of the xml_ruby libraries when compiling. Instead rely on existing checks to ensure that libxml-ruby is installed, the existing checks being: 1) the search for libxml-ruby's installation path in extconf.rb, and 2) the .gemspec dependency upon libxml-ruby.
(We'd also like to remove this section of the extconf.rb because it prevents libxml-ruby from building on Rubinius 1.3.3, due to the linker throwing "undefined reference" errors at this stage. Rubinius does not generate any library/shared object files to link against.)
This is tested on Debian Wheezy (libxml 2.8.0, libxml 1.1.26) with both ruby MRI 1.8.7-p358 and rubinius 1.3.3.