andrjohns / QuickJSR

R interface for the QuickJS lightweight javascript engine
Other
18 stars 0 forks source link

Installation issues on `R 4.0` and `R 3.6` #45

Closed IndrajeetPatil closed 5 months ago

IndrajeetPatil commented 5 months ago

Hi, thanks for the great package.

Our CI builds on R 3.6 are failing because compilation seems to fail for this R version:

  ../inst/include/quickjsr/JSValue_to_SEXP.hpp:72:37: note: candidate: ‘quickjsr::JSValue_to_SEXP_vector(JSContext*, const JSValue&)::<lambda(int&&)>’
     72 |                                     [len](auto&& vec) { return vec.size() == len; });
        |                                     ^
  ../inst/include/quickjsr/JSValue_to_SEXP.hpp:72:37: note:   no known conversion for argument 1 from ‘std::vector<double>’ to ‘int&&’
  make: *** [/opt/R/3.6.3/lib/R/etc/Makeconf:177: quickjsr.o] Error 1
  ERROR: compilation failed for package ‘QuickJSR’
  * removing ‘/tmp/Rtmpg2EnHZ/pkg-lib12f411d6546/QuickJSR’

Is this expected? If yes, can you please set the minimum needed R version in DESCRIPTION? Thanks.

IndrajeetPatil commented 5 months ago

Actually, it also fails on R 4.0.

andrjohns commented 5 months ago

It looks like the package is being compiled using C++11, while it needs c++14 features:

  g++ -std=gnu++11 -I"/opt/R/3.6.3/lib/R/include" -DNDEBUG -I"../inst/include/" -I"../inst/include/quickjs" -D_GNU_SOURCE -DCONFIG_BIGNUM -DCONFIG_VERSION=\"2024-01-13\"  -I/usr/local/include  -fpic  -g -O2  -Wall -pedantic -c quickjsr.cpp -o quickjsr.o
  In file included from ../inst/include/quickjsr/JSValue_to_Cpp.hpp:4,
                   from ../inst/include/quickjsr/JSValue_to_SEXP.hpp:6,
                   from ../inst/include/quickjsr/SEXP_to_JSValue.hpp:5,
                   from ../inst/include/quickjsr.hpp:4,
                   from quickjsr.cpp:4:
  ../inst/include/quickjsr/type_traits.hpp:15:32: error: ‘decay_t’ in namespace ‘std’ does not name a template type; did you mean ‘decay’?
     15 |     using type = typename std::decay_t<T>;
        |                                ^~~~~~~
        |                                decay

I don't set a c++ standard in the package, since CRAN gives a NOTE for anything other than the default c++17

andrjohns commented 5 months ago

I'll have a look at what would need to change to support c++11, and if it's not too much then I'll update

andrjohns commented 5 months ago

@IndrajeetPatil Backwards compatibility fixed, I'll submit an update to CRAN in a week or two

IndrajeetPatil commented 5 months ago

Thanks a lot for the quick fix! 🙌