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

TypeError on whipper offset find #263

Closed
Phaaxood opened this issue Apr 21, 2018 · 7 comments
Closed

TypeError on whipper offset find #263

Phaaxood opened this issue Apr 21, 2018 · 7 comments
Labels
Bug Generic bug: can be used together with more specific labels

Comments

@Phaaxood
Copy link

If the calculation of a checksum fails in program/arc.py, accuraterip_checksum can return None. command/offset.py's Find._arcs fails in this case with a TypeError when trying to format the None as a number in its return value.

(For context: accuraterip_checksum fails in my case due to an illegal hardware instruction and dumping core when trying to compute a v2 checksum. Thus the returncode is nonzero and you might want to die with the TypeError and not treat this as a bug.)

@JoeLametta
Copy link
Collaborator

Thanks for the bug report!

(For context: accuraterip_checksum fails in my case due to an illegal hardware instruction and dumping core when trying to compute a v2 checksum.

That's quite strange: could you provide a sample file file which shows this behaviour?

Cheers,
Joe

@JoeLametta JoeLametta added the Bug Generic bug: can be used together with more specific labels label Apr 26, 2018
@Phaaxood
Copy link
Author

Phaaxood commented May 1, 2018

The problem is in my laptop being an old 32bit one and accuraterip_checksum not using the right data types for the operations there. See leo-bogert/accuraterip-checksum#2.

@JoeLametta
Copy link
Collaborator

The problem is in my laptop being an old 32bit one and accuraterip_checksum not using the right data types for the operations there. See leo-bogert/accuraterip-checksum#2.

Did you install accuraterip-checksum from the source code provided in whipper's repository? I'm asking this because the version shipped in this repository has @MerlijnWajer's fixes:

I've submitted your code for inclusion in whipper/morituri - #37 with my fixes. Hope that's OK (I think the license matches)

@Phaaxood
Copy link
Author

No, and I just see that the arch package uses the correct sources, but doesn't build accuraterip-checksum. It just uses the packaged version of it, which comes from leo-bogert's repo. I'll file a bug report there.

So for here it boils down to whether the Exception that might result from accurateip_checksum returning None is considered a bug or not.

@JoeLametta
Copy link
Collaborator

No, and I just see that the arch package uses the correct sources, but doesn't build accuraterip-checksum. It just uses the packaged version of it, which comes from leo-bogert's repo. I'll file a bug report there.

It seems the package in Arch Linux's community repository was marked as x86_64 only in 2017-11-15: commit 08f26c6.
Anyway it seems there's no bug report filed against that package.

So for here it boils down to whether the Exception that might result from accurateip_checksum returning None is considered a bug or not.

Uhm, if that step fails I think it's better to leave it failing hard (Exception) as I don't see a sensible recovery solution anyway...

@leo-bogert
Copy link
Contributor

The most recent code of accuraterip-checksum at https://github.com/leo-bogert/accuraterip-checksum is fixed to work on 32 bit platforms.
Sorry for the inconvenience!

@mtdcr
Copy link
Contributor

mtdcr commented Jun 29, 2018

Please note that PR #274 would remove the dependency on an installed executable by converting accuraterip-checksum to a Python module.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Generic bug: can be used together with more specific labels
Projects
None yet
Development

No branches or pull requests

4 participants