-
Notifications
You must be signed in to change notification settings - Fork 69
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
(MODULES-8521) Fix $chocolatey_version parameter #103
Conversation
The Chocolatey version parameter is documented to be informational only. I regret having added it for the confusion that it brings when people see it. |
templates/InstallChocolatey.ps1.erb
Outdated
@@ -21,7 +21,7 @@ $ErrorActionPreference = 'Stop' | |||
#try { | |||
|
|||
# variables | |||
$url = '<%= @download_url %>' | |||
$url = '<%= @download_url %><% if @chocolatey_version.to_s =~ /\d+\./ %>chocolatey.<%= @chocolatey_version %>.nupkg<% end %>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation states that the download url should download a nupkg if you go to it on a bare url. So this change would not be enough.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moreover it would be an incorrect change (to clarify).
Are you interested in making it work or do you prefer it the way it is? In this case, pls close the PR but maybe leave the bug open. No harm done :) |
templates/InstallChocolatey.ps1.erb
Outdated
@@ -21,7 +21,7 @@ $ErrorActionPreference = 'Stop' | |||
#try { | |||
|
|||
# variables | |||
$url = '<%= @download_url %>' | |||
$url = '<%= @download_url %><% if @chocolatey_version.to_s =~ /\d+\./ %><%= @chocolatey_version %><% end %>' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ferventcoder - Right, this should work now: Download URI renders to https://chocolatey.org/api/v2/package/chocolatey/0.10.10
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A catch would be if the last /
is missing from @download_url
. Should I add another regex? In this case, it would look nicer in it's own code block.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem is you are only showing how to make it work with the community repo. What about Artifactory, Nexus, ProGet, Chocolatey Simple Server, a file location, or any other the other repository types?
90% of the time the expectation is that an organization is using Puppet and Chocolatey, which means not using the community package repository, even to install Chocolatey. See https://chocolatey.org/docs/how-to-setup-offline-installation for our guide on organizational use.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think all these use cases are fairly easy to cover; we only need to check if our download URL matches something like /^http(s):\/\/.*api/v2/package.*\/$/
. IMHO the version param is only applicable when using a nuget server for downloads anyway.
which means not using the community package repository
This is also our use case; however we do sync the choco package from community to or internal nuget and therefore I most likely end up installing the most recent version instead the version I specify in my manifest.
manifests/install.pp
Outdated
true => '7zip', | ||
default => 'windows' | ||
} | ||
|
||
if "${_download_url}" =~ /^http(s):\/\/.*api\/v2\/package.*\/$/ and "${chocolatey_version}" =~ /\d+\./ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ferventcoder, you think this covers it?
c247009
to
08037cc
Compare
Hello @helge000 , In order to merge this PR, please resolve the conflicts and rebase it with the latest code version. Thanks! |
@ferventcoder - How do you envision people ensuring they have the version of chocolatey they want installed with chocolatey_version being information only? I've decided to do the following:
On first run will result in chocolatey installing whatever version the module uses, and then upgrade itself, subsequent runs do nothing regarding the class instantiation but chocolatey can be upgraded using the package resource. I suspect this is the way to go for now unless you have plans for chocolatey_version to be more than informational? Thanks for your time, I appreciate it, and of course, thanks for Chocolatey!
|
Hello @helge000 , |
08037cc
to
63f8cc8
Compare
e51629e
to
947c460
Compare
Fixes $chocolatey_version to acually bootstrap the given version of chocolatey.
947c460
to
7fa2fc4
Compare
@sheenaajay - rebased. Hopefully this gets merged after 1.5 years... |
Fixes $chocolatey_version to acually bootstrap the given version of
chocolatey.