From 752162a434db0b4ef3a83f04d6da1456a05e70b4 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Fri, 29 May 2020 14:03:27 +0000 Subject: [PATCH 1/6] Fix two flake8 errors Signed-off-by: JoeLametta --- whipper/extern/task/task.py | 4 ++-- whipper/test/test_image_toc.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/whipper/extern/task/task.py b/whipper/extern/task/task.py index 7e08fb8d..f70e68c0 100644 --- a/whipper/extern/task/task.py +++ b/whipper/extern/task/task.py @@ -233,8 +233,8 @@ def addListener(self, listener): def _notifyListeners(self, methodName, *args, **kwargs): if self._listeners: - for l in self._listeners: - method = getattr(l, methodName) + for listener in self._listeners: + method = getattr(listener, methodName) try: method(self, *args, **kwargs) # FIXME: catching too general exception (Exception) diff --git a/whipper/test/test_image_toc.py b/whipper/test/test_image_toc.py index 1f651124..177622b3 100644 --- a/whipper/test/test_image_toc.py +++ b/whipper/test/test_image_toc.py @@ -216,7 +216,7 @@ def testMusicBrainz(self): self.assertEqual(self.toc.table.getMusicBrainzDiscId(), "KnpGsLhvH.lPrNc1PBL21lb9Bg4-") self.assertEqual(self.toc.table.getMusicBrainzSubmitURL(), - "https://musicbrainz.org/cdtoc/attach?toc=1+12+195856+150+15687+31841+51016+66616+81352+99559+116070+133243+149997+161710+177832&tracks=12&id=KnpGsLhvH.lPrNc1PBL21lb9Bg4-") # noqa: E501 + "https://musicbrainz.org/cdtoc/attach?toc=1+12+195856+150+15687+31841+51016+66616+81352+99559+116070+133243+149997+161710+177832&tracks=12&id=KnpGsLhvH.lPrNc1PBL21lb9Bg4-") # noqa: E501 # FIXME: I don't trust this toc, but I can't find the CD anymore From 2fe4292a9bf1aeb290170ad1f13c7379b968d0a7 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Fri, 29 May 2020 14:26:27 +0000 Subject: [PATCH 2/6] Update README - Updated bugs information about the `libcdio-utils` package - Added missing entries to ToC Signed-off-by: JoeLametta --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 1285ad9b..efd10495 100644 --- a/README.md +++ b/README.md @@ -24,13 +24,15 @@ In order to track whipper's latest changes it's advised to check its commit hist * [Package](#package) - [Building](#building) 1. [Required dependencies](#required-dependencies) - 2. [Fetching the source code](#fetching-the-source-code) - 3. [Finalizing the build](#finalizing-the-build) + 2. [Optional dependencies](#optional-dependencies) + 3. [Fetching the source code](#fetching-the-source-code) + 4. [Finalizing the build](#finalizing-the-build) - [Usage](#usage) - [Getting started](#getting-started) - [Configuration file documentation](#configuration-file-documentation) - [Running uninstalled](#running-uninstalled) - [Logger plugins](#logger-plugins) + * [Official logger plugins](#official-logger-plugins) - [License](#license) - [Contributing](#contributing) - [Developer Certificate of Origin (DCO)](#developer-certificate-of-origin-dco) @@ -125,7 +127,7 @@ Whipper relies on the following packages in order to run correctly and provide a - [cd-paranoia](https://github.com/rocky/libcdio-paranoia), for the actual ripping - To avoid bugs it's advised to use `cd-paranoia` versions ≥ **10.2+0.94+2-2** - - The package named `libcdio-utils`, available on Debian and Ubuntu, is affected by a bug (except for Debian testing/sid): it doesn't include the `cd-paranoia` binary (needed by whipper). For more details see: [#888053 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888053), [#889803 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=889803) and [#1750264 (Ubuntu)](https://bugs.launchpad.net/ubuntu/+source/libcdio/+bug/1750264). + - The package named `libcdio-utils`, available on Debian and Ubuntu, is affected by a bug (except for Debian testing/sid where a separate `cd-paranoia` package has been added): it doesn't include the `cd-paranoia` binary (needed by whipper). For more details see: [#888053 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=888053), [#889803 (Debian)](https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=889803) and [#1750264 (Ubuntu)](https://bugs.launchpad.net/ubuntu/+source/libcdio/+bug/1750264). - [cdrdao](http://cdrdao.sourceforge.net/), for session, TOC, pre-gap, and ISRC extraction - [GObject Introspection](https://wiki.gnome.org/Projects/GObjectIntrospection), to provide GLib-2.0 methods used by `task.py` - [PyGObject](https://pypi.org/project/PyGObject/), required by `task.py` From 0a960d991bdc1ecc51241f0ec07350bb917f1bb2 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Fri, 29 May 2020 15:56:11 +0000 Subject: [PATCH 3/6] Change docker alias in README to use '${HOME}' rather than '~' Inlcudes another unrelated change to the README. Fixes #482. Signed-off-by: JoeLametta --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index efd10495..976a2118 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ It's recommended to create an alias for a convenient usage: ```bash alias whipper="docker run -ti --rm --device=/dev/cdrom \ - --mount type=bind,source=~/.config/whipper,target=/home/worker/.config/whipper \ + --mount type=bind,source=${HOME}/.config/whipper,target=/home/worker/.config/whipper \ --mount type=bind,source=${PWD}/output,target=/output \ whipperteam/whipper" ``` @@ -97,7 +97,7 @@ You should put this e.g. into your `.bash_aliases`. Also keep in mind to substit Essentially, what this does is to map the /home/worker/.config/whipper and ${PWD}/output (or whatever other directory you specified) on your host system to locations inside the Docker container where the files can be written and read. These directories need to exist on your system before you can run the container: -`mkdir -p ~/.config/whipper "${PWD}"/output` +`mkdir -p "${HOME}/.config/whipper" "${PWD}/output"` Finally you can test the correct installation: From 8802c5482edf150a94ce430d74ad3239ce803780 Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Fri, 29 May 2020 16:24:30 +0000 Subject: [PATCH 4/6] Improve error message for unconfigured drive offset Fixes #478. Signed-off-by: JoeLametta --- whipper/command/cd.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/whipper/command/cd.py b/whipper/command/cd.py index daa11f0f..10a646f6 100644 --- a/whipper/command/cd.py +++ b/whipper/command/cd.py @@ -317,12 +317,12 @@ def handle_arguments(self): validate_template(self.options.disc_template, 'disc') if self.options.offset is None: - raise ValueError("Drive offset is unconfigured.\n" - "Please install pycdio and run 'whipper offset " - "find' to detect your drive's offset or set it " - "manually in the configuration file. It can " - "also be specified at runtime using the " - "'--offset=value' argument") + raise SystemExit( + "Error: drive offset unconfigured. Please install pycdio and " + "run 'whipper offset find' to detect your drive's offset or " + "set it manually in the configuration file. It can also be " + "specified at runtime using the '--offset=value' argument" + ) if self.options.working_directory is not None: self.options.working_directory = os.path.expanduser( From fa7c50d3a6f7299094768e6dfece1f341aee7a3e Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Sat, 30 May 2020 09:38:42 +0000 Subject: [PATCH 5/6] Replace 'sys.exit()' and 'exit()' instructions with 'SystemExit()' equivalents - `SystemExit` doesn't require importing the `sys` module - `exit()` depends on the `site` module (for this reason its usage is discouraged in production code) Signed-off-by: JoeLametta --- scripts/accuraterip-checksum | 4 ++-- whipper/__main__.py | 3 +-- whipper/command/basecommand.py | 5 ++--- whipper/command/image.py | 4 +--- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/scripts/accuraterip-checksum b/scripts/accuraterip-checksum index 9ad3999b..52a5ade0 100644 --- a/scripts/accuraterip-checksum +++ b/scripts/accuraterip-checksum @@ -7,7 +7,7 @@ import sys if len(sys.argv) == 2 and sys.argv[1] == '--version': print('accuraterip-checksum version 2.0') - exit(0) + raise SystemExit() use_v1 = None if len(sys.argv) == 4: @@ -22,7 +22,7 @@ elif len(sys.argv) == 5: if use_v1 is None: print('Syntax: accuraterip-checksum [--version / --accuraterip-v1 / --accuraterip-v2 (default)] filename track_number total_tracks') - exit(1) + raise SystemExit(1) filename = sys.argv[offset + 1] track_number = int(sys.argv[offset + 2]) diff --git a/whipper/__main__.py b/whipper/__main__.py index 6a8e40b4..98a11ae5 100644 --- a/whipper/__main__.py +++ b/whipper/__main__.py @@ -2,7 +2,6 @@ # vi:si:et:sw=4:sts=4:ts=4 import os -import sys from whipper.command.main import main @@ -11,4 +10,4 @@ # Make accuraterip_checksum be found automatically if it was built local_arb = os.path.join(os.path.dirname(__file__), '..', 'src') os.environ['PATH'] = ':'.join([os.getenv('PATH'), local_arb]) - sys.exit(main()) + raise SystemExit(main()) diff --git a/whipper/command/basecommand.py b/whipper/command/basecommand.py index 98315069..64be91cf 100644 --- a/whipper/command/basecommand.py +++ b/whipper/command/basecommand.py @@ -3,7 +3,6 @@ import argparse import os -import sys from whipper.common import config, drive @@ -109,11 +108,11 @@ def __init__(self, argv, prog_name, opts): if hasattr(self, 'subcommands'): if not self.options.remainder: self.parser.print_help() - sys.exit(0) + raise SystemExit() if not self.options.remainder[0] in self.subcommands: logger.critical("incorrect subcommand: %s", self.options.remainder[0]) - sys.exit(1) + raise SystemExit(1) self.cmd = self.subcommands[self.options.remainder[0]]( self.options.remainder[1:], prog_name + " " + self.options.remainder[0], diff --git a/whipper/command/image.py b/whipper/command/image.py index a49b2852..329f6c94 100644 --- a/whipper/command/image.py +++ b/whipper/command/image.py @@ -18,8 +18,6 @@ # You should have received a copy of the GNU General Public License # along with whipper. If not, see . -import sys - from whipper.command.basecommand import BaseCommand from whipper.common import accurip, config, program from whipper.extern.task import task @@ -63,7 +61,7 @@ def do(self): print('AccurateRip entry not found') accurip.print_report(prog.result) if not verified: - sys.exit(1) + raise SystemExit(1) class Image(BaseCommand): From ec1598e97d69dbe7cf6d1054740fd4c43f02268f Mon Sep 17 00:00:00 2001 From: JoeLametta Date: Sat, 30 May 2020 10:12:07 +0000 Subject: [PATCH 6/6] Replace 'master' with 'develop' branch in README links Signed-off-by: JoeLametta --- README.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 976a2118..8738be8c 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # Whipper -[![license](https://img.shields.io/github/license/whipper-team/whipper.svg)](https://github.com/whipper-team/whipper/blob/master/LICENSE) -[![Build Status](https://travis-ci.com/whipper-team/whipper.svg?branch=master)](https://travis-ci.com/whipper-team/whipper) +[![license](https://img.shields.io/github/license/whipper-team/whipper.svg)](https://github.com/whipper-team/whipper/blob/develop/LICENSE) +[![Build Status](https://travis-ci.com/whipper-team/whipper.svg?branch=develop)](https://travis-ci.com/whipper-team/whipper) [![GitHub (pre-)release](https://img.shields.io/github/release/whipper-team/whipper/all.svg)](https://github.com/whipper-team/whipper/releases/latest) [![IRC](https://img.shields.io/badge/irc-%23whipper%40freenode-brightgreen.svg)](https://webchat.freenode.net/?channels=%23whipper) [![GitHub Stars](https://img.shields.io/github/stars/whipper-team/whipper.svg)](https://github.com/whipper-team/whipper/stargazers) @@ -64,7 +64,7 @@ https://web.archive.org/web/20160528213242/https://thomas.apestaart.org/thomas/t ## Changelog -See [CHANGELOG.md](https://github.com/whipper-team/whipper/blob/master/CHANGELOG.md). +See [CHANGELOG.md](https://github.com/whipper-team/whipper/blob/develop/CHANGELOG.md). For detailed information, please check the commit history. @@ -80,7 +80,7 @@ You can easily install whipper without needing to care about the required depend Please note that, right now, Docker Hub only builds whipper images for the `amd64` architecture: if you intend to use them on a different one, you'll need to build the images locally (as explained below). -Alternatively, in case you prefer building Docker images locally, just issue the following command (it relies on the [Dockerfile](https://github.com/whipper-team/whipper/blob/master/Dockerfile) included in whipper's repository): +Alternatively, in case you prefer building Docker images locally, just issue the following command (it relies on the [Dockerfile](https://github.com/whipper-team/whipper/blob/develop/Dockerfile) included in whipper's repository): `docker build -t whipperteam/whipper .` @@ -155,12 +155,12 @@ Some dependencies aren't available in the PyPI. They can be probably installed u - [git](https://git-scm.com/) or [mercurial](https://www.mercurial-scm.org/) - [libdiscid](https://musicbrainz.org/doc/libdiscid) -PyPI installable dependencies are listed in the [requirements.txt](https://github.com/whipper-team/whipper/blob/master/requirements.txt) file and can be installed issuing the following command: +PyPI installable dependencies are listed in the [requirements.txt](https://github.com/whipper-team/whipper/blob/develop/requirements.txt) file and can be installed issuing the following command: `pip3 install -r requirements.txt` ### Optional dependencies -- [pillow](https://pypi.org/project/Pillow/), for completely supporting the cover art feature (`embed` and `complete` option values won't work otherwise). +- [Pillow](https://pypi.org/project/Pillow/), for completely supporting the cover art feature (`embed` and `complete` option values won't work otherwise). This dependency isn't listed in the `requirements.txt`, to install it just issue the following command: