Skip to content

Torrent creation: add support for mtime-based file sorting #550

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.

Sign up for GitHub

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

Open
loskutov wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Torrent creation: add support for mtime-based file sorting #550

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
352c9ae
Torrent creation: add support for mtime-based file sorting
loskutov Oct 2, 2025
a07f301
Merge branch 'master' into master
casey Oct 3, 2025
152cace
Review fixes
loskutov Oct 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ pub(crate) enum Error {
input: InputTarget,
source: MetainfoError,
},
#[snafu(display("Modification time invalid/missing"))]
ModificationTimeMissing,
#[snafu(display("Network error: {}", source))]
Network { source: io::Error },
#[snafu(display("Failed to invoke opener: {}", source))]
Expand Down
6 changes: 6 additions & 0 deletions src/file_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,10 @@ pub(crate) struct FileInfo {
with = "unwrap_or_skip"
)]
pub(crate) md5sum: Option<Md5Digest>,
#[serde(
skip_serializing_if = "Option::is_none",
default,
with = "unwrap_or_skip"
)]
pub(crate) mtime: Option<SystemTime>,
}
1 change: 1 addition & 0 deletions src/hasher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ impl Hasher {

files.push(FileInfo {
path: file_path.clone(),
mtime: None,
md5sum,
length,
});
Expand Down
2 changes: 2 additions & 0 deletions src/metainfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ impl Metainfo {
files: vec![FileInfo {
length: Bytes(32 * 1024),
path: FilePath::from_components(&["DIR", "FILE"]),
mtime: None,
md5sum: Some(Md5Digest::from_hex("000102030405060708090a0b0c0d0e0f")),
}],
};
Expand Down Expand Up @@ -231,6 +232,7 @@ impl Metainfo {
mode: Mode::Multiple {
files: vec![FileInfo {
length: Bytes(1024),
mtime: None,
md5sum: None,
path: FilePath::from_components(&["a", "b"]),
}],
Expand Down
1 change: 1 addition & 0 deletions src/mode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ mod tests {
length: Bytes(10),
path: FilePath::from_components(&["foo", "bar"]),
md5sum: Some(Md5Digest::from_hex("000102030405060708090a0b0c0d0e0f")),
mtime: None,
}],
};

Expand Down
1 change: 1 addition & 0 deletions src/sort_key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::common::*;
pub(crate) enum SortKey {
Path,
Size,
Mtime,
}

impl SortKey {
Expand Down
5 changes: 5 additions & 0 deletions src/sort_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use crate::common::*;
use snafu::OptionExt;

#[derive(Clone, Copy, Debug, PartialEq)]
pub(crate) struct SortSpec {
Expand All @@ -25,6 +26,10 @@ impl SortSpec {
let ordering = match self.key {
SortKey::Path => a.path.cmp(&b.path),
SortKey::Size => a.length.cmp(&b.length),
SortKey::Mtime => {
a.mtime.context(error::ModificationTimeMissing).unwrap()
.cmp(&b.mtime.context(error::ModificationTimeMissing).unwrap())
}
};

match self.order {
Expand Down
11 changes: 10 additions & 1 deletion src/subcommand/torrent/create.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ Examples:
long = "sort-by",
value_name = "SPEC",
help = "Set the order of files within a torrent. `SPEC` should be of the form `KEY:ORDER`, \
with `KEY` being one of `path` or `size`, and `ORDER` being `ascending` or \
with `KEY` being one of `path`, `size` or `mtime`, and `ORDER` being `ascending` or \
`descending`. `:ORDER` defaults to `ascending` if omitted. The `--sort-by` flag may \
be given more than once, with later values being used to break ties. Ties that remain \
are broken in ascending path order.
Expand Down Expand Up @@ -1274,11 +1274,13 @@ mod tests {
path: FilePath::from_components(&["bar"]),
length: Bytes(4),
md5sum: Some(Md5Digest::from_data("5678")),
mtime: None,
},
FileInfo {
path: FilePath::from_components(&["foo"]),
length: Bytes(4),
md5sum: Some(Md5Digest::from_data("1234")),
mtime: None,
},
],
}
Expand Down Expand Up @@ -1422,6 +1424,7 @@ mod tests {
&[FileInfo {
length: Bytes(3),
md5sum: Some(Md5Digest::from_hex("37b51d194a7513e45b56f6524f2d51f2")),
mtime: None,
path: FilePath::from_components(&["bar"]),
},]
);
Expand Down Expand Up @@ -1457,6 +1460,7 @@ mod tests {
&[FileInfo {
length: Bytes(3),
md5sum: None,
mtime: None,
path: FilePath::from_components(&["bar"]),
},]
);
Expand Down Expand Up @@ -1495,16 +1499,19 @@ mod tests {
&[
FileInfo {
length: Bytes(3),
mtime: None,
md5sum: Some(Md5Digest::from_hex("900150983cd24fb0d6963f7d28e17f72")),
path: FilePath::from_components(&["a"]),
},
FileInfo {
length: Bytes(3),
mtime: None,
md5sum: Some(Md5Digest::from_hex("857c4402ad934005eae4638a93812bf7")),
path: FilePath::from_components(&["h"]),
},
FileInfo {
length: Bytes(3),
mtime: None,
md5sum: Some(Md5Digest::from_hex("d16fb36f0911f878998c136191af705e")),
path: FilePath::from_components(&["x"]),
},
Expand Down Expand Up @@ -1993,11 +2000,13 @@ Content Size 9 bytes
FileInfo {
length: Bytes(3),
md5sum: Some(Md5Digest::from_hex("37b51d194a7513e45b56f6524f2d51f2")),
mtime: None,
path: FilePath::from_components(&["bar"]),
},
FileInfo {
length: Bytes(3),
md5sum: Some(Md5Digest::from_hex("73feffa4b7f6bb68e44cf984c85f6e88")),
mtime: None,
path: FilePath::from_components(&["dir", "baz"]),
},
]
Expand Down
1 change: 1 addition & 0 deletions src/walker.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ impl Walker {
file_infos.push(FileInfo {
path: file_path,
length: Bytes(len),
mtime: metadata.modified().ok(),
Copy link
Owner

@casey casey Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should return an error if we can't read the modificiation time.

Copy link
Author

@loskutov loskutov Oct 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Owner

@casey casey Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@loskutov loskutov Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Owner

@casey casey Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

@loskutov loskutov Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

md5sum: None,
});
}
Expand Down