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

Cannot change 'Path' in registry #48

Closed
jack460 opened this issue Mar 20, 2023 · 7 comments · Fixed by #49
Closed

Cannot change 'Path' in registry #48

jack460 opened this issue Mar 20, 2023 · 7 comments · Fixed by #49
Labels
bug Something isn't working

Comments

@jack460
Copy link

jack460 commented Mar 20, 2023

Hi @WHYBBE & @rashil2000 ,
After #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
Copy link
Member

r15ch13 commented Mar 20, 2023

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

@jack460
Copy link
Author

jack460 commented Mar 21, 2023

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
Copy link
Contributor

WHYBBE commented Mar 22, 2023

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
Copy link
Contributor

WHYBBE commented Mar 22, 2023

Maybe similar to #45 and #46

@r15ch13
Copy link
Member

r15ch13 commented Mar 22, 2023

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
Copy link
Member

😅 Problem can date back to 2009 PowerShell/PowerShell#16989 (comment)

Need to broadcast the change after updating env vars.

@chawyehsu chawyehsu added the bug Something isn't working label Mar 22, 2023
@chawyehsu
Copy link
Member

chawyehsu commented Mar 22, 2023

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 microsoft/terminal#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. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants