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

Environment variables #2

Closed
Ash258 opened this issue Feb 1, 2019 · 3 comments · Fixed by #4
Closed

Environment variables #2

Ash258 opened this issue Feb 1, 2019 · 3 comments · Fixed by #4

Comments

@Ash258
Copy link
Contributor

Ash258 commented Feb 1, 2019

Actual implementation should be respected. Upstream implementation is possible to set using environament varibles (not script parameters).
There is no need to set -InstallPath parameter, when you set $env:SCOOP (SCOOP_GLOBAL, SCOOP_CACHE)

https://github.com/scoopinstaller/install/blob/a4e4238c6b234626ae8d973529135e4398a1314b/install.ps1#L68-L73

All these variables should be surrounded by if ($env:SCOOP) { $SCOOP_DIR = $env:SCOOP} else { $SCOOP_DIR = $ScoopDir}

@chawyehsu
Copy link
Member

chawyehsu commented Feb 1, 2019

https://github.com/lukesampson/scoop/blob/8dd09661b1db75d9bf3693045f3215f65a4bd945/lib/core.ps1#L6-L19

Currently, the installer doesn't save $SCOOP_GLOBAL_DIR and $SCOOP_CACHE_DIR to environment variables, so it's a bug.

This is a known issue. $SCOOP_GLOBAL_DIR and $SCOOP_CACHE_DIR variables haven't been used in the script currently. Although the actual implementation rely on them as environment variables, I was considering to save these three variables into .scoop config file instead of polluting environment variables. But this will require refactoring the core.

But yeah, exposing $env:SCOOP $env:SCOOP_GLOBAL and $env:SCOOP_CACHE make sense, the actual implementation should be respected.

$SCOOP_DIR = if ($env:SCOOP) { $env:SCOOP } else { $ScoopDir } # Scoop root directory
$SCOOP_GLOBAL_DIR = if ($env:SCOOP_GLOBAL) { $env:SCOOP_GLOBAL } else { $ScoopGlobalDir } # Scoop global apps directory
$SCOOP_CACHE_DIR = if ($env:SCOOP_CACHE) { $env:SCOOP_CACHE } elseif ($env:SCOOP) { "$SCOOP_DIR\cache" } else { $ScoopCacheDir } # Scoop cache directory
$SCOOP_SHIMS_DIR = "$SCOOP_DIR\shims" # Scoop shims directory
$SCOOP_APP_DIR = "$SCOOP_DIR\apps\scoop\current" # Scoop itself directory
$SCOOP_CORE_BUCKET_DIR = "$SCOOP_DIR\buckets\core" # Scoop core bucket directory

@r15ch13 @lukesampson any idea?

@chawyehsu
Copy link
Member

chawyehsu commented Feb 12, 2019

#3 should close this, but I would like to format all variables in .scoop file to camelCase naming, see https://github.com/scoopinstaller/install/pull/3/files#diff-00ded903b18eba5a23a1d8ec86c58e94R313

$SCOOP  ===> rootPath
$SCOOP_CACHE  ===> cachePath
$SCOOP_GLOBAL ===> globalPath
lastupdate ===> lastUpdate

# more
show_update_log ===> showUpdateLog
SCOOP_REPO ===> scoopRepo
SCOOP_BRANCH ===> scoopBranch

@r15ch13 @Ash258 please take a review

@chawyehsu
Copy link
Member

chawyehsu commented Mar 15, 2019

@Ash258 @r15ch13 new implementation in #4 , please take a review and give advice.

Introduction:
The orginal environment variables method stays, but the priority changed:

  • if the user set custom directory environment variables using [Environment]::SetEnvironmentVariable before executing the installer, custom directory variables will NOT BE saved into ~/config/scoop/config.json config file.
  • else if the user set custom directory environment variables using $env:<whatever_SCOOP_GLOBAL_or_SCOOP_CACHE> before executing the installer, and is running as administrator, custom directory variables will be saved into system environment variables.
  • else if the user set any custom directory environment variables, those custom directory environment variables will BE saved into ~/config/scoop/config.json config file.
  • no custom directory, no variables will be saved into ~/config/scoop/config.json config file. (this one will be most of the cases, i.e., the typical Installation)

chawyehsu pushed a commit that referenced this issue May 10, 2019

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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 a pull request may close this issue.

2 participants