From 311cc557ff5c5c2269f5f691c46274f2d81734f4 Mon Sep 17 00:00:00 2001
From: blueblots <63152708+blueblots@users.noreply.github.com>
Date: Sat, 30 Jan 2021 23:38:12 +0000
Subject: [PATCH 1/7] Added --keep-going option to cd rip command

Implemented the option (`-k`, `--keep-going`) to continue ripping
the CD even if one track fails to rip (as @xmixahlx suggested in #128).

Requested in #128
Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>

Changed line-lengths/indentation of some code

Travis-CI was failing on account of lines being
under-indented or too long, this should correct it.

Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>
---
 whipper/command/cd.py | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index 853902b..12ee1d2 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -310,6 +310,11 @@ Log files will log the path to tracks relative to this directory.
                                  "{}; 0 means "
                                  "infinity.".format(DEFAULT_MAX_RETRIES),
                                  default=DEFAULT_MAX_RETRIES)
+        self.parser.add_argument('-k', '--keep-going',
+                                 action='store_true',
+                                 help="continue ripping further tracks "
+                                 "instead of giving up if a track "
+                                 "can't be ripped")
 
     def handle_arguments(self):
         self.options.output_directory = os.path.expanduser(
@@ -476,9 +481,14 @@ Log files will log the path to tracks relative to this directory.
                     tries -= 1
                     logger.critical('giving up on track %d after %d times',
                                     number, tries)
-                    raise RuntimeError("track can't be ripped. "
-                                       "Rip attempts number is equal to %d",
-                                       self.options.max_retries)
+                    if self.options.keep_going:
+                        logger.warning("track %d failed to rip. "
+                                       "Continuing to next track", number)
+                    else:
+                        raise RuntimeError("track can't be ripped. "
+                                           "Rip attempts number is equal "
+                                           "to %d",
+                                           self.options.max_retries)
                 if trackResult.testcrc == trackResult.copycrc:
                     logger.info('CRCs match for track %d', number)
                 else:

From ae596df8343f1aa93544636367a4d77a54a8caf6 Mon Sep 17 00:00:00 2001
From: blueblots <63152708+blueblots@users.noreply.github.com>
Date: Sun, 7 Feb 2021 17:34:30 +0000
Subject: [PATCH 2/7] Fixed error and added log output

Made changes to fix `KeyError` and implemented logging
of skipped tracks. For instance:

if any tracks are skipped, the log will show:
`Health status: Some tracks were not ripped (skipped)`.

the tracks that are skipped will show:
`Status: Track not ripped (skipped)`.

Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>
---
 whipper/command/cd.py    | 40 ++++++++++++++++++++++++++++++----------
 whipper/result/logger.py |  9 ++++++++-
 whipper/result/result.py |  1 +
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index 12ee1d2..2c9a407 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -220,6 +220,9 @@ class Info(_CD):
 class Rip(_CD):
     summary = "rip CD"
     # see whipper.common.program.Program.getPath for expansion
+    skipped_tracks = []
+    # this holds tracks that fail to rip -
+    # currently only used when the --keep-going option is used
     description = """
 Rips a CD.
 
@@ -483,21 +486,32 @@ Log files will log the path to tracks relative to this directory.
                                     number, tries)
                     if self.options.keep_going:
                         logger.warning("track %d failed to rip. "
-                                       "Continuing to next track", number)
+                                       "Continuing to next track" % number)
+                        logger.debug("adding %s to skipped_tracks"
+                                     % trackResult.filename)
+                        self.skipped_tracks.append(trackResult.filename)
+                        logger.debug(f"skipped_tracks = {self.skipped_tracks}")
+                        trackResult.skipped = True
+                        logger.debug('trackResult.skipped = True')
                     else:
                         raise RuntimeError("track can't be ripped. "
                                            "Rip attempts number is equal "
                                            "to %d",
                                            self.options.max_retries)
-                if trackResult.testcrc == trackResult.copycrc:
-                    logger.info('CRCs match for track %d', number)
+                if trackResult.filename in self.skipped_tracks:
+                    print("Skipping CRC comparison for track %d "
+                          "due to rip failure"
+                          % number)
                 else:
-                    raise RuntimeError(
-                        "CRCs did not match for track %d" % number
-                    )
+                    if trackResult.testcrc == trackResult.copycrc:
+                        logger.info('CRCs match for track %d', number)
+                    else:
+                        raise RuntimeError(
+                            "CRCs did not match for track %d" % number
+                        )
 
-                print('Peak level: %.6f' % (trackResult.peak / 32768.0))
-                print('Rip quality: {:.2%}'.format(trackResult.quality))
+                    print('Peak level: %.6f' % (trackResult.peak / 32768.0))
+                    print('Rip quality: {:.2%}'.format(trackResult.quality))
 
             # overlay this rip onto the Table
             if number == 0:
@@ -516,8 +530,14 @@ Log files will log the path to tracks relative to this directory.
                     self.itable.setFile(1, 0, trackResult.filename,
                                         self.itable.getTrackStart(1), number)
             else:
-                self.itable.setFile(number, 1, trackResult.filename,
-                                    self.itable.getTrackLength(number), number)
+                if trackResult.filename in self.skipped_tracks:
+                    logger.debug("track %d (%s) "
+                                 "has been skipped; not adding to self.itable"
+                                 % number, trackResult.filename)
+                else:
+                    self.itable.setFile(number, 1, trackResult.filename,
+                                        self.itable.getTrackLength(number),
+                                        number)
 
         # check for hidden track one audio
         htoa = self.program.getHTOA()
diff --git a/whipper/result/logger.py b/whipper/result/logger.py
index 459a32f..b7043ad 100644
--- a/whipper/result/logger.py
+++ b/whipper/result/logger.py
@@ -15,6 +15,7 @@ class WhipperLogger(result.Logger):
     _accuratelyRipped = 0
     _inARDatabase = 0
     _errors = False
+    _skippedTracks = False
 
     def log(self, ripResult, epoch=time.time()):
         """Return logfile as string."""
@@ -139,6 +140,8 @@ class WhipperLogger(result.Logger):
 
         if self._errors:
             message = "There were errors"
+        elif self._skippedTracks:
+            message = "Some tracks were not ripped (skipped)"
         else:
             message = "No errors occurred"
         data["Health status"] = message
@@ -242,8 +245,12 @@ class WhipperLogger(result.Logger):
                 data["Result"] = "Track not present in AccurateRip database"
             track["AccurateRip %s" % v] = data
 
+        # Check if track has been skipped
+        if trackResult.skipped:
+            track["Status"] = "Track not ripped (skipped)"
+            self._skippedTracks = True
         # Check if Test & Copy CRCs are equal
-        if trackResult.testcrc == trackResult.copycrc:
+        elif trackResult.testcrc == trackResult.copycrc:
             track["Status"] = "Copy OK"
         else:
             self._errors = True
diff --git a/whipper/result/result.py b/whipper/result/result.py
index 461e408..47d14a0 100644
--- a/whipper/result/result.py
+++ b/whipper/result/result.py
@@ -38,6 +38,7 @@ class TrackResult:
     copycrc = None
     AR = None
     classVersion = 3
+    skipped = False
 
     def __init__(self):
         """

From 9f36d323bbbf6e66f7c22ee2050f13744d8b97bb Mon Sep 17 00:00:00 2001
From: blueblots <63152708+blueblots@users.noreply.github.com>
Date: Wed, 10 Feb 2021 22:59:36 +0000
Subject: [PATCH 3/7] Added non-zero exit, altered logger format strings

Added an exit status of 5 when tracks are skipped
during a rip attempt.

Fixed a TypeError caused by a syntax error in the
format string on line 537 in `whipper/command/cd.py`.

Changed f-string to printf-style format string
on line 493 in `whipper/command/cd.py`.

Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>
---
 whipper/command/cd.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index 2c9a407..2c87e98 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -192,13 +192,13 @@ class _CD(BaseCommand):
             cdio.Device(self.device).get_hwinfo()
         self.program.result.metadata = self.program.metadata
 
-        self.doCommand()
+        ret = self.doCommand()
 
         if (self.options.eject == 'success' and self.eject or
                 self.options.eject == 'always'):
             utils.eject_device(self.device)
 
-        return None
+        return ret
 
     def doCommand(self):
         pass
@@ -490,7 +490,8 @@ Log files will log the path to tracks relative to this directory.
                         logger.debug("adding %s to skipped_tracks"
                                      % trackResult.filename)
                         self.skipped_tracks.append(trackResult.filename)
-                        logger.debug(f"skipped_tracks = {self.skipped_tracks}")
+                        logger.debug("skipped_tracks = %s"
+                                     % self.skipped_tracks)
                         trackResult.skipped = True
                         logger.debug('trackResult.skipped = True')
                     else:
@@ -531,9 +532,9 @@ Log files will log the path to tracks relative to this directory.
                                         self.itable.getTrackStart(1), number)
             else:
                 if trackResult.filename in self.skipped_tracks:
