Skip to content
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

Fixed missing GEOS_LIBS #47

Closed
wants to merge 2 commits into from
Closed

Fixed missing GEOS_LIBS #47

wants to merge 2 commits into from

Conversation

justbennet
Copy link
Contributor

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 justbennet mentioned this pull request Nov 28, 2019
@justbennet
Copy link
Contributor Author

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.

edzer added a commit that referenced this pull request Nov 29, 2019
@justbennet
Copy link
Contributor Author

I just updated this PR.

@justbennet
Copy link
Contributor Author

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
Copy link
Member

edzer commented Jun 28, 2020

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

@justbennet
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

@justbennet justbennet closed this Jun 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants