-
Notifications
You must be signed in to change notification settings - Fork 445
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 a Dockerfile and Travis CI integration for p4c #251
Conversation
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 for setting this up.
MAINTAINER Seth Fowler <seth.fowler@barefootnetworks.com> | ||
|
||
# Install dependencies. | ||
RUN apt-get update && \ |
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.
I don't know how Docker works, but where are the other dependences for testing, such as the behavioral simulator? Testing will run without it (there is a check in configure), but will test much less.
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.
We'll be adding bmv2 as a dependence to run the tests. But we need to checkout the source and build it since we don't have ubuntu packages.
|
||
script: | ||
- docker run -w /p4c/build p4c sh -c 'make check VERBOSE=1 -j8' | ||
# We should enable this once 'make cpplint' comes back clean. |
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.
cpplint should always come back clean.
I was looking at the console output from travis for the PR #249, and for some reason after running all tests it started recompiling again. |
Regarding the issue with recompiling after tests are finished, I think this is an issue with timestamps since Travis is running this stuff in a VM. I noticed it too, but I can't reproduce it locally. I'll see what I can do going forward. |
This PR adds two things:
A Dockerfile for p4c. The Dockerfile builds against Ubuntu 14.04, which the oldest version we support. It allows us to build and test p4c in a 100% reproducible way and ensure that we don't sneak any unexpected dependencies into the build or accidentally break compatibility with older versions of GCC or Ubuntu. This is handy, but its impact will really be felt only if we run automated tests against this image, which is why this PR also includes...
Travis CI integration. The included
.travis.yml
builds and runs p4c via Docker, so if there are test failures they'll be 100% reproducible locally. Currently Travis is configured to run themake check
test suite. Once we clean things up so thatmake cpplint
comes back clean, we should enable that in.travis.yml
as well.Travis CI will run tests against every new commit automatically, so we'll get tests running against
master
, against any feature branches we may have, and most importantly against all PRs. Travis will automatically attach information to each PR so that we can immediately see whether they break the build or cause any tests to fail, before they get merged. This assists reviewers, prevents bustage from affecting the productivity of other developers, and reduces the need to run tests locally in the case of simple changes.Travis is totally free for open source projects such as this one. Setting it up is pretty easy, though I don't have the required access. @cc10512, can you do it? =) Once this PR is merged, just click on
Settings
at the top of the repo page, go toIntegrations & services
, click onAdd service
and selectTravis CI
. You then need to enable things on the Travis CI side; I believe it'll give you a link when you add the service, but basically you just need to perform steps 1 and 2 as described here. After that point, things should just work - I've already done the other steps in this PR.