Torrent creation: add support for mtime-based file sorting #550
Conversation
casey
left a comment
casey
left a comment
There was a problem hiding this comment.
Seems reasonable to me! This needs a test.
| file_infos.push(FileInfo { | ||
| path: file_path, | ||
| length: Bytes(len), | ||
| mtime: metadata.modified().ok(), |
There was a problem hiding this comment.
We should return an error if we can't read the modificiation time.
There was a problem hiding this comment.
Should that happen here (e.g. the FS has no mtime support at all) or later if we try to actually sort based on mtime?
There was a problem hiding this comment.
Oh, interesting, so Err here indicates that the system doesn't support mtime? I assumed it was an I/O error. Then yes, in that case we should throw the error later, if we actually try to sort on modification time.
There was a problem hiding this comment.
An I/O error would result in an error in the former metadata() call. As of the modified() call, it seems that in practice it can only fail if stat returns a timespec with a strange tv_nsec value on UNIX platforms (https://github.com/rust-lang/rust/blob/master/library/std/src/sys/pal/unix/time.rs#L95), which should never happen unless the kernel has a bug.
Do you believe a.mtime.unwrap().cmp(&b.mtime.unwrap()) would be a sensible thing to do in that case? Storing io::Result in FileInfo would not work because errors don't implement PartialEq, Serialize/Deserialize and so on.
There was a problem hiding this comment.
I think storing it as an Option<SystemTime> is reasonable. On comparison I would do:
a.mtime.context(error::ModificationTimeMissing)?
.cmp(b.mtime.context(error::ModificationTimeMissing)?)
with a new error variant, ModificationTimeMissing. So the error is only returned if the modification time is missing and the user tries to sort on it.
There was a problem hiding this comment.
The standard slices' sort_by requires the comparator to return Ordering, so it seems that panicking is the only way to express an error there.
2 participants
This can be useful for file collections that are periodically updated, so that the new files are appended to the end of the torrent, and the piece boundaries for the old ones are intact.