-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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] CRAN issues #629
Comments
|
@guolinke CRAN Windows machines do not have MSBuild or Runtimes for Visual Studio compiled code. They use MinGW 4.9 and Rtools to compile code and create packages. If using precompiled stuff, we have to check with them about how we can get the package accepted on CRAN. CRAN usually compiles packages themselves in an automated fashion and there should be no error/warning/note when running the check on all platforms they have (unless we have a special reason for the check). The compiled package by CRAN is what is proposed to R users as direct download. As all CRAN / R users (who compile stuff like for Rcpp) have Rtools, they forcibly have MinGW 4.9 hence why the requirement to switch to MinGW as default: VS compiled code will not run nor load on CRAN machines (DLL error), leading to the package being rejected. |
@Laurae2 |
@guolinke vc runtime DLL is MIT license? Everything in the R package must be MIT license if the R package is MIT in CRAN. There are no exception for this rule in CRAN. |
This is the rule for using binary packages for Windows / Mac:
You can also check here online to try to pass a fake Windows CRAN check: https://win-builder.r-project.org/ |
So the dll of vc runtime is ok ? Another solution: upload our dlls (include vc runtime) to another website (like nuget), and download it when install R-package ? |
@guolinke We need to ask uwe.ligges@r-project.org to see if it is feasible or not to use Visual Studio DLL requiring VC runtime. The whole installation and the DLL compilation must be done by Uwe Ligges. |
Hello, I am going to start with the vignette. Is there any point you want to highlight?. Thanks! |
@Laurae2 okay, can you help to ask Uwe Ligges ? |
@guolinke yes, but I'm currently on a project so I don't have enough time anymore. Perhaps @dselivanov can? |
I doubt cran will accept anything which requires vc or something not open sourced. I'm out of context, could you point me why vc is used instead of mingw? |
@dselivanov VS is used by default for performance. But now we have an automatic fallback to Rtools' MinGW, so it is fine. |
@Laurae2 can we enable the test of R package in travis / appveyor? |
@guolinke Yes, we also need to add more appropriate tests to testthat folder (it's very outdated, from 6 months ago). https://github.com/Microsoft/LightGBM/tree/master/R-package/tests/testthat |
how to install lightgbm in the R-language? |
@Royhuiy read https://github.com/Microsoft/LightGBM/tree/master/R-package and open a new issue if you still are not managing to install the R package. If everything is setup correctly, then it requires only one line to install the R package. |
@Laurae2 any updates or plans for CRAN ? |
@guolinke no update yet, we would need to clean all the check errors/warnings/notes first |
@Laurae2 any updates ? |
@guolinke Currently not having enough time, perhaps @jameslamb can help us. |
@Laurae2 I've fixed a few more of these today, will submit a PR in the next few days. You can assign this issue to me if you'd like |
@jameslamb do you know which tests cause these errors? |
You can see the logs here: https://builder.r-hub.io/status/lightgbm_2.3.2.tar.gz-38bbdb220ecd4ea288a2986c8f5717eb Unfortunately I can't tell from those logs which test is breaking. Running the jobs on R Hub are pretty easy though...maybe tomorrow I can build a version of the R package that writes a lot more log messages, so we can narrow it down. |
|
!!!!! ooooooo interesting, I missed that! Ok so one thing we could do is add |
@shiyu1994 could you help to locate the mis-align errors? |
I'm trying that now. By the way, to test that specific issue in the environment where I saw it, you can run this from the root of the repo sh build-cran-package.sh then in R (but change the email address) result <- rhub::check(
path = "lightgbm_2.3.2.tar.gz"
, email = "jaylamb20@gmail.com"
, check_args = c(
"--as-cran"
, "--use-valgrind"
)
, platform = "linux-x86_64-rocker-gcc-san"
, env_vars = c(
"R_COMPILE_AND_INSTALL_PACKAGES" = "always"
)
) |
Ok. I'll look into this too. |
Ok now I think that the failing unit tests and the misalignment errors are not related. Look at this build: https://builder.r-hub.io/status/original/lightgbm_2.3.2.tar.gz-9a1b2e49f6bf473892a6b9eccfa9f045 So I see
but also
That is in a version of the package with these tests (#629 (comment)) skipped. |
I just had a thought about some of these...if we pick up #1944 and create R |
@jameslamb FYI, you can download Oracle Solaris VM from this link. The link also shows how to install R in Solaris. It came in handy when XGBoost failed CRAN checks for Solaris target and I had to debug it. |
ooooooooo that's awesome, thank you! Will be a lot faster than using R Hub to debug 😂 |
@hcho3 are the notes at that link why you decided to add |
@jameslamb It had to do with supporting Windows: dmlc/xgboost#2994. |
ooooo thanks! |
thanks @guolinke ! Ok I know how to fix the Debian note, and how to get a better error on Windows. I'll create a PR tonight. Thanks for your help, we'll have to do this a few times 😬 |
@jameslamb no problem! |
I just re-submitted to win-builder now that #3307 has been merged. Can confirm we are passing all checks there! The only NOTE is the one all new packages get, which can be safely ignored. When CRAN comes back from their time off (August 24th), we can re-submit and I think we have a very good chance of being accepted. How I submitted sh build-cran-package.sh devtools::check_win_release(
pkg = "lightgbm_r/",
args = NULL,
manual = TRUE,
email = "jaylamb20@gmail.com",
quiet = FALSE,
) logs 00check.log (1 NOTE)
00install.out
|
I want to provide an update for anyone who is subscribed to this issue:
See #3338 (comment) for a summary, and the full discussion in #3338 for lots of details on progress towards CRAN. We're working on replicating and then fixing those failing checks (#3439, #3443), and will attempt another submission in November. Thanks to all the |
|
Environment info
Operating System: Windows 8.1 Pro
CPU: i7-4600U
R version: 3.4
To make a release on CRAN, we will need first to fix all the errors / warnings / notes. Currently testing on Windows, but we will also need to test on Linux. If some of them cannot be fixed, we will need to have an explanation for each of those which will not be fixed by us. @guolinke
Maybe time to add some vignettes @coforfe if you want to work on them.
00install.out:
Windows (fake) CRAN log:
The text was updated successfully, but these errors were encountered: