New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fails to build with GEOS 3.6.0 #634

Closed
sebastic opened this Issue Oct 25, 2016 · 11 comments

Comments

Projects
None yet
6 participants
@sebastic
Contributor

sebastic commented Oct 25, 2016

GEOS 3.6.0 has been released, but unfortunately osm2pgsql fails to build with it:

/usr/bin/c++   -DBOOST_TEST_DYN_LINK -DFIXED_POINT -DOSM2PGSQL_DATADIR=\"/usr/share/osm2pgsql\" -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -I/build/osm2pgsql-0.90.1+ds -I/build/osm2pgsql-0.90.1+ds/obj-x86_64-linux-gnu -isystem /build/osm2pgsql-0.90.1+ds/contrib/libosmium -I/usr/include/lua5.2 -I/usr/include/postgresql  -g -O2 -fdebug-prefix-map=/build/osm2pgsql-0.90.1+ds=. -fPIE -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2    -Wall -std=c++11 -o CMakeFiles/osm2pgsql_lib.dir/geometry-builder.cpp.o -c /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In member function 'geometry_builder::pg_geom_t geometry_builder::get_wkb_simple(const nodelist_t&, int) const':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:230:25: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:230:25: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In member function 'geometry_builder::pg_geoms_t geometry_builder::get_wkb_split(const nodelist_t&, int, double) const':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:266:25: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:266:25: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In static member function 'static int geometry_builder::parse_wkb(const char*, multinodelist_t&, bool*)':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:343:21: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
     GeometryFactory gf;
                     ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:343:21: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
     GeometryFactory gf;
                     ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In member function 'geometry_builder::pg_geoms_t geometry_builder::build_polygons(const multinodelist_t&, bool, osmid_t) const':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:398:25: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:398:25: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In member function 'geometry_builder::pg_geom_t geometry_builder::build_multilines(const multinodelist_t&, osmid_t) const':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:536:25: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:536:25: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp: In member function 'geometry_builder::pg_geoms_t geometry_builder::build_both(const multinodelist_t&, int, int, double, osmid_t) const':
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:560:25: error: 'geos::geom::GeometryFactory::GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:420:2: note: declared protected here
  GeometryFactory();
  ^~~~~~~~~~~~~~~
/build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:560:25: error: 'virtual geos::geom::GeometryFactory::~GeometryFactory()' is protected within this context
         GeometryFactory gf;
                         ^~
In file included from /build/osm2pgsql-0.90.1+ds/geometry-builder.cpp:39:0:
/usr/include/geos/geom/GeometryFactory.h:474:10: note: declared protected here
  virtual ~GeometryFactory();
          ^

This change seems to be the culprit.

- C++ API changes:
  - Automatic memory management for GeometryFactory objects
@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Oct 26, 2016

Collaborator

Unless someone provides a patch it's unlikely the maintainers will be providing support for the geos API changes in 3.6.0. Given the chance for memory leaks in any change, if someone did supply a patch it's unlikely it would be backported to 0.90.x.

Long term we'd like to move away from geos, but this won't help with 0.90.x either.

I'm leaving this open because

  • we need to document the maximum geos version and adjust the cmake checks. This should be easy to backport
  • more input is welcome
  • The chance that someone might come forward with a patch and it's something we can reasonably backport
Collaborator

pnorman commented Oct 26, 2016

Unless someone provides a patch it's unlikely the maintainers will be providing support for the geos API changes in 3.6.0. Given the chance for memory leaks in any change, if someone did supply a patch it's unlikely it would be backported to 0.90.x.

Long term we'd like to move away from geos, but this won't help with 0.90.x either.

I'm leaving this open because

  • we need to document the maximum geos version and adjust the cmake checks. This should be easy to backport
  • more input is welcome
  • The chance that someone might come forward with a patch and it's something we can reasonably backport
@mmd-osm

This comment has been minimized.

Show comment
Hide comment
@mmd-osm

mmd-osm Nov 13, 2016

Contributor

Found this related discussion: http://lists.osgeo.org/pipermail/geos-devel/2016-October/007573.html
"I guess C++ API users should make a wrapper to construct and destroy GeometryFactories. For 3.6+ construction uses a factory, and destruction is automatic. Those tools should really consider switching to the C-API instead, which was introduced for the exact purpose of keeping it stable."

Contributor

mmd-osm commented Nov 13, 2016

Found this related discussion: http://lists.osgeo.org/pipermail/geos-devel/2016-October/007573.html
"I guess C++ API users should make a wrapper to construct and destroy GeometryFactories. For 3.6+ construction uses a factory, and destruction is automatic. Those tools should really consider switching to the C-API instead, which was introduced for the exact purpose of keeping it stable."

@AMDmi3

This comment has been minimized.

Show comment
Hide comment
@AMDmi3

AMDmi3 Nov 23, 2016

Contributor

FYI, FreeBSD has already switched to geos 3.6.0. It's no more possible to build osm2pgsql there.

Contributor

AMDmi3 commented Nov 23, 2016

FYI, FreeBSD has already switched to geos 3.6.0. It's no more possible to build osm2pgsql there.

@AMDmi3

This comment has been minimized.

Show comment
Hide comment
@AMDmi3

AMDmi3 Nov 28, 2016

Contributor

FreeBSD user have submitted a patch to make it compile with geos 3.6.0, and it was confirmed to work. I'd like upstream comments though.

https://bz-attachments.freebsd.org/attachment.cgi?id=177390

Contributor

AMDmi3 commented Nov 28, 2016

