boku-ilen / geodot-plugin

Godot plugin for loading geospatial data
GNU General Public License v3.0
108 stars 18 forks source link

won't compile on arch linux because of gdal headers location #61

Closed VRichardJP closed 3 years ago

VRichardJP commented 3 years ago

I am trying to build the plugin on Arch Linux, I get the following include errors :

$ scons platform=linux
scons: Reading SConscript files ...
Compilation Database creation is not supported. Please consider upgrading to SCons 4.x.x!
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
g++ -o GeoRaster.o -c -std=c++17 -fPIC -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global GeoRaster.cpp
GeoRaster.cpp:11:10: fatal error: gdal/cpl_error.h: No such file or directory
   11 | #include <gdal/cpl_error.h>
      |          ^~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [GeoRaster.o] Error 1
scons: building terminated because of errors.
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
g++ -o Feature.o -c -std=c++17 -fPIC -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global Feature.cpp
Feature.cpp:8:10: fatal error: gdal/ogr_feature.h: No such file or directory
    8 | #include <gdal/ogr_feature.h>
      |          ^~~~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [Feature.o] Error 1
scons: building terminated because of errors.
scons: done reading SConscript files.
scons: Building targets ...
g++ -o src/gdlibrary.os -c -std=c++17 -fPIC -g3 -Og -fPIC -I. -Igodot-cpp/godot_headers -Igodot-cpp/include -Igodot-cpp/include/core -Igodot-cpp/include/gen -Isrc/raster-tile-extractor -Isrc/vector-extractor -Isrc -Isrc/global src/gdlibrary.cpp
In file included from src/geodot.h:10,
                 from src/gdlibrary.cpp:1:
src/vector-extractor/VectorExtractor.h:16:10: fatal error: gdal/cpl_port.h: No such file or directory
   16 | #include <gdal/cpl_port.h>
      |          ^~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [src/gdlibrary.os] Error 1
scons: building terminated because of errors.

