Skip to content

Commit f6ea649

Browse files
yaauieyaauie
committed
plugins: improve remove command to support multiple plugins
Removal works in a single pass by finding plugins that would have unmet dependencies if all of the specified plugins were to be removed, and proceeding with the removal only if no conflicts were created. > ~~~ > ╭─{ rye@perhaps:~/src/elastic/logstash@main (pluginmanager-remove-multiple ✘) } > ╰─● bin/logstash-plugin remove logstash-input-syslog logstash-filter-grok > Using system java: /Users/rye/.jenv/shims/java > Resolving dependencies...... > Successfully removed logstash-input-syslog > Successfully removed logstash-filter-grok > [success (00:00:05)] ~~~
1 parent 78c3446 commit f6ea649

13 files changed

+380
-93
lines changed

‎lib/pluginmanager/bundler/logstash_uninstall.rb‎

Lines changed: 51 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -34,54 +34,50 @@ def initialize(gemfile_path, lockfile_path)
3434
@lockfile_path = lockfile_path
3535
end
3636

37-
# To be uninstalled the candidate gems need to be standalone.
38-
def dependants_gems(gem_name)
39-
builder = Dsl.new
40-
builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "original gemfile"), File.read(gemfile_path))
41-
definition = builder.to_definition(lockfile_path, {})
42-
43-
definition.specs
44-
.select { |spec| spec.dependencies.collect(&:name).include?(gem_name) }
45-
.collect(&:name).sort.uniq
46-
end
47-
48-
def uninstall!(gem_name)
49-
unfreeze_gemfile do
50-
dependencies_from = dependants_gems(gem_name)
51-
52-
if dependencies_from.size > 0
53-
display_cant_remove_message(gem_name, dependencies_from)
54-
false
55-
else
56-
remove_gem(gem_name)
57-
true
37+
def uninstall!(gems_to_remove)
38+
gems_to_remove = Array(gems_to_remove)
39+
40+
unsatisfied_dependency_mapping = Dsl.evaluate(gemfile_path, lockfile_path, {}).specs.each_with_object({}) do |spec, memo|
41+
next if gems_to_remove.include?(spec.name)
42+
deps = spec.runtime_dependencies.collect(&:name)
43+
deps.intersection(gems_to_remove).each do |missing_dependency|
44+
memo[missing_dependency] ||= []
45+
memo[missing_dependency] << spec.name
5846
end
5947
end
60-
end
48+
if unsatisfied_dependency_mapping.any?
49+
unsatisfied_dependency_mapping.each { |gem_to_remove, gems_depending_on_removed| display_cant_remove_message(gem_to_remove, gems_depending_on_removed) }
50+
LogStash::PluginManager.ui.info("No plugins were removed.")
51+
return false
52+
end
53+
54+
with_mutable_gemfile do |gemfile|
55+
gems_to_remove.each { |gem_name| gemfile.remove(gem_name) }
6156

62-
def remove_gem(gem_name)
63-
builder = Dsl.new
64-
file = File.new(gemfile_path, "r+")
57+
builder = Dsl.new
58+
builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "gemfile to changes"), gemfile.generate)
6559

66-
gemfile = LogStash::Gemfile.new(file).load
67-
gemfile.remove(gem_name)
68-
builder.eval_gemfile(::File.join(::File.dirname(gemfile_path), "gemfile to changes"), gemfile.generate)
60+
# build a definition, providing an intentionally-empty "unlock" mapping
61+
# to ensure that all gem versions remain locked
62+
definition = builder.to_definition(lockfile_path, {})
6963

70-
definition = builder.to_definition(lockfile_path, {})
71-
definition.lock(lockfile_path)
72-
gemfile.save
64+
# lock the definition and save our modified gemfile
65+
definition.lock(lockfile_path)
66+
gemfile.save
67+
68+
gems_to_remove.each do |gem_name|
69+
LogStash::PluginManager.ui.info("Successfully removed #{gem_name}")
70+
end
7371

74-
LogStash::PluginManager.ui.info("Successfully removed #{gem_name}")
75-
ensure
76-
file.close if file
72+
return true
73+
end
7774
end
7875

7976
def display_cant_remove_message(gem_name, dependencies_from)
80-
message = <<-eos
81-
Failed to remove \"#{gem_name}\" because the following plugins or libraries depend on it:
82-
83-
* #{dependencies_from.join("\n* ")}
84-
eos
77+
message = <<~EOS
78+
Failed to remove \"#{gem_name}\" because the following plugins or libraries depend on it:
79+
* #{dependencies_from.join("\n* ")}
80+
EOS
8581
LogStash::PluginManager.ui.info(message)
8682
end
8783

@@ -93,10 +89,22 @@ def unfreeze_gemfile
9389
end
9490
end
9591

96-
def self.uninstall!(gem_name, options = { :gemfile => LogStash::Environment::GEMFILE, :lockfile => LogStash::Environment::LOCKFILE })
97-
gemfile_path = options[:gemfile]
98-
lockfile_path = options[:lockfile]
99-
LogstashUninstall.new(gemfile_path, lockfile_path).uninstall!(gem_name)
92+
##
93+
# Yields a mutable gemfile backed by an open, writable file handle.
94+
# It is the responsibility of the caller to send `LogStash::Gemfile#save` to persist the result.
95+
# @yieldparam [LogStash::Gemfile]
96+
def with_mutable_gemfile
97+
unfreeze_gemfile do
98+
File.open(gemfile_path, 'r+') do |file|
99+
yield LogStash::Gemfile.new(file).load
100+
end
101+
end
102+
end
103+
104+
def self.uninstall!(gem_names, options={})
105+
gemfile_path = options[:gemfile] || LogStash::Environment::GEMFILE
106+
lockfile_path = options[:lockfile] || LogStash::Environment::LOCKFILE
107+
LogstashUninstall.new(gemfile_path, lockfile_path).uninstall!(Array(gem_names))
100108
end
101109
end
102110
end

‎lib/pluginmanager/remove.rb‎

Lines changed: 32 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -20,48 +20,51 @@
2020
require "pluginmanager/command"
2121

2222
class LogStash::PluginManager::Remove < LogStash::PluginManager::Command
23-
parameter "PLUGIN", "plugin name"
23+
parameter "PLUGIN ...", "plugin name(s)"
2424

2525
def execute
26+
signal_error("One or more plugins must be specified") if plugin_list.empty?
2627
signal_error("File #{LogStash::Environment::GEMFILE_PATH} does not exist or is not writable, aborting") unless File.writable?(LogStash::Environment::GEMFILE_PATH)
2728

2829
LogStash::Bundler.prepare({:without => [:build, :development]})
2930

30-
if LogStash::PluginManager::ALIASES.has_key?(plugin)
31-
unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
32-
aliased_plugin = LogStash::PluginManager::ALIASES[plugin]
33-
puts "Cannot remove the alias #{plugin}, which is an alias for #{aliased_plugin}; if you wish to remove it, you must remove the aliased plugin instead."
34-
return
31+
plugin_list.each do |plugin|
32+
if LogStash::PluginManager::ALIASES.has_key?(plugin)
33+
unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
34+
aliased_plugin = LogStash::PluginManager::ALIASES[plugin]
35+
puts "Cannot remove the alias #{plugin}, which is an alias for #{aliased_plugin}; if you wish to remove it, you must remove the aliased plugin instead."
36+
return
37+
end
3538
end
36-
end
3739

38-
# If a user is attempting to uninstall X-Pack, present helpful output to guide
39-
# them toward the OSS-only distribution of Logstash
40-
LogStash::PluginManager::XPackInterceptor::Remove.intercept!(plugin)
40+
# If a user is attempting to uninstall X-Pack, present helpful output to guide
41+
# them toward the OSS-only distribution of Logstash
42+
LogStash::PluginManager::XPackInterceptor::Remove.intercept!(plugin)
4143

42-
# if the plugin is provided by an integration plugin. abort.
43-
if integration_plugin = LogStash::PluginManager.which_integration_plugin_provides(plugin, gemfile)
44-
signal_error("This plugin is already provided by '#{integration_plugin.name}' so it can't be removed individually")
45-
end
44+
# if the plugin is provided by an integration plugin. abort.
45+
if integration_plugin = LogStash::PluginManager.which_integration_plugin_provides(plugin, gemfile)
46+
signal_error("The plugin `#{plugin}` is provided by '#{integration_plugin.name}' so it can't be removed individually")
47+
end
4648

