It seems like there is a major inconsistency in our Python linting infrastructure between these two categories of builds:
- source builds
- package-based builds
In case 1, for source builds, we instruct to install Flake8 via pip, and receive version 3.7
For case 2, for package-based builds, we install Flake8 via apt, and receive version 3.5
This is the first discrepancy - I ran into a hanging bug in flake8 3.5 in this PR, for valid Python syntax. The bug did not appear in either my local workspace or in the ci.ros2.org build, but did appear in the automated build.ros2.org PR build.
Next, the wiki instructions list a set of flake8 extensions to install.
python3 -m pip install -U \
argcomplete \
flake8 \
flake8-blind-except \
flake8-builtins \
flake8-class-newline \
flake8-comprehensions \
flake8-deprecated \
flake8-docstrings \
flake8-import-order \
flake8-quotes \
pytest-repeat \
pytest-rerunfailures \
pytest \
pytest-cov \
pytest-runner \
setuptools
Of the above extensions, only flake8-docstrings is present in the bionic apt repositories. So, PR builds do not enforce a subset of the linting requirements introduced by those extensions.
In conclusion, we have these problems today:
- A PR can pass automated linting, be merged, and break the CI nightly and packaging builds because it did not meet all the linter extension requirments
- A PR can break in automated PR build, but pass both locally and in CI builds.
(note that the above also applies to the pytest version, I see there is an open issue for that https://github.com/ros2/ci/issues/230)
How might we begin to address this? It’s a situation that we don’t want to stay in. Not even sure where issues might be opened, there are a few possibilities.
2 Likes
The goal on Ubuntu is to only rely on Debian packages since packages installed via pip overlay those Debian packages but don’t get updated by the regular process (apt dist-upgrade).
On Ubuntu 18.04 several of our Python dependencies are too old for what we need. So we have to fall back to install pip packages atm at least. With the upcoming Ubuntu 20.04 that shouldn’t be the case and we can stick to Debian packages again.
The fact that not all the Python packages we want to use are available as a Debian package is unfortunate. The only option I see there is that someone starts contributing them into Debian packages. I don’t think we should drop any of these flake8 plugins since the help us keeping a consistent style / catch “bad” code.
That being said about Ubuntu for macOS and Windows it will always be the case that users will install the latest versions via pip. So on those platforms we have to pass the linter tests with the latest versions (assuming we don’t want to pin specific versions which would e.g. be different between ROS distros and likely not being used by all users anyway).
So I don’t think we are able to get rid of the difference between the platforms.
That being said the obvious solution to the problem is that we are able to run tests on both types of environments for a PR / CI. Unfortunately build.ros2.org providing the PR jobs for repos only has Linux nodes. The goal is to add automatically provisioned macOS and Windows nodes to it, being able to install dependencies from binary packages (otherwise builds take way too long) as well as update the ros_buildfarm logic to support those platforms. But realistically that is a lot of work and will likely take quite some time before it becomes reality.
This makes sense to me - the “scattershot” approach should provide some consistency by providing lots of datapoints. And, I hadn’t thought about that bigger part of the problem, that those tier1 builds could be broken by PRs that pass the automated build.
Are there issues already open (on a project?) tracking adding osx and win builds for PRs?
No, not atm. Those goals are only on our internal infrastructure roadmap.
Where could this plan be publicly displayed? Having that roadmap visible is valuable to the community.
1 Like