-                    logger.debug("track %d (%s) "
+                    logger.debug("track %d (%s)"
                                  "has been skipped; not adding to self.itable"
-                                 % number, trackResult.filename)
+                                 % (number, trackResult.filename))
                 else:
                     self.itable.setFile(number, 1, trackResult.filename,
                                         self.itable.getTrackLength(number),
@@ -581,6 +582,12 @@ Log files will log the path to tracks relative to this directory.
 
         self.program.writeLog(discName, self.logger)
 
+        if len(self.skipped_tracks) > 0:
+            logger.warning('%d tracks have been skipped from this rip attempt'
+                           % len(self.skipped_tracks))
+            return 5
+        return None
+
 
 class CD(BaseCommand):
     summary = "handle CDs"

From 13e1ab6c1489b25a31a626b2fc6775ed5eaa6fb8 Mon Sep 17 00:00:00 2001
From: blueblots <63152708+blueblots@users.noreply.github.com>
Date: Mon, 15 Feb 2021 16:23:18 +0000
Subject: [PATCH 4/7] Changed logging strings, removed unnecessary return

Removed `return None` from the `Rip.doCommand` method
as suggested in review comments.

Changed logging strings to use logger arguments rather
than printf-string, as suggested in review comments.

Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>
---
 whipper/command/cd.py | 25 +++++++++++--------------
 1 file changed, 11 insertions(+), 14 deletions(-)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index 2c87e98..ffd0067 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -485,13 +485,12 @@ Log files will log the path to tracks relative to this directory.
                     logger.critical('giving up on track %d after %d times',
                                     number, tries)
                     if self.options.keep_going:
-                        logger.warning("track %d failed to rip. "
-                                       "Continuing to next track" % number)
-                        logger.debug("adding %s to skipped_tracks"
-                                     % trackResult.filename)
+                        logger.warning("track %d failed to rip.", number)
+                        logger.debug("adding %s to skipped_tracks",
+                                     trackResult.filename)
                         self.skipped_tracks.append(trackResult.filename)
-                        logger.debug("skipped_tracks = %s"
-                                     % self.skipped_tracks)
+                        logger.debug("skipped_tracks = %s",
+                                     self.skipped_tracks)
                         trackResult.skipped = True
                         logger.debug('trackResult.skipped = True')
                     else:
@@ -501,8 +500,7 @@ Log files will log the path to tracks relative to this directory.
                                            self.options.max_retries)
                 if trackResult.filename in self.skipped_tracks:
                     print("Skipping CRC comparison for track %d "
-                          "due to rip failure"
-                          % number)
+                          "due to rip failure" % number)
                 else:
                     if trackResult.testcrc == trackResult.copycrc:
                         logger.info('CRCs match for track %d', number)
@@ -532,9 +530,9 @@ Log files will log the path to tracks relative to this directory.
                                         self.itable.getTrackStart(1), number)
             else:
                 if trackResult.filename in self.skipped_tracks:
-                    logger.debug("track %d (%s)"
-                                 "has been skipped; not adding to self.itable"
-                                 % (number, trackResult.filename))
+                    logger.debug("track %d (%s) has been skipped; "
+                                 "not adding to self.itable",
+                                 number, trackResult.filename)
                 else:
                     self.itable.setFile(number, 1, trackResult.filename,
                                         self.itable.getTrackLength(number),
@@ -583,10 +581,9 @@ Log files will log the path to tracks relative to this directory.
         self.program.writeLog(discName, self.logger)
 
         if len(self.skipped_tracks) > 0:
-            logger.warning('%d tracks have been skipped from this rip attempt'
-                           % len(self.skipped_tracks))
+            logger.warning('%d tracks have been skipped from this rip attempt',
+                           len(self.skipped_tracks))
             return 5
-        return None
 
 
 class CD(BaseCommand):

From 65055914621407b8bacb988d503f488d4d4397a8 Mon Sep 17 00:00:00 2001
From: blueblots <63152708+blueblots@users.noreply.github.com>
Date: Sat, 20 Feb 2021 15:15:22 +0000
Subject: [PATCH 5/7] Fixed m3u and cue sheet generation

Added conditional to `program.write_m3u()` to
ignore skipped tracks.

Added skipped_tracks support to the `Program` and
`image.ImageVerifyTask` classes to avoid crashing
when a file for a skipped track doesn't exist.

Added conditional to `accurip.calculate_checksums`
to check if a path exists before trying to calculate
checksums, this prevents `accuraterip-checksum.c` from
emitting an error message (`sf_open failed!`) when a
path doesn't exist (as when a track is skipped).

Signed-off-by: blueblots <63152708+blueblots@users.noreply.github.com>
---
 whipper/command/cd.py     | 21 +++++++++------------
 whipper/common/accurip.py |  7 ++++++-
 whipper/common/program.py | 11 ++++++++++-
 whipper/image/image.py    | 14 ++++++++++++--
 4 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index ffd0067..9b03cf6 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -487,18 +487,17 @@ Log files will log the path to tracks relative to this directory.
                     if self.options.keep_going:
                         logger.warning("track %d failed to rip.", number)
                         logger.debug("adding %s to skipped_tracks",
-                                     trackResult.filename)
-                        self.skipped_tracks.append(trackResult.filename)
+                                     trackResult)
+                        self.skipped_tracks.append(trackResult)
                         logger.debug("skipped_tracks = %s",
                                      self.skipped_tracks)
                         trackResult.skipped = True
