rndusr / torf-cli

CLI tool for creating, reading and editing torrent files
GNU General Public License v3.0
134 stars 13 forks source link

Provide a way to infer a basename for verification #20

Closed sinic closed 4 years ago

sinic commented 4 years ago

This probably cannot be merged as-is, but maybe you'd consider implementing something similar to it.

Utilities I've used up to this point for verifying downloads have included the torrent's name string as the last component in the path at which they expect the contents to be. Now I'd have to resort to two steps: first extract the name, then append it to the path for verification. This is especially troublesome if I want to verify many torrents at once. With a flag like this one, I could simply call torf in a loop without having to change the path on each iteration.

rndusr commented 4 years ago

Thank you for your feedback. I don't use this feature much so this is useful.

torf already has so many options. If possible, I'd like to avoid adding another option that's only relevant for verifying torrents.

What about auto-detecting what the user meant? If the last component of PATH is not equal to the torrent's name, append the name before passing it to verify().

The PATH documenation in torf -h is ambiuous enough, and the exact behaviour could be documented in the man page.

Can you think of any scenarios in which this would result in wrong or unexpected behaviour?

sinic commented 4 years ago

Torrent clients, while commonly putting contents of a torrent in a directory named after the torrent's name, often provide a way to choose a different directory. This is important for when there are two torrents with the same name and different contents, otherwise their files would end up in the same directory and possibly even overwrite each other.

For this reason, it might be quite common to want to verify a directory with a name that's different from the torrent's name (even though in the majority of cases it will be the same). In the end, there seems to be no way to do without some sort of switch to explicitly select the desired behaviour, unfortunately.

rndusr commented 4 years ago

What about using the --name option to change the torrent's name before it is verified?

sinic commented 4 years ago

That works, but then we're back at requiring two steps. At least it's only in the less common case.

Or did you mean that the verify code path should make use of that option? Unfortunately specifying --name implies torrent editing. Otherwise its argument could have been made optional, presenting a choice between specifying it manually (with argument), inferring it automatically (with no argument), or keeping the old behaviour of not taking the name into account (by default, with no flag at all).

rndusr commented 4 years ago

The --name option doesn't imply editing, and it shouldn't, in my opion. If you don't provide -o, there is nothing to store your changes.

So making use of the --name option would be an option. But now I realize it doesn't help either. This works for me right now, so changing the name doesn't seem to affect the verification anyway:

$ torf -i foo.torrent path/to/foo
$ ln -s path/to/foo bar
$ torf -i foo.torrent ./bar

Just to make sure I understand correctly what you need: Given the directory "path/to/foo" that contains "foo.torrent"'s files, you want this to work?

$ torf -i foo.torrent path/to
sinic commented 4 years ago

The --name option doesn't imply editing, and it shouldn't, in my opion. If you don't provide -o, there is nothing to store your changes.

I meant to describe the current behaviour, not a conceptual necessity. So it could certainly be changed, but doing so would break the interface, and you might not want to increment the major version right now.

So making use of the --name option would be an option. But now I realize it doesn't help either. This works for me right now, so changing the name doesn't seem to affect the verification anyway:

No, it doesn't yet, but that's just a matter of looking at cfg['name'] instead of cfg['basename']. Aside from being an interface-breaking change, the only real headache is the documentation.

Just to make sure I understand correctly what you need: Given the directory "path/to/foo" that contains "foo.torrent"'s files, you want this to work?

$ torf -i foo.torrent path/to

Not necessarily without any additional options, but yes: I'd like to have some way to achieve this.

rndusr commented 4 years ago

I meant to describe the current behaviour

Oh, right, I forgot about that change.

Now that I fully understand what you mean, using --name is a bad idea. It takes an argument and what you want is a switch that appends the torrent's name to PATH.

What is your opinion on auto-detecting?

"torf -i foo.torrent path/to" could look for any of the torrent's files in "path/to" and "path/to/foo" and using the first path with a hit. Unless I'm missing something, that approach should work with "path/to", "path/to/foo" and even with the edge case "path/to/foo/foo".

The documentation is currently ambiguous about the torrent's name being a part of PATH or not, so implementing it like that would be great. On the other hand, vague interfaces aren't so great.

sinic commented 4 years ago

I share your sentiment regarding vague interfaces; this is bound to get confusing fast.

Say you have a torrent of a video DVD with name DVD and containing a file VIDEO_TS/VTS_02_0.VOB. You verify it with torf -i DVD.torrent ~/Downloads, no problem so far. Then a slightly botched torrent of a different video DVD comes along, this time with name VIDEO_TS and containing a file VTS_02_0.VOB. You again verify it with torf -i VIDEO_TS.torrent ~/Downloads and everything seems alright. Then, some other day, you want to verify the first DVD again and suddenly the old command reports a mismatch, even though nobody touched this torrent's contents!

I think the cleanest solution would be to always look for the contents in a directory named after the torrents name, unless overridden by an argument to --name. (But that obviously breaks the interface, so it should be grouped together with more important interface-breaking changes at some point in the future.)

rndusr commented 4 years ago

Yes, I was thinking about collisions like that. They would probably be extremely rare and could be fixed by the user specifying PATH with the torrent's name at the end. But if torf was used in a script, things would break for a single torrent for no apparent reason. And the rarest bugs are arguably the worst.

I agree that's not a good solution. But I also don't like the other solutions we've discussed so far. Let's sit this for a while.

rndusr commented 4 years ago

I had another idea. rsync uses a trailing slash to do something similar.

Both of these commands would look for foo's files in "path/to/foo":

$ torf -i foo.torrent path/to/foo
$ torf -i foo.torrent path/to/
sinic commented 4 years ago

Suits me well (in fact, on BSD I'm already used to add a trailing slash for copying directory contents). It might be worth pointing out, however, that this still constitutes an interface-breaking change for people who used trailing slashes in their scripts.

rndusr commented 4 years ago

OK, great. Do you want to implement it?

interface-breaking

The documentation doesn't specify exactly how this works so we'd only be "breaking user space". But I'm not Linus and torf is not as widely used and mission critical as Linux. In fact, I'd be surprised if anyone would notice.

sinic commented 4 years ago

Do you want to implement it?

Like this? Documentation is still missing, though.

rndusr commented 4 years ago

Whoops, sorry, I missed your reply.

I can do the documentation and adding tests. But you're also welcome to make another PR.