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

[R] Set compilation standard to C++17 on Windows, remove it on Unix (Closes #3407) #3408

Merged
merged 1 commit into from
Feb 12, 2023

Conversation

eddelbuettel
Copy link
Contributor

This PR suggests to increase the compilation standard from C++14 to C++17 to preempt a nag from CRAN.

With the suggested change, and on a (very recent, just updated) r-devel we now get a new line (that I just tweeted about too) that is actually just a comment after an OK instead of the nag I reported in #3407.

The full log with this change (as well as with #3405 addressing my #3406) we get the (good, passing) check below. Note that for that check I also increased the version number locally to make it distinct, this is not part of the PR.

edd@rob:/tmp/rcpp$ rdcc.sh mlpack_4.0.1.1.tar.gz                                                          
* using log directory ‘/tmp/mlpack.Rcheck’
* using R Under development (unstable) (2023-02-09 r83797)
* using platform: x86_64-pc-linux-gnu (64-bit)
* R was compiled by          
    gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0
    GNU Fortran (Ubuntu 12.2.0-3ubuntu1) 12.2.0
* running under: Ubuntu 22.10                      
* using session charset: UTF-8                     
* using option ‘--as-cran’          
* checking for file ‘mlpack/DESCRIPTION’ ... OK
* this is package ‘mlpack’ version ‘4.0.1.1’                                                             
* package encoding: UTF-8                      
* checking CRAN incoming feasibility ... [5s/11s] Note_to_CRAN_maintainers
Maintainer: ‘Ryan Curtin <ryan@ratml.org>* checking package namespace information ... OK
* checking package dependencies ... OK           
* checking if this is a source package ... OK                                                            
* checking if there is a namespace ... OK       
* checking for executable files ... OK                                                                   
* checking for hidden files and directories ... OK 
* checking for portable file names ... OK
* checking for sufficient/correct file permissions ... OK
* checking whether package ‘mlpack’ can be installed ... [534s/414s] OK
* used C++ compiler: ‘g++ (Ubuntu 12.2.0-3ubuntu1) 12.2.0’
* checking C++ specification ... OK
  Not all R platforms support C++17
* checking installed package size ... NOTE
  installed size is 18.7Mb             
  sub-directories of 1Mb or more:       
    libs  18.3Mb                                                                                         
* checking package directory ... OK                 
* checking for future file timestamps ... OK
* checking DESCRIPTION meta-information ... OK
* checking top-level files ... OK
* checking for left-over files ... OK
* checking index information ... OK
* checking package subdirectories ... OK
* checking R files for non-ASCII characters ... OK
* checking R files for syntax errors ... OK
* checking whether the package can be loaded ... OK
* checking whether the package can be loaded with stated dependencies ... OK
* checking whether the package can be unloaded cleanly ... OK
* checking whether the namespace can be loaded with stated dependencies ... OK
* checking whether the namespace can be unloaded cleanly ... OK
* checking loading without being on the library search path ... OK
* checking use of S3 registration ... OK
* checking dependencies in R code ... OK
* checking S3 generic/method consistency ... OK
* checking replacement functions ... OK
* checking foreign function calls ... OK
* checking R code for possible problems ... OK
* checking Rd files ... OK
* checking Rd metadata ... OK
* checking Rd line widths ... OK
* checking Rd cross-references ... OK
* checking for missing documentation entries ... OK
* checking for code/documentation mismatches ... OK
* checking Rd \usage sections ... OK
* checking Rd contents ... OK
* checking for unstated dependencies in examples ... OK
* checking line endings in shell scripts ... OK
* checking line endings in C/C++/Fortran sources/headers ... OK
* checking line endings in Makefiles ... OK
* checking compilation flags in Makevars ... OK
* checking for GNU extensions in Makefiles ... OK
* checking for portable use of $(BLAS_LIBS) and $(LAPACK_LIBS) ... OK
* checking use of PKG_*FLAGS in Makefiles ... OK
* checking use of SHLIB_OPENMP_*FLAGS in Makefiles ... OK
* checking pragmas in C/C++ headers and code ... OK
* checking compilation flags used ... OK
* checking compiled code ... OK
* checking examples ... OK
* checking for unstated dependencies in ‘tests’ ... OK
* checking tests ...
  Running ‘testthat.R’
 OK
* checking PDF version of manual ... OK
* checking HTML version of manual ... OK
* checking for non-standard things in the check directory ... OK
* checking for detritus in the temp directory ... OK
* DONE

Status: 1 NOTE
See
  ‘/tmp/mlpack.Rcheck/00check.log’
for details.

$ 

@eddelbuettel eddelbuettel changed the title [R] Set compilation standard to C++17 (Close #3407) [R] Set compilation standard to C++17 (Closes #3407) Feb 10, 2023
@eddelbuettel
Copy link
Contributor Author

Further revision is to keep the compilation standard setting for Windows, but roll it to C++17. We can remove it once R upgraded.

There was a recently found issue that Windows did not properly progagate the default compilation standard to C++14 so by removing it we likely fell back to C++11 which is why Windows failed.

@eddelbuettel eddelbuettel changed the title [R] Set compilation standard to C++17 (Closes #3407) [R] Set compilation standard to C++17 on Windows, remove it on Unix (Closes #3407) Feb 10, 2023
Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks---I appreciate this! Always good to stay ahead of the CRAN nags. 👍

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit 9e81e35 into mlpack:master Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants