Correct Nushell source command

[Yethal]

Nushell uses $env.HOME instead of $HOME syntax for environment variables. However since source is a parser keyword it cannot access environment variables which is why it should use $nu.home-path instead
Additionally, the original command will cause nushell to error out on next config evaluation

[rami3l]

Note that this is not necessary the $CARGO_HOME! It's just its default value...

[Yethal]

Is there a scenario where $CARGO_HOME is set to a value different than default on a system and a human user will actually read this prompt? Or is it something that's only overriden in CI and such?

[rami3l]

@Yethal A human user will actually read the prompt. We allow $CARGO_HOME to be set to something else beforehand, actually IIRC we have suggested in an earlier note (before the actual installation, see https://github.com/search?q=repo%3Arust-lang%2Frustup+%22This+can+be+modified+with+the%22&type=code) that if you want it to be elsewhere you can set that and return to the installer.

[Yethal]

So will it suffice to add a note that if $CARGO_HOME was overriden then the path will need to be adjusted accordingly?

[rami3l]

@Yethal I am still inclined to doing this properly.

For example, you probably can get inspired from this logic:

rustup/src/cli/self_update.rs

Lines 469 to 487 in 21d4b27

	fn canonical_cargo_home(process: &Process) -> Result<Cow<'static, str>> {
	let path = process.cargo_home()?;
	let default_cargo_home = process
	.home_dir()
	.unwrap_or_else(|| PathBuf::from("."))
	.join(".cargo");
	Ok(if default_cargo_home == path {
	cfg_if! {
	if #[cfg(windows)] {
	r"%USERPROFILE%\.cargo".into()
	} else {
	"$HOME/.cargo".into()
	}
	}
	} else {
	path.to_string_lossy().into_owned().into()
	})
	}

... and figure out a proper way to print the exact path for env.nu for nushell. Maybe instead of using {cargo_home}, you can add an extra format argument {cargo_home_nushell}.

The only detail I don't know is how $nu.home-path is defined in nushell. However the spirit stays the same: if dirname is ($nu.home-path)/.cargo you can print source $"($nu.home-path)/.cargo/env.nu", otherwise just print out the full path.

[ChrisDenton]

Might this be considered a nushell bug? I think they could use std::env::home_dir since the implementation was fixed in 1.85 (though it won't be undeprecated until 1.87).

[Yethal]

What, the fact that environment variables are not parse-time safe? To the best of my knowledge this is by design.

[rami3l]

What, the fact that environment variables are not parse-time safe? To the best of my knowledge this is by design.

I believe @ChrisDenton meant that nushell relying on dirs/fn.home_dir.html's behavior could be considered a bug in this case... So you are basically free to ignore the "%USERPROFILE set to something else" case for now.

[ChrisDenton]

No, I mean the fact that it only uses {FOLDERID_Profile} and doesn't try %USERPROFILE% first. That would at least mean the behaviour is consistent.

[Yethal]

If the behavior was fixed in 1.85 then it might be too early for Nushell since the policy is to stay two versions behind latest stable

[rami3l]

AIUI if $CARGO_HOME is something else then this will get out of sync with other commands.

I believe we should add a more friendly note instead of just using the default value, if the equivalent cannot be achieved in nushell.

[rami3l]

It looks like we need something like https://www.nushell.sh/cookbook/foreign_shell_scripts.html but I'm not so sure... cc @LittleJianCH

[alluring-mushroom]

@rami3l I don't think you need the tricks in that link. The canonical way in nushell to resolve dynamic solutions is to place the code that generates in nushell/env.nu and the code that sources in nushell/config.nu. For example, most completion scripts work like this.

[Yethal]

An alternative approach would be to place the file in one of nushell's autoload directories so it could be sourced by name only with no relative path

[rami3l]

@rami3l I don't think you need the tricks in that link. The canonical way in nushell to resolve dynamic solutions is to place the code that generates in nushell/env.nu and the code that sources in nushell/config.nu. For example, most completion scripts work like this.

@alluring-mushroom I agree, and I now think plain substring substitutions would be fine: it's mostly about pretty-printing the absolute path by truncating it after all.

[Yethal]

@rami3l What else do I need to do to get this merged and/or the bug being resolved in any other way?

[rami3l]

@Yethal Sorry for the late reply. I haven't forgotten about this PR, it's just that I've been busy with other stuff for a while. That said, I'd definitely want it to become part of v1.28.2.

After a second look, I think although I suggested a string replacement previously, that replacement should not happen when printing the note. Instead, it should happen in the form of a refactored cargo_home() function that either outputs a path with HOME/USERPROFILE or one with the correct nushell-type of path.

Anyway, the overall outcome I'd like to see is:

• Accurate hints (modulo the HOME difference on Windows as discussed above)
• Use the full path if and only if necessary

PS: If you think that's too much for you, I can offer to fix this patch with your consent.

[rami3l]

So, as a quick summary, this $HOME-adding logic happens in two places:

1. rustup/src/cli/self_update.rs

   Line 469 in 364f72a

   fn canonical_cargo_home(process: &Process) -> Result<Cow<'static, str>> {

   
   However this doesn't affect any note on Windows, so we can safely replace that note for Unix only.

2. rustup/src/cli/self_update/shell.rs

   Line 54 in 364f72a

   pub(crate) fn cargo_home_str(process: &Process) -> Result<Cow<'static, str>> {

   
   This has more impact as it actually influences two places:
   i.

   rustup/src/cli/self_update/shell.rs

   Lines 42 to 51 in 364f72a

   	impl ShellScript {
   	pub(crate) fn write(&self, process: &Process) -> Result<()> {
   	let home = process.cargo_home()?;
   	let cargo_bin = format!("{}/bin", cargo_home_str(process)?);
   	let env_name = home.join(self.name);
   	let env_file = self.content.replace("{cargo_bin}", &cargo_bin);
   	utils::write_file(self.name, &env_name, &env_file)?;
   	Ok(())
   	}
   	}

   
   ii.

   rustup/src/cli/self_update/shell.rs

   Lines 301 to 303 in 364f72a

   	fn source_string(&self, process: &Process) -> Result<String> {
   	Ok(format!(r#"source "{}/env.nu""#, cargo_home_str(process)?))
   	}

... and as we must consider Windows in such scenarios now, I suggest using the full paths for nushell and don't bother with cargo_home_str() any more.

[rami3l]

@Yethal I understand that the command sourcing env.nu is wrong. However, does the generated env.nu work for you, or it needs correction as well?

[Yethal]

@Yethal I understand that the command sourcing env.nu is wrong. However, does the generated env.nu work for you, or it needs correction as well?

Opposite actually, command sourcing is ok but the generated env.nu is incorrect

[rami3l]

Closing in favor of #4265.