-
Notifications
You must be signed in to change notification settings - Fork 189
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
Need to specify iconv location to compile. #11
Comments
Would this have worked? $ gem install tiny_tds -- --with-libiconv-prefix=/some/dir |
I just did this commit too. |
Trying
did not work for me, with or without the new fix. I should have mentioned that with the fix I did have to install the gem with the following command, which is similar to what you suggested, but this command didn't work before the fix.
Thanks for the quick response! |
OK, good to know. Thanks too! |
This breaks compilation on Debian (and presumably anything using glibc), which includes iconv in libc (so gcc bails when you try to link iconv). |
@michaelkirk Iconv is important for FreeTDS to compile against. Can you help me find some middle ground and proper way to structure extconf.rb? We recently added that for the future cross compilation on Windows to work which is also important for compiling locally. Dose something like this not work for you?
Or maybe I should make the iconv optional in some fashion. Not sure. I am very curious to find out how FreeTDS is installed on your system now. If you do As a note to myself, I may go back to using iconv as the compile locally option, give a usage above to specify for @JuliaDana but ultimately not stop the installation process. To that end, here is a code snippet for me.
|
Or maybe this?
|
@metaskills I'll happilly work with you to get this resolved. Disclaimer: most of this stuff I figured out last night while trying to get tiny_tds to work, my opinion on what's happening should probably be taken with a grain of salt. I don't think the --with-iconv-dir=/some/dir will work, the problem is that there is no "iconv lib" on glibc systems (most anything linux). Instead, it's included as part of the basic c library, thus no explicit linking is required. Since there is no separate iconv lib, trying to link to it will break the compile process.
It's worth noting that there is an iconv header file. For me it lives at /usr/include/iconv.h, and is part of the libc6-dev package. I'll get back to you about |
re: I've just tried installing the 0.4.1 version of the gem, but to no avail. Here's the command line generated:
notice it's trying to link a library called "false" If you'd like, before you publish a new version, I can test it on my environment. |
I'll do that, give me a few to redo this iconv thing. |
OK 0.4.2 is out, let me know if that fixes it. @JuliaDana, if you need to specify iconv, you should be able to pass options down like this.
|
succeeded! Thanks On Thu, Mar 24, 2011 at 11:10 AM, metaskills <
|
w00t |
I just came across this error but from a different angle. I'm on cygwin (1.7). When running I confirmed that libiconv was installed, but I couldn't work out why the gem wasn't linking against it. Once I found the line in extconf What was really confusing was I didn't have this problem on another cygwin machine. After hunting around I discovered that if you have libiconv previously installed (which I did on one machine and not the other) when you build freetds it will be compiled with iconv without you having to do anything (if you don't have libiconv it will compile fine too). But if your freetds was compiled with iconv, then the only way to get tiny_tds to install is to pass the enable_iconv argument. I think that the README should include some text explaining that if you are having problems installing, and you suspect that your freetds was compiled with iconv then you will need to pass the enable-iconv argument. It should also show an example since a lot of windows devs won't be familiar with how to pass args to the extension builder from gem install. |
Interesting, but TinyTDS has native gems for Windows precompiled so no "need" for anyone Windows to download and compile FreeTDS or libiconv since it is all statically linked in the x86-mingw32 platform gem. https://rubygems.org/gems/tiny_tds/versions I'm not Windows specialist, but I believe cygwin is nothing akin to mingw32 and hence you would not get that by default, but wouldn't it still work if you explicitly installed that platform version? Much unknowns to me on that topic? @luislavena any opinions? @asdavey, can you work up what I should include in the README or something? |
I believe in the case of cygwin, it should be used like POSIX, meaning: install libiconv and install FreeTDS linking to libiconv, and then install tiny_tds, the need for Cygwin has a package manager so installing libiconv shouldn't be complicated, then compile FreeTDS should be able to detect iconv (and if not, you can force that). AFAIK, haven't used cygwin in many years but that will be the right behavior. |
@luislavena there is a good chance that I'm doing something wrong given I don't have a great deal of experience in getting ruby extensions to compilet (my 3 years ruby experience has all been in the cozy confines of jruby). But if you don't supply the On my machine, cygwin has libiconv installed as a package, and when I built freetds it picked up libiconv without any configuration changes (I assume that But tiny_tds would not install. Running This portion of output from make might help diagnose whats going on: gcc -shared -s -o tiny_tds.so client.o result.o tiny_tds_ext.o -L. -L/home/Andrew/.rvm/rubies/ruby-1.9.2-p290/lib -L/usr/local/lib -L. -L/usr/local/lib -Wl,--enable-auto-image-base,--enable-auto-import -lruby191 -lsybdb -lpthread -lrt -ldl -lcrypt
/usr/local/lib/libsybdb.a(iconv.o): In function `tds_set_iconv_name':
/home/Andrew/sdk/freetds-0.91/src/tds/iconv.c:206: undefined reference to `_libiconv_open' I was surprised that the error message lists the location of the source code that was used to build freetds (this probably just highlights my lack of compiling C stuff experience). I guess I was thinking the error would have been in the source of the extension, not because some third-party binary can't find one of it's dependencies. Is there some sort of dynamic link going on here vs static link? @metaskills I'll get something for you to look at shortly, assuming that @luislavena doesn't point out that I did something stupid. |
Seems you're right, without I believe a warning/note should be added to the README so others do not suffer the same issue you had. |
@metaskills, the update to the README could read (as the last paragraph of the Install section):
But I was wondering if it is better to just fix the build process so that it detects if libiconv is available, and if so, links against it. So I made some small modifications to
I believe that the modifications should still support the scenarios raised by @JuliaDana (you can still tell extconf where to find libiconv) and @michaelkirk (if libiconv is not on the system the compile or glibc is being used instead then the compile will still work). Though full disclaimer, I'm pretty new to all this compiling C code business. Here are the changes. In summary, I've removed iconv being conditionally added to diff --git a/ext/tiny_tds/extconf.rb b/ext/tiny_tds/extconf.rb
index 9aa6cc5..817622c 100644
--- a/ext/tiny_tds/extconf.rb
+++ b/ext/tiny_tds/extconf.rb
@@ -1,10 +1,9 @@
require 'mkmf'
FREETDS_LIBRARIES = ['sybdb']
-FREETDS_LIBRARIES.unshift 'iconv' if enable_config('iconv')
FREETDS_HEADERS = ['sybfront.h', 'sybdb.h']
-dir_config('iconv') if enable_config('iconv')
+dir_config('iconv')
dir_config('freetds')
def root_paths
@@ -70,6 +69,8 @@ def have_freetds?
find_freetds_libraries_path && find_freetds_include_path
end
+have_library("iconv")
+
if enable_config("lookup", true)
unless have_freetds?
abort "-----\nCan not find FreeTDS's db-lib or include directory.\n-----"
@@ -79,6 +80,6 @@ else
unless have_freetds_libraries?(*FREETDS_LIBRARIES) && have_freetds_headers?(*FREETDS_HEADERS)
abort "-----\nCan not find FreeTDS's db-lib or include directory.\n-----"
end
-end
+end
create_makefile('tiny_tds/tiny_tds')
|
Wow, this is a lot to consume, really good information too! I had always assumed that if FreeTDS was compiled and found libiconv that it would negate TinyTDS needing to do the same. But after thinking about this, seems the later makes sense too. I am going to apply the diff and see if we can get some people to test. Repoened the issue too. |
I like the patch to the build too. Confirmed that this works well on both local rake compile as well as during a gem install without miniportile. I think I'll release a 0.5.1.rc1 to see how it goes. |
I had to do this commit too so cross compile works. Basically we have to put iconv in the standard libraries we need and take out that root |
OK, I have done a 0.5.1.rc1 release. Would love to know how it goes. Please let me know. |
I was able to test this today on my cygwin box and everything worked as expected. Later I'll be able to do the mingw and ubuntu tests. Out of curiosity why does extconf.rb have the exotic path search vs using stock standard 'have_library' calls? (not trying to sound snarky - I am genuinely interested). |
No worries. I copied that path first search loop concept from other native gems like nokogiri, mysql2, etc. It turns out to be a win for us because when we are running our local tests or building native gems using miniportile, the path is altered so the local downloaded libraries have precedence. I suspect this process plays nicely with other build scenarios too. |
Hi, unluckily adding back iconv in the list of required freetds libraries breaks compilation on Linux again :-). I'd suggest, because it's a MacOS specific issue to link against libiconv, to add the library only on that platform. Or, to not try to link against iconv on Linux - as it is included in the glibc. What do you think? |
I would be willing to open this ticket and apply another patch, but I am not sure what you suggested is best. There has to be some way to make extconf.rb happy for all. Other gems have solved this, say like nokogiri. https://github.com/tenderlove/nokogiri/blob/master/ext/nokogiri/extconf.rb#L94-107 They have a few open tickets about iconv too, but I think that code above is fine? https://github.com/tenderlove/nokogiri/issues/381 So my question is, what are others doing, what have they learned and how can I make this work for everyone? |
I'd go with have_func('iconv_open', 'iconv.h') or have_library('iconv', 'iconv_open', 'iconv.h') -- this way if iconv_open is available in glibc (Linux) it'll go ok, otherwise it'll look for the function in libiconv (MacOS, BSD, ...). The "asplode" bit is über nice as well, tenderlove style :D -- and the search paths as well. Anyway I'd refrain from grabbing too much from that source, less is more. My .2c, I'll be able to work on this tomorrow (CEST) PS. we're maintaining a Sybase adapter for Rails that uses TinyTDS, if you're interested, on https://github.com/ifad/activerecord-sybase-adapter Thanks!!1oneone ~Marcello (via iPhone) ~ marcello.barnaba@gmail.com On Dec 19, 2011, at 8:19 PM, Ken Collins reply@reply.github.com wrote:
|
I reopened the ticket. Throw me a patch and I will put this in RC2. |
Library compiles both on Linux and OSX, now making all tests pass on Sybase ASE 15.0 :-). |
Is there a patch or pull request? |
Still not - I'm verifying the cross compilation is OK with the new extconf. I'll send you a pull soon. |
Pull #68 is on for your review :-) |
Please let me know how the latest master works and if I should re-open this or not. If not, I will create an RC2. |
FYI, the last change we had was in ticket #69 |
Hello, I am hacing the libiconv is missing error while trying to install the gem :
I tried several variation of the --with-iconv-dir but can not get it to work. LMaybe I'm just poiting to the wrond dir... which directory should it be ? |
@tbuyle needs to be the directory of a installed libiconv installation. If you manually installed libiconv, then will be the directory defined by |
I installed libiconv using HomeBrw - should the dir then be |
@tbuyle no idea about homebrew, probably you should read their documentation or ask them. |
On a 64-bit version of OS X (Snow Leopard), I am unable to compile the gem as-is. The error I get is:
I was able to resolve the problem by using another version of iconv (that is not installed in the standard location), but this required adding the following line to ext/tiny_tds/extconf.rb
Could this be included as a permanent fix for other people who have iconv in a non-standard location? I'm new to attempting to contribute, so I'm not sure if this was the right way submit this. Let me know if there is something else I need to do.
Julia
The text was updated successfully, but these errors were encountered: