search

whipper-team / whipper

Python CD-DA ripper preferring accuracy over speed
GNU General Public License v3.0
1.11k stars 88 forks source link

Rip process fails due to ruamel.yaml change #605

Closed nitro322 closed 2 months ago

nitro322 commented 6 months ago

As of 0.18.0, ruamel.yaml has deprecated dump(), among other functions. With at least 0.18.5, it's not completely broken, resulting in a whipper crash:

INFO:whipper.common.program:2 AccurateRip response(s) found
track  1: rip accurate     (confidence   7 of   8) v1 [050e81fa], v2 [c569f3de], DB [c569f3de]
track  2: rip accurate     (confidence   7 of   8) v1 [e95f176a], v2 [fbf38a0a], DB [fbf38a0a]
track  3: rip accurate     (confidence   7 of   8) v1 [18741649], v2 [a12e64e3], DB [a12e64e3]
track  4: rip accurate     (confidence   6 of   8) v1 [956c2ad6], v2 [139ccb5a], DB [139ccb5a]
track  5: rip accurate     (confidence   7 of   8) v1 [7683eac8], v2 [af585d85], DB [af585d85]
Traceback (most recent call last):
  File "/usr/lib/python-exec/python3.11/whipper", line 8, in <module>
    sys.exit(main())
             ^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/command/main.py", line 56, in main
    ret = cmd.do()
          ^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/command/basecommand.py", line 141, in do
    return self.cmd.do()
           ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/command/basecommand.py", line 141, in do
    return self.cmd.do()
           ^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/command/cd.py", line 203, in do
    ret = self.doCommand()
          ^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/command/cd.py", line 589, in doCommand
    self.program.writeLog(discName, self.logger)
  File "/usr/lib/python3.11/site-packages/whipper/common/program.py", line 699, in writeLog
    log = txt_logger.log(self.result)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/result/logger.py", line 22, in log
    return self.logRip(ripResult, epoch)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/whipper/result/logger.py", line 151, in logRip
    riplog = yaml.dump(
             ^^^^^^^^^^
  File "/usr/lib/python3.11/site-packages/ruamel/yaml/main.py", line 1251, in dump
    error_deprecation('dump', 'dump', arg="typ='unsafe', pure=True")
  File "/usr/lib/python3.11/site-packages/ruamel/yaml/main.py", line 1039, in error_deprecation
    raise AttributeError(s, name=None)
AttributeError: 
"dump()" has been removed, use

  yaml = YAML(typ='unsafe', pure=True)
  yaml.dump(...)

instead of file "/usr/lib/python3.11/site-packages/whipper/result/logger.py", line 151

        riplog = yaml.dump(

Details about this change can be found here: https://yaml.readthedocs.io/en/latest/

Downgrading to ruamel.yaml 0.17.40 "fixed" the issue for me so I was able to continue ripping CDs, but it'd be great to get this updated so whipper works with newer versions of the library.

github-actions[bot] commented 6 months ago

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

To help make it easier for us to investigate your issue, please follow the contributing instructions.

MerlijnWajer commented 2 months ago

This seems related: https://github.com/whipper-team/whipper/pull/543

mweinelt commented 2 months ago

It is unclear to me from the documentation what to replace dump with. The basic usage guide and examples still advertise load/dump etc.

Edit: Calling load/dump methods on the YAML object does not look deprecated.

the functions scan, parse, compose, load, emit, serialize, dump and their variants (all, safe, roundtrip, etc) have been deprecated (the same named methods on YAML() instances are, of course, still there.

MerlijnWajer commented 2 months ago

To me it looks like the AttributeError itself tells us how to change the code:

AttributeError: 
"dump()" has been removed, use

  yaml = YAML(typ='unsafe', pure=True)
  yaml.dump(...)

instead of file "/usr/lib/python3.11/site-packages/whipper/result/logger.py", line 151

        riplog = yaml.dump(

I'm happy to make that change in the develop branch now.

MerlijnWajer commented 2 months ago

Actually, it looks like this has already been merged to develop in commit e0942417a1c267781a8b676789730457dcb2e6fa - so I think we can close this issue.

mweinelt commented 2 months ago

Oh, that is what confused me. I was comparing it to my change.

Can we get a new release? I know at least NixOS and Arch are using that patch, and soon also Gentoo.

MerlijnWajer commented 2 months ago

Yeah, I think it's about time for another release. Maybe we should make a ticket for a new release, and see if there's any outstanding minor or major issues that need to be fixed before we make a new release.

I haven't seen @JoeLametta here for a while and he has (busy) after his name, so I don't think he'll mind if I work on a new release.

MerlijnWajer commented 2 months ago

(If you know of any other patches that gentoo and others are carrying, please do let me know and we can try to integrate them)

mweinelt commented 2 months ago

No, that is the only one I found when I browsed repology.