Skip to content
This repository has been archived by the owner on Feb 19, 2019. It is now read-only.

Allowing ChocolateyInstall from System & Proc Env #135

Closed
wants to merge 2 commits into from

Conversation

abombss
Copy link

@abombss abombss commented Aug 1, 2012

Based on the discussion in the google group
https://groups.google.com/forum/?fromgroups#!topic/chocolatey/84nMqjleMcI

This patch adds support for System and Proc based environment variable when installing chocolatey.

This still sets the path only for the user, but I am assuming if using system or proc the person installing will manually override the path.

@ghost ghost assigned ferventcoder Aug 8, 2012
if ($userVar -ne $folder) {
write-host "Creating $chocInstallVariableName as a User Environment variable and setting it to `'$folder`'"
[Environment]::SetEnvironmentVariable($chocInstallVariableName, $folder, [System.EnvironmentVariableTarget]::User)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

So when you are a regular user who has never installed before, how do you get the environment variable set? Am I misunderstanding that process environment variable will aways be set?

Fixing logic around setting the chocolatey path.
This allows a user to preset a system or user environment variable
or specify a process based environment variable.  We only set a user variable
if the install directory came from a process based env or if it was the default.

Usage for Proc Based which will set a permanent user env var:
SET ChocolateyInstall=C:\Opt && @PowerShell -NoProfile -ExecutionPolicy unrestricted -Command "iex ((new-object net.webclient).DownloadString('http://bit.ly/psChocInstall'))" && SET PATH=%PATH%;%ChocolateyInstall%\bin
@abombss
Copy link
Author

abombss commented Aug 8, 2012

That was a good catch. In my testing I realize I always had a var set in which case the default value was always falling through. The idea I am thinking of is that I only want to set the variable if its the Default or if the User overrides it via the "Process" scope environment variable. Otherwise the variable came from System or User scope in which case an explicit User scoped environment variable doesn't need to be written because something already exists.

Does that make sense? I updated the pull request to fix the issue, let me know if there is anything else you would like me to do.

@rismoney
Copy link
Contributor

i'd want to use exclusively system variables. so what do i need to do exactly?

is there a problem with passing params to InstallChocolatey.ps1 that this can't be done within it? isn't that what's in the bit.ly url that gets invoked?

@abombss
Copy link
Author

abombss commented Aug 10, 2012

I am all for passing parameters, is that possible, seems like a big change?
There still would need a fix for explicitly checking for a user scoped
environment var.
On Aug 9, 2012 8:42 PM, "Rich Siegel" [email protected] wrote:

i'd want to use exclusively system variables. so what do i need to do
exactly?

is there a problem with passing params to InstallChocolatey.ps1 that this
can't be done within it? isn't that what's in the bit.ly url that gets
invoked?


Reply to this email directly or view it on GitHubhttps://github.com//pull/135#issuecomment-7632159.

@ferventcoder
Copy link
Contributor

system vars require admin - that's why the default goes to user.

To go to system, it would just take adding them manually and applying this fix for now.

I think in the future we could make the installer smarter.

@rismoney
Copy link
Contributor

I was shooting for an optional switch if specified, while running under admin that could do it for me :) I have been installing chocolatey on 400 servers and I have seen some bizzare issues with environment path modification and powershell putting in 2 semi-colons or none.

@ferventcoder
Copy link
Contributor

Weird. Interesting. It might be a bug somewhere in chocolatey's installer script.

@abombss
Copy link
Author

abombss commented Aug 14, 2012

I actually just run my own bootstrap first that configures
chocolateyinstall and bin_root, then download and install chocolatey. Our
requirements are unique such that everything has to come from the inside.
No external packages are allowed. The only hangup I had was chocolatey
looking explicitly to user var instead of just $env:. This should fix that
for now.

The installer could use some work. The biggest issue is the lack of named
parameter support for invoking commands, only positional args that I am
aware of with ps.

Maybe a dictionary hash or reference to a simple property file as the only
installer arg?
On Aug 14, 2012 9:06 AM, "Rich Siegel" [email protected] wrote:

I was shooting for an optional switch if specified, while running under
admin that could do it for me :) I have been installing chocolatey on 400
servers and I have seen some bizzare issues with environment path
modification and powershell putting in 2 semi-colons or none.


Reply to this email directly or view it on GitHubhttps://github.com//pull/135#issuecomment-7726761.

@ferventcoder
Copy link
Contributor

I would be fine with more named parameters in there.

Keep in mind that whatever is done, it should work without the supplied parameters or property files.

@rismoney
Copy link
Contributor

I saw a discussion here - http://stackoverflow.com/questions/4225748/how-do-i-pass-named-parameters-with-invoke-command which uses script blocks- somewhat convoluted.

@rismoney
Copy link
Contributor

@ferventcoder

To go to system, it would just take adding them manually and applying this fix for now. I think in the future we could make the installer smarter.

Agree. Can we do a git fetch upstream and git rebase upstream/master (assuming your chocolatey/chocolatey remote is upstream). Lets get the innovation, but lets get @abombss to a git rebase -i fixup on the 2nd commit, especially since it mods the same file, and we want every commit to stand on its own as workable going forward.

@ferventcoder
Copy link
Contributor

Think I'm getting ready to pull this in. We'll also be rewriting this to make it a ton cleaner as well.

@ferventcoder
Copy link
Contributor

This being the installer, not your code. :)

@ferventcoder
Copy link
Contributor

Definitely want to get this into .21

@ferventcoder
Copy link
Contributor

I don't think is necessary now. 0.9.8.24 had some major improvements to how it handles environment variables during install.

This is superseded by #453 which was later superseded by #486.

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

Successfully merging this pull request may close these issues.

3 participants