chocolatey / cChoco

Community resource to manage Chocolatey
Apache License 2.0
154 stars 99 forks source link

(GH-139) Added Chocolatey installation from local script #142

Closed wizarden1 closed 3 years ago

wizarden1 commented 4 years ago

Fixes and changes:

remove trailing spaces for newer version of test
rewrite cChocoInstaller for use nuget source(chocolate nuget source) directly
pauby commented 4 years ago

Thanks for the submission @wizarden1.

It looks like you are replacing the installation by script method (the script URL was in $ChocoInstallScriptUrl) by downloading the Chocolatey package and executing the chocolateyInstall.ps1 file?

wizarden1 commented 4 years ago

Thanks for the submission @wizarden1.

It looks like you are replacing the installation by script method (the script URL was in $ChocoInstallScriptUrl) by downloading the Chocolatey package and executing the chocolateyInstall.ps1 file? Yes, I move what do install script in to a module. As I understand, that choco do natively.

pauby commented 4 years ago

We can't remove that functionality as the Chocolatey install script preps the system for install of Chocolatey. That's not something that is done inside the chocolateyInstall.ps1 of the Chocolatey package.

In addition that functionality allows your own script to be defined and used so it can't be removed as others may well be using this.

pauby commented 4 years ago

I've also noticed that in Get-ChocoPackageUrl you are querying the Chocolatey API directly (Packages()?$filter=((Id%20eq%20%27chocolatey%27)%20and%20(not%20IsPrerelease))%20and%20IsLatestVersion') which isn't supported. The only supported methods to query the Chocolatey API is via choco.exe or using ChocolateyLib - neither of which you can use here because Chocolatey isn't installed.

I don't want to start a review of the PR until these major show stoppers are discussed.

wizarden1 commented 4 years ago

We can't remove that functionality as the Chocolatey install script preps the system for install of Chocolatey. That's not something that is done inside the chocolateyInstall.ps1 of the Chocolatey package.

In addition that functionality allows your own script to be defined and used so it can't be removed as others may well be using this.

That interesting, I investigate install.ps1 for system preparation and that i got from it:

  1. Use system env for install - I replace it by parameters
  2. Fix-PowerShellOutputRedirectionBug - fixes for powershell 2/3 than can't use dsc at all. So it's not possible this problem will appear
  3. Use system proxy - really, I didn't implement it.
  4. $url = 'https://chocolatey.org/api/v2/Packages()?$filter=((Id%20eq%20%27chocolatey%27)%20and%20(not%20IsPrerelease))%20and%20IsLatestVersion' this link has been taken from install.ps1
  5. Use 7z, I didn't use it, because Expand-Archive already exists in powershell 5 and upper, yes powershell 4 doesn't have it, is it a problem?
  6. After that install.ps1 extract downloaded file and run "chocolateyInstall.ps1" - the same I do

Did I miss something?

pauby commented 4 years ago

I'm not sure how that answers the question about not removing the existing functionality for the install script?

wizarden1 commented 4 years ago

I'm not sure how that answers the question about not removing the existing functionality for the install script?

I have an Idea, I can write an addition, that will use install.ps1 script for people who want it. So it will be two solutions. How about that?

wizarden1 commented 4 years ago

We can't remove that functionality as the Chocolatey install script preps the system for install of Chocolatey. That's not something that is done inside the chocolateyInstall.ps1 of the Chocolatey package.

I'm curious "preps the system" what for example?

In addition that functionality allows your own script to be defined and used so it can't be removed as others may well be using this.

I thought all day and really have no idea for what else can be used install.ps1 except install choco. Do you have any examples? You meas someone will write own install script.

I want to do best for others and I'll be really appreciated if you help with this.

pauby commented 4 years ago

You meas someone will write own install script.

That's exactly it. We recommend organizations internalize the Chocolatey install script so they will have their own, tweaked, versions of it.

In addition to that, this is functionality that has been there for some time and we cannot remove it.

I'm curious "preps the system" what for example?

As I said above, organizations may have their own install scripts so I can't give you an example beyond what is done inside the Chocolatey install script. What I don't want to do is get bogged down in a discussion around the merits of the Chocolatey install script. It really comes down that we cannot remove that functionality.

I want to do best for others and I'll be really appreciated if you help with this.

We all appreciate that and appreciate the time you have taken to submit this PR. This is what community is all about so please don't take my comments as negative. My goal here is to ensure we continue to support the community who use the module and in particular use the functionality that is being removed.

wizarden1 commented 4 years ago

Thanks for your answer and explanation. I'll return this functionality back and make it usage primary and set my functionality as alternative, this solution will be acceptable?

wizarden1 commented 4 years ago

Hello Paul I made changes that you request, have you review them?

pauby commented 4 years ago

Two comments on this PR:

  1. All of the formatting changes should be removed and put into a separate commit as it makes reviewing the PR very difficult;
  2. I don't think the creation of a separate resource to allow installation of Chocolatey from an internal source is required. A better way of doing this would be to look at the user supplying their own script and installing from there as well as installing from Chocolatey.org as a default.
wizarden1 commented 4 years ago

You mean to return to way as it was, am I right?

pauby commented 4 years ago

Now that I've seen the code for this I don't believe it is something we could merge as it stands now.

As I said in the previous comment reviewing the PR when you've mixed up formatting changes with code changes is very difficult. I'm unsure why we need a separate resource to install Chocolatey from a local script?

wizarden1 commented 4 years ago

The main idea to replace script for native install was like that:

pauby commented 4 years ago

Closed in error. Reopened.

For large deployment from one ip I get an error (rate limit) from choco servers. Because each server want to download and install choco, and this script can download choco package ether from choco (nuget) server, or form my web server.

As this is being used in an organization you might want to follow the organizational best practice. These links may also help:

Create my own web server with choco package, and write my own script. And do update of choco package manually on this server. Or write another script and put it into cron for upgrading. Also except of this script I need resource where this script and web server will be running. So I need +1 VM for this task, and this is money spending for just install choco.

If you set up the repositories mentioned above you can use them to host the internal Chocolatey package. Or alternatively you can put the Chocolatey package on a file share (not recommended but will work) which doesn't require a server (assuming you have file shares). Running a script in a cron job doesn't require a new server. You could also run an 'update script manually to create the updated Chocolatey package locally.

The issues that you have, and are trying to solve with this PR, can be worked around following the organizational best practices for Chocolatey using the links I mentioned. Using Chocolatey.org directly, for organizational use, is not recommended and will result in rate limiting or IP blocking at some point. Following the best practice will avoid this.

Having said that I'm, not against having the option to install Chocolatey from a local script (local as in internal to your 'network'). But, as I said, I don't see why we need a separate resource. Nothing you've said convinces me that we do. The implementation here, in this Pull Request, as it stands now, is not something that can be merged.

pauby commented 3 years ago

There has been no update to this in over 5 months. Closing.