FreeBSD user have submitted a patch to make it compile with geos 3.6.0, and it was confirmed to work. I'd like upstream comments though.

https://bz-attachments.freebsd.org/attachment.cgi?id=177390

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Nov 28, 2016

Collaborator

FreeBSD user have submitted a patch to make it compile with geos 3.6.0, and it was confirmed to work. I'd like upstream comments though.

https://bz-attachments.freebsd.org/attachment.cgi?id=177390

Without testing I can't be positive, but I think that would break compilation on Geos 3.5

Collaborator

pnorman commented Nov 28, 2016

FreeBSD user have submitted a patch to make it compile with geos 3.6.0, and it was confirmed to work. I'd like upstream comments though.

https://bz-attachments.freebsd.org/attachment.cgi?id=177390

Without testing I can't be positive, but I think that would break compilation on Geos 3.5

@AMDmi3

This comment has been minimized.

Show comment
Hide comment
@AMDmi3

AMDmi3 Dec 1, 2016

Contributor

That would be likely. If they've broken an API, the code is likely to work on one of versions only. If that's a correct fix to 3.6.0, it could be upstreamed with proper #ifdef's

Contributor

AMDmi3 commented Dec 1, 2016

That would be likely. If they've broken an API, the code is likely to work on one of versions only. If that's a correct fix to 3.6.0, it could be upstreamed with proper #ifdef's

@gdt

This comment has been minimized.

Show comment
Hide comment
@gdt

gdt Jan 22, 2017

I am the maintainer of the geos package in pkgsrc, and I am wondering if it's time to update to 3.6.1 from 3.5.1. From the pkgsrc viewpoint, a version of osm2pgsql that requires geos >= 3.6.0 is fine, since that's easy to express. Absent a released version of osm2pgsql (pkgsrc attempts to package only releases), I am inclined to hold off a bit on geos 3.6.1.

gdt commented Jan 22, 2017

I am the maintainer of the geos package in pkgsrc, and I am wondering if it's time to update to 3.6.1 from 3.5.1. From the pkgsrc viewpoint, a version of osm2pgsql that requires geos >= 3.6.0 is fine, since that's easy to express. Absent a released version of osm2pgsql (pkgsrc attempts to package only releases), I am inclined to hold off a bit on geos 3.6.1.

@lonvia

This comment has been minimized.

Show comment
Hide comment
@lonvia

lonvia Jan 22, 2017

Collaborator

There are no plans to officially support geos >= 3.6.0. We plan to get completely rid of geos instead. It might be a while, though, before an official release without geos is available. We are aware that this creates a problem for distributions. The problem is that it seems that the only viable way forward with geos 3.6 is to switch to the C API and that is about as much work as getting rid of it.

Collaborator

lonvia commented Jan 22, 2017

There are no plans to officially support geos >= 3.6.0. We plan to get completely rid of geos instead. It might be a while, though, before an official release without geos is available. We are aware that this creates a problem for distributions. The problem is that it seems that the only viable way forward with geos 3.6 is to switch to the C API and that is about as much work as getting rid of it.

gentoo-bot pushed a commit to gentoo/gentoo that referenced this issue Feb 10, 2017

sci-geosciences/osm2pgsql: fix dep bug #607456
Suggested-by: bugtrack@web.de
Upstream-bug: openstreetmap/osm2pgsql#634

Package-Manager: portage-2.3.0
@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Mar 3, 2017

Collaborator

Closed in #684, where we no longer support geos.

For backports, anything which requires geos >= 3.6.0 is not an option, as earlier versions of geos must continue to work. An option which supports both >=3.6 and <3.6 is probably too invasive to be backported into a patch release, and would require extensive testing.

Collaborator

pnorman commented Mar 3, 2017

Closed in #684, where we no longer support geos.

For backports, anything which requires geos >= 3.6.0 is not an option, as earlier versions of geos must continue to work. An option which supports both >=3.6 and <3.6 is probably too invasive to be backported into a patch release, and would require extensive testing.

@pnorman pnorman closed this Mar 3, 2017

@gdt

This comment has been minimized.

Show comment
Hide comment
@gdt

gdt Sep 1, 2017

(I'm the maintainer of the geos package within pkgsrc, and have been delaying updating to 3.6 for most of a year. I've been distracted, and returned to this because geos 3.7 is about to be released.)

Seeing this bug closed, I just tried to build the latest release (0.92.1) without geos, and it failed, which on reading the README wasn't surprising. Am I correct that there is no release without geos (or that works with 3.6)? Any timeline for one (presumably 0.94)?

gdt commented Sep 1, 2017

(I'm the maintainer of the geos package within pkgsrc, and have been delaying updating to 3.6 for most of a year. I've been distracted, and returned to this because geos 3.7 is about to be released.)

Seeing this bug closed, I just tried to build the latest release (0.92.1) without geos, and it failed, which on reading the README wasn't surprising. Am I correct that there is no release without geos (or that works with 3.6)? Any timeline for one (presumably 0.94)?

@pnorman

This comment has been minimized.

Show comment
Hide comment
@pnorman

pnorman Sep 1, 2017

Collaborator

Am I correct that there is no release without geos

Yes.

Any timeline for one (presumably 0.94)

No timeline, but probably quite soon. We're reviewing if we're ready.

Collaborator

pnorman commented Sep 1, 2017

Am I correct that there is no release without geos

Yes.

Any timeline for one (presumably 0.94)

No timeline, but probably quite soon. We're reviewing if we're ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment