ScoopInstaller / Install

📥 Next-generation Scoop (un)installer
https://get.scoop.sh
The Unlicense
742 stars 94 forks source link

Cannot change 'Path' in registry #48

Closed jack460 closed 1 year ago

jack460 commented 1 year ago

Hi @WHYBBE & @rashil2000 , After https://github.com/ScoopInstaller/Install/pull/44 , we cannot install scoop successfully via Azure Run command (https://learn.microsoft.com/en-us/azure/virtual-machines/windows/run-command). The root cause is Write-Env does not effect via Azure Run command. I try to output '$EnvRegisterKey.GetValue($name)' after SetValue, the value is changed. But when I open Registry Editor, I found it remains the original value. I try to run Write-Env directly inside VM, it works.

r15ch13 commented 1 year ago

Can you provide a short explanation for setting up a test environment on Azure?

jack460 commented 1 year ago

Can you provide a short explanation for setting up a test environment on Azure?

  1. Provision an Azure VM
  2. At left panel, open 'Run Command' under 'Operations' section.
  3. Select 'RunPowerShellScript', execute Invoke-Expression "& {$(Invoke-RestMethod get.scoop.sh)} -RunAsAdmin"
  4. Close 'RunPowerShellScript' and reopen, execute scoop --version, 'scoop' command is unavailable.

In fact, it is caused by '$EnvRegisterKey.SetValue($name)' does not affect.

When we execute: Invoke-Expression "& {$(Invoke-RestMethod get.scoop.sh)} -RunAsAdmin" scoop --version in the same session, it's OK since they are in the same session, the $env:PATH = "$SCOOP_SHIMS_DIR;$env:PATH" in Add-ShimsDirToPath affects.

After we execute Invoke-Expression "& {$(Invoke-RestMethod get.scoop.sh)} -RunAsAdmin", even simply copy 'Write-Env' to 'RunPowerShellScript' console, the registry value is not changed. That's cause we cannot run scoop command in new sessions.

WHYBBE commented 1 year ago

I don't have an azure vm to test, and may need other people to help solve it. I just did some tests on my local virtual machine and made some discoveries, hope it helps.

Since the changes of #44 are mostly inherited from ScoopInstaller/Scoop#5395, and I didn't find a hidden problem in it, so maybe your problem is caused by this.

When there is no scoop in the environment variable, and using irm get.scoop.sh | iex similar commands to install scoop, my code will add the scoop dir to the Path, in fact it also adds.

But I don’t know why, if you use the right-click Windows start key (Windows 11) to invoke Powershell or use the search (Windows 11) to get cmd or Powershell, you can’t get new environment variables after installation.

But if you pin Windows Terminal inthe taskbar, the cmd/Powershell you get after clicking it can get new environment variables. There are two other ways: restart or manually save the editor of the environment variable.

I think the session of the environment variable caused by the new code may be abnormal, but I don't have time to explore until the weekend (I don’t have time on weekdays).

It is worth mentioning that ScoopInstaller/Scoop will also be affected (can verify by installing Python, because installing Python will add new environment variables), so we need to fix this problem before upgrading to 0.3.2.

I am sorry for the trouble caused to you by my modification of the code, currently you can install scoop by using the historical version of install.ps1.

WHYBBE commented 1 year ago

Maybe similar to #45 and #46

r15ch13 commented 1 year ago

Changing the registry value is not enough. This is why choco runs setx and calls a user32.dll function: https://github.com/chocolatey/choco/blob/ac806ee5ce03dea28f01c81f88c30c17726cb3e9/src/chocolatey.resources/helpers/functions/Set-EnvironmentVariable.ps1#L99-L124 The function call is probably not needed, because we don't care what explorer.exe does.

chawyehsu commented 1 year ago

😅 Problem can date back to 2009 https://github.com/PowerShell/PowerShell/issues/16989#issuecomment-1364615686

Need to broadcast the change after updating env vars.

chawyehsu commented 1 year ago

The function call is probably not needed

@r15ch13 the native SendMessageTimeout function call is required, I just tested before your comment. 😅

refs: https://stackoverflow.com/questions/6979741/setting-environment-variables-requires-reboot-on-64-bit https://gist.github.com/alphp/78fffb6d69e5bb863c76bbfc767effda https://github.com/AzureAD/microsoft-authentication-cli/pull/167/files https://mnaoumov.wordpress.com/2012/07/24/powershell-add-directory-to-environment-path-variable/ https://www.powershellgallery.com/packages/PSCI/1.0.4/Content/core%5Cutils%5CUpdate-EnvironmentVariables.ps1 https://github.com/nodejs/node/pull/613/files

Edit: PS. Windows Terminal still does not work even if an env broadcasting patch is applied to Scoop. Because the current (and previous) version(s) of WT does not listen for the WM_SETTINGCHANGE message. And guess what, I noticed this yestoday seeing https://github.com/microsoft/terminal/pull/14999 started last week and got merged few days ago. And I believe you will have to wait until next WT release if you are using WT. 😅