Add std::os::unix::fs::DirEntryExt2::file_name_ref(&self) -> &OsStr

[arennow]

Greetings!

This is my first PR here, so please forgive me if I've missed an important step or otherwise done something wrong. I'm very open to suggestions/fixes/corrections.

This PR adds a function that allows std::fs::DirEntry to vend a borrow of its filename on Unix platforms, which is especially useful for sorting. (Windows has (as I understand it) encoding differences that require an allocation.) This new function sits alongside the cross-platform file_name(&self) -> OsString function.

I pitched this idea in an internals thread, and no one objected vehemently, so here we are.

I understand features in general, I believe, but I'm not at all confident that my whole-cloth invention of a new feature string (as required by the compiler) was correct (or that the name is appropriate). Further, there doesn't appear to be a test for the sibling ino function, so I didn't add one for this similarly trivial function either. If it's desirable that I should do so, I'd be happy to [figure out how to] do that.

The following is a trivial sample of a use-case for this function, in which directory entries are sorted without any additional allocations:

use std::os::unix::fs::DirEntryExt;
use std::{fs, io};

fn main() -> io::Result<()> {
    let mut entries = fs::read_dir(".")?.collect::<Result<Vec<_>, io::Error>>()?;
    entries.sort_unstable_by(|a, b| a.file_name_ref().cmp(b.file_name_ref()));

    for p in entries {
        println!("{:?}", p);
    }

    Ok(())
}

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @sfackler (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[joshtriplett]

Seems reasonable to me, and the feature gate you used seems fine. This shouldn't be insta-stable, though; it should be marked as unstable for a while, and stabilized once we have some experience with it.

Some logistical complications: these traits aren't currently sealed, which means theoretically, someone could have implemented them for a type outside the standard library. We'll need to see if anyone has done so. The easiest way to do that would be to first make a PR to seal these traits, and run that through crater. If that doesn't turn up any failures, then we can merge it, and subsequently add methods to these traits.

[arennow]

As it turns out, m-ou-se recently did a crater run of sealing a number of traits, including DirEntryExt, and a few projects (including a little one you may not have heard about called tokio) implement that trait (at least as of a couple of months ago).

I can seal the trait and submit the PR for another run if you like, but I wouldn't expect significant changes since January.

Should I assume that, since breaking existing, working code is frowned upon, this is a bit of a non-starter?

[joshtriplett]

Sealing that one is probably a non-starter, then, but we could make a new sealed extension trait.

[arennow]

Sorry for the delay — life stuff.

If that's an allowable solution to this problem, I can certainly do that. I'm not yet intuitively good at Rust–style naming. What would be a good name for the new, sealed trait? It's like std::os::unix::fs::DirEntryExt but needs to be distinguishable from that. If it were just this one method, I could see something like DirEntBorrowExt, but I worry that that might be too narrow for future behavior to be added to it.

[sfackler]

I'd probably just go with DirEntryExt2 - it's basically the same thing as DirEntryExt but we just need to fix the sealing.

[arennow]

If that's aesthetically acceptable, I can make that change and update the PR today.

[bors]

☔ The latest upstream changes (presumably #84200) made this pull request unmergeable. Please resolve the merge conflicts.

[m-ou-se]

@arennow Can you open a tracking issue and replace the #[stable] attributes by #[unstable]? Thanks!

We can discuss things like the name of the trait after this is merged (and before it is stabilized) on that tracking issue.

[crlf0710]

@arennow Ping from triage, would you mind follow the instructions above? Thanks!

[arennow]

@crlf0710

Yes, sorry. Life again. I should be able to get that done this weekend. I just want to make sure I do things slowly to make sure I do them correctly. Trying to be a good open source citizen

[arennow]

Update: That was less intimidating than I expected it to be.

There are several things I'm not sure I did completely right, but I'd be more than happy to correct… whatever they may be. I'm pretty new to contributing to open source projects, and this is a big one.

[JohnTitor]

r? @m-ou-se

[m-ou-se]

@bors r+

[bors]

📌 Commit 469f467 has been approved by m-ou-se