search

r-spatial / lwgeom

bindings to the liblwgeom library
https://r-spatial.github.io/lwgeom/
58 stars 23 forks source link

Fixed missing GEOS_LIBS #47

Closed justbennet closed 4 years ago

justbennet commented 4 years ago

The original was only passing -lgeos_c and not the location of the libraries that contain it. Replacing the hard-coded -lgeos_c with the information from geos-config stored in $GEOS_LIBS in configure.ac and running autoreconf -i generated the configure script here.

I ran that with

$ autoreconf --version
autoreconf (GNU Autoconf) 2.69

on Centos 7.6 and tested the resulting configure with

$ echo $PROJ_DIR
/sw/arcts/centos7/specials/proj/4.9.2

$ echo $GEOS_DIR
/sw/arcts/centos7/specials/geos/3.5.0

$ ./configure --with-proj-include=$PROJ_DIR/include \
    --with-proj-lib=$PROJ_DIR/lib \
    --with-proj-share=$PROJ_DIR/share/proj \
    --with-geos-config=$GEOS_DIR/bin/geos-config

and the resulting src/Makevars contained

PKG_CPPFLAGS= -I/sw/arcts/centos7/specials/proj/4.9.2/include -I/sw/arcts/centos7/specials/geos/3.5.0/include -DPOSTGIS_GEOS_VERSION=35 -I./liblwgeom -DHAVE_LIBGEOM_INTERNAL_H
PKG_LIBS= -L/sw/arcts/centos7/specials/proj/4.9.2/lib  -lproj -L/sw/arcts/centos7/specials/geos/3.5.0/lib -lgeos_c
CXX_STD=CXX11

which should be correct.

My original comments were based on the contents of the package distributed on CRAN. See Issue #45. If there is something I can do to help get a patched version created and distributed to CRAN, please let me know.

justbennet commented 4 years ago

I believe that the CI test from travis-ci may not be because of the changes in the PR. The text suggests to me that this is because of a deprecation warning from the CI framework. See line 4666 of the ci outpu from Details, then line 4809-4810, where the warning states

> checking top-level files ... WARNING
  A complete check needs the 'checkbashisms' script

and lines 4814-4825,

Warnings found in rcmdcheck::rcmdcheck(), and `errors_on = "warning"` is set.
Backtrace:
  1. tic::script()
  2. tic::run_stage("script", stages = stages)
  3. stage$run_all()
  4. private$run_one(step)
 11. step$run()
 12. tic:::stopc("Warnings found in rcmdcheck::rcmdcheck(), ", "and `errors_on = \"warning\"` is set.")
Error: A step failed in stage "script": script.
In addition: Warning message:
'add_package_checks' is deprecated.
Use 'do_package_checks' instead.
justbennet commented 4 years ago

I just updated this PR.

justbennet commented 4 years ago

This appears to be the line that causes and error.

g++ -std=gnu++11 -I"/opt/R/3.5.3/lib/R/include" -DNDEBUG -DACCEPT_USE_OF_DEPRECATED_PROJ_API_H -I/usr/include -DPOSTGIS_GEOS_VERSION=35 -I./liblwgeom -DHAVE_LIBGEOM_INTERNAL_H -I"/home/runner/work/_temp/Library/Rcpp/include" -I"/home/runner/work/_temp/Library/sf/include" -I/usr/local/include   -fpic  -g -O2 -c geodetic.cpp -o geodetic.o
##[error]Error: R CMD check found ERRORs

It doesn't say why, but this doesn't seem to be anything to do with the change I made.

The changes that commented these lines from .travis.yml

cache:
  - packages
  - ccache

seemed to change the error message, but it still seems to be outside the changes here.

I could try to start over, as there have been a number of changes since last Nov, when this was originally opened. If you would like me to pursue this.

edzer commented 4 years ago

Yes, it would be great if you could sync with master and see if the issue still exists.

justbennet commented 4 years ago

OK. I'll give it a try. There seem to be a number of the R geospatial libraries that are tangled with autoconf. Spent part of today looking at rgdal, too.

justbennet commented 4 years ago

Hi, Edzer,

I am going to close this PR since it's got out of sync with master and I am not good enough with Git to get everything on track. I will open a new PR based on the current master.