47-
not_installed_message = "This plugin has not been previously installed"
48-
plugin_gem_spec = LogStash::PluginManager.find_plugins_gem_specs(plugin).any?
49-
if plugin_gem_spec
50-
# make sure this is an installed plugin and present in Gemfile.
51-
# it is not possible to uninstall a dependency not listed in the Gemfile, for example a dependent codec
52-
signal_error(not_installed_message) unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
53-
else
54-
# locally installed plugins are not recoginized by ::Gem::Specification
55-
# we may ::Bundler.setup to reload but it resets all dependencies so we get error message for future bundler operations
56-
# error message: `Bundler::GemNotFound: Could not find rubocop-1.60.2... in locally installed gems`
57-
# instead we make sure Gemfile has a definition and ::Gem::Specification recognizes local gem
58-
signal_error(not_installed_message) unless !!gemfile.find(plugin)
49+
not_installed_message = "The plugin `#{plugin}` has not been previously installed"
50+
plugin_gem_spec = LogStash::PluginManager.find_plugins_gem_specs(plugin).any?
51+
if plugin_gem_spec
52+
# make sure this is an installed plugin and present in Gemfile.
53+
# it is not possible to uninstall a dependency not listed in the Gemfile, for example a dependent codec
54+
signal_error(not_installed_message) unless LogStash::PluginManager.installed_plugin?(plugin, gemfile)
55+
else
56+
# locally installed plugins are not recoginized by ::Gem::Specification
57+
# we may ::Bundler.setup to reload but it resets all dependencies so we get error message for future bundler operations
58+
# error message: `Bundler::GemNotFound: Could not find rubocop-1.60.2... in locally installed gems`
59+
# instead we make sure Gemfile has a definition and ::Gem::Specification recognizes local gem
60+
signal_error(not_installed_message) unless !!gemfile.find(plugin)
5961

60-
local_gem = gemfile.locally_installed_gems.detect { |local_gem| local_gem.name == plugin }
61-
signal_error(not_installed_message) unless local_gem
62+
local_gem = gemfile.locally_installed_gems.detect { |local_gem| local_gem.name == plugin }
63+
signal_error(not_installed_message) unless local_gem
64+
end
6265
end
6366

64-
exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin)
67+
exit(1) unless ::Bundler::LogstashUninstall.uninstall!(plugin_list)
6568
LogStash::Bundler.genericize_platform
6669
remove_unused_locally_installed_gems!
6770
rescue => exception

‎qa/integration/fixtures/plugins/generate-gems.sh‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
#!/usr/bin/env sh
2+
3+
cd "$( dirname "$0" )"
4+
find . -name '*.gemspec' | xargs -n1 gem build

‎qa/integration/fixtures/plugins/logstash-filter-four_depends_on_one_and_three.gemspec‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
Gem::Specification.new do |s|
2+
s.name = File.basename(__FILE__, ".gemspec")
3+
s.version = '0.1.1'
4+
s.licenses = ['Apache-2.0']
5+
s.summary = "A dummy plugin with two plugin dependencies"
6+
s.description = "This plugin is only used in the acceptance test"
7+
s.authors = ["Elasticsearch"]
8+
s.email = 'info@elasticsearch.com'
9+
s.homepage = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
10+
s.require_paths = ["lib"]
11+
12+
# Files
13+
s.files = [__FILE__]
14+
15+
# Tests
16+
s.test_files = []
17+
18+
# Special flag to let us know this is actually a logstash plugin
19+
s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
20+
21+
# Gem dependencies
22+
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
23+
24+
s.add_runtime_dependency "logstash-filter-one_no_dependencies", "~> 0.1"
25+
s.add_runtime_dependency "logstash-filter-three_no_dependencies", "~> 0.1"
26+
end