-                        logger.debug('trackResult.skipped = True')
                     else:
                         raise RuntimeError("track can't be ripped. "
                                            "Rip attempts number is equal "
                                            "to %d",
                                            self.options.max_retries)
-                if trackResult.filename in self.skipped_tracks:
+                if trackResult in self.skipped_tracks:
                     print("Skipping CRC comparison for track %d "
                           "due to rip failure" % number)
                 else:
@@ -529,14 +528,9 @@ Log files will log the path to tracks relative to this directory.
                     self.itable.setFile(1, 0, trackResult.filename,
                                         self.itable.getTrackStart(1), number)
             else:
-                if trackResult.filename in self.skipped_tracks:
-                    logger.debug("track %d (%s) has been skipped; "
-                                 "not adding to self.itable",
-                                 number, trackResult.filename)
-                else:
-                    self.itable.setFile(number, 1, trackResult.filename,
-                                        self.itable.getTrackLength(number),
-                                        number)
+                self.itable.setFile(number, 1, trackResult.filename,
+                                    self.itable.getTrackLength(number),
+                                    number)
 
         # check for hidden track one audio
         htoa = self.program.getHTOA()
@@ -571,6 +565,9 @@ Log files will log the path to tracks relative to this directory.
         logger.debug('writing m3u file for %r', discName)
         self.program.write_m3u(discName)
 
+        if len(self.skipped_tracks) > 0:
+            self.program.skipped_tracks = self.skipped_tracks
+
         try:
             self.program.verifyImage(self.runner, self.itable)
         except accurip.EntryNotFound:
diff --git a/whipper/common/accurip.py b/whipper/common/accurip.py
index 8f504fd..9be9a94 100644
--- a/whipper/common/accurip.py
+++ b/whipper/common/accurip.py
@@ -21,6 +21,7 @@
 
 import struct
 import whipper
+import os
 from urllib.error import URLError, HTTPError
 from urllib.request import urlopen, Request
 
@@ -111,7 +112,11 @@ def calculate_checksums(track_paths):
     logger.debug('checksumming %d tracks', track_count)
     # This is done sequentially because it is very fast.
     for i, path in enumerate(track_paths):
-        v1_sum, v2_sum = accuraterip_checksum(path, i+1, track_count)
+        if os.path.exists(path):
+            v1_sum, v2_sum = accuraterip_checksum(path, i+1, track_count)
+        else:
+            logger.warning('Can\'t checksum %s; path doesn\'t exist', path)
+            v1_sum, v2_sum = None, None
         if v1_sum is None:
             logger.error('could not calculate AccurateRip v1 checksum '
                          'for track %d %r', i + 1, path)
diff --git a/whipper/common/program.py b/whipper/common/program.py
index 302a276..58cd9a4 100644
--- a/whipper/common/program.py
+++ b/whipper/common/program.py
@@ -57,6 +57,7 @@ class Program:
     metadata = None
     outdir = None
     result = None
