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 1 commit
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
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 @@ -9,5 +9,11 @@ pub(crate) struct FileInfo {
default,
with = "unwrap_or_skip"
)]
pub(crate) mtime: Option<SystemTime>,
Comment thread
loskutov marked this conversation as resolved.
Outdated
#[serde(
skip_serializing_if = "Option::is_none",
default,
with = "unwrap_or_skip"
)]
pub(crate) md5sum: Option<Md5Digest>,
}
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
1 change: 1 addition & 0 deletions src/sort_spec.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ 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.cmp(&b.mtime),
};

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(),

@casey casey Oct 2, 2025

Copy link
Copy Markdown
Owner

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.

@loskutov loskutov Oct 2, 2025

Copy link
Copy Markdown
Author

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?

@casey casey Oct 3, 2025

Copy link
Copy Markdown
Owner

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.

@loskutov loskutov Oct 3, 2025

Copy link
Copy Markdown
Author

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.

@casey casey Oct 3, 2025

Copy link
Copy Markdown
Owner

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.

@loskutov loskutov Oct 3, 2025

Copy link
Copy Markdown
Author

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.

@casey casey Mar 19, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I'd rather not panic. When we would sort by modification time, somewhere outside of the actual sorting, we should check if mtime isn't present, and if so, throw an error before sorting.

md5sum: None,
});
}
Expand Down
Loading