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

Rework Add-Path to avoid expanding and request an environment refresh #1658

Merged

Conversation

DavisVaughan
Copy link
Contributor

@DavisVaughan DavisVaughan commented Dec 19, 2024

Closes #1614
Closes #1657

A rework of #1657 after everything we have learned from #1657 (comment). Please read that comment / PR as it describes everything in detail.

I will once again require some help with snapshot updating!

Key points:

  • Powershell installers no longer forcibly expand "expandable variables" in your PATH, like %EXPAND%
  • Powershell installers no longer change your user-level Path registry property from REG_EXPAND_SZ to REG_SZ
  • Powershell installers no longer require a system restart, and now only require a shell restart, like Unix shell installers

I tested this myself by:

  • Completely resetting my Path to a state before dist touched it
  • Adding %TEMP% into my Path as a test "expandable" variable
  • Running a patched powershell installer with these changes, resulting in:
    • The Path stayed a REG_EXPAND_SZ data type
    • The Path retained my unexpanded %TEMP%
    • On a shell restart, I could load the cli tool I installed
  • Running the patched powershell installer again, nothing happened because it detected the existing install directory in the Path

I even went so far as to wipe out my Path property in the registry to see if it could create it "from scratch" if it wasn't there already, and it worked and was created with the right REG_EXPAND_SZ type.

Screenshot 2024-12-19 at 4 34 21 PM

@duckinator
Copy link
Contributor

FYI: We're probably not going to get a chance to review this until after the holidays.

Thank you again for the help, by the way! I know lots of folks are looking forward to not having to reboot after installing stuff, and looks like this PR improves other Path-related stuff too. 🙂

@mistydemeo mistydemeo force-pushed the feature/windows-path-updating branch from b82ae41 to c7ffe96 Compare January 6, 2025 22:24
@mistydemeo
Copy link
Contributor

Thanks again for the comprehensive writeup! I've updated the snapshots, and will do some testing before we merge this.

Copy link
Contributor

@mistydemeo mistydemeo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested, and confirmed working! Thanks so much for your work on this. I was also able to confirm that this preserves the expected string type for the PATH in the registry.

I did notice that if the user had an existing PATH entry that's a REG_SZ, this will replace it with a REG_EXPAND_SZ. Since we were previously unconditionally creating it as REG_SZ, this is IMO still a step up from before and not a blocker.

@mistydemeo mistydemeo merged commit ccac81c into axodotdev:main Jan 7, 2025
17 checks passed
@mistydemeo mistydemeo mentioned this pull request Jan 8, 2025
@zanieb
Copy link
Contributor

zanieb commented Jan 8, 2025

Thanks @DavisVaughan :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows: Modify PATH without requiring a system restart?
4 participants