Deprecate `std::env::home_dir` and fix incorrect documentation

[soc]

Compare std::env::home_dirs claim:

Returns the value of the 'HOME' environment variable if it is set and not equal to the empty string.

... with its actual behavior:

std::env::set_var("HOME", "");
println!("{:?}", std::env::var_os("HOME")); // Some("")
println!("{:?}", std::env::home_dir());     // Some("")

The implementation is incorrect in two cases:

• $HOME is set, but empty.
• An entry for the user exists in /etc/passwd, but it's pw_dir is empty.

In both cases Rust considers an empty string to be a valid home directory. This contradicts the documentation, and is wrong in general.

[rust-highfive]

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

[soc]

Discussion: https://internals.rust-lang.org/t/deprecate-or-break-fix-std-env-home-dir/7315/15

[shepmaster]

The code seems reasonable to me, but I'm unclear on if such a change is acceptable.

randomly reassigning to...

r? @KodrAus

[KodrAus]

So it looks like the docs were correct once upon a time, but since we stabilized with the current behaviour I think I'd be more comfortable updating the docs to reflect that, unless we do have the described behaviour on Windows.

[soc]

@KodrAus Windows is comparably broken (just in different ways) – see https://internals.rust-lang.org/t/deprecate-or-break-fix-std-env-home-dir/7315.

Preferably, I'd fix all the issues, but fixing the bugs on Windows has already been rejected. May it be possible to reevaluate the earlier decision given the new circumstances?

[soc]

The reason I'm trying to figure out what's the position here is because I want to work on rust-lang/cargo#5183, which is blocked by the 1.0 release of the dirs/directories crates, which are in turn blocked by the current home_dir situation.

From a user/third-party library author perspective I pretty much have three options:

1. Keep dirs::home_dir() -> PathBuf calling env::home_dir().unwrap() on Unix-likes. This means:

• Live with the incorrect behavior on Unix.
• Use my own implementation for Windows, because env::home_dir is just too broken on Windows.
• Not having to maintain code for are large number of niche systems which I won't be able to test.

2. Copy the implementation of env::home_dir and fix the bugs

• Change the result type of dirs::home_dir from PathBuf to Option<PathBuf>, because there are now realistic cases where home_dir can fail.
• Correct implementation on both Unix and Windows.
• Large maintenance burden for all the different platforms that env::home_dir was supposed to cover.

3. Fix env::home_dir to make sense and simply use it in dirs and directories.

• Able to drop workarounds for Windows.
• No workarounds necessary on Linux.
• Everyone benefits, not just me.

I'd really prefer it if we could make 3. work, although I'm slowly approaching the pain threshold of further delaying 1.0 releases for the crates and the things they keep blocking down the line. So I'd appreciate some communication about whether hoping for fixes is realistic.

[steveklabnik]

Nominating for @rust-lang/libs discussion, as they'd have to sign off on major changes here.

[SimonSapin]

Do we know of a situation other than a test case where the environment variable might exist and be empty?

[soc]

https://www.reddit.com/r/rust/comments/8o5rmz/why_envhome_dir_returns_optionpathbuf_instead/e01ft8z

[SimonSapin]

@soc (Assuming this is in response to me.) This post mentions not having a home directory. Is $HOME typically the empty string in that case, rather than unset?

[soc]

@SimonSapin It depends. It seems like creating a user and using the flags to not create a home directory usually causes some non-existing dir to end up in /etc/passwd. That may depend on the tool being used, though.
$HOME is usually derived from that and there is nothing preventing someone from having an entry like

soc:x:1001:1001:::/bin/false

std::env::home_dir should not pretend that there is some valid home directory in that case by returning Some("").

[SimonSapin]

The libs team discussed this today and consensus was that, since this function is already not doing what people want on Windows, we’d rather deprecate it in favor of something on crates.io and not change its behavior.

@rfcbot fcp close

[rfcbot]

Team member @SimonSapin has proposed to close this. The next step is review by the rest of the tagged teams:

• @Kimundi
• @SimonSapin
• @alexcrichton
• @dtolnay
• @sfackler
• @withoutboats

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[soc]

I released 1.0 versions of dirs and directories with correct implementations of home_dir. What needs to be done here now?

Shall I change this PR to deprecate std::env::home_dir?

[soc]

@SimonSapin Done!

[SimonSapin]

@bors r+

[bors]

📌 Commit 0afc16a has been approved by SimonSapin

[bors]

⌛️ Testing commit 0afc16a with merge 99b0ddb...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: SimonSapin
Pushing 99b0ddb to master...

[tjkirch]

I don't understand this decision. Removing the API seems like a bigger breakage than fixing the behavior, particularly when it works in normal cases.

[sfackler]

Deprecation is not removal.

[tjkirch]

Deprecation usually implies future removal. And now anyone using this API gets yelled at by the compiler, and so can't maintain a clean build without bringing in a dependency.

[sfackler]

https://play.rust-lang.org/?gist=ca688815ab9f69a9e87e7550ed389cdc&version=nightly&mode=debug&edition=2015

[soc]

@tjkirch Yeah, I'm not too happy with the decision. I'm now basically on the hook for supporting every platform ever invented under the sun, which I'm practically unable to test.