Is there some reason the 'FileTimes' and 'set_times' API is File only?

[kimono-koans]

I don't see any discussion of this in the feature: #98245

Of course, it is possible the change the mtime and atime on a directory. However, this fails re: the current FileTimes API because one can't open a directory as one would a file.

Shouldn't the FileTimes API be available for all paths?

Tasks

Preview Give feedback

No tasks being tracked yet.

[the8472]

On unix directories are considered files and can be opened. They mere are not regular files. Which is why FileType has is_dir and is_file (the latter ought to be called is_regular_file... at least the prose mentions it).

So File::open("/tmp") should work on unix at least. It probably does on windows too, but I haven't tested that.

[kimono-koans]

On unix directories are considered files and can be opened. They mere are not regular files. Which is why FileType has is_dir and is_file (the latter ought to be called is_regular_file... at least the prose mentions it).

So File::open("/tmp") should work on unix at least. It probably does on windows too, but I haven't tested that.

Then perhaps it's not on the file open? My use is/was: I would open a directory, read times from the source, and then do a set_times on the destination. At some point in this sequence I was getting a EISDIR error, which would blow up my program.

FYI the filetime crate works for this use case, and has the API I'm speaking of. See: https://github.com/alexcrichton/filetime/blob/main/src/unix/linux.rs

All in all, does it seem a little goofy to open a file to change its metadata? Even if this is what actually happens under the hood?

[the8472]

Check with strace which syscall is failing.

[kimono-koans]

Seems as if the issue is the file open:

% sudo strace -e status=failed -f -- httm --roll-forward=rpool/program@snap_2024-04-09-22:45:09_ProgrammingSync
...
pid 2809161] openat(AT_FDCWD, "/srv/program/ppa/.git/objects/f8", O_WRONLY|O_CLOEXEC) = -1 EISDIR (Is a directory)
...
httm roll forward failed for the following reason: Could not overwrite "/srv/program/ppa/.git/objects/f8" with snapshot file version "/srv/program/.zfs/snapshot/snap_2024-04-09-22:45:09_ProgrammingSync/ppa/.git/objects/f8".

[the8472]

man 2 open:

    EISDIR pathname refers to a directory and the access requested involved writing (that is, O_WRONLY or O_RDWR is set).

So open it as readable.

This works on the playground:

use std::fs::FileTimes;
use std::time::SystemTime;
use std::fs::File;
fn main() {
    let home = std::env::home_dir().unwrap();
    let home = File::open(home).unwrap();
    home.set_times(FileTimes::new().set_modified(SystemTime::now())).unwrap();
}

[kimono-koans]

So open it as readable.

Excuse me. This blows up when the "source" path is opened as a file. And I have really no idea where in my program I would be doing that.

I'm going to have to dig into where this happens, but my best guess is that this is happening under the hood, in the standard library, because one doesn't need to open the file to read its metadata/mtime/atime?

[the8472]

I'm confused what you're doing and where it's failing. A reproducer would help. Stacktraces might also provide some context (strace may be able to provide those).

[kimono-koans]

I'm confused what you're doing and where it's failing. A reproducer would help. Stacktraces might also provide some context (strace may be able to provide those).

That's fair. See: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=c8ffc679c3beae232aeeae5a2d69da0a

I'm struggling to see what the issue might be.

[the8472]

.write(true)

Remove that part, open it as read-only as I said earlier. Then you can set the metadata.

[kimono-koans]

Remove that part, open it as read-only as I said earlier. Then you can set the metadata.

Aw, didn't understand that was what you were saying earlier. This appears to work, and I very much appreciate your help.

I will close the issue, but this does seem like confusing semantics. In order to write file metadata, the file must be open read only? That just seems wrong, at the very least, so counterintuitive there should be a note describing this use case at set_times.

[the8472]

In order to write file metadata, the file must be open read only

There are two steps. Opening the file, setting the metadata. And in this case opening a directory has additional preconditions.

There is a syscall to do it without the opening step, but the standard library currently doesn't expose that as a free function.

[kimono-koans]

In order to write file metadata, the file must be open read only

There are two steps. Opening the file, setting the metadata. And in this case opening a directory has additional preconditions.

Agreed, and I still think that's wildly counterintuitive. See the set_times doc comment where a regular file is opened writable for use with the API: https://doc.rust-lang.org/src/std/fs.rs.html#701-703

I'm supposing I simply copied the doc comment's code. And then it failed when I used the same with a directory. And perhaps I looked again and again at the doc comment and couldn't understand why it was failing.

You're saying "Hey go look at open". When this error pops, it doesn't tell me in what context it happens. It doesn't give me a path name and/or syscall. It just says EISDIR. There should be more here to assist in root causing this error. IMHO, at the very least, I would add a note to the function call and set the file as read-only in the doc comment example. And IMHO hiding this further complication behind a function/method on Path makes even more sense.

I'd be pleased to file another issue, if you think that is best.

[the8472]

When this error pops, it doesn't tell me in what context it happens. It doesn't give me a path name and/or syscall.

This is due to the standard library being close to the system API which provides no backtraces or line information because those would come at extra costs, which can be prohibitive in cases where errors are frequent (such as trying to open lots of files that might not exist). Which is why I suggested using strace.
You can opt-in into additional context by using crates like anyhow.

IMHO, at the very least, I would add a note to the function call and set the file as read-only in the doc comment example.

Setting file times is rare, setting file times on directories is even rarer. So this wasn't considered so far. That said, updating the documentation is a good idea.

And IMHO hiding this further complication behind a function/method on Path makes even more sense.

If you want to propose a new API you can file an API Change Proposal here: https://github.com/rust-lang/libs-team/issues