[GitHub] [arrow] nealrichardson commented on a change in pull request #10414: [C++]: C++17 test PR

[GitBox <gi...@apache.org>]

nealrichardson commented on a change in pull request #10414:
URL: https://github.com/apache/arrow/pull/10414#discussion_r645001388



##########
File path: r/configure
##########
@@ -196,20 +196,10 @@ if [ "$ARROW_R_CXXFLAGS" ]; then
 fi
 
 # Test configuration
-CXXCPP="`${R_HOME}/bin/R CMD config CXX11` -E"
-if [ $? -eq 0 ]; then
-  # Great, CXX11 exists for this version of R (current);
-  # now let's set the other two variables
-  CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX11FLAGS`
-  CXX11STD=`"${R_HOME}"/bin/R CMD config CXX11STD`
-else
-  # For compatibility with R < 3.4, when these were called CXX1X
-  CXXCPP="`${R_HOME}/bin/R CMD config CXX1X` -E"
-  CXX11FLAGS=`"${R_HOME}"/bin/R CMD config CXX1XFLAGS`
-  CXX11STD=`"${R_HOME}"/bin/R CMD config CXX1XSTD`
-fi
+CXX17FLAGS=`"${R_HOME}"/bin/R CMD config CXX17FLAGS`

Review comment:
       I don't think you actually want/need to delete this `if` block, but regardless the deletion was overzealous and we still need to define `CXXCPP` for the test command on L202. This should fix the Linux R builds (or the reason they're currently failing).
   
   ```suggestion
   CXXCPP="`${R_HOME}/bin/R CMD config CXX17` -E"
   CXX17FLAGS=`"${R_HOME}"/bin/R CMD config CXX17FLAGS`
   ```




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org