GOBIN pwsh ps1 profile syntax issue

W1M0R commented 1 month ago

What version?

0.38.1

Which command?

proto use

What happened?

installation

# I run this in PowerShell Core 7.4.3
# When prompted to install the profile, I select None, since I manage it myself.
irm https://moonrepo.dev/install/proto.ps1 | iex

.prototools

go = "1.22.5"

[settings]
detect-strategy = "prefer-prototools"
telemetry = false
auto-install = true
auto-clean = true

proto use

❯ proto use
...
Go 1.22.5 has been installed to ~\.proto\tools\go\1.22.5!
Added GOBIN to shell profile ~\Documents\PowerShell\Microsoft.PowerShell_profile.ps1
Successfully installed tools!
...

Microsoft.PowerShell_profile.ps1

# go
$env:GOBIN = Join-Path $HOME "go" "bin";
$env:PATH = @(
  (Join-Path $env:GOBIN),
  $env:PATH
) -join [IO.PATH]::PathSeparator;

What happens

When I open a new pwsh session, I am greeted with the following error:

cmdlet Join-Path at command pipeline position 1
Supply values for the following parameters:
ChildPath:

If I delete the lines added by proto from Microsoft.PowerShell_profile.ps1, then pwsh works as expected.

What should happen

Ideally, I don't want proto use to modify my profile at all, since I manage my own GOBIN and PATH per project.

I see that the go plugin can be configured to not inject GOBIN: https://github.com/moonrepo/go-plugin?tab=readme-ov-file#configuration. Adding the following lines to my .prototools file solves the issue for me:

[tools.go]
gobin = false

Perhaps proto use can gain a --no-profile flag to skip the profile update part. Tool plugins can be taught to respect that flag and then set their configuration automatically based on that, e.g. gobin=false in the case of the go plugin.

Alternatively, proto use can detect whether GOBIN is already set, and if so, not make any changes to the profile.

For those that do prefer the profile to be updated, the current implementation must be corrected. The powershell syntax seems odd to me. The following version works as expected:

# go
$Env:GOBIN = Join-Path $Env:USERPROFILE "go/bin"
$Env:PATH = @(
  $Env:GOBIN
  $Env:PATH
) -join [IO.Path]::PathSeparator

This is quite long. Let me know if the issue is unclear, then I can paste it here.

Operating system?

Windows

Architecture?

x64

milesj commented 1 month ago

Thanks for the report, it's very detailed! Much appreciated. It seems like theres a few things going on here:

Our pwsh syntax needs to be fixed when only an env var is used (and not a full path).
So we do check if the profile contains the env var, but not in all cases, and I think this hit that other case. I'll look into normalizing it.
The gobin setting should be false, so that's a bug. We made all options opt-in a while back but looks like we missed one.

milesj commented 1 month ago

Come on github, closing an issue in another repo shouldn't close this one...

milesj commented 1 month ago

I'll update gobin in the next big release, since it's technically a breaking change.

W1M0R commented 1 month ago

Thank you very much for your time and effort to resolve this issue.

moonrepo / proto