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.

Sign up for GitHub

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

Jump to bottom

CI: build OS X on master and bors merges to it #1625

Merged
merged 1 commit into from May 13, 2020
Merged

CI: build OS X on master and bors merges to it #1625

merged 1 commit into from May 13, 2020

Conversation

ppannuto
Copy link
Member

Pull Request Overview

This pull request attempts to implement the idea suggested in #1568.

Testing Strategy

This pull request was tested by... well, you're looking at it. I've successfully demonstrated with this branch that an OS X build is correctly not yet triggered, but linux builds are. Then the question will be if/when we use bors to merge this whether an OS X build runs first.

TODO or Help Wanted

If someone's more a travis guru than me and sees a flaw, holler, but I think this is right.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make formatall.

@ppannuto ppannuto requested a review from bradjc February 25, 2020 03:24
@bradjc
Copy link
Contributor

bradjc commented Feb 25, 2020

This is the most reasonable travis on mac approach I have seen. I guess I support this if we can agree to aggressively revert this if it gets in the way of merging PRs in the future.

I think it's worth noting this doesn't actually help the major (only?) issue we've had with mac v. linux: the scripts in the tools folder which use commands that differ from linux and BSD.

@alistair23
Copy link
Contributor

In libtock-rs we are starting to add run tests (running Tock and apps in QEMU). Doing that on OS X is difficult, just something else to keep in mind. Adding more obscure targets makes it harder to add more complex testing in the future.

@alevy
Copy link
Member

alevy commented Mar 3, 2020

ibex doesn't seem to compile on OS X with this change (see the Travis PR run).

@ppannuto
Copy link
Member Author

ppannuto commented Mar 4, 2020

ibex doesn't run tests successfully on my machine either -- part of what motivated this PR was that stuff has started slipping in that fails on OS X. They will need to be fixed first / concurrently.

@ppannuto ppannuto added the blocked Waiting on something, like a different PR or a dependency. label Mar 4, 2020
@ppannuto ppannuto removed the blocked Waiting on something, like a different PR or a dependency. label Apr 2, 2020
@ppannuto
Copy link
Member Author

ppannuto commented Apr 2, 2020

Okay, I believe I have an easy fix for this (as ELF care less about the characters used in section names than Mach-o, just adhere to the latter's conventions), but I'd like for someone from @tock/opentitan-wg to chime in and confirm this is an okay change.

@ppannuto ppannuto requested a review from a team April 8, 2020 02:54
@ppannuto ppannuto mentioned this pull request Apr 8, 2020
Merged
2 tasks
@ppannuto
Copy link
Member Author

@alistair23 @gkelly @silvestrst @jon-flatley @bradjc -- can someone confirm/deny that renaming the section identifier is benign?

@ppannuto ppannuto mentioned this pull request Apr 21, 2020
Merged
2 tasks
@ppannuto
Copy link
Member Author

This has been rebased on lastest master, and should be ready to go again (post 1.5)

@ppannuto ppannuto mentioned this pull request May 1, 2020
Merged
2 tasks
@hudson-ayers
Copy link
Contributor

Given that github actions does not have the issues with OSX builds that Travis does, should we just transition this to the github actions workflow instead of travis?

It could still be a separate workflow that only runs on bors merges.

If this is desired I am happy to push a commit that moves this to gh-actions instead of travis.

@ppannuto
Copy link
Member Author

ppannuto commented May 5, 2020

If there's no high-latency issue, let's just do it on every build as an action. Please do push a commit!

@brghena
Copy link
Contributor

brghena commented May 5, 2020

What's different about how Github is doing MacOS support that makes it faster? The Travis problem, I believe, was that they had less dedicated Apple machines and therefore could just not keep up with demand. Is the belief that Github is just willing to buy more machines, or just that Github actions aren't currently used enough to stress MacOS demand.

@hudson-ayers
Copy link
Contributor

What's different about how Github is doing MacOS support that makes it faster? The Travis problem, I believe, was that they had less dedicated Apple machines and therefore could just not keep up with demand. Is the belief that Github is just willing to buy more machines, or just that Github actions aren't currently used enough to stress MacOS demand.

I am basing this mostly off of the observation that OpenSK Mac builds finish as quickly as Ubuntu builds. There is also the bonus that the MacOS builds run in parallel with ubuntu builds whereas Travis ran them in series.

@brghena
Copy link
Contributor

brghena commented May 5, 2020

Alright, that seems sufficient to go with MacOS builds whenever we do Linux ones for now. If things slow down on the MacOS side again, we can go back to the current method.

@hudson-ayers
Copy link
Contributor

Hmm, this is perhaps slightly trickier than I hoped. Does OSX travis actually work for testing the tools? Its not clear to me how libusb-1.0-0-dev is installed in Travis for OSX.

Also, installing QEMU on MacOS is a whole other project, and a pain for me to iterate on because I don't have a Mac.

@ppannuto
Copy link
Member Author

ppannuto commented May 5, 2020

Admittedly this PR pre-dates the QEMU merge, but it did pass travis then: https://travis-ci.org/github/tock/tock/jobs/678628547

I can also confirm that I can run make ci-travis locally on latest master, but that could easily be a result of my machine having lots of dependencies already installed that are needed for the QEMU build. If you don't have a Mac, then I think iterating this on your end is probably silly. I'll pick up this thread again later.

@hudson-ayers
Copy link
Contributor

I went ahead and pushed what I had. This doesn't run emulation-check on mac, but runs everything else on mac. ci-tests fails because of something related to libusb -- I tried brew install libusb and that didn't fix the problem but I didn't go any further.

Hopefully this is a helpful starting point @ppannuto

@gendx
Copy link
Contributor

gendx commented May 7, 2020

What's different about how Github is doing MacOS support that makes it faster? The Travis problem, I believe, was that they had less dedicated Apple machines and therefore could just not keep up with demand. Is the belief that Github is just willing to buy more machines, or just that Github actions aren't currently used enough to stress MacOS demand.

I am basing this mostly off of the observation that OpenSK Mac builds finish as quickly as Ubuntu builds. There is also the bonus that the MacOS builds run in parallel with ubuntu builds whereas Travis ran them in series.

Yes, whatever infrastructure Github is using under the hood, it works well as we didn't notice any MacOS issues on Github actions for OpenSK, despite having a few "heavy" workflows (e.g. building the Tock boards we support to double-check on OpenSK's side).

One thing to note is that if you have multiple runners for a single workflow (e.g. ubuntu & macos), if one runner fails then the whole workflow is marked as failing and all the other runners of the workflow are immediately cancelled by Github actions. This makes sense to save resources, but can be surprising at first glance.
On the other hand, one workflow failing doesn't prevent other workflows from running to completion.

@bradjc
Copy link
Contributor

bradjc commented May 7, 2020

No latency issue?
image
That makes it look like mac takes 2x as long, with significant variance. I'm not sure using github actions changes anything.

If we do want to give up the time benefits of switching to github actions, is it possible to selectively run specific workflows on mac?

@gendx
Copy link
Contributor

gendx commented May 7, 2020

No latency issue?
image
That makes it look like mac takes 2x as long, with significant variance. I'm not sure using github actions changes anything.

Travis itself took 12 minutes to run everything. So, the fact that Linux Github actions are 2x faster than MacOS Github actions doesn't undermine the improvements of Github actions w.r.t. Travis.

Also, the emulation check seems quite slower, and runs only on Linux, so the bottleneck is more likely there than on MacOS.

IMO 10 minutes are totally acceptable (especially given that most of the time there won't be any failures specific to MacOS, so it won't be much of a bottleneck for development).

If we do want to give up the time benefits of switching to github actions, is it possible to selectively run specific workflows on mac?

Yes it's possible to split the actions into various workflows, and for each workflow decide whether to run it on MacOS or not.

gendx
gendx reviewed May 12, 2020
View reviewed changes
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@ppannuto
Copy link
Member Author

I think this is ready to go now

@bradjc bradjc changed the title travis: build OS X on master and bors merges to it CI: build OS X on master and bors merges to it May 12, 2020
@hudson-ayers hudson-ayers added the last-call Final review period for a pull request. label May 12, 2020
@bradjc bradjc merged commit 29a9329 into master May 13, 2020
@bors bors bot deleted the limited-mac-builds branch May 13, 2020 17:06
@hudson-ayers hudson-ayers mentioned this pull request May 14, 2020
Merged
2 tasks
bors bot added a commit that referenced this pull request May 14, 2020
@bors @hudson-ayers
Merge #1851
cc2f0ff 
1851: Fix bors by updating status names to include OS r=ppannuto a=hudson-ayers

### Pull Request Overview

This pull request updates the status names that Bors blocks on before merging PRs. #1625  introduced jobs which run on multiple different OSes, so Github reports job names to Bors that include the OS of the runner for that job. Bors is expecting the old names, and so eventually times out.

This PR updates the names to match the names reported by Github.

### Testing Strategy

This pull request has not been tested. The easiest test would be for someone to approve this and tell bors to merge it and see if it works -- if it works its correct, if not it doesn't merge!


### TODO or Help Wanted

N/A


### Documentation Updated

- [x] No updates are required.

### Formatting

- [x] Ran `make formatall`.


Co-authored-by: Hudson Ayers <hayers@stanford.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gendx gendx gendx left review comments

@bradjc bradjc bradjc approved these changes

@hudson-ayers hudson-ayers hudson-ayers approved these changes

Assignees
No one assigned
Labels
last-call Final review period for a pull request.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@ppannuto @bradjc @alistair23 @alevy @hudson-ayers @brghena @gendx