Don't allow ripping without an explicit offset, and make pycdio a required dependency

[tobbez]

Right now, if you don't set the offset for a drive, the ripping process with proceed with just a warning ("WARNING: using default offset 0").

It would be better to force the user to set it explicitly (either using rip offset find, or manually setting it in the config file), and error out if it's not set.

In line with this it would be good to also make the pycdio dependency a required one rather than optional. It would additionally allow to simplify some code (by no longer having to handle pycdio being present or not), and make it more user friendly (just install from the package system and you have everything you need).

[JoeLametta]

Looks good.
I'll try to implement this as soon I have enough time: if you have already worked on this one, just submit it as pull request.

[JoeLametta]

@tobbez
Done (issue23 branch), please let me know what you think about this.
Now whipper throws an exception either if pycdio import fails or if ripping without a configured offset.

Traceback (most recent call last):                 
  File "/usr/local/bin/rip", line 46, in <module>
    h.handleImportError(e)
  File "/usr/local/bin/rip", line 41, in <module>
    sys.exit(main.main(sys.argv[1:]))
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/main.py", line 50, in main
    h.handleImportError(e)
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/main.py", line 45, in main
    ret = c.parse(argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/main.py", line 123, in parse
    logcommand.LogCommand.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 401, in parse
    return self.subCommands[command].parse(args[1:])
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 401, in parse
    return self.subCommands[command].parse(args[1:])
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 363, in parse
    ret = self.do(args)
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/cd.py", line 171, in do
    raise ImportError("Pycdio module import failed.\n"
ImportError: Pycdio module import failed.
This is a hard dependency: if not available please install it

Traceback (most recent call last):
  File "/usr/local/bin/rip", line 41, in <module>
    sys.exit(main.main(sys.argv[1:]))
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/main.py", line 45, in main
    ret = c.parse(argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/main.py", line 123, in parse
    logcommand.LogCommand.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 401, in parse
    return self.subCommands[command].parse(args[1:])
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 401, in parse
    return self.subCommands[command].parse(args[1:])
  File "/usr/local/lib/python2.7/site-packages/morituri/common/logcommand.py", line 62, in parse
    command.Command.parse(self, argv)
  File "/usr/local/lib/python2.7/site-packages/morituri/extern/command/command.py", line 325, in parse
    ret = self.handleOptions(self.options)
  File "/usr/local/lib/python2.7/site-packages/morituri/rip/cd.py", line 274, in handleOptions
    raise ValueError("Drive offset is unconfigured.\n"
ValueError: Drive offset is unconfigured.
Please install pycdio and run 'rip offset find' to detect your drive's offset or set it manually in the configuration file. It can also be specified at runtime using the '--offset=value' argument

This commit could be improved, in the future, providing a way to override pycdio hard dependency.

[JoeLametta]

Overriding pycdio hard dependency is now possible (README updated).

[JoeLametta]

@neitsab
Could you please update the whipper-git package in AUR marking python2-pycdio as required dependency?

Thanks.

[neitsab]

@JoeLametta Done. Note that README still says "optional": want me to prepare a pull request?

[JoeLametta]

Thanks. That's because it's still unmerged (I was waiting for your reply first).
The updated README looks like this:

• pycdio, for drive identification (it can be overridden placing a blank file named PYCDIO_IGNORE into whipper's config path)
  • Required for drive offset and caching behavior to be stored in the config file

Do you think it's OK (enough clear)?

In the meantime I'm merging this branch into master one: I'll wait for your reply before closing the issue.

[neitsab]

You're welcome! The proposed version looks good to me, and therefore this issue can be closed :)

[tobbez]

The idea was to remove any logic around the imports of pycdio, and just crash if you tried running without it.

That shouldn't have a negative impact on regular users, since the package manager would install pycdio for them.

[JoeLametta]

@tobbez

I know but now whipper acts nicely by default (throwing exceptions if the required conditions aren't met).
The override is still possible for power users but discouraged and only documented into the README (not in the error messages).

To me that seems a reasonable solution: code didn't get that complex, users will have pycdio installed by default through their package manager, tinkerers will have the freedom to override the dependency in a simple way.
Am I wrong?

[tobbez]

Most, if not all users will need to do offset detection before they start ripping, which means pycdio is required.

Sure, there might be some user for which installing pycdio is a deal-breaker, but you would have to have very special requirements for that to be the case.

In all, I'd say it's needless complexity to ship a code path that will barely (if at all) get any use. And if a user really needs to avoid pycdio, they may patch out the dependency themselves.

[JoeLametta]

@tobbez

Done ;)

67ea220