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

Allow selecting alternative *paranoia implementations #220

Open
xmixahlx opened this issue Jan 28, 2018 · 23 comments · May be fixed by #571
Open

Allow selecting alternative *paranoia implementations #220

xmixahlx opened this issue Jan 28, 2018 · 23 comments · May be fixed by #571
Assignees
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: patch A pull request is required Priority: medium Medium priority Sprintable Small enough to sprint on
Milestone

Comments

@xmixahlx
Copy link

errors with master. i've installed all the cdio packages I can find.
Traceback (most recent call last): File "/usr/local/bin/whipper", line 11, in <module> load_entry_point('whipper==0.5.1', 'console_scripts', 'whipper')() File "/usr/local/lib/python2.7/dist-packages/whipper-0.5.1-py2.7.egg/whipper/command/main.py", line 40, in main ret = cmd.do() File "/usr/local/lib/python2.7/dist-packages/whipper-0.5.1-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.5.1-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.5.1-py2.7.egg/whipper/command/cd.py", line 172, in do cdparanoia.getCdParanoiaVersion() File "/usr/local/lib/python2.7/dist-packages/whipper-0.5.1-py2.7.egg/whipper/program/cdparanoia.py", line 587, in getCdParanoiaVersion return getter.get() File "/usr/local/lib/python2.7/dist-packages/whipper-0.5.1-py2.7.egg/whipper/common/common.py", line 305, in get raise MissingDependencyException(self._dep) whipper.common.common.MissingDependencyException: cd-paranoia

reverting to prior than #213 resolves.

@xmixahlx
Copy link
Author

This looks to be related to the debian marillat libcdio-paranoia packages: the cd-paranoia executable isn't included or something. Installing the various cdio packages from git resolves...

@45054
Copy link

45054 commented Jan 28, 2018

You don't even have to install anything from git: https://packages.debian.org/stretch/libcdio-utils

@JoeLametta JoeLametta added the Expected Behaviour Bug report of intended functionality label Jan 28, 2018
@MerlijnWajer
Copy link
Collaborator

This will cause errors for Gentoo users at least, since their binary is not called 'cd-paranoia'. It's either 'cdparanoia' (depending on the symlink) or libcdio-paranoia and cdparanoia-paranoia.

Let's keep this bug report open for the 'allow selecting alternative paranoia implementation' feature request.

@MerlijnWajer MerlijnWajer self-assigned this Jan 28, 2018
@MerlijnWajer MerlijnWajer added Bug Generic bug: can be used together with more specific labels and removed Expected Behaviour Bug report of intended functionality labels Jan 28, 2018
@xmixahlx
Copy link
Author

libcdio-utils hasn't included cd-paranoia upstream for a while, breaking out libcdio-cdparanoia.

The marillat lbcdio-cdparanoia package doesn't package the binary. I emailed the maintainer for comment.

I am on Debian Sid.

@JoeLametta
Copy link
Collaborator

libcdio-utils hasn't included cd-paranoia upstream for a while, breaking out libcdio-cdparanoia.

??? (cd-paranoia is definitely included):

https://packages.debian.org/sid/amd64/libcdio-utils/filelist

I think I've misunderstood something... 🤣

@xmixahlx
Copy link
Author

Sid has libcdio v1.0. Marillat has v2.0 and cd-paranoia is now separated upstream.

Marillat is also purposefully removing the cd-paranoia binary in debian rules. I asked him about this and awaiting his reply.

@spvkgn
Copy link

spvkgn commented Feb 6, 2018

@JoeLametta cd-paranoia binary doesn't include in libcdio-utils v1.0 .deb package:

