pgRouting / osm2pgrouting

Import tool for OpenStreetMap data to pgRouting database
https://pgrouting.org
GNU General Public License v2.0
291 stars 111 forks source link

osm2pgrouting does not compile with libpqxx v 7.1.1, 7.2.0. Compiles okay with 6.4.7 #284

Closed jmarca closed 3 years ago

jmarca commented 3 years ago

Describe the bug Building osm2pgrouting in my docker image. Build fails with libpqxx 7.1.1, 7.2.0, and master. Build succeeds with libpqxx set to 6.4.7

To Reproduce

(I can upload my entire Dockerfile if you want...)

FROM postgres:alpine
....
#ARG LIBPQXX_VERSION=7.2.0
ARG LIBPQXX_VERSION=7.1.1
RUN wget -O libpqxx.tar.gz "https://github.com/jtv/libpqxx/archive/$LIBPQXX_VERSION.tar.gz" \
        && tar xvf libpqxx.tar.gz \
        && cd libpqxx-$LIBPQXX_VERSION  \
        && ./configure prefix=/usr --disable-documentation \
        && make -j9 \
        && make install \
        && cd .. && rm -rf libpqxx*
RUN  git clone --depth 1 "https://github.com/pgRouting/osm2pgrouting" \
        && cd osm2pgrouting  \
        && mkdir build \
        && cd build \
        && cmake -DCMAKE_INSTALL_PREFIX=/usr .. \
        && make \
        && make install \
        && cd ../.. && rm -rf osm2pgrouting*

If I change the version of libpqxx to 6.4.7, the build passes

The compile error starts off as:

[ 34%] Building CXX object CMakeFiles/osm2pgrouting.dir/src/database/pois_config.cpp.o
In file included from /usr/include/pqxx/array.hxx:18,
                 from /usr/include/pqxx/array:4,
                 from /usr/include/pqxx/pqxx:2,
                 from /osm2pgrouting/include/database/Export2DB.h:25,
                 from /osm2pgrouting/src/database/Export2DB.cpp:22:
/usr/include/pqxx/internal/encodings.hxx:24:42: error: 'pqxx::internal::encoding_group pqxx::internal::enc_group' redeclared as different kind of entity
   24 | encoding_group enc_group(std::string_view);
      |                                          ^
/usr/include/pqxx/internal/encodings.hxx:23:16: note: previous declaration 'pqxx::internal::encoding_group pqxx::internal::enc_group(int)'
   23 | encoding_group enc_group(int /* libpq encoding ID */);
      |                ^~~~~~~~~
/usr/include/pqxx/internal/encodings.hxx:24:31: error: 'string_view' is not a member of 'std'
   24 | encoding_group enc_group(std::string_view);
      |                               ^~~~~~~~~~~
/usr/include/pqxx/internal/encodings.hxx:24:31: note: 'std::string_view' is only available from C++17 onwards
/usr/include/pqxx/internal/encodings.hxx:39:28: error: 'std::string_view' has not been declared
   39 |   encoding_group enc, std::string_view haystack, char needle,
      |                            ^~~~~~~~~~~
/usr/include/pqxx/internal/encodings.hxx:44:28: error: 'std::string_view' has not been declared
   44 |   encoding_group enc, std::string_view haystack, std::string_view needle,
... lots more lines ...

The releases page for libpqxx (https://github.com/jtv/libpqxx/releases) notes some changes to string processing, which might align with the error above. But I'm not going to pretend to know enough C++ to hazard a guess as to what is going on.

Expected behavior Compile successfully with v 7+ version of libpqxx

Sample Data See above

Specifications (please complete the following information): Use the commands:

SELECT version();
SELECT postgis_full_version();
SELECT pgr_version();

NA

In my Dockerfile, I am compiling

ARG POSTGIS_VERSION=3.0.2
ARG PGROUTING_VERSION=3.1.0
ARG GEOS_VERSION=3.8.1

I am building osm2pgrouting master, but I have also tried and failed with version OSM2PGROUTING_VERSION=2.3.6

Additional context NA

dkastl commented 3 years ago

pgRouting has a well maintained Docker repository and as far as I know they all images contain osm2pgrouting as well: https://github.com/pgRouting/docker-pgrouting/ . Could you maybe compare what is different in the way you are creating a Docker image?

jmarca commented 3 years ago

It seems maybe they ran into the same problem? For example, looking at the dockerfile for the Postgres 13 version,

https://github.com/pgRouting/docker-pgrouting/blob/c0ae2c671ed730157970f397e60d803cb972b209/13-3.0-3.1.0/extra/Dockerfile

They pin osm2pgrouting version to 2.3.6 and also fix the version of libpqxx:

ENV OSM2PGROUTING_VERSION 2.3.6

RUN apt update \
 && apt install -y \
        libpqxx-6.2 \
...
cayetanobv commented 3 years ago

These errors from compiler are because Libpqxx v7.x require C++17. Related to issue #289

cayetanobv commented 3 years ago

This is not a bug. It's an improvement so I'm going to change the tag.

jmarca commented 3 years ago

Cool! Glad there is movement on this.

ildar15 commented 3 years ago

In addition to compile flag -std=c++17, I also had to change

diff --git a/src/osm_elements/osm2pgrouting.cpp b/src/osm_elements/osm2pgrouting.cpp
index e11fb2f..37e6232 100644
--- a/src/osm_elements/osm2pgrouting.cpp
+++ b/src/osm_elements/osm2pgrouting.cpp
@@ -140,7 +140,7 @@ int main(int argc, char* argv[]) {
                 cout << "Can't open database" << endl;
                 return 1;
             }
-            C.disconnect ();
+            C.close ();
         }catch (const std::exception &e){
             cerr << e.what() << std::endl;
             return 1;

with my version of pqxx:

user:build$ grep VERSION /usr/include/pqxx/version.hxx 
#ifndef PQXX_H_VERSION
#  define PQXX_VERSION "7.3.1"
#  define PQXX_VERSION_MAJOR 7
#  define PQXX_VERSION_MINOR 3
#  define PQXX_VERSION_CHECK                                                  \
    check_pqxx_version_##PQXX_VERSION_MAJOR##_##PQXX_VERSION_MINOR
PQXX_LIBEXPORT int PQXX_VERSION_CHECK() noexcept;
cvvergara commented 3 years ago

@jmarca Sorry for the delay, the PR is ready if you can review #292 and leave a comment on the PR The most critical part is the change on the minimum standard for C++

I also modified FindPQXX that detects the version, can you verify that it works as it should.

jmarca commented 3 years ago

I may not get to it for a few days as I am quite busy, but I have added a reminder for Wednesday to spend an hour checking this with my setup.

On Mon, May 31, 2021 at 11:12:31AM -0700, Vicky Vergara wrote:

@jmarca Sorry for the delay, the PR is ready if you can review #292 and leave a comment on the PR The most critical part is the change on the minimum standard for C++

  • C++ minimum standard depends on libpqxx
    • For version libpqxx 6: C++14
    • For version libpqxx 7: C++17 see here

I also modified FindPQXX that detects the version, can you verify that it works as it should.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/pgRouting/osm2pgrouting/issues/284#issuecomment-851623068

--

James E. Marca Activimetrics LLC