| Message ID | 1520910585-19097-6-git-send-email-ricardo.martincoski@gmail.com |
|---|---|
| State | Accepted |
| Commit | 14aa15a5a583cf1621880f422f27296180a3d73a |
| Headers | show |
| Series | fix Python code style v3 | expand |
Ricardo, All, On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly: > Use the latest version of the tool because it is actively maintained. > But use a fixed version of the tool and its dependencies to get stable > results. It can be manually bumped from time to time. > > Before installing any Python packages, ensure pip, setuptools, and wheel > are up to date as recommended in the docs [1]. > > [1] https://packaging.python.org/tutorials/installing-packages/ > > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Yann E. MORIN <yann.morin.1998@free.fr> Acked-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN. > --- > Changes v2 -> v3: (suggested by Yann E. MORIN) > - minimise the number of intermediate layers; > - explain why install setuptools separately using the latest version > (I actually just used the exact command line from the docs and > referenced it in the commit log); > - use a single package on each line, sorted. > > Changes v1 -> v2: (suggested by Yann E. MORIN) > - install flake8 to the base docker image instead of adding sudo to > install tools on the fly. > --- > support/docker/Dockerfile | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile > index ce3fdd9..f01ac25 100644 > --- a/support/docker/Dockerfile > +++ b/support/docker/Dockerfile > @@ -27,10 +27,19 @@ RUN dpkg --add-architecture i386 && \ > cvs bzr git mercurial subversion wget \ > cpio unzip \ > libncurses5-dev \ > - python-nose2 python-pexpect qemu-system-arm qemu-system-x86 && \ > + python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \ > + python-pip && \ > apt-get -y autoremove && \ > apt-get -y clean > > +# For check-flake8 > +RUN python -m pip install --upgrade pip setuptools wheel && \ > + pip install -q \ > + flake8==3.5.0 \ > + mccabe==0.6.1 \ > + pycodestyle==2.3.1 \ > + pyflakes==1.6.0 > + > # To be able to generate a toolchain with locales, enable one UTF-8 locale > RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \ > /usr/sbin/locale-gen > -- > 2.7.4 >
>>>>> "Ricardo" == Ricardo Martincoski <ricardo.martincoski@gmail.com> writes: > Use the latest version of the tool because it is actively maintained. > But use a fixed version of the tool and its dependencies to get stable > results. It can be manually bumped from time to time. > Before installing any Python packages, ensure pip, setuptools, and wheel > are up to date as recommended in the docs [1]. > [1] https://packaging.python.org/tutorials/installing-packages/ > Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> > Cc: Arnout Vandecappelle <arnout@mind.be> > Cc: Peter Korsgaard <peter@korsgaard.com> > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> > Cc: Yann E. MORIN <yann.morin.1998@free.fr> > --- > Changes v2 -> v3: (suggested by Yann E. MORIN) > - minimise the number of intermediate layers; > - explain why install setuptools separately using the latest version > (I actually just used the exact command line from the docs and > referenced it in the commit log); > - use a single package on each line, sorted. > Changes v1 -> v2: (suggested by Yann E. MORIN) > - install flake8 to the base docker image instead of adding sudo to > install tools on the fly. Committed, thanks.
Ricardo, All, On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly: > Use the latest version of the tool because it is actively maintained. > But use a fixed version of the tool and its dependencies to get stable > results. It can be manually bumped from time to time. [--SNIP--] > +# For check-flake8 > +RUN python -m pip install --upgrade pip setuptools wheel && \ So, back when you submnitted this patch, I ACKed it. But I missed that this command really means we can't reproduce the docker file, because it will use versions of pip, setuptools, and wheel that are current at the time the command is run. I.e. today I could very well get something that is different from the version we got back at 20180205.0730 when we last built the docker image. This is not really nice... :-( Do you think we could get away without running this command at all, and just run the pip-install one, below, since we force the version of the modules we isntall? Regards, Yann E. MORIN. > + pip install -q \ > + flake8==3.5.0 \ > + mccabe==0.6.1 \ > + pycodestyle==2.3.1 \ > + pyflakes==1.6.0 > + > # To be able to generate a toolchain with locales, enable one UTF-8 locale > RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \ > /usr/sbin/locale-gen > -- > 2.7.4 >
Hello, First of all: I don't think we need to hurry to fix it in the master branch. The release is so close. Unless of course we need to generate a new docker image right now for some reason. Do you disagree? On Sun, May 27, 2018 at 01:42 PM, Yann E. MORIN wrote: > On 2018-03-13 00:09 -0300, Ricardo Martincoski spake thusly: >> Use the latest version of the tool because it is actively maintained. >> But use a fixed version of the tool and its dependencies to get stable >> results. It can be manually bumped from time to time. > [--SNIP--] >> +# For check-flake8 >> +RUN python -m pip install --upgrade pip setuptools wheel && \ > > So, back when you submnitted this patch, I ACKed it. But I missed that > this command really means we can't reproduce the docker file, because it > will use versions of pip, setuptools, and wheel that are current at the > time the command is run. > > I.e. today I could very well get something that is different from the > version we got back at 20180205.0730 when we last built the docker > image. > > This is not really nice... :-( You are correct. Sorry I missed it too. And this is not the whole problem... I fixed the version of the direct dependencies of flake8. But a better approach for the general case would be to fix all versions, as dependencies between Python packages can include ranges: package foo 1.0 can depend on bar >= 1.1 > Do you think we could get away without running this command at all, and > just run the pip-install one, below, since we force the version of the > modules we isntall? No. But certainly we can come up with some command that generates reproducible images. By trying to install packages without that line we would get errors [1] and [2]. The only version that we don't *need* to upgrade is pip. apt-get installs 9.0.1, and the pip --upgrade installed version 9.0.2 at the time the current image was generated. An image generated today would have pip 10.0.1, demonstrating the problem you found in the current Dockerfile. Using 'pip freeze' and 'pip list' inside the current image we can dump all the current package versions [3]. Since the list of packages to install will become longer we could even move it to a separate file, as suggested in [4] and also in the example (but not the text) from the 'Best practices' guide which url you shared a while ago [5]. So a way to generate an image equivalent to the current one would be: @ support/docker/Dockerfile: # For check-flake8 COPY requirements.txt /tmp/ RUN pip install -q \ pip==9.0.2 \ setuptools==39.0.1 \ wheel==0.30.0 && \ pip install -q -r /tmp/requirements.txt @ support/docker/requirements.txt bzr==2.8.0.dev1 configobj==5.0.6 configparser==3.5.0 enum34==1.1.6 flake8==3.5.0 mccabe==0.6.1 mercurial==4.0 nose2==0.6.5 pexpect==4.2.1 ptyprocess==0.5.1 pycodestyle==2.3.1 pyflakes==1.6.0 six==1.10.0 And maybe a comment at the first line of the new file would be nice: # output from 'pip freeze' What do you think about this approach? Are you preparing a patch? [1] error message without setuptools and wheel: Collecting configparser; python_version < "3.2" (from flake8==3.5.0) Using cached https://files.pythonhosted.org/packages/7c/69/c2ce7e91c89dc073eb1aa74c0621c3eefbffe8216b3f9af9d3885265c01c/configparser-3.5.0.t ar.gz Complete output from command python setup.py egg_info: Traceback (most recent call last): File "<string>", line 1, in <module> ImportError: No module named setuptools [2] error message with setuptools but without wheel: Building wheels for collected packages: configparser Running setup.py bdist_wheel for configparser ... error Complete output from command /usr/bin/python -u -c "import setuptools, tokenize;__file__='/tmp/pip-build-HGBa5l/configparser/setup.py';f=get attr(tokenize, 'open', open)(__file__);code=f.read().replace('\r\n', '\n');f.close();exec(compile(code, __file__, 'exec'))" bdist_wheel -d /tm p/tmpAST7T9pip-wheel- --python-tag cp27: usage: -c [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...] or: -c --help [cmd1 cmd2 ...] or: -c --help-commands or: -c cmd --help error: invalid command 'bdist_wheel' [3] versions for python packages used in buildroot/base:20180318.1724 : br-user@fa92a79b0862:~$ pip freeze bzr==2.8.0.dev1 configobj==5.0.6 configparser==3.5.0 enum34==1.1.6 flake8==3.5.0 mccabe==0.6.1 mercurial==4.0 nose2==0.6.5 pexpect==4.2.1 ptyprocess==0.5.1 pycodestyle==2.3.1 pyflakes==1.6.0 six==1.10.0 You are using pip version 9.0.2, however version 10.0.1 is available. You should consider upgrading via the 'pip install --upgrade pip' command. br-user@fa92a79b0862:~$ pip list --format=columns | grep 'pip\|setuptools\|wheel' pip 9.0.2 setuptools 39.0.1 wheel 0.30.0 [4] https://unix.stackexchange.com/questions/349227/dockerfile-docker-image-and-reproducible-environment [5] https://docs.docker.com/v17.09/engine/userguide/eng-image/dockerfile_best-practices/#add-or-copy Regards, Ricardo
Ricardo, All, On 2018-05-28 00:00 -0300, Ricardo Martincoski spake thusly: > First of all: I don't think we need to hurry to fix it in the master branch. > The release is so close. Unless of course we need to generate a new docker > image right now for some reason. > Do you disagree? Absolutely no urgency at all. ;-) [--SNIP--] > > Do you think we could get away without running this command at all, and > > just run the pip-install one, below, since we force the version of the > > modules we isntall? > > No. But certainly we can come up with some command that generates reproducible > images. > > By trying to install packages without that line we would get errors [1] and [2]. > The only version that we don't *need* to upgrade is pip. apt-get installs 9.0.1, > and the pip --upgrade installed version 9.0.2 at the time the current image was > generated. > An image generated today would have pip 10.0.1, demonstrating the problem you > found in the current Dockerfile. > > Using 'pip freeze' and 'pip list' inside the current image we can dump all the > current package versions [3]. > Since the list of packages to install will become longer we could even move it > to a separate file, as suggested in [4] and also in the example (but not the > text) from the 'Best practices' guide which url you shared a while ago [5]. > > So a way to generate an image equivalent to the current one would be: > > @ support/docker/Dockerfile: > # For check-flake8 > COPY requirements.txt /tmp/ > RUN pip install -q \ > pip==9.0.2 \ > setuptools==39.0.1 \ > wheel==0.30.0 && \ > pip install -q -r /tmp/requirements.txt > > @ support/docker/requirements.txt > bzr==2.8.0.dev1 > configobj==5.0.6 > configparser==3.5.0 > enum34==1.1.6 > flake8==3.5.0 > mccabe==0.6.1 > mercurial==4.0 > nose2==0.6.5 > pexpect==4.2.1 > ptyprocess==0.5.1 > pycodestyle==2.3.1 > pyflakes==1.6.0 > six==1.10.0 > > And maybe a comment at the first line of the new file would be nice: > # output from 'pip freeze' > > What do you think about this approach? Why do we even use pip to install those? Can't we just rely on the verions actually packaged in the distro instead? I.e. apt-get install python-configobj python-configparser etc... Of course, provided that the module is indeed packaged in the distro... As far as I can see, there should be everything we need, no? The only two for which I have a doubt are bzr (python-bzrlib) and mercurial (python-hglib); all the others are in stretch-20171210. But if we apt-get install python-flake8, then it should bring all its dependencies I guess: root@39b33276de75:/# apt-get install python-flake8 Reading package lists... Done Building dependency tree Reading state information... Done The following additional packages will be installed: dh-python javascript-common libjs-jquery libjs-sphinxdoc libjs-underscore libmpdec2 libpython3-stdlib libpython3.5-minimal libpython3.5-stdlib pyflakes pyflakes3 python-configparser python-enum34 python-mccabe python-pycodestyle python-pyflakes python-setuptools python3 python3-minimal python3-pkg-resources python3-pyflakes python3.5 python3.5-minimal Suggested packages: apache2 | lighttpd | httpd python-enum34-doc python-mock python-setuptools-doc python3-doc python3-tk python3-venv python3-setuptools python3.5-venv python3.5-doc binfmt-support The following NEW packages will be installed: dh-python javascript-common libjs-jquery libjs-sphinxdoc libjs-underscore libmpdec2 libpython3-stdlib libpython3.5-minimal libpython3.5-stdlib pyflakes pyflakes3 python-configparser python-enum34 python-flake8 python-mccabe python-pycodestyle python-pyflakes python-setuptools python3 python3-minimal python3-pkg-resources python3-pyflakes python3.5 python3.5-minimal Note that I don't care if stretch packages a different version than what we curently have; I'd prefer that we use the distro-packaged versions, since we know that apt-get *is* reproducible as we use a fixed version of the distro. > Are you preparing a patch? When we agree on the directions, yes I can. Regards, Yann E. MORIN.
Hello, On Mon, May 28, 2018 at 04:44 PM, Yann E. MORIN wrote: [snip] > Why do we even use pip to install those? Can't we just rely on the > verions actually packaged in the distro instead? I.e. > > apt-get install python-configobj python-configparser etc... > > Of course, provided that the module is indeed packaged in the distro... > As far as I can see, there should be everything we need, no? Indeed. Yes. > The only two for which I have a doubt are bzr (python-bzrlib) and > mercurial (python-hglib); all the others are in stretch-20171210. Don't worry about these. They were probably installed by the apt-get command, pip only listed them. > But if we apt-get install python-flake8, then it should bring all its > dependencies I guess: Yes. We don't even need to install python-pip. [snip] > Note that I don't care if stretch packages a different version than what > we curently have; I'd prefer that we use the distro-packaged versions, > since we know that apt-get *is* reproducible as we use a fixed version > of the distro. OK. Simple and effective solution. Notice we will be downgrading flake8 from 3.5.0 (mccabe: 0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0) from 2017-10-23 to 3.2.1 (pyflakes: 1.3.0, pycodestyle: 2.2.0, mccabe: 0.5.3) from 2016-11-21 Anyone curious can see the changelog here: http://flake8.pycqa.org/en/latest/release-notes/index.html#x-release-series >> Are you preparing a patch? > > When we agree on the directions, yes I can. Reproducible images with less code and less maintenance effort in exchange of using a not-so-bleeding-edge version. Makes sense to me. I am only in doubt how future-proof is this solution, if some day we will *need* a new version, i.e. to something related to Python3 in 2020 (it's only a wild guess!, I don't know any current limitation, I don't know the internals of flake8). But then probably debian will have a newer version of flake8, so we can update the stretch version. And we can always go back to using pip if really needed. Regards, Ricardo
Ricardo, All, On 2018-05-29 00:43 -0300, Ricardo Martincoski spake thusly: > On Mon, May 28, 2018 at 04:44 PM, Yann E. MORIN wrote: > > Why do we even use pip to install those? Can't we just rely on the > > verions actually packaged in the distro instead? I.e. [--SNIP--] > >> Are you preparing a patch? > > When we agree on the directions, yes I can. > > Reproducible images with less code and less maintenance effort in exchange of > using a not-so-bleeding-edge version. Makes sense to me. OK, so I'll send a patch to implement this soilution. Thanks! :-) > I am only in doubt how future-proof is this solution, if some day we will > *need* a new version, i.e. to something related to Python3 in 2020 (it's only a > wild guess!, I don't know any current limitation, I don't know the internals of > flake8). > But then probably debian will have a newer version of flake8, so we can update > the stretch version. And we can always go back to using pip if really needed. Yes, then we can update to a newer version of the distro. There was also a wild suggestion that we provide more Docker images, for each of the mainstream distros. Because sometimes, build issues only occur on some distros and not others, e.g. arch-linux, which uses musl, sometimes break the build of some host packages, and it is difficult to diagnose and fix... I was also looking into providing additional Dockerfiles for additional distros. I'll get this in the same series. Soon. Sooooon.... ;-) Regards, Yann E. MORIN.
diff --git a/support/docker/Dockerfile b/support/docker/Dockerfile index ce3fdd9..f01ac25 100644 --- a/support/docker/Dockerfile +++ b/support/docker/Dockerfile @@ -27,10 +27,19 @@ RUN dpkg --add-architecture i386 && \ cvs bzr git mercurial subversion wget \ cpio unzip \ libncurses5-dev \ - python-nose2 python-pexpect qemu-system-arm qemu-system-x86 && \ + python-nose2 python-pexpect qemu-system-arm qemu-system-x86 \ + python-pip && \ apt-get -y autoremove && \ apt-get -y clean +# For check-flake8 +RUN python -m pip install --upgrade pip setuptools wheel && \ + pip install -q \ + flake8==3.5.0 \ + mccabe==0.6.1 \ + pycodestyle==2.3.1 \ + pyflakes==1.6.0 + # To be able to generate a toolchain with locales, enable one UTF-8 locale RUN sed -i 's/# \(en_US.UTF-8\)/\1/' /etc/locale.gen && \ /usr/sbin/locale-gen
Use the latest version of the tool because it is actively maintained. But use a fixed version of the tool and its dependencies to get stable results. It can be manually bumped from time to time. Before installing any Python packages, ensure pip, setuptools, and wheel are up to date as recommended in the docs [1]. [1] https://packaging.python.org/tutorials/installing-packages/ Signed-off-by: Ricardo Martincoski <ricardo.martincoski@gmail.com> Cc: Arnout Vandecappelle <arnout@mind.be> Cc: Peter Korsgaard <peter@korsgaard.com> Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com> Cc: Yann E. MORIN <yann.morin.1998@free.fr> --- Changes v2 -> v3: (suggested by Yann E. MORIN) - minimise the number of intermediate layers; - explain why install setuptools separately using the latest version (I actually just used the exact command line from the docs and referenced it in the commit log); - use a single package on each line, sorted. Changes v1 -> v2: (suggested by Yann E. MORIN) - install flake8 to the base docker image instead of adding sudo to install tools on the fly. --- support/docker/Dockerfile | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)