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

Add Dockerfile #237

Merged
merged 5 commits into from
Sep 24, 2018
Merged

Add Dockerfile #237

merged 5 commits into from
Sep 24, 2018

Conversation

thomas-mc-work
Copy link
Contributor

Here is my second approach to adding a Docker environment.

This makes it easier to create an isolated package to build, distribute and inherit for downstream projects (e.g. Most possible Unattended Rip Workflow).

Usage example:

docker build -t whipper/whipper:0.6 -t whipper/whipper:latest .
alias whipper="docker run -ti --rm --device=/dev/cdrom \
  -v ${PWD}/config:/home/worker/.config/whipper \
  -v ${PWD}/output:/output \
  whipper/whipper"
whipper -v
whipper drive list
…

Finally I'd appreciate an upload of the current version v0.6 to Dockerhub (the public repository of prepared Docker images). I could assist in this step if desired.

@thomas-mc-work thomas-mc-work mentioned this pull request Feb 23, 2018
@JoeLametta
Copy link
Collaborator

Thanks for the PR!

With the provided Dockerfile I expect whipper to be non-working. I say that because it now depends on libcdio-paranoia instead of Xiph's cdparanoia (starting from v0.6.0) which unfortunately is wrongfully packaged in Debian (with a missing binary needed to make it useful for whipper).
Here's a recap of the situation: LINK.
BTW: libcdio-paranoia's needed binary is packaged in the deb-multimedia repository (for Debian Testing & Sid).

The easier way to fix that for Docker is probably to build both libcdio and libcdio-paranoia from their sources. Here's a script I had started writing for Travis CI which should be OK for Debian Stretch.

#!/bin/sh

set -eu

# Synchronize package index
sudo apt-get -qq update

# Build-time dependencies?
sudo apt-get -qq install autoconf libtool

# Install libcdio 0.94 from tarball
wget -O 'libcdio-0.94.tar.gz' 'https://ftp.gnu.org/gnu/libcdio/libcdio-0.94.tar.gz'
printf '96e2c903f866ae96f9f5b9048fa32db0921464a2286f5b586c0f02699710025a  libcdio-0.94.tar.gz' | sha256sum -c
tar xf libcdio-0.94.tar.gz && cd libcdio-0.94/
autoreconf -fi
./configure --disable-dependency-tracking --disable-cxx --disable-example-progs --disable-static
sudo make install
cd ../

# Install cd-paranoia from tarball
wget -O 'libcdio-paranoia-10.2+0.94+2.tar.gz' 'https://ftp.gnu.org/gnu/libcdio/libcdio-paranoia-10.2+0.94+2.tar.gz'
printf 'd60f82ece97eeb92407a9ee03f3499c8983206672c28ae5e4e22179063c81941  libcdio-paranoia-10.2+0.94+2.tar.gz' | sha256sum -c
tar xf libcdio-paranoia-10.2+0.94+2.tar.gz && cd libcdio-paranoia-10.2+0.94+2/
autoreconf -fi
./configure --disable-dependency-tracking --disable-example-progs --disable-static
sudo make install
cd ../

@thomas-mc-work
Copy link
Contributor Author

With the provided Dockerfile I expect whipper to be non-working.

For me it was working. Are you only relying on libcdio-paranoia 0.94 or are you able to use both versions?

I hope the packaging bug (in case it is one actually) will get resolved soon to get over this uncertain situation.

@JoeLametta
Copy link
Collaborator

@thomas-mc-work I've tested this yesterday against master's head and it seems to have troubles...

  1. It didn't find my CD-DA drive (the test machine has got only one):
CRITICAL:whipper.command.basecommand:No CD-DA drives found!
Traceback (most recent call last):
  File "/usr/local/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.6.0', 'console_scripts', 'whipper')()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/main.py", line 39, in main
    cmd = Whipper(sys.argv[1:], os.path.basename(sys.argv[0]), None)
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 102, in __init__
    self.options
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 102, in __init__
    self.options
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 72, in __init__
    raise IOError(msg)
IOError: No CD-DA drives found!
  1. Whipper complains about the missing cd-paranoia dependency and won't proceed ripping (as expected).
Traceback (most recent call last):
  File "/usr/local/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.6.0', 'console_scripts', 'whipper')()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/main.py", line 40, in main
    ret = cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/basecommand.py", line 124, in do
    return self.cmd.do()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/command/debug.py", line 265, in do
    version = cdparanoia.getCdParanoiaVersion()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/program/cdparanoia.py", line 567, in getCdParanoiaVersion
    return getter.get()
  File "/usr/local/lib/python2.7/dist-packages/whipper-0.6.0-py2.7.egg/whipper/common/common.py", line 304, in get
    raise MissingDependencyException(self._dep)
whipper.common.common.MissingDependencyException: cd-paranoia

Cheers,
Joe

@JoeLametta
Copy link
Collaborator

@thomas-mc-work Sorry, ignore the first error: it's probably my fault as I've run the generated docker image without the --device command line argument.

Copy link
Collaborator

@JoeLametta JoeLametta left a comment

Choose a reason for hiding this comment

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

Whipper complains about the missing cd-paranoia dependency and won't proceed ripping.

@thomas-mc-work
Copy link
Contributor Author

Sorry for the long delay. I'll check it next week.

@thomas-mc-work
Copy link
Contributor Author

thomas-mc-work commented Apr 13, 2018

But the build process has finished without errors? (docker build -t whipper/whipper:0.6 -t whipper/whipper:latest .)

So now I get the same error. I think I'll try to build the image based on either debain/testing or ubuntu/18.04 which both are shipping with the required dependency.

@thomas-mc-work
Copy link
Contributor Author

@JoeLametta When I look into the Debian Package cdparanoia then I can find the binary cdparanoia. But it's written without a dash between the words. But whipper expects it different as can be seen here. Maybe it'd be enough to try both versions, cdparanoia || cd-paranoia? Where is the version with the dash actually from? I couldn't find this notation even in older packages.

The actual situation leads to an expected error:

whipper.common.common.MissingDependencyException: cd-paranoia

@thomas-mc-work
Copy link
Contributor Author

thomas-mc-work commented Jul 27, 2018

Ok, I've found the issue which is addressing this issue. It's confusing to see cd-paranoia among the files being installed on the Debian packages site. But finally the file is missing. Did someone ever filed an issue in the Debian bug tracker or tried to contact the maintainer/packager?

@thomas-mc-work
Copy link
Contributor Author

I've send an e-mail. But there's not much hope it'll be fixed soon:

O: This package has been orphaned and needs a maintainer.

😢

@thomas-mc-work
Copy link
Contributor Author

I'v tried the manual way as you've described above @JoeLametta. That one seems to be working. I'd love to hear from your test results!

This version is hopefully just an intermediate step to a proper solution based on the fixed upstream packages by Debain.

@JoeLametta
Copy link
Collaborator

@thomas-mc-work Will test it ASAP. As the dockerfile is compiling libcdio itself, I'd recommend to use libcdio 2.0.0 instead and pip install pycdio 2.0.0 (cd-paranoia is already at the latest version available).

Cheers,
Joe

@thomas-mc-work
Copy link
Contributor Author

Ok, i'll update libcdio and make at the beginning of the next week.

pycdio is already at 2.0.0:

# pip list
…
pycdio (2.0.0)

@JoeLametta
Copy link
Collaborator

JoeLametta commented Sep 8, 2018

@thomas-mc-work I've updated the Dockerfile, hopefully it makes sense.

FROM debian:buster

RUN apt-get update \
  && apt-get install -y autoconf cdrdao curl eject flac gir1.2-glib-2.0 \
  libiso9660-dev libsndfile1-dev libtool locales make pkgconf python-cddb \
  python-gi python-musicbrainzngs python-mutagen python-pip python-requests \
  python-setuptools sox swig && pip install pycdio==2.0.0

# libcdio-utils, libcdio-dev are actually required
RUN curl -o 'libcdio-2.0.0.tar.gz' 'https://ftp.gnu.org/gnu/libcdio/libcdio-2.0.0.tar.gz'
RUN printf '1b481b5da009bea31db875805665974e2fc568e2b2afa516f4036733657cf958  libcdio-2.0.0.tar.gz' | sha256sum -c
RUN tar xf libcdio-2.0.0.tar.gz && cd libcdio-2.0.0/ \
&& autoreconf -fi \
&& ./configure --disable-dependency-tracking --disable-cxx --disable-example-progs --disable-static \
&& make install

# Install cd-paranoia from tarball
RUN curl -o 'libcdio-paranoia-10.2+0.94+2.tar.gz' 'https://ftp.gnu.org/gnu/libcdio/libcdio-paranoia-10.2+0.94+2.tar.gz'
RUN printf 'd60f82ece97eeb92407a9ee03f3499c8983206672c28ae5e4e22179063c81941  libcdio-paranoia-10.2+0.94+2.tar.gz' | sha256sum -c
RUN tar xf libcdio-paranoia-10.2+0.94+2.tar.gz && cd libcdio-paranoia-10.2+0.94+2/ \
&& autoreconf -fi \
&& ./configure --disable-dependency-tracking --disable-example-progs --disable-static \
&& make install

RUN ldconfig

# install whipper
RUN mkdir /whipper
COPY . /whipper/
RUN cd /whipper/src && make && make install \
  && cd /whipper && python2 setup.py install \
  && rm -rf /whipper

# add user
RUN useradd -m worker -G cdrom \
  && mkdir -p /output /home/worker/.config/whipper \
  && chown worker: /output /home/worker/.config/whipper
VOLUME ["/home/worker/.config/whipper", "/output"]

# setup locales + cleanup
RUN echo "LC_ALL=en_US.UTF-8" >> /etc/environment \
  && echo "en_US.UTF-8 UTF-8" >> /etc/locale.gen \
  && echo "LANG=en_US.UTF-8" > /etc/locale.conf \
  && locale-gen en_US.UTF-8 \
  && apt-get clean && apt-get autoremove -y

ENV LC_ALL=en_US.UTF-8
ENV LANG=en_US
ENV LANGUAGE=en_US.UTF-8
ENV PYTHONIOENCODING=utf-8

USER worker
WORKDIR /output
RUN whipper -v
ENTRYPOINT ["whipper"]

I've tested it: apart from some permission errors, regarding both the output and config folders (fixed with a chmod), it seems to works as expected. I've only run the commands whipper drive analyze and whipper cd rip ....

P.S. Don't know if there are redundant things included in the Dockerfile.

@thomas-mc-work
Copy link
Contributor Author

I've updated the Dockerfile. My test were all successful. I've also simplified the instructions further.

@JoeLametta JoeLametta merged commit 0a762e2 into whipper-team:master Sep 24, 2018
@JoeLametta
Copy link
Collaborator

Merged, thanks!
Will add a mention about this Dockerfile in whipper's README: could you provide a usage example?

@thomas-mc-work
Copy link
Contributor Author

Fantastic! Yes' I'll create another PR for that (hopefully still today).

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