The files on Arch are provided by the gdal packages and installed in /usr/include/ (see https://archlinux.pkgs.org/rolling/archlinux-community-x86_64/gdal-3.2.2-1-x86_64.pkg.tar.zst.html) On other major distributions like Ubuntu or Fedora these files are located in /usr/include/gdal/* as expected (https://ubuntu.pkgs.org/21.04/ubuntu-universe-amd64/libgdal-dev_3.2.2+dfsg-1ubuntu1_amd64.deb.html)

Basically Arch Linux follows the same hierarchy than Windows and OSX, and other Linux distros chose a different path =/

Easily fixed with: $ grep -rnI "<gdal/" src -l | xargs sed 's/<gdal\//\</g' -i

I'm fine with hacking, but is there any clean way to handle this?

kb173 commented 3 years ago

Thanks for the detailed report! Unfortunately it's not as easy as checking for Windows/Linux/OSX, as there's no built-in macro for Arch Linux. However, it's possible to get the Linux distribution in Python via:

import distro
print(distro.id()) # should be 'arch' in your case

so we could put that into the SConstruct script and pass a custom macro to check for in the C++ includes.

I want to condense all those platform-specific includes into a single separate file for clarity, so I'll tackle this issue in the process of doing that.

kb173 commented 3 years ago

This should be fixed with 08c5816 - can you verify whether it's compiling with no modifications now?

VRichardJP commented 3 years ago

Hi, Thanks for the update

it looks like the -D_ARCH is missing for some files:

$ scons platform=linux
scons: Reading SConscript files ...
Compilation Database creation is not supported. Please consider upgrading to SCons 4.x.x!
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
g++ -o GeoRaster.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global GeoRaster.cpp
g++ -o RasterTileExtractor.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global RasterTileExtractor.cpp
ar rc build/libRasterTileExtractor.a GeoRaster.o RasterTileExtractor.o
ranlib build/libRasterTileExtractor.a
scons: done building targets.
scons: Reading SConscript files ...
scons: done reading SConscript files.
scons: Building targets ...
g++ -o Feature.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global Feature.cpp
g++ -o LineFeature.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global LineFeature.cpp
g++ -o PointFeature.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global PointFeature.cpp
g++ -o PolygonFeature.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global PolygonFeature.cpp
g++ -o VectorExtractor.o -c -std=c++17 -fPIC -D_ARCH -I. -I/home/vrichard/Projects/godot/geodot-plugin/src/global VectorExtractor.cpp
ar rc build/libVectorExtractor.a Feature.o LineFeature.o PointFeature.o PolygonFeature.o VectorExtractor.o
ranlib build/libVectorExtractor.a
scons: done building targets.
scons: done reading SConscript files.
scons: Building targets ...
g++ -o src/gdlibrary.os -c -std=c++17 -fPIC -g3 -Og -fPIC -I. -Igodot-cpp/godot_headers -Igodot-cpp/include -Igodot-cpp/include/core -Igodot-cpp/include/gen -Isrc/raster-tile-extractor -Isrc/vector-extractor -Isrc -Isrc/global src/gdlibrary.cpp
In file included from src/vector-extractor/VectorExtractor.h:4,
                 from src/geodot.h:10,
                 from src/gdlibrary.cpp:1:
src/global/gdal-includes.h:20:10: fatal error: gdal/cpl_error.h: No such file or directory
   20 | #include <gdal/cpl_error.h>
      |          ^~~~~~~~~~~~~~~~~~
compilation terminated.
scons: *** [src/gdlibrary.os] Error 1
scons: building terminated because of errors.
kb173 commented 3 years ago

You're right, I missed something - sorry! It should be working now, I tested it on a machine with Manjaro, and it successfully compiled there. Would be great if you could also test it one more time!

(In the process, discovered that there's distro.like() which returns 'arch' for all distributions similar to Arch, so this also works with Manjaro, which is pretty handy.)

VRichardJP commented 3 years ago

unfortunately the trick distro.like() == 'arch' does not work on my machine. -D_ARCH is no more defined, and checking quickly with python confirmed it does not want to work this way:

Python 3.9.5 (default, May 12 2021, 17:14:51) 
[GCC 10.2.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import distro
>>> distro.like()
''
>>> 

playing with distro makes it even clearer:

$ distro -j
{
    "codename": "n/a",
    "id": "arch",
    "like": "",
    "version": "rolling",
    "version_parts": {
        "build_number": "",
        "major": "",
        "minor": ""
    }
}

TL;DR: arch linux is not like arch linux :) (I don't know if it is a mistake or if ubuntu isn't like ubuntu either)

So this is what I changed to make it work:

$ cat diff.txt
diff --git a/SConstruct b/SConstruct
index 1efd676..975a187 100644
--- a/SConstruct
+++ b/SConstruct
@@ -115,10 +115,10 @@ elif env['platform'] in ('x11', 'linux'):
         env.Append(CCFLAGS=['-fPIC', '-g3', '-Og'])
     else:
         env.Append(CCFLAGS=['-fPIC', '-g', '-O3'])
-    
+
     # Arch needs different includes!
     import distro
-    if distro.like() == "arch":
+    if distro.like() == "arch" or distro.id() == "arch":
         env.Append(CPPDEFINES=["_ARCH"])

 elif env['platform'] == "windows":
diff --git a/src/raster-tile-extractor/SConstruct b/src/raster-tile-extractor/SConstruct
index 3d08f45..5df77d1 100644
--- a/src/raster-tile-extractor/SConstruct
+++ b/src/raster-tile-extractor/SConstruct
@@ -35,7 +35,7 @@ if env['platform'] in ('x11', 'linux'):

     # Arch needs different includes!
     import distro
-    if distro.like() == "arch":
+    if distro.like() == "arch" or distro.id() == "arch":
         env.Append(CPPDEFINES=["_ARCH"])

 elif env['platform'] == "windows":
diff --git a/src/vector-extractor/SConstruct b/src/vector-extractor/SConstruct
index 3044d24..b138cac 100644
--- a/src/vector-extractor/SConstruct
+++ b/src/vector-extractor/SConstruct
@@ -35,7 +35,7 @@ if env['platform'] in ('x11', 'linux'):

     # Arch needs different includes!
     import distro
-    if distro.like() == "arch":
+    if distro.like() == "arch" or distro.id() == "arch":
         env.Append(CPPDEFINES=["_ARCH"])

 elif env['platform'] == "windows":
kb173 commented 3 years ago

Haha oh, wow, I didn't expect that. I'd hope that's a bug! Thanks for researching and fixing that. I added your changes in 2e1218f, so that should finally fix it now!