Preserve ToC file generated by cdrdao

[JoeLametta]

Whipper uses cdrdao during its ripping process. With this commit it will now store cdrdao's generated tocfile in the ripping path.
Preserving the tocfile allows users to easily burn ripped discs having a non-compliant cue sheet.

Fixes #214.

Previous comments are available in #277.

[MerlijnWajer]

Why do you pass toc_bpath when it's not used the functions? Or am I blind? :)

[Freso]

Using: whipper cd --device='/dev/cdrom' rip --cdr --prompt --working-directory='/tmp/rip-disc.Xur' --output-directory='' --track-template='%A - %d (%y) [%X]/%t. %a - %n' --disc-template='%A - %d (%y) [%X]/%A - %d' --release-id=''

The .toc file was placed under the current directory, while all the other rip files were placed in the directory specified by --working-directory. Maybe the other files need to be moved (but it hasn't been a problem before?), or there's something faulty in the logic of where this new code saves the .toc file.

[JoeLametta]

Why do you pass toc_bpath when it's not used the functions? Or am I blind? :)

Made a mess while moving/squashing commits: right nowtoc_bpath is in fact useless: removed.

or there's something faulty in the logic of where this new code saves the .toc file.

Hm, the reviewed implementation doesn't take care of that option: I've just pushed a new commit which, hopefully, solves this issue...

[Freso]

--track-template='%A - %d (%y) [%X]/%t. %a - %n' is now broken with this PR:

ValueError: disc template string contains invalid variable(s): %t, %a, %n.

git bisect log:

git bisect start
# good: [30785b8d1b95611c808e0e2d1c035f6511a5b39f] Push whipper v0.7.2 release
git bisect good 30785b8d1b95611c808e0e2d1c035f6511a5b39f
# good: [30785b8d1b95611c808e0e2d1c035f6511a5b39f] Push whipper v0.7.2 release
git bisect good 30785b8d1b95611c808e0e2d1c035f6511a5b39f
# bad: [a6eb8edeed5596319a1a03633c08ca06d37f71d6] WIP: fix ignored working directory [2/2]
git bisect bad a6eb8edeed5596319a1a03633c08ca06d37f71d6
# bad: [9c72ebccd31f32c78bdc4741971233f8792db7b7] Merge pull request #322 from whipper-team/feature/issue-279-disc-template-keyerror
git bisect bad 9c72ebccd31f32c78bdc4741971233f8792db7b7

Full output:

> env WHIPPER_DEBUG=DEBUG WHIPPER_LOGFILE=whipper.log whipper cd --device='/dev/cdrom' rip --cdr --prompt --working-directory=(mktemp -d) --output-directory='' --track-template='%A - %d (%y) [%X]/%t. %a - %n' --disc-template='%A - %d (%y) [%X]/%A - %d' --release-id=''
Using configured read offset 6
Checking device /dev/sr0
eject: Cd-rom-kommandoen luk skuffen mislykkedes: Inddata/uddata-fejl
Reading TOC...
CDDB disc id: 0d0b7e14
MusicBrainz disc id RJQhphcoFZQzhRYwCXgMgs2NecM-
MusicBrainz lookup URL https://musicbrainz.org/cdtoc/attach?toc=1+20+220812+150+11554+21905+33040+44335+50916+60363+71077+81941+94287+106967+114802+124022+131275+145203+152515+164551+177876+196428+207121&tracks=20&id=RJQhphcoFZQzhRYwCXgMgs2NecM-
Disc duration: 00:49:02.160, 20 audio tracks

Matching releases:

Artist  : Kraja
Title   : Vackert väder
Duration: 00:49:02.152
URL     : https://musicbrainz.org/release/010c91c6-b6f6-440a-9499-6533a1027515
Release : 010c91c6-b6f6-440a-9499-6533a1027515
Type    : Album
Barcode : 7393844010407
Cat no  : DROCD040

creating output directory Kraja - Vackert väder (2005) [FLAC]
Traceback (most recent call last):
  File "/home/freso/.local/share/virtualenvs/whipper-dev/bin/whipper", line 11, in <module>
    load_entry_point('whipper==0.7.2', 'console_scripts', 'whipper')()
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/main.py", line 36, in main
    ret = cmd.do()
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/basecommand.py", line 139, in do
    return self.cmd.do()
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/cd.py", line 181, in do
    self.doCommand()
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/cd.py", line 471, in doCommand
    _ripIfNotRipped(i + 1)
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/command/cd.py", line 353, in _ripIfNotRipped
    track_number=number) + '.flac'
  File "/home/freso/.local/share/virtualenvs/whipper-dev/lib/python2.7/site-packages/whipper-0.7.2-py2.7.egg/whipper/common/program.py", line 202, in getPath
    'variable(s): {}.'.format(', '.join(matches)))
ValueError: disc template string contains invalid variable(s): %t, %a, %n.

whipper.log

[JoeLametta]

--track-template='%A - %d (%y) [%X]/%t. %a - %n' is now broken with this PR:

ValueError: disc template string contains invalid variable(s): %t, %a, %n.

Thanks for reporting the issue!
Unfortunately that happens because of 9c72ebc (should be fixed now with #325).
I've just merged the develop branch into this one to apply the bugfix.

[Freso]

Ah, right, I was testing comparing to master, didn't spot that this PR was on top of develop.

[Freso]

I still don't get a .toc file. 😭

Rip went smoothly: whipper.log.gz

But no .toc with the other resulting files:

> ls -A /tmp/tmp.SHtYL9TjHV/Julie\ Fowlis\ -\ Uam\ \(2009\)\ \[FLAC\]/
'01. Julie Fowlis - M_ fhearann saidhbhir _My land is rich_ - Nellie Garvey_s Favourite - _G ioman nan gamhan _s mi muladach - Jerry_s Pipe Jig.flac'
'02. Julie Fowlis - Bothan Àirigh am Bràigh Raithneach.flac'
'03. Julie Fowlis - Wind and Rain.flac'
'04. Julie Fowlis - Thig am Bàta.flac'
'05. Julie Fowlis - A Chatrion_ Òg.flac'
'06. Julie Fowlis - Hé Gràdh, Hò Gràdh.flac'
'07. Julie Fowlis - Cò Nì Mire Rium_.flac'
'08. Julie Fowlis - A_ Chiad Cheum.flac'
'09. Julie Fowlis - Brògan Ùr Agam A-Nochd.flac'
'10. Julie Fowlis - Rugadh Mi _n Teis Meadhan na Mara.flac'
'11. Julie Fowlis - Bodachan Cha Phòs Mi.flac'
'12. Julie Fowlis - A Mhic Dhùghaill _ic Ruairidh.flac'
'13. Julie Fowlis - Hò Bha Mi, Hé Bha Mi.flac'
'Julie Fowlis - Uam.cue'
'Julie Fowlis - Uam.log'
'Julie Fowlis - Uam.m3u'

[JoeLametta]

I still don't get a .toc file. 😭

Thanks for the test, unfortunately I can't check the logfile right now. Will inspect it later today while trying to understand what isn't working as should...

[JoeLametta]

I still don't get a .toc file. 😭

Rip went smoothly: whipper.log.gz

But no .toc with the other resulting files:

@Freso

Hi, I still haven't found the time to debug this issue but while I was sleeping an idea came to my mind: maybe the pull request is actually OK and this behaviour is due to a caching issue. Whipper, with its current architecture, caches information about the ToCs of already "seen" discs: in those cases (ripping a disc with a cached ToC) cdrdao's tocfiles won't be moved as the cdrdao read-toc step is completely skipped. So, if the disc you've tried to rip without getting any tocfile had already been ripped before (without clearing the cache in the meantime), that could explain the observed outcome.

[Freso]

So, if the disc you've tried to rip without getting any tocfile had already been ripped before (without clearing the cache in the meantime), that could explain the observed outcome.

That still sounds like unexpected behaviour (ie., a bug) and probably needs resolving though… But I'll try another test run with a cleared cache and/or a "new" CD.

[Freso]

I tried again clearing the cache first (rm -rf .cache/whipper/*/*), but there's still no .toc in the final directory:

> ls /tmp/tmp.*/*
'01. Julie Fowlis - M_ fhearann saidhbhir _My land is rich_ - Nellie Garvey_s Favourite - _G ioman nan gamhan _s mi muladach - Jerry_s Pipe Jig.flac'
'02. Julie Fowlis - Bothan Àirigh am Bràigh Raithneach.flac'
'03. Julie Fowlis - Wind and Rain.flac'
'04. Julie Fowlis - Thig am Bàta.flac'
'05. Julie Fowlis - A Chatrion_ Òg.flac'
'06. Julie Fowlis - Hé Gràdh, Hò Gràdh.flac'
'07. Julie Fowlis - Cò Nì Mire Rium_.flac'
'08. Julie Fowlis - A_ Chiad Cheum.flac'
'09. Julie Fowlis - Brògan Ùr Agam A-Nochd.flac'
'10. Julie Fowlis - Rugadh Mi _n Teis Meadhan na Mara.flac'
'11. Julie Fowlis - Bodachan Cha Phòs Mi.flac'
'12. Julie Fowlis - A Mhic Dhùghaill _ic Ruairidh.flac'
'13. Julie Fowlis - Hò Bha Mi, Hé Bha Mi.flac'
'Julie Fowlis - Uam.cue'
'Julie Fowlis - Uam.log'
'Julie Fowlis - Uam.m3u'

Log: whipper.log(.gz)

[JoeLametta]

I tried again clearing the cache first (rm -rf .cache/whipper/*/*), but there's still no .toc in the final directory:

I've just rebased on develop and pushed a new commit which, in my tests, fixes the issue: please let me know if it works on your end.

That still sounds like unexpected behaviour (ie., a bug) and probably needs resolving though… But I'll try another test run with a cleared cache and/or a "new" CD.

Well, I think that's all I can do in this pull request: maybe dropping the cache entirely won't be a bad idea... (#196 (comment)). It seems our implementation of the cache is the cause of some bugs too...

P.S.: Travis CI's failures are caused by failing tests which are unrelated to this pull request (AccurateRip related).

[JoeLametta]

Forced pushed (commits rebased on the develop branch (which includes the fix for the failing tests)).

[JoeLametta]

Merged.

Regarding to the caching issue, see #335.