Skip to content
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.

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

Try detecting sudo when running rustup-init #617

Merged
merged 3 commits into from
Jul 29, 2016

Conversation

inejge
Copy link
Contributor

@inejge inejge commented Jul 26, 2016

Re #608. Not all sudo setups keep HOME, e.g., the ones in RH-derived distros don't. For those, a sudo'd installation will not end up in user's home directory, which may be unexpected, but not ugly like rust-lang/cargo#2892. On Debian/Ubuntu and OS X the detection works as expected.

@inejge
Copy link
Contributor Author

inejge commented Jul 26, 2016

The Windows build failure is spurious.

Other failures are much more interesting.

  1. Linux builds triggered a "can't get user data" warning inside the sudo check, and messed up expected test output. Unless /etc/passwd is nonexistent/unreadable, which is doubtful, the only other way it could've failed is if the euid under which the program is running didn't have a passwd entry. Not critical, but annoying.
  2. OS X builds massively failed because the test harness modifies HOME, confusing the sudo check. If Linux builds haven't failed at (1), they would've hit this.

Since the sudo check seems fundamentally incompatible with the way in which the test harness works, I've added an override which skips it when testing. It's a kludge; maybe a #cfg would be cleaner?

@brson
Copy link
Contributor

brson commented Jul 29, 2016

Looks great @inejge! I'd prefer this code be sunk into the self_update module, which has some other pre-flight checks. I want to do a release shortly though, so I'll do that refactoring myself. Hope you don't mind.

@brson brson merged commit 699e141 into rust-lang:master Jul 29, 2016
@inejge inejge deleted the sudo-detect branch July 31, 2016 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants