Richard/cmake

[r-barnes]

Switches code building and documentation generation to cmake. Switches to including shapelib as a submodule. Rewrites installation instructions as markdown, for better viewing on Github. Adds more stringent compilation warnings.

[r-barnes]

I've updated this to incorporate the latest on master.

[r-barnes]

I've updated this to incorporate the latest on master.

[r-barnes]

Updated to run with master.

[sahrk]

I just pushed version 7.1, which can build with or without GDAL depending on whether or not USE_GDAL is defined. I have renamed all the existing make files with a file extension .noCMake, so that our two build systems can temporarily co-exist. This means I can merge your pull request more quickly once it's updated (I won't need to test a bunch of production configurations).

[r-barnes]

@sahrk : I've just updated my PR to include your changes. The header files live in a separate place for the cmake version, so the two build systems may not work simultaneously, unfortunately.

[sahrk]

@r-barnes I started picking away at the memory leaks and ended up completing that task first. Could you update your pull request to version 7.2? Thanks!

[r-barnes]

@sahrk : That's great!

I've just updated my PR so that it is synced with master.

[sahrk]

@r-barnes OK I'm hoping we can have this PR done quickly. Just a couple of things I see to work through. First, it looks like this version doesn't use GDAL. Can you change this to optionally build with GDAL? Building with GDAL should also use the -DUSE_GDAL flag which is used inside the source code.

[sahrk]

@sahrk : I've just updated my PR to include your changes. The header files live in a separate place for the cmake version, so the two build systems may not work simultaneously, unfortunately.

I'm uncomfortable with the directory nesting lib/dglib/include/dglib. I'm not very familiar with cmake but isn't there some way to specify arbitrary include file directories?

[sahrk]

Please add -std=c++11 to the C++ compilation flags. That will get rid of a lot (most?) of the warnings (which are actually just a C++ version issue).

[r-barnes]

I'm uncomfortable with the directory nesting lib/dglib/include/dglib. I'm not very familiar with cmake but isn't there some way to specify arbitrary include file directories?

@sahrk : Could you be more specific about which part of it bothers you?

• The lib/ is there, if I recall correctly, to preserve as much of your original directory structure as I could.
• The dglib/ is there because lib also contains proj4lib.
• The dglib at the end is important, and standard, because it allows folks to write includes as #include <dglib/header> rather than #include <header>. The #include <dglib/XXX> way of writing includes is desirable because it helps prevent namespace conflicts with other libraries.

That said, a possible rearrangement would be to remove the src directory in favour of moving proj4lib to submodules, dglib/lib to ./src, dglib/include to ./include or (./api), and dglib/apps to ./apps or some such.

The only part of this that I think is really important is having the headers live inside of a directory named dglib so that the #include interface is clean.

I'll look at the GDAL and C++11 issues now.

[r-barnes]

@sahrk : I've added the C++11 compilation flags. This appears in the cmake files as target_compile_features(dggrid PRIVATE cxx_std_11)

[r-barnes]

@sahrk : I've added the GDAL flags you mentioned. CMake should automatically identify GDAL and generate the appropriate compilation flags, including -DUSE_GDAL, if GDAL is present. Otherwise, it will compile without GDAL (likely emitting a warning).

[sahrk]

I'm uncomfortable with the directory nesting lib/dglib/include/dglib. I'm not very familiar with cmake but isn't there some way to specify arbitrary include file directories?

@sahrk : Could you be more specific about which part of it bothers you?

The redundant dglib. I understand your reasoning now on the directory structure, and if we go this way I think it should be src with directories apps and libs, and then inside libs have a dglib and proj4lib directory.

• The dglib at the end is important, and standard, because it allows folks to write includes as #include <dglib/header> rather than #include <header>. The #include <dglib/XXX> way of writing includes is desirable because it helps prevent namespace conflicts with other libraries.

I have seen this done, but it feels to me like using #include "file.h" (rather than #include <file.h>) essentially does the same thing without the extra characters. I did it this way in H3 and they haven't changed the approach (they're not shy about improving upon my code). Is this something that R requires?

That said, a possible rearrangement would be to remove the src directory in favour of moving proj4lib to submodules, dglib/lib to ./src, dglib/include to ./include or (./api), and dglib/apps to ./apps or some such.

I have forks of DGGRID that add other libraries in the lib directory, some of which may find their way into the public version. So I'd like to keep a library directory.

Which brings up what I think is my last question on this PR: why did you choose to make shapelib a submodule? I wanted to have a version that was as self-contained as possible, with no external dependencies, and the submodule seems like extra syntax and potential source of issues.