+    skipped_tracks = None
 
     def __init__(self, config, record=False):
         """
@@ -612,7 +613,12 @@ class Program:
         """
         cueImage = image.Image(self.cuePath)
         # assigns track lengths
-        verifytask = image.ImageVerifyTask(cueImage)
+        if self.skipped_tracks is not None:
+            verifytask = image.ImageVerifyTask(cueImage,
+                                               [os.path.basename(t.filename)
+                                                for t in self.skipped_tracks])
+        else:
+            verifytask = image.ImageVerifyTask(cueImage)
         runner.run(verifytask)
         if verifytask.exception:
             logger.error(verifytask.exceptionMessage)
@@ -627,6 +633,7 @@ class Program:
         ])
         if not (checksums and any(checksums['v1']) and any(checksums['v2'])):
             return False
+
         return accurip.verify_result(self.result, responses, checksums)
 
     def write_m3u(self, discname):
@@ -637,6 +644,8 @@ class Program:
                 if not track.filename:
                     # false positive htoa
                     continue
+                if track.skipped:
+                    continue
                 if track.number == 0:
                     length = (self.result.table.getTrackStart(1) /
                               common.FRAMES_PER_SECOND)
diff --git a/whipper/image/image.py b/whipper/image/image.py
index 224af4a..7a356b7 100644
--- a/whipper/image/image.py
+++ b/whipper/image/image.py
@@ -120,7 +120,7 @@ class ImageVerifyTask(task.MultiSeparateTask):
     description = "Checking tracks"
     lengths = None
 
-    def __init__(self, image):
+    def __init__(self, image, skipped_tracks=[]):
         task.MultiSeparateTask.__init__(self)
 
         self._image = image
@@ -147,7 +147,17 @@ class ImageVerifyTask(task.MultiSeparateTask):
             length = cue.getTrackLength(track)
 
             if length == -1:
-                path = image.getRealPath(index.path)
+                try:
+                    path = image.getRealPath(index.path)
+                except KeyError:
+                    logger.debug('Path not found; Checking '
+                                 'if %s is a skipped track', index.path)
+                    if index.path in skipped_tracks:
+                        logger.warning('Missing file %s due to skipped track',
+                                       index.path)
+                        continue
+                    else:
+                        raise
                 assert isinstance(path, str), "%r is not str" % path
                 logger.debug('schedule scan of audio length of %r', path)
                 taskk = AudioLengthTask(path)

From c97f2ce5472791f29992d37fea51b55cc884c4b4 Mon Sep 17 00:00:00 2001
From: JoeLametta <JoeLametta@users.noreply.github.com>
Date: Sun, 28 Mar 2021 15:45:32 +0000
Subject: [PATCH 6/7] Add warning about missing files referenced in the cue
 sheet

This happens when a track fails to be ripped and gets skipped.

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
---
 whipper/command/cd.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/whipper/command/cd.py b/whipper/command/cd.py
index 9b03cf6..205e650 100644
--- a/whipper/command/cd.py
+++ b/whipper/command/cd.py
@@ -566,6 +566,9 @@ Log files will log the path to tracks relative to this directory.
         self.program.write_m3u(discName)
 
         if len(self.skipped_tracks) > 0:
+            logger.warning("the generated cue sheet references %d track(s) "
+                           "which failed to rip so the associated file(s) "
+                           "won't be available", len(self.skipped_tracks))
             self.program.skipped_tracks = self.skipped_tracks
 
         try:

From 2c8acd61a2bd882c295dc6a6d6ecf0fd8a671e27 Mon Sep 17 00:00:00 2001
From: JoeLametta <JoeLametta@users.noreply.github.com>
Date: Sun, 28 Mar 2021 15:46:46 +0000
Subject: [PATCH 7/7] Add 'keep-going' option to whipper-cd-rip's manpage

Signed-off-by: JoeLametta <JoeLametta@users.noreply.github.com>
---
 man/whipper-cd-rip.rst | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/man/whipper-cd-rip.rst b/man/whipper-cd-rip.rst
index b09566f..2b8447b 100644
--- a/man/whipper-cd-rip.rst
+++ b/man/whipper-cd-rip.rst
@@ -69,6 +69,10 @@ Options
 |     Number of rip attempts before giving up if can't rip a track. This
 |     defaults to 5; 0 means infinity.
 
+| **-k** | **--keep-going**
+|     continue ripping further tracks instead of giving up if a track can't be
+|     ripped
+
 Template schemes
 ================