[R-package] Apply patch for R4.2 on Windows #4923
Conversation
|
Following is the link to the patches provided by CRAN |
jameslamb
changed the title
There was a problem hiding this comment.
Linking #4881.
Thanks for this! It's nice that they sent you a patch directly. Could you please paste in the text of the email that CRAN sent, so we can include it in R-package/cran-comments.md during the next release?
I'll try testing master and this patch against the R Hub UCRT R-devel environment later today to confirm that that environment is sufficient to reproduce the issue, and that this patch is sufficient to fix it.
we need a minor version update for this patch to update our CRAN package.
This is another reason I'd like to see the full text of the email CRAN sent you. I'd like to know if they said anything about when we need to upload a patched version by.
Either way, I think the next release should be a major release, v4.0. There are significant breaking changes already on master, and per #3210 we release all components at the same time. See https://github.com/microsoft/LightGBM/releases/tag/untagged-cdc3eab37af1a2dcafa7 (this link is viewable by maintainers only).
To avoid the package being removed from CRAN, I think that after this patch is merged we should move forward with a 4.0 release. Other changes that are seem to still not be ready for review, like #4630 and #3234 can be in a future, v5.0, release.
|
/gha run r-solaris Workflow Solaris CRAN check has been triggered! 🚀 solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.3.1.99.tar.gz-37a5f47dae964f62b290133de329b614 |
|
/gha run r-valgrind Workflow R valgrind tests has been triggered! 🚀 Status: success ✔️. |
Unfortunately it seems it's not possible to use R Hub to try to replicate whatever bug CRAN saw, because CRAN's patch for LightGBM is automatically applied on R Hub 😱 😱 😱 I built the package from code for submitting to R Hub (click me)sh build-cran-package.shlibrary(rhub)
package_tarball <- paste0("lightgbm_", readLines("VERSION.txt")[1], ".tar.gz")
rhub::check(
path = package_tarball
, email = # YOUR EMAIL HERE
, check_args = "--as-cran"
, platform = c(
"windows-x86_64-devel"
)
, env_vars = c(
"R_COMPILE_AND_INSTALL_PACKAGES" = "always"
)
)I was stunned to see the following in the build logs (link). full logs (link)NOTE: I removed 1000+ lines of Eigen warnings from this, since they're not specific to the UCRT build of R and since including them makes |
I'm really impressed! OK, great! Then this fact removes urgency for releasing new version of LightGBM, right? |
I'm not sure. R Hub is just a testing service, so the fact that patches are automatically applied there isn't necessarily informative about how CRAN will handle such patches. The official announcement from CRAN that I linked to in #4881 (link) says the following
This leads me to believe we don't need to act urgently, but CRAN is being intentionally vague here about when they'll force us to create a new release. I'd like to see the full text of the email(s) @shiyu1994 received from CRAN to see if they gave any indication about how urgently we need to do a new release. Either way....I support these changes if they pass they solaris and valgrind checks, and I'm ok with not doing a new release just because of this if CRAN isn't forcing us to do so. |
|
The full text of the email from CRAN is pasted below. Dear maintainers, This concerns the CRAN packages Cairo cepreader gpboost httpuv ipaddress lightgbm proj4 prophet maintained by one of you: Andreas Blaette andreas.blaette@uni-due.de: RcppCWB your packages need to be updated for R-devel/R 4.2 to work on Windows, Sorry for the group message, please feel free to respond individually I've created patches for you, so please review them and fix your packages: You can apply them as follows tar xfz package_1.0.0.tar.gz patch --binary < package.diff These patches are currently automatically applied by R-devel on Windows For more information, please see https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdeveloper.r-project.org%2FBlog%2Fpublic%2F2021%2F12%2F07%2Fupcoming-changes-in-r-4.2-on-windows%2F&data=04%7C01%7Cyushi2%40microsoft.com%7C8e6c353d1a8842c81eeb08d9bef5d835%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637750786169848244%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=SY77zgtbDbHvTxTgPLOoe%2Fw5OZDhXvJoxpVOoEaKoYo%3D&reserved=0 Once you add your patches/fix the issues, your package will probably If you end up just applying the patch as is, there is probably no need If you wanted to test locally on your Windows machine and do not have a Currently, the new R-devel can be downloaded from If you end up testing locally, you can use R_INSTALL_TIME_PATCHES If you wanted to find libraries to link for yourself, e.g. in a newer If you wanted to try in a virtual machine, but did not have a license, (but that needs a very good and un-metered network connection to install) Please let us know if you have any questions. Thanks, |
|
And in another email, it is said that we'd better do it in early Jaunuary.
As always, the sooner the better. Best, |
In that case, maybe we still need a minor version release first? Or shall we make it 4.0? |
Thanks for sharing that! Based on that reaction from CRAN asking for a new release in January, I think we should proceed with BOTH...put up a new release AND make that a 4.0. Since there are so many significant breaking changes already on |
Now we know that auto-patching is done by R-devel
and we can disable this by
|
|
I think we can have a new |
I totally support that. Just thought from the discussion in #3210 that you felt strongly that all components for LightGBM should always be released together. |
Yes... I still have the same position. I mean, we will have |
I see, ok. I totally support that, but will definitely need your help with it. As I understand it a lot of LightGBM's release process uses automation that pulls from |
|
I hope there won't be a lot of changes in CI. 🙏 |
|
This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this. |
Recently I've got an email from CRAN. LightGBM needs a small fix to work for updated R-devel/R 4.2 on Windows. I've applied the patch provided by CRAN in this PR.
Maybe we need a minor version update for this patch to update our CRAN package.