‎qa/integration/fixtures/plugins/logstash-filter-one_no_dependencies.gemspec‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
Gem::Specification.new do |s|
2+
s.name = File.basename(__FILE__, ".gemspec")
3+
s.version = '0.1.1'
4+
s.licenses = ['Apache-2.0']
5+
s.summary = "A dummy plugin with no dependencies"
6+
s.description = "This plugin is only used in the acceptance test"
7+
s.authors = ["Elasticsearch"]
8+
s.email = 'info@elasticsearch.com'
9+
s.homepage = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
10+
s.require_paths = ["lib"]
11+
12+
# Files
13+
s.files = [__FILE__]
14+
15+
# Tests
16+
s.test_files = []
17+
18+
# Special flag to let us know this is actually a logstash plugin
19+
s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
20+
21+
# Gem dependencies
22+
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
23+
end

‎qa/integration/fixtures/plugins/logstash-filter-three_no_dependencies.gemspec‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
Gem::Specification.new do |s|
2+
s.name = File.basename(__FILE__, ".gemspec")
3+
s.version = '0.1.1'
4+
s.licenses = ['Apache-2.0']
5+
s.summary = "A dummy plugin with no dependencies"
6+
s.description = "This plugin is only used in the acceptance test"
7+
s.authors = ["Elasticsearch"]
8+
s.email = 'info@elasticsearch.com'
9+
s.homepage = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
10+
s.require_paths = ["lib"]
11+
12+
# Files
13+
s.files = [__FILE__]
14+
15+
# Tests
16+
s.test_files = []
17+
18+
# Special flag to let us know this is actually a logstash plugin
19+
s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
20+
21+
# Gem dependencies
22+
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
23+
end

‎qa/integration/fixtures/plugins/logstash-filter-two_depends_on_one.gemspec‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
Gem::Specification.new do |s|
2+
s.name = File.basename(__FILE__, ".gemspec")
3+
s.version = '0.1.1'
4+
s.licenses = ['Apache-2.0']
5+
s.summary = "A dummy plugin with one plugin dependency"
6+
s.description = "This plugin is only used in the acceptance test"
7+
s.authors = ["Elasticsearch"]
8+
s.email = 'info@elasticsearch.com'
9+
s.homepage = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
10+
s.require_paths = ["lib"]
11+
12+
# Files
13+
s.files = [__FILE__]
14+
15+
# Tests
16+
s.test_files = []
17+
18+
# Special flag to let us know this is actually a logstash plugin
19+
s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
20+
21+
# Gem dependencies
22+
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
23+
24+
s.add_runtime_dependency "logstash-filter-one_no_dependencies", "~> 0.1"
25+
end

‎qa/integration/fixtures/plugins/logstash-filter-zero_no_dependencies.gemspec‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
Gem::Specification.new do |s|
2+
s.name = File.basename(__FILE__, ".gemspec")
3+
s.version = '0.1.1'
4+
s.licenses = ['Apache-2.0']
5+
s.summary = "A dummy plugin with no dependencies"
6+
s.description = "This plugin is only used in the acceptance test"
7+
s.authors = ["Elasticsearch"]
8+
s.email = 'info@elasticsearch.com'
9+
s.homepage = "http://www.elasticsearch.org/guide/en/logstash/current/index.html"
10+
s.require_paths = ["lib"]
11+
12+
# Files
13+
s.files = [__FILE__]
14+
15+
# Tests
16+
s.test_files = []
17+
18+
# Special flag to let us know this is actually a logstash plugin
19+
s.metadata = { "logstash_plugin" => "true", "logstash_group" => "filter" }
20+
21+
# Gem dependencies
22+
s.add_runtime_dependency "logstash-core-plugin-api", ">= 1.60", "<= 2.99"
23+
end

‎qa/integration/framework/settings.rb‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
# under the License.
1717

1818
require 'yaml'
19+
require 'delegate'
1920

2021
# All settings for a test, global and per test
2122
class TestSettings
@@ -76,4 +77,23 @@ def feature_config_dir
7677
feature = feature_flag
7778
feature.empty? ? nil : File.join(FIXTURES_DIR, feature)
7879
end
80+
81+
def override(kv_map)
82+
Override.new(self, kv_map)
83+
end
84+
85+
class Override < SimpleDelegator# quacks like TestSettings
86+
def initialize(test_settings, overrides)
87+
super(test_settings)
88+
@overrides = overrides
89+
end
90+
91+
def is_set?(key)
92+
@overrides.include?(key) || super
93+
end
94+
95+
def get(key)
96+
@overrides.fetch(key) { super }
97+
end
98+
end
7999
end

0 commit comments

Comments
 (0)