2

I tried to use the rocksdb inside R package. I used the following src/Makevars:

CXX_STD = CXX11

PKG_CPPFLAGS = -I./rocksdb/include/
PKG_LIBS = rocksdb/librocksdb.a -lbz2 -lz -lzstd -llz4 -lsnappy

$(SHLIB): rocksdb/librocksdb.a

rocksdb/librocksdb.a: rocksdb/Makefile
    CFLAGS="$(CFLAGS) $(CPICFLAGS)" AR="$(AR)" RANLIB="$(RANLIB)" LDFLAGS="$(LDFLAGS)" \
       $(MAKE) -d --jobs=3 --directory=rocksdb static_lib

clean:
    $(MAKE) --directory=rocksdb clean

Package installation failed with many errors (see build log below).

You could reproduce this case using Docker container:

Necessary commands:

docker run --rm -ti rocker/r-ver:latest bash

Execute in container:

apt-get update
# install system deps
apt-get install -y libgflags-dev libsnappy-dev zlib1g-dev libbz2-dev liblz4-dev libzstd-dev
apt-get install -y git-core
# install R deps
install2.r Rcpp checkmate R6 tinytest
cd /tmp
git clone https://gitlab.com/artemklevtsov/rocksdb
cd rocksdb/
git submodule init
git submodule update
R CMD INSTALL .

But I can successfully run make directly in the rocksdb source directory:

cd src/rocksdb/
make static_lib

How can I fix src/Makevars to build rocksdb during R package installation?

Links:

Artem Klevtsov
  • 9,193
  • 6
  • 52
  • 57
  • 3
    I won't have time to play with this in detail but the broad strokes look correct. What a few packages to is to farm out the _creation_ of the required static library to the `configure` script and then just depend on it. Other package sometimes just invoke a help script from the `src/Makevars` too. It really is more about R build system wrangling at this point... – Dirk Eddelbuettel Sep 29 '19 at 22:37

2 Answers2

4

Not a full answer but an observation (for now):

I tried to reproduce this in a docker container. The R package build failed, but also the plain build when using the same flags as used by R, but without parallel jobs and make's debug output:

root@e8749c4bca63:/tmp/rocksdb/src# CFLAGS="-g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wdate-time -D_FORTIFY_SOURCE=2 -g  -fpic" AR="ar" RANLIB="ranlib" LDFLAGS="-L/usr/local/lib" \
>    make --directory=rocksdb static_lib
[...]
  CC       util/bloom.o
  CC       util/build_version.o
util/build_version.cc:5:42: error: macro "__DATE__" might prevent reproducible builds [-Werror=date-time]
 const char* rocksdb_build_compile_date = __DATE__;
                                          ^~~~~~~~
cc1plus: all warnings being treated as errors
Makefile:2029: recipe for target 'util/build_version.o' failed
make: *** [util/build_version.o] Error 1
make: Leaving directory '/tmp/rocksdb/src/rocksdb'

So it looks as if -Wdate-time got promoted to -Werror=date-time.

Ralf Stubner
  • 26,263
  • 3
  • 40
  • 75
  • Thank you for the answer. I think we can omit all R flags except `-fpic`. – Artem Klevtsov Sep 30 '19 at 08:41
  • This flags defined in the `/usr/local/lib/R/etc/Makeconf`. – Artem Klevtsov Sep 30 '19 at 09:09
  • Also `-Werror` flag added in the `src/rosckdb/Makefile` (lines 350-352). – Artem Klevtsov Sep 30 '19 at 09:17
  • 3
    @ArtemKlevtsov ACK, and without the flag rocksdb builds fine outside of the R package, but there are still errors when build from `Makevars`. I would go for building the library in a `configure` script (maybe after checking that it is not already installed) as hinted by Dirk. See the `nloptr` package for an example. – Ralf Stubner Sep 30 '19 at 09:19
3

To solve the problem we should reset MAKEFLAGS variable. So the right way to do it:

rocksdb/librocksdb.a: rocksdb/Makefile
    CFLAGS="$(CCFLAGS) $(CPICFLAGS)" MAKEFLAGS="" \
       $(MAKE) -C rocksdb DISABLE_WARNING_AS_ERROR=1 static_lib

MAKEFLAGS content:

MAKEFLAGS= -- OBJECTS=RcppExports.o\ backup.o\ checkpoint.o\ db.o\ del.o\ exists.o\ get.o\ keys.o\ list.o\ options.o\ property.o\ put.o\ size.o\ sst.o\ utils.o\ version.o\ wrap.o SHLIB=rocksdb.so SHLIB_LD=$$(SHLIB_CXX11LD) SHLIB_LDFLAGS=$$(SHLIB_CXX11LDFLAGS) CXXPICFLAGS=$$(CXX11PICFLAGS) CXXFLAGS=$$(CXX11FLAGS) CXX=$$(CXX11)\ $$(CXX11STD)
Artem Klevtsov
  • 9,193
  • 6
  • 52
  • 57