-
Notifications
You must be signed in to change notification settings - Fork 75
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
CI: try Github Actions as CI provider #688
Conversation
GH Actions initial setup, building on all platforms
395b314
to
6e9433e
Compare
Win does not handle #TODO at end of pip install -r requirements.txt #TODO
7886ca3
to
21da78c
Compare
which is needed for OSX
21da78c
to
cbe73a4
Compare
as it's the common location and easier to find for new developers.
6b60eb4
to
46514b2
Compare
python setup.py test runs both c++ and python tests
on Linux
as in CI, other platforms run slow.
4e54cbb
to
346cbdd
Compare
e2cd0a1
to
94e482a
Compare
uses ubuntu-latest as platform
94e482a
to
4cf5087
Compare
@dkeeney some more things we need to do for this PR:
For next PR will be:
|
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.
@dkeeney please review, when you have time. I'd like to merge to test the new Release & PYPI functionality.
Some overview from porting to Github Actions:
Some things are much easier now: 👍
- publishing to PYPI
- binary Releases to Github
- multi-platform build using a matrix of all OSs (Linux, OSX, Win)
More problematic parts:
- setting custom c++ compiler (eg gcc-8 for Ubuntu)
- this is due to each step being "blank new", not inheriting
env
from other steps (bcs steps can run in parallel)
- this is due to each step being "blank new", not inheriting
- passing env variables is not possible
Overall, GitHub Actions is quite great CI! I had the multiplatform build running within an hour. (then spent hours on debugging the Debug and gcc-8 issues). Highlights: multiplatform support, integration with services and GH (native).
Other changes:
- python setup.py test now runs also the c++ tests (for convenience)
- split requirements/extras dependencies
- name: Install dependencies (gcc-8) | ||
if: matrix.os == 'ubuntu-18.04' | ||
env: | ||
CC: gcc-8 |
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.
setting custom gcc (gcc-8, so we use c++17 and not :boost) is quite challenging here
python -m pip install --upgrade pip setuptools wheel | ||
|
||
- name: build htmcore with setup.py | ||
run: python setup.py install --user |
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.
multi-platform Release build is darn easy now!
cmake --build . --config Release --target package # make package | ||
|
||
- name: Release (deploy) | ||
# from https://github.com/marketplace/actions/gh-release |
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.
GH Release is easy now
ls dist | ||
|
||
- name: Publish to PyPI | ||
if: github.event_name == 'push' && startsWith(github.event.ref, 'refs/tags') |
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.
PYPI is easy
.github/workflows/htmcore.yml
Outdated
|
||
|
||
build-arm64-docker: | ||
name: Build for ARM64 on Docker |
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.
ARM64 build
python-dev \ | ||
python-numpy \ | ||
python-pip | ||
python3-minimal \ |
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.
updated our Dockerfile to python3
@@ -64,7 +64,7 @@ Fork or download the HTM-Community htm.core repository from https://github.com/h | |||
``` | |||
cd to-repository-root | |||
python -m pip install --user --upgrade pip setuptools setuptools-scm wheel | |||
python -m pip install --no-cache-dir --user -r bindings/py/packaging/requirements.txt | |||
python -m pip install --no-cache-dir --user -r requirements.txt |
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.
moved requirements here, TODO might need move back (for pypi dist/ build)
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.
Yes, as I mentioned before, in order to get the requirements.txt file into the wheel it needs to somehow get into the Release/distr/dist/
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.
fixed by copying /requirements.txt to build/Release/distr/dist during make install
step
@@ -340,7 +347,13 @@ def generateExtensions(platform, build_type): | |||
"htm.bindings": ["*.so", "*.pyd"], | |||
"htm.examples": ["*.csv"], | |||
}, | |||
extras_require = {}, | |||
#install extras by `pip install htm.core[examples]` | |||
extras_require={'scikit-image>0.15.0':'examples', |
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.
split requirements.txt to hard-core dependencies (requirements.txt) , and optional (in setup.py extras_required)
This is due to Arm64 which failed on some of the new pip dependencies (scikit ?)
#ifndef _MSC_VER | ||
const size_t CI_avg_time = (size_t)floor(20*Timer::getSpeed()); //sec | ||
#ifdef NTA_OS_LINUX | ||
const size_t CI_avg_time = (size_t)floor(99*Timer::getSpeed()); //sec //FIXME the CI speed broken for docker linux |
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.
TODO re-enable the time check? Time was agan different on the new CI
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.
Great job. I am glad you were able to pull this together. 🥇
had problem with ../../requirements.txt on Windows, so now call it first (=from ./requirements.txt)
as it's quite time consuming, and needed only for release
as ARM build takes too long (4hrs, due to docker emulation), make it build only nightly (not on each PR)
copy it there during install, needed for pypi
d9292f9
to
85b4e23
Compare
85b4e23
to
5a8f08e
Compare
can call python setup.py configure which installs c++ deps and runs cmake
known issue in 3rd party dependency.
dd9d062
to
7166ffd
Compare
8ec3db0
to
257267f
Compare
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 will now merge, all CI are now passing green 💚
Next steps:
- check PYPI
- check GH Releases
- make GH Actions CI mandatory and other CI only optional
|
||
jobs: | ||
build-arm64-docker: | ||
name: Build for ARM64 on Docker |
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.
ARM build takes 4h, so separated to a standalone file and runs on schedure (nightly at 6am)
|
||
# build: make && make install | ||
if platform != "windows": #TODO since cmake 3.12 "-j4" is directly supported (=crossplatform), for now -- passes other options to make | ||
subprocess.check_call(["cmake", "--build", ".", "--target", "install", "--config", build_type, "--", "-j", "4"]) |
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 now call cmake/make with implicit -j4
, this really speeds up builds using python setup.py
for users and developers.
- on CI the build time is 2x faster (down to 6-10min, from 20+)
- crossplatform approach since cmake 3.12, but I don't find it reasonable to bump dependency, it's too new yet.
- Windows slows down the green PRs now, I tried building with
/MT
instead of/MD
(which I found is equivalent of usingmake -j
, but then unittests complain to be compiled with /MD, so reverted for now. FYI @dkeeney
GH Actions initial setup, building on all platforms
Other:
Fixes #612
Fixes #19
Fixes #361