[sahrk]

@sahrk : I've added the C++11 compilation flags. This appears in the cmake files as target_compile_features(dggrid PRIVATE cxx_std_11)

Thanks. I'll fix the unused variable warnings (and any others that remain).

[sahrk]

@sahrk : I've added the GDAL flags you mentioned. CMake should automatically identify GDAL and generate the appropriate compilation flags, including -DUSE_GDAL, if GDAL is present. Otherwise, it will compile without GDAL (likely emitting a warning).

Is there any way to force a build without GDAL, even if GDAL is somewhere on the system? We had that request from another user, and I'll need to be able to do it for testing.

[sahrk]

Having thought about this more I guess I am fine with with the he #include <dglib/XXX> approach. But I would prefer going with an arrangement (which was unfortunately incomplete in my answer last night) of src with directories apps, lib, and include, and then inside both lib and include have a dglib and proj4lib directory.

[sahrk]

@sahrk : I've added the GDAL flags you mentioned. CMake should automatically identify GDAL and generate the appropriate compilation flags, including -DUSE_GDAL, if GDAL is present. Otherwise, it will compile without GDAL (likely emitting a warning).

Is there any way to force a build without GDAL, even if GDAL is somewhere on the system? We had that request from another user, and I'll need to be able to do it for testing.

I still think I'm going to supply both build systems for at least the next version, to give me time to test and integrate the cmake with my other forks. That means if someone needs a version without GDAL they can use the old approach for now. So we don't need to make this work before merging this PR. We can just add it as an issue for later.

[sahrk]

@r-barnes I really want to get this PR merged before I get too distracted by other things. I have been thinking more and I guess I am fine with everything as it is, except for the shapelib submodule (I need to understand the reasoning for adding that complication first). So unless I hear an objection from you my plan is to merge this PR sometime this weekend, and then immediately walk back the shapelib submodule (we can always restore it) and add my legacy build files, and update the readme files accordingly.

[r-barnes]

Sorry about the delay; I have a few deadlines this week on other projects.

I'll set up a way to build without GDAL.

Having thought about this more I guess I am fine with with the he #include <dglib/XXX> approach. But I would prefer going with an arrangement (which was unfortunately incomplete in my answer last night) of src with directories apps, lib, and include, and then inside both lib and include have a dglib and proj4lib directory.

I'll make it so.

Which brings up what I think is my last question on this PR: why did you choose to make shapelib a submodule? I wanted to have a version that was as self-contained as possible, with no external dependencies, and the submodule seems like extra syntax and potential source of issues.

My primary worry here was that the approach of including shapelib directly doesn't provide good version control between shapelib and DGGRID. If they're good about stamping their files with a version number or if you're good about including this information, then it's possible to figure out which variant of their code DGGRID uses.

I like that the submodule approach makes it unambiguous what code is being used, along with an easy migration path to newer versions: just do a git pull on the submodule and test.

I can see that this makes things less self-contained, though. I could roll the submodule part of the diff back if you'd like to think about this separately.

[sahrk]

I'll set up a way to build without GDAL.

Thanks!

Having thought about this more I guess I am fine with with the he #include <dglib/XXX> approach. But I would prefer going with an arrangement (which was unfortunately incomplete in my answer last night) of src with directories apps, lib, and include, and then inside both lib and include have a dglib and proj4lib directory.

I'll make it so.

Please see my last comment above, in which it seems I continue to be unclear :). After contemplating I realized I was simply being resistant to change, and your directory structure makes sense. So no need to change. Sorry for all the confusion.

shapelib... I can see that this makes things less self-contained, though. I could roll the submodule part of the diff back if you'd like to think about this separately.

Those reasons make sense. But my overriding concern on the proj4/shapelib stuff is to make it as simple as possible for someone to get the system working if they aren't using GDAL. So for now I'd like to keep shapelib as part of the DGGRID package.

I am very close to having a branch ready to go with everything as we've discussed. In order not to duplicate effort I'd encourage you to work on other things for a couple of days and I will get everything we've discussed merged into master. Then we can revisit what needs to be done.

[r-barnes]

@sahrk : Okay. To be clear: I won't make further changes to this PR until I hear back from you. That will leave sorting out the shapelib submodule and making a non-GDAL build mode as follow-up items.

[sahrk]

@r-barnes I have merged this PR and made the changes we discussed. I made new issues for considering the shapelib submodule and cmake non-GDAL build mode. I have not yet tested the final version under all build configurations, nor have I tested it on linux. But I will do so asap.

Thanks for all your help and especially for pushing me to get this all done!!!!!