dark-panda / ffi-geos

ffi-geos is an implementation of the GEOS Ruby bindings in Ruby via FFI.
MIT License
58 stars 15 forks source link

Windows Fixes #6

Closed cfis closed 13 years ago

cfis commented 13 years ago

Hey J,

Looking to update from the geos ruby bindings to ffi-geos. I needed to make these changes to get geos to load on windows:

Note that I kind of wonder if any of this code is necessary. What if we just send the library name to FFI and let it deal with these problems?

Note you should double check on linux/osx, I didn't double check the changes on those platforms.

Thanks - Charlie

dark-panda commented 13 years ago

Cool, thanks. I'll take a look next week once I'm back from vacation. I had attempted to set up a mingw environment on a Windows VM but didn't have much luck compiling the then-unreleased GEOS 3.3.0 so I eventually just gave up and decided to let someone more familiar with Windows development save my arse, and lo and behold, here you are to oblige.

Other than the path and library name issues, how do things look on Windows, anyways? Do the unit tests all pass and whatnot?

On 2011-08-03, at 6:11 PM, cfisreply@reply.github.com wrote:

Hey J,

Looking to update from the geos ruby bindings to ffi-geos. I needed to make these changes to get geos to load on windows:

  • Look for geos_c-1 and geos-3.3.0 - Mingw doesn't create the geos_c and geos symlink that exist on windows.
  • Look in the system path for dlls.
  • Don't hard-code library extensions

Note that I kind of wonder if any of this code is necessary. What if we just send the library name to FFI and let it deal with these problems?

Note you should double check on linux/osx, I didn't double check the changes on those platforms.

Thanks - Charlie

Reply to this email directly or view it on GitHub: https://github.com/dark-panda/ffi-geos/pull/6

cfis commented 13 years ago

Yup, all looks good. I have about 4 or 5 minor little patches after this one that just add a couple methods/tests here and there - but it all seems to working. Updating our tests now.

Thanks - Charlie

cfis commented 13 years ago

Ah by the way instead of my hack of looking for geos_c, geos_c-1.0.0, it dawned on me that using a glob filter (like you do for paths) would be a better and less brittle.

See what you think - I can update the patch if you want.

Thanks - Charlie

dark-panda commented 13 years ago

Less brittle sounds good to me. I took a look at the other patches and they all look good to me. The SRID copying one is something I'll probably roll in with another set of patches I have that copies SRIDs when calling methods like convex_hull and envelope and the like, since to me it makes sense that those kinds of methods should create geometries that inherit the caller's SRID. I can include you in the thread that Daniel Azuma and I were having before I went on vacation if you'd like since there appear to be a few issues with this sort of thing. (For instance, it appears possible in GEOS for different points in a multipoint geometry collection to have different SRIDs from each other and even from the multipoint itself, so we were kind of discussing the behavior we should be seeing from the Ruby perspective.)

Cheers

On 2011-08-04, at 6:12 PM, cfisreply@reply.github.com wrote:

Ah by the way instead of my hack of looking for geos_c, geos_c-1.0.0, it dawned on me that using a glob filter (like you do for paths) would be a better and less brittle.

See what you think - I can update the patch if you want.

Thanks - Charlie

Reply to this email directly or view it on GitHub: https://github.com/dark-panda/ffi-geos/pull/6#issuecomment-1730848

dark-panda commented 13 years ago

Any luck on that aforementioned path globbing? I'm gonna merge these changes in soonish and hopefully get a new version out shortly thereafter if everything looks cool.

For the CoordinateSequence#to_s method, would it make sense to comma-separate the coordinates, since there's no real way of discerning 2d from 3d if everything is just whitespace-separated?

cfis commented 13 years ago

On 8/9/2011 2:24 PM, dark-panda wrote:

Any luck on that aforementioned path globbing? I'm gonna merge these changes in soonish and hopefully get a new version out shortly thereafter if everything looks cool.

Yes, can do.

For the CoordinateSequence#to_s method, would it make sense to comma-separate the coordinates, since there's no real way of discerning 2d from 3d if everything is just whitespace-separated?

I just went with what georss does, which admittedly I always thought was weird. Up to you...

Charlie

cfis commented 13 years ago

Ok, changed pushed. Do a double check on the glob pattern - works here but would be good to have a quick look at it.

Thanks,

Charlie

On 8/9/2011 2:24 PM, dark-panda wrote:

Any luck on that aforementioned path globbing? I'm gonna merge these changes in soonish and hopefully get a new version out shortly thereafter if everything looks cool.

For the CoordinateSequence#to_s method, would it make sense to comma-separate the coordinates, since there's no real way of discerning 2d from 3d if everything is just whitespace-separated?

Charlie Savage http://cfis.savagexi.com

dark-panda commented 13 years ago

Patches should be in master now. Cheers!

cfis commented 13 years ago

Cool - thanks!

Charlie