Add OsStr::from_bytes()

[remram44]

OsString::from_bytes() turns Vec<u8> into OsString. This adds the same thing, from &[u8] to &OsStr:

pub fn OsString::from_bytes(vec: Vec<u8>) -> Option<OsString>
pub fn OsStr::from_bytes(bytes: &[u8]) -> Option<&OsStr>

Problem is that OsStrExt has a conflicting from_bytes() (that always succeeds; unix-only). This PR removes it, which might need an RFC (it was Rust 1.0.0).
OsStrExt then only has an always-succeeding as_bytes(), which could also be replaced by to_bytes().unwrap() without loss of functionality.

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @nikomatsakis (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. 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.

[alexcrichton]

This is currently a stable method, so it can't be removed.

[alexcrichton]

I like the idea of this, but I'm worried about this breaking code in practice. OsStrExt::from_bytes is commonly called as OsStr::from_bytes, so this will break all callers of the method today. We may want to consider a different name for this method.

[remram44]

Sure, that's certainly much more compatible. Though we have to choose between symmetry with OsString (which has from_bytes()) and compatibility (keeping the current OsStrExt). I can do the rename if you want.

[alexcrichton]

We cannot compromise on compatibility, so while it's unfortunate that the symmetry is lost here we don't have any other recourse.

[remram44]

Understood, that is very clear.
Any idea for the new name? from_slice? from_u8? from_u8_slice? from_bytes_checked?

[alexcrichton]

Perhaps from_bytes_opt but I don't think we use the _opt suffix that often any more.

[remram44]

It seems to me that the best approaches are

• to give the OsStr method a name that makes it clear why it's different from the OsString one (something like from_bytes_slice; this is the new patch), or
• to change the name of both of them (make this from_bytes_opt, and rename OsString's from_bytes to from_bytes_opt as well; adding this alias can be done in a 100% compatible way).

[alexcrichton]

I think that the current strategy taken is probably the best course of action, but I'd recommend the name from_bytes_opt for now. The OsString::from_bytes method is currently still unstable so we can rename it as appropriate as well.

[remram44]

Sure thing!

[alexcrichton]

Looks good to me, thanks @remram44! Tagging with libs/needs-decision so this comes up in triage.

[alexcrichton]

Ok we talked about this in the triage meeting for libs this week, and we reached two conclusions:

• The suffix _opt should be avoided as we've done it before and it never panned out too well
• The OsString::from_bytes method should be renamed to whatever this method is.

Unfortunately there weren't any new insights about what the method should be called, but it was suggested that it should emphasize perhaps the parsing aspect for some platforms (e.g. str::from_utf8 on Windows).

[aturon]

from_platform_bytes perhaps?

[alexcrichton]

Closing due to inactivity, but feel free to reopen with the a renaming!

[remram44]

I did the rename, you can reopen. Not sure if the old OsString::from_bytes() should be deprecated or just go away (it was never stable). Currently trying to run tests.

[alexcrichton]

Oh weird it looks like github doesn't allow me to reopen after a force push of the branch... Could you make a new PR?