-
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
[ci] use GitHub Actions for R CI jobs (fixes #2353) #3119
Conversation
I'm very confused by this message in the mac jobs:
That is not something I saw on my fork with the exact same For the Failed windows + Basically sometimes if a process writes to |
909821b
to
80c07f1
Compare
I learned tonight that there if any task in GitHub Actions fails, all tasks must be re-run. Because of that, for now I'm going to put |
I spent a few hours today trying to get around errors like this on the Windows jobs and couldn't get them to reliably pass: I seems that sometimes processes writing to
At this point, I'm out of ideas and haven't found any other tips from search engines. Hopefully GitHub Support will respond to me this week. For reviewers, here are other links I found while investigating that held possible answers.
Suggestions welcomed! |
Maybe |
That sounds exactly what I need! Thank you! I'll try that tonight. |
I tried many things tonight and still cannot get Windows jobs to reliably succeed. At this point I just don't believe the documentation saying that overriding the default I'm going to look around for other R packages using GitHub Actions and see if I can pick up any tricks. |
covr I also found this absolutely cursed description of error handling in Powershell. It is very complicated, and seems like there is even an issue where setting |
Ok I have some evidence of what I suspected to be true: the way GitHub Actions powershell handles treatment of Then I pushed 10 empty commits.
I really really think these "errors" are just "some code writing benign messages to stderr", and not actual failures of the calls. The two types of errors I encountered are shown below. The most common is what I've called ERR1 ERR2 Given this finding, I'm sad to say that at this point, we should not move any of the Windows CI builds too GitHub Actions. It is too unreliable. I'm planning to freeze the PR on my fork and use it in a post on the Github Actions Community Forum. If that post returns any useful information, or GitHub Support ever responds to the ticket I submitted, maybe we'll be able to add GitHub Actions jobs for Windows in the future. |
I've created a GitHub Community Forum post with the contents of #3119 (comment): https://github.saobby.my.eu.orgmunity/t/powershell-steps-fail-nondeterministically/115496 |
@jameslamb Seems that we are not alone! Check this ticket: https://github.saobby.my.eu.orgmunity/t/windows-builds-failing-randomly/18172. The proposed solution is "great":
😄 |
BTW, did you try to set |
I did! No luck :( I think we should use this PR just for Linux / Mac builds. It will still help us get more coverage (like all combinations of R version and compiler), and we can come back and add Windows if / when GitHub responds to me. What do you think? |
@StrikerRUS I got some very quick help on that GitHub Forum post about this issue!! https://github.saobby.my.eu.orgmunity/t/powershell-steps-fail-nondeterministically/115496/4?u=jameslamb It seems that GitHub Actions is using a version of Powershell which may have introduced a regression in the handling of this issue: PowerShell/PowerShell#11036 In this PR, I'm going to revert all the Windows-specific changes and make this PR just Mac and Linux. That will at least cut some of the burden on Travis and make it easier to add tests on R 4.0, CRAN, etc. |
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 a lot @jameslamb !
Great job!
One question. Will GitHub Actions be used exclusively for R-package tests? Or we may use this CI service for other tests in the future? If later, please push a test commit with
- TASK=regular
- TASK=sdist
- TASK=bdist
- TASK=if-else
- TASK=lint
- TASK=check-docs
- TASK=mpi METHOD=source
- TASK=mpi METHOD=pip
- TASK=gpu METHOD=source
- TASK=gpu METHOD=pip
tasks to ensure that everything works OK. After that test swapped compilers as well.
.github/workflows/main.yml
Outdated
@@ -0,0 +1,55 @@ | |||
name: GitHub Actions | |||
|
|||
on: [push] |
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.
Will it work for PRs?
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.
You're right! I think to avoid problems like #3096 , we want this:
on:
push:
branches:
- master
pull_request:
branches:
- master
So push: branches: master
will build when we merge to master
and pull_request: branches: master
will only build on pull requests into master
.
more references:
I've been reading Events that trigger workflows.
I think we'll get a pull_request
event when a PR is opened (doc):
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.
one other thing...after this is merged, a repo admin with more permissions than me will need to make these checks required on pull requests
That can be done by going to Settings
--> Branches
and then clicking Edit
on master
. Screenshots below are from a personal repo of mine, just to show the interface
I am proposing in this PR that we use it for R packages. The results of trying |
OK. You removed |
ohhh right! Sorry, that was before #3119 (comment) I'll undo those changes |
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.
@jameslamb Wow! Awesome job! Thanks a lot for setting up GitHub Actions with a such neat solution and small-diff PR!
Two comments below for your consideration:
Thanks for the review! As soon as we merge #3133 it should fix Travis here |
3e378bf
to
0710aae
Compare
Just rebased to |
All checks passed! I checked the logs in GitHub Actions (just to be sure we weren't getting a false ✔️ ) and all looks good! Thanks for the review @StrikerRUS . I'm really excited about this one |
@guolinke Please make GitHub Actions required to merge PRs: #3119 (comment) |
Have you tried PS Core for Windows jobs? |
I did :/ But there are enough dimensions that maybe I missed a combination in testing.
|
Ah, OK! 🙁 I was hoping that |
@jameslamb I see this error in
|
@StrikerRUS I've never seen this exact error before. I have occasionally seen issues with the It seems like there was a successful build on #3140 that is more recent than your comment, so hopefully this was something temporary? |
…ft#3119) * GitHub Actions * ok * fixing on list * stuff * directories * directories * things * env variables * working dir * running a bunch of tasks * more builds * PATH * actually use R task * TASK * be right, often * doing stuff * trying stuff * more paths * conda activate * updating PATH * trying bash * where the hell is activate * WHERE IS ACTIVATE * set up conda * more conda * PLEASE WORK * installing cpplint * try r-package * R version * try windows job * make windows work * use powershell * exe * use conda * conda init powershell * different conda approach * make it work * cleaning up * init powershell * fixing windows * more windows * build directory * no way right * maybe it will work * trying Visual Studio * do this * Windows is interesting * put back check-output * set compiler * stuff * more fixes * fix the broken things * updating jobs * continuing * poweshell is bad * ok so maybe not powershell * cmon now * ok so * fixing env variables * maybe this * MINGW job * cleaning up * conda init powershell * moving more R stuff into GitHub Actions * everything else * use powershell * cmon now powershell * ttry to Continue * override powershell * peg MiKTeX URL * what is happening * try powershell -File * trying stuff * path * more testing of output * Matches uppercase * more regex stuff * this is getting ridiculous * back to powershell I guess * more commands * this might work * adding more reliable miktex download * trying to download miktex * installing httr * fix error in MiKTeX script * remove comments * redirect output * redirect output * move linting back to Travis * change redirection * switch back to just mac and linux * put linting exclude back * renamed R_TRAVIS_LINUX * revert changes to non-R tasks and update events * simplify
…ft#3119) * GitHub Actions * ok * fixing on list * stuff * directories * directories * things * env variables * working dir * running a bunch of tasks * more builds * PATH * actually use R task * TASK * be right, often * doing stuff * trying stuff * more paths * conda activate * updating PATH * trying bash * where the hell is activate * WHERE IS ACTIVATE * set up conda * more conda * PLEASE WORK * installing cpplint * try r-package * R version * try windows job * make windows work * use powershell * exe * use conda * conda init powershell * different conda approach * make it work * cleaning up * init powershell * fixing windows * more windows * build directory * no way right * maybe it will work * trying Visual Studio * do this * Windows is interesting * put back check-output * set compiler * stuff * more fixes * fix the broken things * updating jobs * continuing * poweshell is bad * ok so maybe not powershell * cmon now * ok so * fixing env variables * maybe this * MINGW job * cleaning up * conda init powershell * moving more R stuff into GitHub Actions * everything else * use powershell * cmon now powershell * ttry to Continue * override powershell * peg MiKTeX URL * what is happening * try powershell -File * trying stuff * path * more testing of output * Matches uppercase * more regex stuff * this is getting ridiculous * back to powershell I guess * more commands * this might work * adding more reliable miktex download * trying to download miktex * installing httr * fix error in MiKTeX script * remove comments * redirect output * redirect output * move linting back to Travis * change redirection * switch back to just mac and linux * put linting exclude back * renamed R_TRAVIS_LINUX * revert changes to non-R tasks and update events * simplify
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. |
NOTE: moved from #3116 to use a branch on
LightGBM
instead of my fork.This PR proposes adding GitHub Actions CI and moving all R jobs to it.
GitHub Actions gives you 20 concurrent jobs for free and access to Mac, Linux, and Windows environments.
See #2353 and discussion in this thread: #3065 (comment)
This PR should improve CI times (since AppVeyor runs tasks sequentially and now those tasks will be run concurrently), and should give enough extra CI capacity to test more installation paths for the R package(like R 3.6.x AND R 4.0.0x on #3065 )
Summary of Changes
lint
job from Travis to GitHub Actions.ci/download-miktex.R
, to makemiktexsetup.zip
downloads more reliableNotes for reviewers
r-package
job, since it gives us access to Visual Studio 2017. GitHub Actions only has Visual Studio 2019 (reference).ci/download-miktex.R
because maybe 1 out of every 8 builds hit this issue: [ci] peg MiKTeX mirror #3113 (comment)stderr
causes the build to fail, even if the command itself succeeds. I tracked that down to this section of the docs, which state:Other references