RcppCore / RcppArmadillo

Rcpp integration for the Armadillo templated linear algebra library
192 stars 56 forks source link

Add a check message success in config after g++version check #183

Closed coatless closed 6 years ago

coatless commented 6 years ago

Minor blip, but it would be nice to have a new line / msg status after "checking whether g++ version is sufficient"...

e.g.

checking whether g++ version is sufficient... checking LAPACK_LIBS... fallback LAPACK from R 3.3.0 or later used

c.f. @dorianps's configure output in #163

eddelbuettel commented 6 years ago

I don't follow. If it is deemed NOT sufficient we stop on error. Ergo when we don't stop ...

coatless commented 6 years ago

I suppose this is looking for uniformity across configure messages, e.g.

checking whether we are using the GNU C++ compiler... (cached) yes
checking whether clang++ accepts -g... (cached) yes
checking whether g++ version is sufficient... checking LAPACK_LIBS... fallback LAPACK from R 3.3.0 or later used
checking for macOS... not found as on Linux
checking for OpenMP... found

There is a multi-line check that should be split to:

checking whether g++ version is sufficient... yes
checking LAPACK_LIBS... fallback LAPACK from R 3.3.0 or later used
eddelbuettel commented 6 years ago

Works here:

edd@bud:~/git/rcpparmadillo(master)$ ./configure
checking whether the C++ compiler works... yes
checking for C++ compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C++ compiler... yes
checking whether ccache g++ accepts -g... yes
checking how to run the C++ preprocessor... ccache g++ -E
checking whether we are using the GNU C++ compiler... (cached) yes
checking whether ccache g++ accepts -g... (cached) yes
checking whether g++ version is sufficient... (6.3.0) yes
checking LAPACK_LIBS... system LAPACK found
checking for macOS... not found as on Linux
checking for OpenMP... found
configure: creating ./config.status
config.status: creating inst/include/RcppArmadilloConfigGenerated.h
config.status: creating src/Makevars
edd@bud:~/git/rcpparmadillo(master)$

I guess we have a false positive on crapOS then because the code is just

## Check the C++ compiler using the CXX value set
AC_PROG_CXX
## If it is g++, we have GXX set so let's examine it
if test "${GXX}" = yes; then
    AC_MSG_CHECKING([whether g++ version is sufficient])
    gxx_version=$(${CXX} -v 2>&1 | awk '/^.*g.. version/ {print $3}')
    case ${gxx_version} in
        1.*|2.*|3.*|4.0.*|4.1.*|4.2.*|4.3.*|4.4.*|4.5.*|4.6.*|4.7.0|4.7.1)
             AC_MSG_RESULT([no])
             AC_MSG_WARN([Only g++ version 4.7.2 or greater can be used with RcppArmadillo.])
             AC_MSG_ERROR([Please use a different compiler.])   
        ;;
        4.7.[2-9]|4.8.*|4.9.*|5.*|6.*|7.*|8.*)
             gxx_newer_than_45="-fpermissive"
             AC_MSG_RESULT([(${gxx_version}) yes])
        ;;
    esac
fi

and GXX should not be "yes" for you poor souls on the wrongOS. But if it is, we should better get a result from the short shell fragment. And no, I don't think we need s/CXX/GXX/. I think that is a been there done that.

coatless commented 6 years ago

Yeah. Digging into this issue shows that the AC_PROG_CXX check is detecting the __GNUC__ define being issued by clang.

https://lists.gnu.org/archive/html/autoconf/2014-09/msg00025.html

One way around this is the ax_compiler_vendor check, c.f.

https://www.gnu.org/software/autoconf-archive/ax_compiler_vendor.html#ax_compiler_vendor

An alternative is to just check for a difference between __clang__ and __GNUC__ used by the llvm project prior to its switch over to using CMake:

https://github.com/mono/llvm/blob/21492ec92e255a43bc6b687468f1eb18a635d94e/autoconf/configure.ac#L112-L129

Though, I'm a fan of the occam's razor approach of defining a default case like:

    case ${gxx_version} in
        1.*|2.*|3.*|4.0.*|4.1.*|4.2.*|4.3.*|4.4.*|4.5.*|4.6.*|4.7.0|4.7.1)
             AC_MSG_RESULT([no])
             AC_MSG_WARN([Only g++ version 4.7.2 or greater can be used with RcppArmadillo.])
             AC_MSG_ERROR([Please use a different compiler.])   
        ;;
        4.7.[2-9]|4.8.*|4.9.*|5.*|6.*|7.*|8.*)
             gxx_newer_than_45="-fpermissive"
             AC_MSG_RESULT([(${gxx_version}) yes])
        ;;
        *)
             AC_MSG_RESULT([compiler self-identifies as being compliant with GNUC extensions.])
        ;;
    esac
