-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
add --preserve option to plugin install #5224
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ class LogStash::PluginManager::Install < LogStash::PluginManager::Command | |
parameter "[PLUGIN] ...", "plugin name(s) or file", :attribute_name => :plugins_arg | ||
option "--version", "VERSION", "version of the plugin to install" | ||
option "--[no-]verify", :flag, "verify plugin validity before installation", :default => true | ||
option "--preserve", :flag, "preserve current gem options", :default => false | ||
option "--development", :flag, "install all development dependencies of currently installed plugins", :default => false | ||
option "--local", :flag, "force local-only plugin installation. see bin/logstash-plugin package|unpack", :default => false | ||
|
||
|
@@ -90,7 +91,15 @@ def install_gems_list!(install_list) | |
|
||
# Add plugins/gems to the current gemfile | ||
puts("Installing" + (install_list.empty? ? "..." : " " + install_list.collect(&:first).join(", "))) | ||
install_list.each { |plugin, version, options| gemfile.update(plugin, version, options) } | ||
install_list.each do |plugin, version, options| | ||
if preserve? | ||
plugin_gem = gemfile.find(plugin) | ||
puts("Preserving Gemfile gem options for plugin #{plugin}") if plugin_gem && !plugin_gem.options.empty? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think its would be cool to report the options in the logger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is not logger instance in the plugin manager There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. err I should have said, it would be cool to report what the options this plugins is currently using the the puts output. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ahh yes. I'd prefer not too because the installed gem option are not just discarded in favour of the options in the Gemfile, they are actually merged in the Gemfile options but at this layer, we don't know yet the result of that merge. Very practically, I don't think the plugins manager ever pass options to install just yet so it can be assumed today that the current options in the Gemfile will be the final options but that could eventually not be the case ... so I don't think it's really worth investing too much time on this now, if we think it's really valuable we can take another look at that point? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we agreed on zoom to not add this. |
||
gemfile.update(plugin, version, options) | ||
else | ||
gemfile.overwrite(plugin, version, options) | ||
end | ||
end | ||
|
||
# Sync gemfiles changes to disk to make them available to the `bundler install`'s API | ||
gemfile.save | ||
|
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.
maybe add a debug log message here to make the behavior known on a per plugin basis?
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.
yap, added!