Skip to content

Convert CI from travis + appveyor to github actions#2646

Merged
sbc100 merged 4 commits intomasterfrom
github_actions
Apr 21, 2020
Merged

Convert CI from travis + appveyor to github actions#2646
sbc100 merged 4 commits intomasterfrom
github_actions

Conversation

@sbc100
Copy link
Copy Markdown
Member

@sbc100 sbc100 commented Feb 7, 2020

See: #2356

@kripken
Copy link
Copy Markdown
Member

kripken commented Feb 7, 2020

Would actions be faster? (Is there data here already?)

@sbc100
Copy link
Copy Markdown
Member Author

sbc100 commented Feb 7, 2020

From our experience on wabt they are way faster than both appveyor and travis. @binji can attest. We basically don't see any queuing time. Plus they seem really good at aggressively killing stale jobs when you push new versions of PRs.

There are lots of reasons for wanting to move away from travis, and we considered circleci too, but the consensus seems to be github actions (see the linked issue).

@sbc100 sbc100 force-pushed the github_actions branch 2 times, most recently from ab44263 to 78d4091 Compare February 7, 2020 01:42
Copy link
Copy Markdown
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

sgtm

@sbc100 sbc100 mentioned this pull request Apr 16, 2020
@sbc100 sbc100 force-pushed the github_actions branch 5 times, most recently from cd10641 to 893e6f3 Compare April 17, 2020 00:42
@sbc100 sbc100 changed the title [WIP] Add initial github actions Convert CI from travis + appveyor to github actions Apr 20, 2020
The intention is to move away from travis and appveyor which have
become very slow.

See: #2356
@sbc100
Copy link
Copy Markdown
Member Author

sbc100 commented Apr 21, 2020

This is now ready for to land. Can you take anther look? I'm synchronously removing both travis and appveyor now.

@sbc100
Copy link
Copy Markdown
Member Author

sbc100 commented Apr 21, 2020

Sadly there a fair amount of dupliction here between the santizer jobs, but hopefully github actions will add the ability to share/inherit at some point: https://github.community/t5/GitHub-Actions/reusing-sharing-inheriting-steps-between-jobs-declarations/td-p/37849

Copy link
Copy Markdown
Member

@kripken kripken left a comment

Choose a reason for hiding this comment

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

lgtm but it would be good to confirm, if you haven't already, that intentional breaking of tests causes proper errors to be reported (e.g., I don't know github actions well enough to be sure there isn't a typo here preventing the sanitizer flags from being passed, or that even some of these jobs are not run).

python-version: '3.x'
- uses: actions/checkout@v1
with:
submodules: true
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

do we have submodules?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, you are right we could remove those lines. Its harmless though.

if: matrix.os == 'macos-latest'

- name: install ninja (win)
run: choco install ninja
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there's more package managers for windows than I had any idea...

if: matrix.os == 'windows-latest'

- name: mkdir
run: mkdir -p out
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

how does this work on windows?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think the actions run under the git-bash shell by default which is some kind MINGW/MSYS version of bash.

Copy link
Copy Markdown
Contributor

@dcodeIO dcodeIO Apr 22, 2020

Choose a reason for hiding this comment

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

Sorry, late to the party. If you type this into cmd, you'll get two directories -p and out. So this might be what's happening here. Iirc Windows images run PowerShell.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just learned: Under PowerShell mkdir -p out is an alias of mkdir -Path out, which apparently does what mkdir -p does. So, all good.

@sbc100 sbc100 merged commit 8e017e9 into master Apr 21, 2020
@sbc100 sbc100 deleted the github_actions branch April 21, 2020 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants