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.

Sign up for GitHub

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

Jump to bottom

feat: Changed ENV variable name to allow for custom paths #5813

Closed
wants to merge 2 commits into from
Closed

feat: Changed ENV variable name to allow for custom paths #5813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
418ab78
Changed ENV variable name to allow for custom paths
CEbbinghaus Mar 14, 2024
d0f8ee7
Changed from environment variable to config
CEbbinghaus Mar 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 18 additions & 8 deletions lib/core.ps1
View file
Open in desktop
Expand Up @@ -739,6 +739,14 @@ public static extern IntPtr SendMessageTimeout(
}

function env($name, $global, $val = '__get') {
if(-not $name) {
$name = $scoop_path_env
}

if(-not $name) {
throw "Unable to evaluate path environment variable. Please set or remove PATH_ENV in your config.json."
}

$RegisterKey = if ($global) {
Get-Item -Path 'HKLM:\SYSTEM\CurrentControlSet\Control\Session Manager'
} else {
Expand All @@ -748,7 +756,7 @@ function env($name, $global, $val = '__get') {

if ($val -eq '__get') {
$RegistryValueOption = [Microsoft.Win32.RegistryValueOptions]::DoNotExpandEnvironmentNames
$EnvRegisterKey.GetValue($name, $null, $RegistryValueOption)
$EnvRegisterKey.GetValue($name, $null, $RegistryValueOption) + ""
Copy link
Member

Choose a reason for hiding this comment

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

What's this "" for?

Copy link
Author

Choose a reason for hiding this comment

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

If the environment variable is empty we get a null exception later on. so we add the empty string to ensure it always returns a string even if the environment variable is empty.

} elseif ($val -eq $null) {
try { $EnvRegisterKey.DeleteValue($name) } catch { }
Publish-Env
Expand Down Expand Up @@ -1010,7 +1018,7 @@ function get_shim_path() {
}

function search_in_path($target) {
$path = (env 'PATH' $false) + ";" + (env 'PATH' $true)
$path = (env $Null $false) + ";" + (env $Null $true)
foreach($dir in $path.split(';')) {
if(test-path "$dir\$target" -pathType leaf) {
return "$dir\$target"
Expand All @@ -1019,12 +1027,12 @@ function search_in_path($target) {
}

function ensure_in_path($dir, $global) {
$path = env 'PATH' $global
$path = env $Null $global
$dir = fullpath $dir
if($path -notmatch [regex]::escape($dir)) {
write-output "Adding $(friendly_path $dir) to $(if($global){'global'}else{'your'}) path."

env 'PATH' $global "$dir;$path" # for future sessions...
env $Null $global "$dir;$path" # for future sessions...
$env:PATH = "$dir;$env:PATH" # for this session
}
}
Expand Down Expand Up @@ -1113,8 +1121,8 @@ function add_first_in_path($dir, $global) {
$dir = fullpath $dir

# future sessions
$null, $currpath = strip_path (env 'path' $global) $dir
env 'path' $global "$dir;$currpath"
$null, $currpath = strip_path (env $Null $global) $dir
env $Null $global "$dir;$currpath"

# this session
$null, $env:PATH = strip_path $env:PATH $dir
Expand All @@ -1125,10 +1133,10 @@ function remove_from_path($dir, $global) {
$dir = fullpath $dir

# future sessions
$was_in_path, $newpath = strip_path (env 'path' $global) $dir
$was_in_path, $newpath = strip_path (env $Null $global) $dir
if($was_in_path) {
Write-Output "Removing $(friendly_path $dir) from your path."
env 'path' $global $newpath
env $Null $global $newpath
}

# current session
Expand Down Expand Up @@ -1415,6 +1423,8 @@ if ($pathExpected) {
}
$scoopConfig = load_cfg $configFile

$scoop_path_env = get_config PATH_ENV 'PATH'
Copy link
Member

@niheaven niheaven Mar 14, 2024 •
edited

Choose a reason for hiding this comment

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

Recommend using a config option to check if use isolated path, and the PATH_ENV here could be SCOOP_PATH.

Copy link
Author

Choose a reason for hiding this comment

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

I assumed we would want the default behavior to revert to the current behavior of just adding the paths to PATH. So we should have two different config options? A bool to enable custom paths and one to contain the path itself? I feel it makes more sense to just have a single config value that contains the Environment variable scoop should write to. That way we don't need extra checks and the user is always aware which environment variable is being written to.


# Scoop root directory
$scoopdir = $env:SCOOP, (get_config ROOT_PATH), (Resolve-Path "$PSScriptRoot\..\..\..\.."), "$([System.Environment]::GetFolderPath('UserProfile'))\scoop" | Where-Object { -not [String]::IsNullOrEmpty($_) } | Select-Object -First 1

Expand Down
6 changes: 3 additions & 3 deletions lib/install.ps1
View file
Open in desktop
Expand Up @@ -876,16 +876,16 @@ function unlink_current($versiondir) {

# to undo after installers add to path so that scoop manifest can keep track of this instead
function ensure_install_dir_not_in_path($dir, $global) {
$path = (env 'path' $global)
$path = (env $Null $global)

$fixed, $removed = find_dir_or_subdir $path "$dir"
if ($removed) {
$removed | ForEach-Object { "Installer added '$(friendly_path $_)' to path. Removing." }
env 'path' $global $fixed
env $Null $global $fixed
}

if (!$global) {
$fixed, $removed = find_dir_or_subdir (env 'path' $true) "$dir"
$fixed, $removed = find_dir_or_subdir (env $Null $true) "$dir"
if ($removed) {
$removed | ForEach-Object { warn "Installer added '$_' to system path. You might want to remove this manually (requires admin permission)." }
}
Expand Down