search

naturalatlas / node-gdal

Node.js bindings for GDAL (Geospatial Data Abstraction Library)
http://naturalatlas.github.io/node-gdal/classes/gdal.html
Apache License 2.0
567 stars 124 forks source link

Ability to build standalone binaries against GDAL SDK #19

Closed springmeyer closed 10 years ago

springmeyer commented 10 years ago

Appveyor builds are limited to ~30 minutes, which is not enough time to compile both the internally bundled gdal and the bindings.

So, I'm going to take a stab at:

wilhelmberg commented 10 years ago

@brandonreavis @brianreavis I've built gdal with expat, geos and proj on Windows. What is it exactly you would need? Just gdal.lib or anything else?

brianreavis commented 10 years ago

@BergWerkGIS Epic! Just gdal.lib to link node-gdal to. We need to get libpq in eventually (#20), but I haven't had a chance to get it configured in gyp... but maybe that's irrelevant for the windows setup.

wilhelmberg commented 10 years ago

DL here (libpq included): https://mapnik.s3.amazonaws.com/dist/dev/node-gdal_win32_deps.7z

Note: To get a test gdal.lib as fast as possible, I've not used gyp, but the windows build scripts that are included with the sources of each library.

As next step I think it would be good to use your gyp files to have the same system for all plattforms? With that approach we also have to keep in mind that gdal-config is only available on Unix systems. So, for Windows, we will have to create a simple BAT that spits out the libs.

To test locally I would have to replace deps/libgdal/libgdal.gyp:libgdal with <pathto>/gdal.lib?

Correct?

brianreavis commented 10 years ago

I've never done it before, but it looks like it needs to go in "libraries" (like so).

rclark commented 10 years ago

@springmeyer today I took a stab at your second bullet point. It doesn't look like the mapnik sdk bundle includes header files for gdal. From poking around in node-mapnik's build_from_sdk script, it looks to me like the shared libgdal will need gdal-config, headers, and the compiled lib.

In your vision does this mean another TARGET in mapnik-packaging specifically for building gdal?

wilhelmberg commented 10 years ago

@brianreavis @springmeyer For creating a libgdal.lib to download, I switched from my approach to using your repo. I think this makes more sense: only one repo and one batch of build scripts to maintain.

Probably the gyp script could be tweaked to check if there already is a libgdal.lib (has been downloaded before npm install is called) and only if there is none, to compile everything.

I managed to compile geos on Windows. Just added a few additional includes.

With the latest geos snapshots this is not necessary. Would it be possible to upgrade geos to a recent snapshot?

I'm not able to compile node-gdal. Error on AppVeyor Line in source

 gdal_rasterband.cpp
..\src\gdal_rasterband.cpp(210): error C2664: 'CPLErrorHandler CPLSetErrorHandler(CPLErrorHandler)' : cannot convert argument 1 from 'void (__cdecl *)(CPLErr,int,const char *)' to 'CPLErrorHandler' [C:\projects\node-gdal\build\gdal.vcxproj]
None of the functions with this name in scope match the target type

I think to this could be due to the fact that I'm using VS2013/MSBuild 12 (look for Microsoft Visual Studio 12.0 in the AppVeyor output) and you are using MSBuild 10 (look for Microsoft Visual Studio 10.0).

BTW: Which branch should I be working on? master or pre-gyp-publish? I've used pre-gyp-publish.

wilhelmberg commented 10 years ago

With my latest commit local build is successfull.

In src/gdal_rasterband.cpp I changed void statisticsErrorHandler(CPLErr eErrClass, int err_no, const char *msg) to void CPL_STDCALL statisticsErrorHandler(CPLErr eErrClass, int err_no, const char *msg).

AppVeyor is still running: https://ci.appveyor.com/project/BergWerkGIS/node-gdal/build/1.0.2

brianreavis commented 10 years ago

We just merged pre-gyp-publish into master, so probably master (or a new branch that can be merged in)? I just added you as a collaborator, so you should have full push access and such.

I think @springmeyer would know more about the best approach on where to put the build scripts. At one point we were talking about one repository for each dependency, so in the future there could be multiple node bindings without the need for nearly-identical build scripts in each. cc: @yhahn @rclark

For right now though, I think it's totally fine to have in the repo here. In the future we might want to try splitting them out.

Finally, thanks for all the work on this!

rclark commented 10 years ago

@BergWerkGIS I agree that its nice to keep all the build scripts together, however that's what runs us into the original problem: these builds timeout on AppVeyor after working for 30 min.

As I understand, this is why we actually need to split the building of gdal, etc. from building the bindings for this library. @springmeyer definitely is the goto on the vision for how this might look overall.

In the short-term, and for a first / alpha release, we could definitely use @BergWerkGIS's work in this repository to build, package, and pre-gyp publish for Windows, just not automatically using AppVeyor. Thoughts on going this route?

springmeyer commented 10 years ago

I'm thrilled to see the movement here. @rclark - right: in my experience with node-mapnik, I've found it is critical to have fairly quick (~5 min) build times for node-mapnik for easy per-commit testing and also smooth packaging. So yeah, ensuring as much work as possible goes into the SDK of headers and libgdal is good. Will be reviewing more as I catch up this week.

wilhelmberg commented 10 years ago

Let me explain what I meant in more detail:

Does this sound reasonable?

springmeyer commented 10 years ago

On unix systems we can do npm install gdal --shared_gdal and the shared_gdal gets set to something other than false: https://github.com/naturalatlas/node-gdal/blob/5e634d7b9838c88b29594358ac9b8c74ba2669b4/binding.gyp#L4. That triggers building against an external GDAL. @BergWerkGIS: I wonder if we could have this work the same general way on windows?

springmeyer commented 10 years ago

btw, I realize the name --shared_gdal is terrible. But the idea holds: by default builds use bundled gdal, but a command line option can be passed to trigger building against an external gdal. I like the idea of the external GDAL being able to be anywhere rather than just in build\release\libgdal.lib. What do you think?

wilhelmberg commented 10 years ago

I'll have a look at passing parameters to npm install (still on the way making myself familiar with gyp). Of course, custom path for the lib is what we should aim for. build\release\libgdal.lib was just an example, because the current script puts it there.

wilhelmberg commented 10 years ago

@brianreavis @springmeyer

YEAH!

Local build with prepared libs was successful (npm test: 98 passing, 9 pending).

AppVeyor is still running at the moment https://ci.appveyor.com/project/BergWerkGIS/node-gdal/build/1.0.6

I'm passing the directory containg the libs via --shared_gdal=c:/projects/node-gdal/deplibs/.

My changes were these: https://github.com/BergWerkGIS/node-gdal/compare/pre-gyp-publish?expand=1

brianreavis commented 10 years ago

@BergWerkGIS Epic! That looks clean too. How/where is https://mapnik.s3.amazonaws.com/dist/dev/libgdal_win_$env:PLATFORM.7z being built? Is that a manual process based on build-gdal?

wilhelmberg commented 10 years ago

@brianreavis I've abandoned build-gdal for this task. I think it is better to have one build process (this repo) so that the prebuilt libs match exactly what node-gdal is expecting.

This is how I did it:

springmeyer commented 10 years ago

I think it is better to have one build process (this repo) so that the prebuilt libs match exactly what node-gdal is expecting.

Interesting. I trust your judgement on this and would be interested to hear more about the tradeoffs you are seeing by using external scripts and whether/how we might learn to adapt your findings to the mapnik/node-mapnik packaging process for windows.

springmeyer commented 10 years ago

But don't let my curiosity slow you down - just flagging my interest. Getting a first windows tag out of node-gdal is still the priority - thanks for making it possible @BergWerkGIS!

wilhelmberg commented 10 years ago

My statement was based on:

Of course I think the final target should be to have one package/SDK/repo containing the dependencies that are needed by several projects. On the other hand it might be overkill to download a big SDK if just one lib is needed.

Looking forward to a chat to connect the dots and get an overview of the overall direction/needs/involved libs and projects. Of course only after the :fire: has cooled down and you are safe.

rclark commented 10 years ago

@BergWerkGIS this is great and positions us to make a first release, which is :+1: :+1:. A couple questions that come to mind:

wilhelmberg commented 10 years ago

Thanks :smile:

wilhelmberg commented 10 years ago

@rclark Concerning "local": The preferable solution would of course be a dedicated repo for creating/uploading the deps/headers. Like outlined by Dane: mapbox/windows-cpp-collab/issues/50 Current solution is based on priority to get a working Windows release and the fact, that I was not able to get node-gdal working with the libs created by build-gdal, will investigate.

wilhelmberg commented 10 years ago

Build with node 0.8 is successful. Trick is, to do a npm install node-gyp, so that the old node-gyp that is bundled with 0.8 is not used. See AppVeyor

springmeyer commented 10 years ago

The 'mapbox' appveyor account is already on a somewhat custom plan. I worked with Feodor to get a concurrency of 5. I forgot to ask about longer build times. I agree that priority here is node-gdal tag so local build for the SDK is fine, but in the coming weeks an appveyor build would be pretty sweet.

On Jul 17, 2014, at 3:21 PM, Wilhelm Berg notifications@github.com wrote:

Thanks

Yes, they could get out of sync. Each time new gdal headers are introduced to node-gdal, the libs would have to be rebuilt One way to get "local" out of the build process would be to upgrade the AppVeyor account. When I exchanged some emails with Feodor Fitsner (the AppVeyor guy), he offered custom plans. I'm pretty sure we could work out a deal with more CPU or longer build time. AppVeyor is based on Azure so the possible configuration would be one of those: http://msdn.microsoft.com/en-us/library/azure/dn197896.aspx node 0.8: oh yeah, forgot about that: the version of node-gyp that's bundled with 0.8 doesn't know about --msvs_version=2013 yet. I've already solved that somewhere else, I think with node-mapnik (will take a look) ok, will work in this repo from now own — Reply to this email directly or view it on GitHub.

springmeyer commented 10 years ago

closing this as I think we're mostly done. I have longer term plans to make it easier to create the standalone SDK's and this will surface over at https://github.com/mapbox/mason once I start on this project.