$ dpkg-deb -c libcdio-utils_1.0.0-2_amd64.deb 
drwxr-xr-x root/root         0 2017-12-06 13:05 ./
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/bin/
-rwxr-xr-x root/root     23008 2017-12-06 13:05 ./usr/bin/cd-drive
-rwxr-xr-x root/root     43912 2017-12-06 13:05 ./usr/bin/cd-info
-rwxr-xr-x root/root     31248 2017-12-06 13:05 ./usr/bin/cd-read
-rwxr-xr-x root/root     31304 2017-12-06 13:05 ./usr/bin/cdda-player
-rwxr-xr-x root/root     31424 2017-12-06 13:05 ./usr/bin/iso-info
-rwxr-xr-x root/root     27176 2017-12-06 13:05 ./usr/bin/iso-read
-rwxr-xr-x root/root     27128 2017-12-06 13:05 ./usr/bin/mmc-tool
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/share/
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/share/doc/
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/share/doc/libcdio-utils/
-rw-r--r-- root/root      4318 2017-12-06 13:05 ./usr/share/doc/libcdio-utils/changelog.Debian.gz
-rw-r--r-- root/root    134033 2017-11-22 06:50 ./usr/share/doc/libcdio-utils/changelog.gz
-rw-r--r-- root/root      2799 2014-08-22 05:39 ./usr/share/doc/libcdio-utils/copyright
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/share/man/
drwxr-xr-x root/root         0 2017-12-06 13:05 ./usr/share/man/man1/
-rw-r--r-- root/root       757 2017-12-06 13:05 ./usr/share/man/man1/cd-drive.1.gz
-rw-r--r-- root/root      1387 2017-12-06 13:05 ./usr/share/man/man1/cd-info.1.gz
-rw-r--r-- root/root      1163 2017-12-06 13:05 ./usr/share/man/man1/cd-read.1.gz
-rw-r--r-- root/root       867 2017-12-06 13:05 ./usr/share/man/man1/cdda-player.1.gz
-rw-r--r-- root/root       993 2017-12-06 13:05 ./usr/share/man/man1/iso-info.1.gz
-rw-r--r-- root/root       890 2017-12-06 13:05 ./usr/share/man/man1/iso-read.1.gz
-rw-r--r-- root/root       992 2017-12-06 13:05 ./usr/share/man/man1/mmc-tool.1.gz

And I didn't find cd-paranoia.* files in libcdio-release-1.0.0.tar.gz as well.

upd: just found.. it's here in separated dmo package https://www.deb-multimedia.org/dists/unstable/main/binary-amd64/package/cd-paranoia.php

@45054
Copy link

45054 commented Feb 6, 2018

Its really odd, that it is listed in here: https://packages.debian.org/sid/amd64/libcdio-utils/filelist but not really contained in the deb package. Very strange or maybe I'm missing something. Honestly, it smells like a packaging error.

@JoeLametta
Copy link
Collaborator

I'm sorry: I only tried to list the package content (the listing is wrong indeed).
According to snapshot.debian.org the cd-paranoia binary was included up to the 0.83 releases: starting from 0.92-1 up to the latest version (2.0.0-1) it is always missing. The cd-paranoia binary is missing in Ubuntu too.

It also appears that the libcdio source package in Debian has been orphaned and needs a maintainer: bug 881719 (didn't check in Ubuntu).

There's also a bug report, filed two weeks and a half ago against the Debian package, which describes this exact issue (bug 888053):

libcdio-utils: cd-paranoia is absent from deb package at version 1.0.0-2 (currently in testing/sid)

P.S. cd-paranoia 0.83 is too old to work correctly with whipper.

@JoeLametta
Copy link
Collaborator

JoeLametta commented Feb 16, 2018

🎆

Yesterday whipper 0.6.0 has been added to the deb-multimedia repository (announcement here).
Thanks to Christian Marillat!

@spvkgn
Copy link

spvkgn commented Feb 23, 2018

I uploaded a patch bug 889803 for debian source package libcdio-paranoia which adds creating .deb package with cd-paranoia binary as NMU QA upload, but don't know how to send a request to apply it (bug still has status 'Pending upload'). It would be great if anyone can help with it.

@thomas-mc-work
Copy link
Contributor

@spvkgn Maybe you could write him an explicit email. It would be great to get this point cleared for the users.

@xmixahlx
Copy link
Author

xmixahlx commented Mar 5, 2018

Also, to close the loop on my original comment: Marillat's packages were updated/fixed he just didn't reply...

libcdio-paranoia-dmo (10.2+0.94+2-dmo4) unstable; urgency=medium

  • Add the binary cd-paranoia in a new cd-paranoia package

-- Christian Marillat [email protected] Sun, 04 Feb 2018 08:09:40 +0100

@JoeLametta JoeLametta changed the title cd-paranoia error Allow selecting alternative *paranoia implementations Apr 6, 2018
@DamonLane
Copy link

As Merlijn predicts above, I'm a Gentoo user with a cd-paranoia problem. Whipper is available to us through an overlay. After adding that overlay/repo, Whipper installs through our normal command and pulls in its dependencies including cd-paranoia. So this is perplexing:

# emerge -vp cdparanoia

These are the packages that would be merged, in order:
Calculating dependencies... done!
[ebuild   R    ] media-sound/cdparanoia-3.10.2-r6::gentoo  USE="-static-libs" ABI_X86="(64) -32 (-x32)" 0 KiB
Total: 1 package (1 reinstall), Size of downloads: 0 KiB

# exit
$ whipper drive analyze
whipper: error: missing dependency "cd-paranoia"

Based on what little of this thread I can understand, I added a symlink from cd-paranoia to cdparanoia and now Whipper works.

@MerlijnWajer
Copy link
Collaborator

We're working on getting a gentoo ebuild approved: gentoo/gentoo#9525

This should solve your issue(s), see the patches in the ebuild.

@CaseOf
Copy link

CaseOf commented Aug 23, 2018

@DamonLane You can now install whipper from gentoo's portage tree:
https://packages.gentoo.org/packages/media-sound/whipper

@JoeLametta
Copy link
Collaborator

Because of #234 and #302 I think that this one gains higher priority...

@JoeLametta JoeLametta added Accepted Accepted issue on our roadmap Priority: high High priority Feature New feature Sprintable Small enough to sprint on labels Nov 13, 2018
@JoeLametta JoeLametta added Needed: patch A pull request is required and removed Bug Generic bug: can be used together with more specific labels labels Nov 13, 2018
@pieqq
Copy link

pieqq commented Mar 29, 2019

So to fix this issue, we need to modify the calls to the cd-paranoia binary in cdparanoia.py depending on the platform used, correct?

If whipper is to be a Unix-only application, we could rely on platform.linux_distribution() to get the proper info and therefore the proper binary name (maybe with a simple dict to match the proper binary for a given (distname,version) tuple).

The only needed thing would be a list of what Linux versions need what binary...

I'm currently running Ubuntu 18.04, so I know:

{
    ('Ubuntu', '18.04'): 'cdparanoia'
}

but I'm not sure for other distros.

@JoeLametta
Copy link
Collaborator

@pieqq Depends, it may also be implemented in a more general way (like proposed in #244).

@spvkgn
Copy link

spvkgn commented Apr 17, 2019

I've uploaded whipper 0.7.3 snap package to Ubuntu store https://snapcraft.io/whipper
It's in beta channel, not released yet cause I don't have a cd drive on my linux pc for now and can't test rip process. I guess Ubuntu users could test it. Your feedbacks are welcome.

@pieqq
Copy link

pieqq commented Apr 17, 2019

@spvkgn great, thanks!

Do you have a link to the snapcraft.yaml? (just out of curiosity)

Do you know if the snap requires special connect commands for some given plugs/slots?

@spvkgn
Copy link

spvkgn commented Apr 17, 2019

@pieqq snapcraft.yaml

This is first snap package I created, maybe there is something missing there.

@JoeLametta
Copy link
Collaborator

@spvkgn Thanks for the snap package! When you deem it to be ready, let me know so that I'll mention it in whipper's README. 😉

@JoeLametta JoeLametta added Priority: medium Medium priority and removed Priority: high High priority labels Dec 3, 2019
eharris added a commit to eharris/whipper that referenced this issue Sep 15, 2022
Resolves whipper-team#220
Partially addresses whipper-team#244
Provides workaround for whipper-team#234

Signed-off-by: Evan Harris <[email protected]>
@eharris eharris linked a pull request Sep 15, 2022 that will close this issue
eharris added a commit to eharris/whipper that referenced this issue Sep 15, 2022
Resolves whipper-team#220
Partially addresses whipper-team#244
Provides workaround for whipper-team#234

Signed-off-by: Evan Harris <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Feature New feature Needed: patch A pull request is required Priority: medium Medium priority Sprintable Small enough to sprint on
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants