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

Travis CI revamp part 1, refs #2008 #2011

Merged
merged 1 commit into from
Oct 31, 2019

Conversation

AndreMiras
Copy link
Member

Revamping Travis and Docker setup introducing a Makefile.
The idea is to move the CI complexity from .travis.yml to Makefile.
That makes a single entry point via make command and reproducible
builds via Docker.
It makes it easy to run some commands outside docker, such as:

make testapps/python3/armeabi-v7a

Or the same command inside docker:

make docker/run/make/testapps/python3/armeabi-v7a

This pull request also starts introducing some docker layer cache
optimization as needed by #2009 to speed up docker pull/push and
rebuilds from cache.
It also introduces other Docker images good practices like ordering
dependencies alphabetically or always enforcing apt update prior
install, refs:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/
Subsequent pull requests would simplify the process furthermore and
leverage the cache to speed up builds.

Revamping Travis and Docker setup introducing a `Makefile`.
The idea is to move the CI complexity from .travis.yml to `Makefile`.
That makes a single entry point via `make` command and reproducible
builds via Docker.
It makes it easy to run some commands outside docker, such as:
```sh
make testapps/python3/armeabi-v7a
```
Or the same command inside docker:
```sh
make docker/run/make/testapps/python3/armeabi-v7a
```
This pull request also starts introducing some docker layer cache
optimization as needed by kivy#2009 to speed up docker pull/push and
rebuilds from cache.
It also introduces other Docker images good practices like ordering
dependencies alphabetically or always enforcing `apt update` prior
install, refs:
https://docs.docker.com/develop/develop-images/dockerfile_best-practices/
Subsequent pull requests would simplify the process furthermore and
leverage the cache to speed up builds.
@AndreMiras AndreMiras force-pushed the feature/revamp_travis_file branch from a975015 to c7d76e4 Compare October 29, 2019 21:39
@AndreMiras
Copy link
Member Author

I'm pretty happy with the level of simplification of the .travis.yml file. I think there's still room for improvements in the tox part, but let's see that after we merge that one.
I could also leverage some docker caching, which seems to speed up each docker build slightly (~2 minutes). This is because we still get some cache bust due to a bug/feature with the Docker COPY command. This can be overcome to speed things furthermore, by pushing the image from Travis on merge to develop. But let's address it in subsequent pull request.
I also think we can speed up docker build, pull and push to some nice extent by optimizing the image, e.g. using multi-stage builds (e.g. lighter image for CI), also refs #2012

@AndreMiras AndreMiras requested a review from opacam October 30, 2019 11:48
Comment on lines -59 to -61
# Run a background process to make sure that travis will not kill our tests in
# case that the travis log doesn't produce any output for more than 10 minutes
- while sleep 540; do echo "==== Still running (travis, don't kill me) ===="; done &
Copy link
Member

Choose a reason for hiding this comment

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

I suspect that we still may need this lines, not for the basic tests we make but for rebuild_updated_recipes whenever we touch some of the most time demanding recipes (icu, boost+libtorrent...). Anyway, we can leave it out, for now, and reintroduce it later in case that we need it...if not...better for us...less code to maintain and more readable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point for the demanding recipes. Yes I would try without for now as it's more readable, but I can definitely try with some of theses recipes on my fork to see if it builds for too long without providing any output

--requirements libffi,sdl2,pyjnius,kivy,python3,openssl,requests,sqlite3,setuptools
--arch=armeabi-v7a
- <<: *testing
script: make testapps/python3/armeabi-v7a PYTHON_WITH_VERSION=python3
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to pass PYTHON_WITH_VERSION=python3?

We default to python3, so maybe we can remove it right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question, on osx I had to put it because the python3.6 binary itself didn't exist, making it fail when generating the virtualenv: virtualenv --python=$(PYTHON_WITH_VERSION) $(VIRTUAL_ENV).
It's just than in osx they don't seem to symlink it

Comment on lines +53 to +85
autoconf \
automake \
build-essential \
ccache \
cmake \
gettext \
git \
lbzip2 \
libffi-dev \
libgtk2.0-0:i386 \
libidn11:i386 \
libltdl-dev \
libncurses5:i386 \
libpangox-1.0-0:i386 \
libpangoxft-1.0-0:i386 \
libstdc++6:i386 \
libtool \
openjdk-8-jdk \
patch \
pkg-config \
python \
python-pip \
python3 \
python3-dev \
python3-pip \
python3-venv \
sudo \
unzip \
virtualenv \
wget \
zip \
zlib1g-dev \
zlib1g:i386 \
Copy link
Member

Choose a reason for hiding this comment

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

Nice 😄...far more readable sorted and each dependency in one line 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

Comment on lines +38 to +39
python setup_testapp_python2_sqlite_openssl.py apk --sdk-dir $(ANDROID_SDK_HOME) --ndk-dir $(ANDROID_NDK_HOME) \
--requirements sdl2,pyjnius,kivy,python2,openssl,requests,sqlite3,setuptools,numpy
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick: I would put each argument in one line, maybe even sorted, as you did with the apt packages (the same for other tests)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes that could make sense too. I'm not sure how the --requirements argument would digest spaces and line breaks between the commas.
I hope you don't mind I try to address this point in subsequent PR as I'm excited for merging this one 😅

Copy link
Member

@opacam opacam left a comment

Choose a reason for hiding this comment

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

Thanks @AndreMiras, really nice work 👍

@AndreMiras AndreMiras merged commit 0d20e2a into kivy:develop Oct 31, 2019
@AndreMiras AndreMiras deleted the feature/revamp_travis_file branch October 31, 2019 07:12
@AndreMiras AndreMiras mentioned this pull request Nov 9, 2019
opacam added a commit that referenced this pull request Nov 17, 2019
* [bug] Fix rebuild_updated_recipes

* [pep8] Shorten long line for `zope_interface` recipe

To test `rebuild_updated_recipes`
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.

2 participants