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

Install module to 'Program Files' and remove modifying $PROFILE #578

Open
wants to merge 1 commit into
base: v0
Choose a base branch
from
Open

Conversation

pauby
Copy link

@pauby pauby commented May 14, 2018

This is the Chocolatey package install / uninstall files I use which copies the embedded module to the Program Files\WindowsPowerShell\Modules folder.

The module itself is embedded into the package for easier distribution and I do this using an automatic update script. As you're not using that then simply add the code you want to distribute into a folder called posh-git within the tools folder. Inside the posh-git folder should be the code itself. The installer script takes care of creating a version folder if needed for PS 5+.

See my other modules for PSScriptAnalyzer, PSCodeHealth, Plaster, PlatyPS, PSStringTemplate and EditorServicesCommandSuite here for more info.

@dahlbyk
Copy link
Owner

dahlbyk commented May 14, 2018

I'll take a closer look at your new code later, but for now:

  • 👍 for new location
  • 👍 for including only relevant files
  • It seems like we should try to clean up the legacy $poshgitPath on install to the new path, as we currently do on uninstall.
  • Similarly, if we can detect that this package has previously modified someone's $PROFILE with a reference to $poshgitPath (and other legacy cruft), we should continue to clean up after ourselves.
  • As I mentioned in Avoiding creating a C:\tools directory #473 (comment), thoughts on how we could let the user opt out of modifying profile? Or how we could prompt if they wish us to add to profile?

@dahlbyk dahlbyk self-requested a review May 14, 2018 23:03
catch {}
throw
if ($PSVersionTable.PSVersion.Major -lt 4) {
$modulePaths = [Environment]::GetEnvironmentVariable('PSModulePath', 'Machine') -split ';'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we assume no one will try to use Chocolatey to install this on Linux or macOS? If not, you should use [System.IO.Path]::PathSeparator instead of hard-coding in ;.

Copy link
Author

Choose a reason for hiding this comment

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

For the time being we can. As per the next comment this is only for v0 so we don't need to worry about PS Core.

Remove-Module -Name $moduleName -Force -ErrorAction SilentlyContinue

$sourcePath = Join-Path -Path $toolsDir -ChildPath "$modulename\*"
$destPath = Join-Path -Path $env:ProgramFiles -ChildPath "WindowsPowerShell\Modules\$moduleName"
Copy link
Collaborator

@rkeithhill rkeithhill May 14, 2018

Choose a reason for hiding this comment

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

A big part of the v1 release is to support PowerShell Core. If the user has PS Core installed, we should also install into "PowerShell\Modules\$moduleName" if the $env:ProgramFiles\PowerShell dir exists.

Copy link
Author

Choose a reason for hiding this comment

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

I agree. I have been wondering what to do about whether you run Chocolatey under Core or not and where you should put the modules. However this is purely for the v0 branch so PS Core wasn't a consideration.

@rkeithhill
Copy link
Collaborator

RE:

for including only relevant files

Don't mean to sound like a broken record but ... with a build script - or at least a publish script - we could copy the relevant module files for distribution into an out dir. Which I have proposed would be <repo-root>/<version-number>. :-)

@pauby
Copy link
Author

pauby commented May 15, 2018

As I mentioned in #473 (comment), thoughts on how we could let the user opt out of modifying profile? Or how we could prompt if they wish us to add to profile?
We could also reasonably let them specify which profile to modify, assuming someone gets around to fixing chocolatey/choco#1154.

I think the way you should do this is to allow the user to opt in to modifying the profile. Let me try and explain.

If you have an op-out then when I install it I use choco install poshgit --params='/optout' and there is no profile modification. When I come to upgrade it (and everything else) I use choco upgrade all and everything is upgraded but because I didn't specify that parameter it goes off an modifies my profile again.

If you have opt-in then when I install it I use choco install poshgit --params='/optin' and my profile is modified. When I come to upgrade I do choco upgrade all and because I didn't specify the parameter my profile is not modified. But it was modified at install the first time so no need to do it again.

The difference is an opt-out will try and modify my profile every single time the package is installed / upgraded and I need to try and stop it. An opt-in only modifies the profile at install (with the parameter) and not every time I install / upgrade. But it only has to modify the profile once anyway.

If a user doesn't opt-in but wishes they had they can always reinstall the package or simply use the Add-PoshGitToProfile cmdlet.

@dahlbyk
Copy link
Owner

dahlbyk commented May 15, 2018

Can our Chocolatey installer tell if it's a clean install or an upgrade?

Assuming we're installing into a location in $Env:PSModulePath, profiles shouldn't need more than Import-Module posh-git. Consequently, it seems safe for upgrades to assume posh-git is already installed to the user's profile. Or, accounting for legacy Chocolatey installs, perhaps we make that assumption only if the legacy $poshgitPath doesn't exist?

Ultimately for v0 I'm trying to avoid a breaking change in how users are accustomed to a Chocolatey install of posh-git working.

@pauby
Copy link
Author

pauby commented May 15, 2018

Can our Chocolatey installer tell if it's a clean install or an upgrade?

You can simply detect if the module exists as a loadable module or not. But you'll need to add some extra code around checking the tools folder.

@dahlbyk dahlbyk added this to the v0.8.0 milestone May 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants