Installation stops if .profile is a directory.

[follower]

Problem

When installing, if .profile is a directory, an error occurs and installation stops.

Ideally, if an error occurs when modifying .profile, the change of .profile would be skipped (displaying a warning or giving choice) but rest of changes/installation continues.

This is, admittedly very much a corner case, but I encountered it in the "real world" as a result of simlu/voxelshop#305 which (mis)uses what seems to be a semi-common Java application framework/pattern.

$ ./rustup-init 

Welcome to Rust!

[...]

This path will then be added to your PATH environment variable by
modifying the profile files located at:

  /Users/<user>/.profile
  /Users/<user>/.bash_profile

You can uninstall at any time with rustup self uninstall and
these changes will be reverted.

Current installation options:


   default host triple: x86_64-apple-darwin
     default toolchain: stable (default)
               profile: default
  modify PATH variable: yes

1) Proceed with installation (default)
2) Customize installation
3) Cancel installation
>1

error: could not read rcfile file: '/Users/<user>/.profile'
error: caused by: Is a directory (os error 21)

Steps

1. mkdir ~/.profile
2. ./rustup-init

Possible Solution(s)

It may be sufficient to modify the first conditional here in do_add_to_path() to also ensure the "rcfile" is not a directory if the path exists:

rustup/src/cli/self_update/unix.rs

Lines 89 to 94 in d3bb61a

	if !rc.is_file() || !utils::read_file("rcfile", &rc)?.contains(&source_cmd) {
	utils::append_file("rcfile", &rc, &source_cmd).chain_err(|| {
	ErrorKind::WritingShellProfile {
	path: rc.to_path_buf(),
	}
	})?;

The .profile path is always used for POSIX:

rustup/src/cli/self_update/shell.rs

Lines 118 to 122 in 4cbc785

	fn update_rcs(&self) -> Vec<PathBuf> {
	// Write to .profile even if it doesn't exist. It's the only rc in the
	// POSIX spec so it should always be set up.
	self.rcfiles()
	}

The do_add_to_path() routine is called from:

rustup/src/cli/self_update.rs

Line 340 in 4ee141e

do_add_to_path()?;

The path removal code around here may also need to be modified--but probably not because it seems the check here is for is_file():

rustup/src/cli/self_update/unix.rs

Lines 60 to 61 in d3bb61a

	// Check more files for cleanup than normally are updated.
	for rc in sh.rcfiles().iter().filter(|rc| rc.is_file()) {

Tests would also need to be modified/added but it seems from this test the error may be intentional:

rustup/tests/cli-paths.rs

Lines 108 to 119 in 8e987b3

	#[test]
	fn install_errors_when_rc_cannot_be_updated() {
	clitools::setup(Scenario::Empty, &|config| {
	let rc = config.homedir.join(".profile");
	fs::File::create(&rc).unwrap();
	let mut perms = fs::metadata(&rc).unwrap().permissions();
	perms.set_readonly(true);
	fs::set_permissions(&rc, perms).unwrap();
	expect_err(config, &INIT_NONE, "amend shell");
	});
	}

Part of me wonders if an error should only occur if zero rcfiles can be updated but another part of me wonders if a warning is likely to be missed & result in more confusion in other situations...

Notes

Output of rustup-init --version:

rustup-init 1.22.1 (b01adbbc3 2020-07-08)

Output of rustup show: (not a command for rustup-init)

Additional (semi-)related code/issues

• https://github.com/rust-lang/rustup/blob/d8eed08b0274b175fd2684f5da466bb48e11ac9d/tests/cli-inst-interactive.rs#L111-L113
• Implement shell PATH fixes via UnixShell trait #2387
• Error: "could not amend shell profile" when HOME is unwritable #2040

[workingjubilee]

Those changes were merged after the last release.
Have you verified this issue happens with master?

[follower]

Those changes were merged after the last release.
Have you verified this issue happens with master?

That is a good point and an excellent question which I admit I did not consider. :)

(Perhaps a "Verify against master branch" would be a good thing to be added to the issue template.)

Checking against master is something I'm open to doing but despite extensive searching around (via docs, issues, CI, TOML & source) I couldn't work out if there are pre-built nightly/master versions of rustup-init available or not (due in part to my understanding that rustup archives are based on version number rather than date?).

Does a check against current master require me to build rustup-init from source?

Thanks.

[rbtcollins]

Yes, you'll need to build it yourself at this point, as we don't publish the builds of master / PRs.

[workingjubilee]

Yes, for rustup you should be able to just walk into the directory and cargo build.

[kinnison]

@follower Have you been able to reproduce this with master or is it fixed?

[follower]

Unfortunately I'm unlikely to get through my pre-existing yaks in order to test this with master any time soon.

I'll endeavour to remember to take another look when a new version is released.

Given that rust-init is the beginning of journey for installing Rust, and is dependent on nightly, I was wondering if there was a specific reason for not having pre-built binaries for master other than lack of resources?

It doesn't seem entirely unlikely that someone might encounter an issue with rust-init and not be able to install the toolchain but still want to test an updated pre-release version.

---

Potential todo's for me if I get back to this:

• Test with new release.
• Add note about testing with master to issue template.
• Add note about no pre-built non-release binaries (README or issue template?).

[kinnison]

Precisely because rustup is the gateway for many (if not most) people to Rust, and people are version junkies so will often follow a master line if one is available, we do not make it too easy for people to try out master of rustup. That way, a huge support nightmare would ensue. It is pretty easy for someone reasonably well versed in Rust to obtain and install rustup from git though.

[kinnison]

@follower Since 1.23 is now out with the revamped code by @workingjubilee in place, could you have another go and see how things behave? If we still need to make improvements then we will.