-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Check existence of inet_pton for win32 in CMakeLists.txt (fixes #5019) #5159
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this!
Could you please add inet_pton
existence check in configure.win
too like it was stated in #5019 (comment)?
This is essential because CRAN doesn't use CMake and builds there will stop working with removed #ifndef _UCRT
guard with some recent versions of MSYS according to #4923.
@StrikerRUS Thanks for the reminder. I've added that in ff8c604. Could you please help to check? BTW, do you have any idea why some actions of R packages with gcc and R4.1 hangs? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
R changes look good to me, thanks! I'm going to kick off the Solaris and valgrind jobs to give us greater confidence that this will work on CRAN.
/gha run r-solaris |
/gha run r-valgrind |
/gha run r-valgrind |
It looks like the valgrind build is being triggered by comments correctly For example, https://github.com/microsoft/LightGBM/runs/6122965595?check_suite_focus=true But is then failing prior to even building the R package. https://github.com/microsoft/LightGBM/runs/6122967527?check_suite_focus=true
I suspect that was just missed in #5152. Sorry about that! Will put up a PR shortly. |
Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
/gha run r-solaris Workflow Solaris CRAN check has been triggered! 🚀 solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.3.2.99.tar.gz-66562bfd23ac42ff942995a915f1b396 |
/gha run r-valgrind Workflow R valgrind tests has been triggered! 🚀 Status: success ✔️. |
The r-solaris check fails with the following information
|
/gha run r-solaris Workflow Solaris CRAN check has been triggered! 🚀 solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.3.2.99.tar.gz-4f49e421e97542b4876d80c696893832 |
Still fails. Maybe worth reporting to https://github.com/r-hub/rhub/issues? |
I agree! I see the following error at the top of the logs from the Solaris jobs, which I suspect is the root cause.
I found one issue with a similar error, where maintainers point to "GitHub rate limiting" as a problem: r-hub/rhub#492 (comment). I also see the following in the logs:
Anyway, I can try tonight submitting to R-hub from |
Co-authored-by: Nikita Titov <nekit94-08@mail.ru>
I just built the package like this: sh build-cran-package.sh Then manually triggered Solaris builds like this: package_tarball <- paste0("lightgbm_", readLines("VERSION.txt")[1], ".tar.gz")
rhub::check(
path = package_tarball
, email = "FILL_IN_EMAIL_HERE"
, check_args = "--as-cran"
, platform = c(
"solaris-x86-patched"
, "solaris-x86-patched-ods"
)
, env_vars = c(
"R_COMPILE_AND_INSTALL_PACKAGES" = "always"
)
) With Statuses should be sent there once the jobs complete. Links: |
Hasn't this meant that our token was overwritten? LightGBM/.ci/run_rhub_solaris_checks.R Lines 12 to 15 in 0018206
LightGBM/.ci/run_rhub_solaris_checks.R Lines 24 to 27 in 0018206
|
I just get it! LightGBM/.github/workflows/r_solaris.yml Line 12 in 0018206
LightGBM/.github/workflows/r_solaris.yml Lines 40 to 42 in 0018206
LightGBM/.ci/test_r_package_solaris.sh Lines 8 to 10 in 0018206
Job fails even before submitting the LightGBM package to RHub. LightGBM/.ci/test_r_package_solaris.sh Line 15 in 0018206
LightGBM/.ci/run_rhub_solaris_checks.R Line 37 in 0018206
|
It would surprise me if that was true. I assumed that when I was prompted for a new token, R-hub was issuing me an additional token for use with that email, and that you could have multiple associated with the same email address. If that's not true and we find out that the token has been overwritten, then I apologize and I'll put up a PR to fix the comment-triggered jobs. |
ooooh I see! I was wondering where @shiyu1994 saw failures related to I guess then, based on #5159 (comment), we need to install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you for the fix!
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. |
This is to fix #5019. We manually check the existence of
inet_pton
inCMakeLists.txt
to avoidinet_pton
being redefined insrc/network/socket_wrapper.hpp
.