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

Add Github Actions #1880

Merged
merged 1 commit into from
Nov 28, 2019
Merged

Add Github Actions #1880

merged 1 commit into from
Nov 28, 2019

Conversation

Luni-4
Copy link
Collaborator

@Luni-4 Luni-4 commented Nov 27, 2019

As requested by @lu-zero, I've ported most of the CI tasks to the Github Actions system with the exception of the arm tests that will remain on Travis and AppVeyor.

The "Build and Coveralls" test is now done using grcov instead of kcov and can be run only on the nightly because of the -Z profile feature.

The entire CI process takes approximately 25 minutes.

Thanks in advance for your review! :)

@barrbrain
Copy link
Collaborator

My initial feedback is that it would be good to have a transition period. To that point, we should keep the existing Travis builds where they do not conflict (e.g. pushing artifacts) with GitHub Actions. During such a transition we could contrast the CI systems and optimise our new configuration.

Following are some considerations for a wholesale switch:

With Travis, we have seen great benefit from sccache integration as our primary caching method.
There are other Rust projects using it in combination with GitHub Actions.

We could skip building libaom and dav1d by using packages from Debian Sid and Debian Multimedia:

Considering the above, we might benefit from registering a Docker image with our build-deps preinstalled.

@Luni-4
Copy link
Collaborator Author

Luni-4 commented Nov 28, 2019

First off, thanks for your review! I forgot to show you the effective results of this PR, you can find them here.

My initial feedback is that it would be good to have a transition period. To that point, we should keep the existing Travis builds where they do not conflict (e.g. pushing artifacts) with GitHub Actions. During such a transition we could contrast the CI systems and optimise our new configuration.

It seems fair. I will leave the old CI system as-is then. For what concerns pushing the artifacts, I had already implemented that feature through Github Actions. I was thinking to open a new PR for that, but perhaps I could merge that here. What do you think about it @barrbrain ?

Following are some considerations for a wholesale switch:

With Travis, we have seen great benefit from sccache integration as our primary caching method.
There are other Rust projects using it in combination with GitHub Actions.

Github Actions implements an independent cache system using an action called cache. I could try both sscache and cache in order to see which one is better. But what should I cache though? The libaom and libdav1d packages? Anyway, building libaom from scratch takes 1 minute, libdav1d 46 seconds.

We could skip building libaom and dav1d by using packages from Debian Sid and Debian Multimedia:

* https://packages.debian.org/sid/libaom-dev

* https://packages.debian.org/sid/libaom0

* http://www.deb-multimedia.org/dists/unstable/main/binary-amd64/package/libdav1d-dev

* http://www.deb-multimedia.org/dists/unstable/main/binary-amd64/package/libdav1d3

I agree, we should be able to speed up a bit the entire process in this way.

Considering the above, we might benefit from registering a Docker image with our build-deps preinstalled.

We could add that later in my opinion, let's see how it works without Docker first.

@coveralls
Copy link
Collaborator

Coverage Status

Coverage remained the same at 75.657% when pulling a11ce64 on Luni-4:github-actions into 32e0b52 on xiph:master.

Copy link
Collaborator

@barrbrain barrbrain left a comment

Choose a reason for hiding this comment

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

This is a good baseline, let's iterate from here.

@barrbrain
Copy link
Collaborator

It seems fair. I will leave the old CI system as-is then. For what concerns pushing the artifacts, I had already implemented that feature through Github Actions. I was thinking to open a new PR for that, but perhaps I could merge that here. What do you think about it @barrbrain ?

Let's follow that up in a new PR.

Github Actions implements an independent cache system using an action called cache. I could try both sscache and cache in order to see which one is better. But what should I cache though? The libaom and libdav1d packages? Anyway, building libaom from scratch takes 1 minute, libdav1d 46 seconds.

The libaom and libdav1d builds do benefit from caching but they are not the main target. The Rust compiler does a lot of work, and we repeat most of it between builds for dependencies. Other than the compiler output, almost all artifacts are downloaded. (Mostly via git or binary distribution.) We have found that explicitly caching only the compiler outputs is a good compromise between blocking on CPU cycles and blocking on network access.
I expect that the sccache binary can cooperate with the cache action to form a solution. I will likely follow that up in a new PR.

@barrbrain barrbrain merged commit a03237a into xiph:master Nov 28, 2019
@Luni-4 Luni-4 deleted the github-actions branch November 28, 2019 13:20
@Luni-4
Copy link
Collaborator Author

Luni-4 commented Nov 28, 2019

Thanks for landing it!

The libaom and libdav1d builds do benefit from caching but they are not the main target. The Rust compiler does a lot of work, and we repeat most of it between builds for dependencies. Other than the compiler output, almost all artifacts are downloaded. (Mostly via git or binary distribution.) We have found that explicitly caching only the compiler outputs is a good compromise between blocking on CPU cycles and blocking on network access.
I expect that the sccache binary can cooperate with the cache action to form a solution. I will likely follow that up in a new PR.

I see, thanks for the explanation! I will try to fiddle with them

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