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

Fixes for R checks. #8330

Merged
merged 10 commits into from Oct 19, 2022
Merged

Fixes for R checks. #8330

merged 10 commits into from Oct 19, 2022

Conversation

trivialfis
Copy link
Member

@trivialfis trivialfis commented Oct 11, 2022

Fixes #8329 .

  • Bump configure.ac version.
  • Remove amalgamation to reduce the build time for a single object with the added benefit that we can use parallel build during development.
  • Fix c function prototype warning.
  • Remove Windows automake file generation step to make the build script easier to understand.

Will wait for other PRs for 1.7 before getting all the needed fixes. Also, we need to change the CI as kindly suggested by @jameslamb , for unknown reason xgboost's cran test doesn't emit any warning: https://github.com/dmlc/xgboost/actions/runs/3219079146/jobs/5264008989

@jameslamb
Copy link
Contributor

cran test doesn't emit any warning

In case you are using the the rhub/ container images in testing... I found that the Debian ones haven't actually built successfully since August 1st, even though they are supposed to be nightly builds. You might want to subscribe to updates for this fix: r-hub/rhub-linux-builders#62.

@trivialfis trivialfis added this to In progress in 1.7 Roadmap via automation Oct 11, 2022
@trivialfis
Copy link
Member Author

It seems I need to disable network support for R. We have tests on GitHub action with MinGW, but the linking step somehow fails on win-builder:

D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x555dd): undefined reference to `__imp_getaddrinfo'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x556d5): undefined reference to `__imp_htons'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x5572a): undefined reference to `__imp_freeaddrinfo'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x55791): undefined reference to `__imp_htons'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x557b4): undefined reference to `__imp_freeaddrinfo'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x55976): undefined reference to `__imp_WSAGetLastError'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x559a6): undefined reference to `__imp_WSAGetLastError'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x55ab7): undefined reference to `__imp_send'
D:\rtools42\x86_64-w64-mingw32.static.posix\bin/ld.exe: ./amalgamation/xgboost-all0.o:xgboost-all0.c:(.text+0x55c01): undefined reference to `__imp_WSAGetLastError'

...

@trivialfis
Copy link
Member Author

NVM, found the cause. It's the outdated makefile.

@hcho3
Copy link
Collaborator

hcho3 commented Oct 18, 2022

@trivialfis Do you need help with this?

@jameslamb
Copy link
Contributor

@ me too if you need help. I had some trouble with LightGBM's network support + R 4.2 + RTools42 + Windows, might be able to offer some suggestions.

@trivialfis
Copy link
Member Author

Thank you for the offers! I fixed the winbuilder error, it's caused by xgboost's old makefile, not actual linking errors. See the fix here: 5b79322

Now I'm debugging issues with r-hub build error, will keep you updated.

@trivialfis trivialfis changed the title [WIP] Fixes for R checks. Fixes for R checks. Oct 19, 2022
@trivialfis
Copy link
Member Author

trivialfis commented Oct 19, 2022

Test results (to be updated)

@trivialfis
Copy link
Member Author

@jameslamb Hi, are you familiar with the rhub tests? I ran rhub::check_for_cran(), and the tests seem to be failing for weird reasons as listed in #8330 (comment) . We run tests on rhub before for previous releases but haven't seen such a case where all tests failed for unknown reasons.

1.7 Roadmap automation moved this from In progress to Reviewer approved Oct 19, 2022
@trivialfis
Copy link
Member Author

Merging it now for the upcoming 1.7rc.

@trivialfis trivialfis merged commit 28a466a into dmlc:master Oct 19, 2022
1.7 Roadmap automation moved this from Reviewer approved to Done Oct 19, 2022
@trivialfis trivialfis deleted the r-1.7-rel branch October 19, 2022 18:52
@jameslamb
Copy link
Contributor

@trivialfis I'm very familiar with {rhub}, but don't fully understand the errors in the builds you linked.

I will say...usually with {rhub}, the reason for your job being considered a "failure" is not in the log lines near the end, but buried somewhere in the middle.

In the Fedora one, for example, I see

  • checking package dependencies ... ERROR
    Package suggested but not available: ‘float’

I'm not sure how to address each of these specifically, but can say that I've found that submitting with a few customized env_vars does help sometimes:

EMAIL <- "****" # my personal email
PACKAGE_TARBALL <- "lightgbm_3.3.2.99.tar.gz"

result <- rhub::check(
    path = PACKAGE_TARBALL
    , email = EMAIL
    , check_args = c(
        "--as-cran"
    )
    , platforms = c(
        "windows-x86_64-devel"
        , "windows-x86_64-oldrel"
        , "windows-x86_64-patched"
        , "windows-x86_64-release"
    )
    , env_vars = c(
        "R_COMPILE_AND_INSTALL_PACKAGES" = "always"
        , "_R_CHECK_FORCE_SUGGESTS_" = "true"
        , "_R_CHECK_CRAN_INCOMING_USE_ASPELL_" = "true"
    )
)

If activity on https://github.com/r-hub/rhub-linux-builders/commits/master and https://github.com/r-hub/rhub/commits/master are any indication, R Hub maintenance is very bursty (long periods of no activity interrupted by brief periods of lots of maintenance fixes), so it's possible you could resubmit the same code in a few days and see all checks pass.

I'll also add ... the maintainers do accept bug reports and questions on the issue tracker for the R client: https://github.com/r-hub/rhub/issues. So as a last resort, you can try asking there for help.

@trivialfis
Copy link
Member Author

@jameslamb Thank you for sharing and I really appreciate the info! Will try to run more tests with rhub.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[R] CRAN warning: use of prototypes in init.c
3 participants