eddelbuettel commented 6 years ago

Yep, agree on Occam. I'll fold those additional three lines in. Easiest and cleanest.

eddelbuettel commented 6 years ago
modified   configure.ac
@@ -43,6 +43,10 @@ if test "${GXX}" = yes; then
              gxx_newer_than_45="-fpermissive"
              AC_MSG_RESULT([(${gxx_version}) yes])
         ;;
+        *)
+             AC_MSG_RESULT([nope])
+             AC_MSG_WARN([Compiler self-identifies as being compliant with GNUC extensions but is not g++.])
+        ;;
     esac
 fi 

Maybe I change nope to almost :) Either is better than the wtf I had first...

coatless commented 6 years ago

Closing this as dee02abec9aed7935971886699d0d5c1b6eaa72d fixed the check for a non-gcc compiler being used.

warning triggered on non-gcc compiler ``` > devtools::install_github("rcppcore/rcpparmadillo") Downloading GitHub repo rcppcore/rcpparmadillo@master from URL https://api.github.com/repos/rcppcore/rcpparmadillo/zipball/master Installing RcppArmadillo '/Library/Frameworks/R.framework/Resources/bin/R' --no-site-file --no-environ --no-save --no-restore --quiet \ CMD INSTALL \ '/private/var/folders/c8/fd3_gkc90657vwn34_bm76fc0000gn/T/RtmpfsfuCb/devtools460f678420dd/RcppCore-RcppArmadillo-dee02ab' \ --library='/Library/Frameworks/R.framework/Versions/3.4/Resources/library' --install-tests * installing *source* package ‘RcppArmadillo’ ... checking whether the C++ compiler works... yes checking for C++ compiler default output file name... a.out checking for suffix of executables... checking whether we are cross compiling... no checking for suffix of object files... o checking whether we are using the GNU C++ compiler... yes checking whether /usr/local/clang4/bin/clang++ accepts -g... yes checking how to run the C++ preprocessor... /usr/local/clang4/bin/clang++ -E checking whether we are using the GNU C++ compiler... (cached) yes checking whether /usr/local/clang4/bin/clang++ accepts -g... (cached) yes checking whether g++ version is sufficient... almost configure: WARNING: Compiler self-identifies as being compliant with GNUC extensions but is not g++. checking LAPACK_LIBS... fallback LAPACK from R 3.3.0 or later used checking for macOS... found configure: WARNING: OpenMP unavailable and turned off. configure: creating ./config.status config.status: creating inst/include/RcppArmadilloConfigGenerated.h config.status: creating src/Makevars ** libs /usr/local/clang4/bin/clang++ -std=gnu++11 -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG -I"/Library/Frameworks/R.framework/Versions/3.4/Resources/library/Rcpp/include" -I/usr/local/include -I../inst/include -fPIC -Wall -g -O2 -c RcppArmadillo.cpp -o RcppArmadillo.o /usr/local/clang4/bin/clang++ -std=gnu++11 -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG -I"/Library/Frameworks/R.framework/Versions/3.4/Resources/library/Rcpp/include" -I/usr/local/include -I../inst/include -fPIC -Wall -g -O2 -c RcppExports.cpp -o RcppExports.o /usr/local/clang4/bin/clang++ -std=gnu++11 -I/Library/Frameworks/R.framework/Resources/include -DNDEBUG -I"/Library/Frameworks/R.framework/Versions/3.4/Resources/library/Rcpp/include" -I/usr/local/include -I../inst/include -fPIC -Wall -g -O2 -c fastLm.cpp -o fastLm.o /usr/local/clang4/bin/clang++ -std=gnu++11 -dynamiclib -Wl,-headerpad_max_install_names -undefined dynamic_lookup -single_module -multiply_defined suppress -L/Library/Frameworks/R.framework/Resources/lib -L/usr/local/clang4/lib -o RcppArmadillo.so RcppArmadillo.o RcppExports.o fastLm.o -L/Library/Frameworks/R.framework/Resources/lib -lRlapack -L/Library/Frameworks/R.framework/Resources/lib -lRblas -L/usr/local/gfortran/lib/gcc/x86_64-apple-darwin15/6.1.0 -L/usr/local/gfortran/lib -lgfortran -lquadmath -lm -F/Library/Frameworks/R.framework/.. -framework R -Wl,-framework -Wl,CoreFoundation ld: warning: directory not found for option '-L/usr/local/gfortran/lib/gcc/x86_64-apple-darwin15/6.1.0' installing to /Library/Frameworks/R.framework/Versions/3.4/Resources/library/RcppArmadillo/libs ** R ** inst ** tests ** preparing package for lazy loading ** help *** installing help indices ** building package indices ** installing vignettes ** testing if installed package can be loaded * DONE (RcppArmadillo) ```