Dockerfile needs GCC or alternative workaround
svenevs opened this issue · 7 comments
This could be me using things incorrectly, but on a fresh clone and install of docker tonight (I don't use it locally), the container cannot build. I don't understand the whole picture, but it's something like
- You require
pylint,pylintrequiresasteroid, andasteroidrequirestyped_ast, andtyped_astneeds GCC.- See "main" discussion here: pylint-dev/pylint#2291
I tried a couple of the fixes there but nothing I found actually got past the issue, and I just want to be able to run the tests. So what I added to the Dockerfile:
--- a/Dockerfile
+++ b/Dockerfile
@@ -6,4 +6,6 @@ COPY . /src/pytest-raises
WORKDIR /src/pytest-raises
+# typed_ast needs gcc, musl-dev provides limits.h
+RUN apk add --no-cache gcc musl-dev
RUN pip install .[develop]It takes probably noticeably longer to install gcc and musl-dev, though. Basically, there seem to be a handful of solutions, but since I don't know much about docker (do I have to sudo make build after every change for sudo make test lol) I leave it to you.
Yeah, looks like this is a new dependency issue. This is what I get for not being a bit stricter on test dependencies. I will check it out.
This is what I get for not being a bit stricter on test dependencies.
Lol, I don't think I follow -- you only depend on pytest and pylint right? This is a (very far) upstream change that affected a lot of projects, but using pylint is 👍 🙂
Good news is I think I have a reliable fix so that the docker image doesn't rebuild every time you want to make test with updated source code! The idea came from this article, where they suggest copying a requirements.txt first, installing them, then copy the whole source folder. So the diff above is bad because the installation of gcc musl-dev happens after the COPY for the whole source tree, meaning it reinstalled both every time (annoying to wait for).
The diff makes it look like everything changed, but really it's just a restructure to "simulate" copying over requirements.txt, because
- There is no
requirements.txtfor this, you only need to install two things. - The existing
setup.pycannot be used in isolation, so just copyingsetup.pyand getting theegg_infofor example won't work.
Basically, this is a valid solution, but if you ever change dependencies you're currently in a position where you have to change two files now. Not ideal, but least-impactful change:
FROM python:3.6-alpine
RUN apk add --no-cache git
RUN apk add gcc musl-dev
# Install dependencies first before copying full tree
# so docker does not re-install everything each time
# a test file is changed.
RUN mkdir -p /src/pytest-raises
RUN python3 -m pip install pytest>=3.2.2
RUN python3 -m pip install pylint==1.7.2
# Changes to source should have docker build cached up to here
WORKDIR /src/pytest-raises
COPY . /src/pytest-raises
RUN pip install .[develop]
Would you like a PR with this? I tried to include the reasoning behind all decisions because as stated docker is unfamiliar to me. If you would like a PR (heads up, another one I'm really excited about inbound too)
- Yes vs no about pinning install versions in the
Dockerfile. Are those the right versions for you if so? python3 -m pipvspip. I don't know alpine, is their a concern thatpipmay point to python2 somehow on this distro or is that the point? Right now the above is inconsistent with existingpip install .[develop], I'd like to make them be consistent in the PR 🙃
Sorry this is such a long response, I'll add a PR if it looks good to you / with some answers!
Thanks for looking into this and submitting such a detailed response and PR. I've gone through it a bit, and will try to finish my review tomorrow.
Sure thing! There's no rush on my end, now that it's on a fork somewhere I can pip install using git. It seems that currently setting outcome.excinfo in all cases is not currently supported. The pytest / pluggy folks may be able to create an official solution so I think it makes sense to wait and see what they feel is appropriate. This is a rather minute use case that they may not feel is worth the overhead to support.
I think this looks really good. I'm going to merge it, and we can deal with the issues around outcome.excinfo as we figure out a good solution to it.
@Lemmons works for me 🙂
I didn't actually add anything to the README discussing the new additions. I'll add a PR this weekend for that. Would you also like to use codecov.io? I can add the relevant stuff in Travis yaml if the repo owner adds the repo to codecov.
If you aren't sure about that I'll just do a README PR 👍