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-package] [ci] added echoing of R CMD check logs to CI #3009

Merged
merged 1 commit into from
Apr 23, 2020

Conversation

jameslamb
Copy link
Collaborator

No that we use Rscript build_r.R --skip--install, the only build of the R package is from R CMD check. R CMD check suppresses output from the build and writes it to a file. Those logs are useful for us to check for compiler warnings and debug issues.

See https://github.com/microsoft/LightGBM/pull/2936/files#r411607175

Copy link
Collaborator

@StrikerRUS StrikerRUS left a comment

Choose a reason for hiding this comment

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

@jameslamb Thanks!
Seems that it is really usefull!
There are a lot of warnings like this one produced by gcc-9 compiler (macOS job):

/Users/travis/build/microsoft/LightGBM/lightgbm.Rcheck/00_pkg_src/lightgbm/src/src/lightgbm_R.cpp:664:52: warning: cast between incompatible function types from 'LGBM_SER* (*)(LGBM_SE, LGBM_SE, LGBM_SE)' {aka 'LGBM_SER* (*)(LGBM_SER*, LGBM_SER*, LGBM_SER*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  664 |   {"LGBM_GetLastError_R"              , (DL_FUNC) &LGBM_GetLastError_R              , 3},
      |                                                    ^~~~~~~~~~~~~~~~~~~

@jameslamb
Copy link
Collaborator Author

jameslamb commented Apr 21, 2020

@jameslamb Thanks!
Seems that it is really usefull!
There are a lot of warnings like this one produced by gcc-9 compiler (macOS job):

/Users/travis/build/microsoft/LightGBM/lightgbm.Rcheck/00_pkg_src/lightgbm/src/src/lightgbm_R.cpp:664:52: warning: cast between incompatible function types from 'LGBM_SER* (*)(LGBM_SE, LGBM_SE, LGBM_SE)' {aka 'LGBM_SER* (*)(LGBM_SER*, LGBM_SER*, LGBM_SER*)'} to 'DL_FUNC' {aka 'void* (*)()'} [-Wcast-function-type]
  664 |   {"LGBM_GetLastError_R"              , (DL_FUNC) &LGBM_GetLastError_R              , 3},
      |                                                    ^~~~~~~~~~~~~~~~~~~

Thanks! Yeah I believe these can safely be ignored right now, but I also think I know how to fix them eventually.

These are all coming from our call to R_RegisterRoutines (https://github.com/microsoft/LightGBM/blob/master/src/lightgbm_R.cpp#L709). I believe I set that up correctly per CRAN's "Writing R Extensions", but could be wrong. Recall that you'll get an R CMD check warning if you don't call R_RegisterRoutines (#1910). My guess is that we're getting these compiler warnings because we're using our custom R-to-C interface instead of SEXP from R.h.

It would be good to eventually just use the R headers instead of our own, but I don't think that it will be a blocker to getting to CRAN, so I haven't prioritized it yet. I'll create an issue and put it in #2302

@jameslamb
Copy link
Collaborator Author

Added #3016 to capture the work to suppress that compiler warning from #3009 (review)

@jameslamb jameslamb merged commit 02a0089 into microsoft:master Apr 23, 2020
@jameslamb jameslamb deleted the ci/check-logs branch April 25, 2020 17:02
@lock lock bot locked as resolved and limited conversation to collaborators Jun 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants