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

migrate circleci workflows to github actions #9111

Conversation

robandpdx
Copy link
Contributor

This pull request converts the CircleCI workflows to GitHub actions workflows.

Notes

Checkout v4 does not work

The workflows use the flowtype/flow-ci:linux-x86_64 docker image. This images is based on CentOS 7, which unfortunately is missing the version of GLIBC needed to use the checkout action v4. However, checkout action v3 does work.

We could use git to do the checkout. However it's not as simple as running apt-get install -y git because the version of git on CentOS is super old. We would need to install git from source on flowtype/flow-ci:linux-x86_64 docker image. This is how the facebook/rocksdb folks did it...
evolvedbinary/docker-rocksjava@9cc4bef

No GitHub hosted linux arm64 runners

The runtests_linux_arm64 test have been removed because there are no linux arm64 runners. The github_linux_arm64 deploy job has also been removed. You could potentially run self hosted runners to support this workflow.

The artifact dist/libflowparser-linux-arm64.zip is not produced. I have remove the line that copies this artifact from the npm_pack job to aviod failure. You could use a self-hosted runner to produce this artifact, or maybe pull it from another source for this job.

Windows cache breaker

I could not get the windows cache breaker job to work. I reworked the cache key to include the runner.os, runner.arch, and hashfiles of the various files used previously. The only thing missing is the opam version. Since there is only one opam version being used currently, perhaps this is not needed. If in the future you build/test with different opam version, simply use a matrix and pass in this value and add it to the cache key.

Secrets

There are placeholders for secrets at the top level of the workflow file. It would be better to provide the secrets to each job or step as needed.

Not tested

I was unable to test the following jobs:

website_deploy
npm_deploy
github_linux
github_macos
github_macos_arm64
github_win
flow_bin_deploy
try_flow_deploy

Testing

Here is a link to the latest workflow run in my fork.

tool_test_win error

Error
Using flow binary: D:\a\flow\flow\bin\win64\flow.exe
Found 30 suites
Tests will be built in C:\Users\RUNNER~1\AppData\Local\Temp\flow\tests\81dddb1582
Running 30 suites
uncaught exception Error: spawn D:\a\flow\flow\bin\win64\flow.exe ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) {
  errno: -4058,
  code: 'ENOENT',
  syscall: 'spawn D:\\a\\flow\\flow\\bin\\win64\\flow.exe',
  path: 'D:\\a\\flow\\flow\\bin\\win64\\flow.exe',
  spawnargs: [
    'server',
    '--strip-root',
    '--debug',
    '--file-watcher',
    'none',
    '--wait-for-recheck',
    'true',
    'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\flow\\tests\\81dddb1582\\autocomplete\\1'
  ]
} Error: spawn D:\a\flow\flow\bin\win64\flow.exe ENOENT
    at ChildProcess._handle.onexit (node:internal/child_process:286:19)
    at onErrorNT (node:internal/child_process:484:16)
    at process.processTicksAndRejections (node:internal/process/task_queues:82:21) 
25hCleaning up...
Error: Process completed with exit code 1.

https://fburl.com/workplace/f6mz6tmw

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

Copy link
Contributor

@SamChou19815 SamChou19815 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase and comment out all the jobs that fail? I can start to figure out how to deal with that later.

@robandpdx robandpdx force-pushed the convert-facebook-flow-to-actions-20231222-155533 branch from d10ce85 to 28c60e6 Compare March 12, 2024 17:56
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

4 similar comments
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@robandpdx robandpdx force-pushed the convert-facebook-flow-to-actions-20231222-155533 branch from 50ee2ca to 6f67de6 Compare March 12, 2024 18:50
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@robandpdx
Copy link
Contributor Author

Can you rebase and comment out all the jobs that fail? I can start to figure out how to deal with that later.

@SamChou19815 Done!

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 64a8f3c.

SamChou19815 added a commit that referenced this pull request Jun 15, 2024
Summary:
Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Changelog: [internal]

Differential Revision: D58626390
facebook-github-bot pushed a commit that referenced this pull request Jun 15, 2024
Summary:

Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Changelog: [internal]

Differential Revision: D58626390
SamChou19815 added a commit that referenced this pull request Jun 15, 2024
Summary:
Pull Request resolved: #9158

Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Changelog: [internal]

Differential Revision: D58626390
SamChou19815 added a commit that referenced this pull request Jun 15, 2024
Summary:
Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Differential Revision: D58626390
facebook-github-bot pushed a commit that referenced this pull request Jun 15, 2024
Summary:
Pull Request resolved: #9158

Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Changelog: [internal]

Reviewed By: samwgoldman

Differential Revision: D58626390

fbshipit-source-id: 8411b1b4cb11cb6b1082311352600b3b5653c2f7
facebook-github-bot pushed a commit that referenced this pull request Jun 15, 2024
Summary:
Pull Request resolved: #9158

Separate this out from the diff that tries to bump dune build to ocaml 5

- Move away from very old docker containers
  - This requires us to install opam separately on linux jobs
- Fix the syntax of `hashFiles` expression
  - The one initially created in #9111 is completely broken...

Changelog: [internal]

Reviewed By: samwgoldman

Differential Revision:
D58626390

------------------------------------------------------------------------
(from 8411b1b4cb11cb6b1082311352600b3b5653c2f7)

fbshipit-source-id: 9f35920a6bd97c2124b337b46cae7b15a4b02281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants