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

WIP: [ci] switch to r-devel in Solaris builds (fixes #5216) #5217

Closed
wants to merge 2 commits into from

Conversation

jameslamb
Copy link
Collaborator

Fixes #5216.

Proposes using RDscript instead of Rscript when building the R package prior to uploading it to R Hub.

Notes for Reviewers

As mentioned in #5216 (comment), I've opened an issue in https://github.com/wch/r-debug asking about the choice of RDscript vs. Rscript in that image. At least for now, though, I think this PR should be merged if if fixes the Solaris CI builds, to unblock development on #5159 (and the rest of the repo).

@jameslamb
Copy link
Collaborator Author

jameslamb commented May 16, 2022

/gha run r-solaris

Workflow Solaris CRAN check has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/2329432141

Reports also have been sent to LightGBM public e-mail: https://yopmail.com?lightgbm_rhub_checks
Status: failure ❌.

@jameslamb
Copy link
Collaborator Author

jameslamb commented May 16, 2022

/gha run r-solaris

Workflow Solaris CRAN check has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/2329496616

solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.3.2.99.tar.gz-e003453fa4744e8b8d9ac584dfa4cdb7
Reports also have been sent to LightGBM public e-mail: https://yopmail.com?lightgbm_rhub_checks
Status: failure ❌.

@jameslamb
Copy link
Collaborator Author

good news

With the changes in this PR, the Solaris job now successfully builds the R package and pushes it up to R Hub.

bad news

The jobs on R Hub are failing, because one of {lightgbm}'s dependencies is not building successfully on Solaris.

I've opened r-hub/rhub#526 describing this. Essentially, {ps} is failing to build, and it is required through the following dependency chain.

lightgbm
|_testthat
   |_callr
     |_processx
       |_ps

Given that I don't see Solaris checks at any of the following places:

I strongly suspect that Solaris builds might no longer be required by CRAN, now that R 4.2.0 has been released.

I recall seeing another strong signal that Solaris support would no longer be required in R 4.2.0, in this tweet: https://twitter.com/henrikbengtsson/status/1466877096471379970.

image

However, I haven't yet seen something authoritative from CRAN saying that. I will search the mailing lists and CRAN docs tomorrow to see if I can find something definitive. Maybe we'll be able to remove the Solaris requirement entirely.

@jameslamb
Copy link
Collaborator Author

jameslamb commented May 16, 2022

/gha run r-solaris

Workflow Solaris CRAN check has been triggered! 🚀
https://github.com/microsoft/LightGBM/actions/runs/2332516440

solaris-x86-patched: https://builder.r-hub.io/status/lightgbm_3.3.2.99.tar.gz-426a3164aece4ae6b1a5eb5af17334ab
Reports also have been sent to LightGBM public e-mail: https://yopmail.com?lightgbm_rhub_checks
Status: failure ❌.

@jameslamb
Copy link
Collaborator Author

Ok so the conclusion of r-hub/rhub#526 was "a release of {ps} that doesn't build on Solaris made it onto CRAN, so anything depending on it will fail on R Hub Solaris builds".

I think that, from this investigation, I see some strong evidence that we can remove Solaris support in LightGBM.

The evidence:

I really think that given all of this, we could remove Solaris support from LightGBM.

I appreciate Uwe's plea for people to continue writing portable software, but in this case CRAN removing that check means that similar issues to this {ps} one will show up over time which prevent us from testing that {lightgbm} works on Solaris without significant effort (like running our own Solaris server). Without a way to test, we can't really guarantee that it works, and the only reason we added that support at all was to make the R package more easily accessible by getting it onto CRAN.


@StrikerRUS @jmoralez can you please read through this when you have time and let me know what you think about this question: Should we remove Solaris support in LightGBM?.

@jameslamb jameslamb requested a review from jmoralez May 16, 2022 16:05
@jmoralez
Copy link
Collaborator

I agree on removing Solaris. Seems like everyone's happy to do so, so as you say we might face similar issues to ps in the near future.

@StrikerRUS
Copy link
Collaborator

Woo hoo! I'm so exited about that we can remove Solaris support! 🎉

Thank you very much for the deep investigation.

@jameslamb
Copy link
Collaborator Author

Sure, happy to do it! Ok I'll close this PR and open a new one removing Solaris.

And we can use admin powers to merge PRs like #5159 with a failed Solaris GitHub Actions build.

@jameslamb jameslamb closed this May 21, 2022
@jameslamb jameslamb deleted the ci/fix-solaris-build branch May 21, 2022 01:46
@StrikerRUS
Copy link
Collaborator

And we can use admin powers to merge PRs like #5159 with a failed Solaris GitHub Actions build.

Or we can:

  1. wait for a new PR with Solaris job removal will be merged
  2. (hacky way) manually modify /gha run ... comment with Status: success and re-run workflow that checks optional job statuses

@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 19, 2023
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.

[ci] Solaris R hub build is failing: "installation of package ‘Matrix’ had non-zero exit status"
3 participants