Audit use of :run dependencies.

These are a no-op so let's remove them.
This commit is contained in:
Mike McQuaid 2018-03-19 10:11:08 +00:00
parent efe07f4a43
commit d2c23bde6d
10 changed files with 24 additions and 63 deletions

View File

@ -29,3 +29,4 @@ require "compat/ENV/super"
require "compat/utils/shell" require "compat/utils/shell"
require "compat/extend/string" require "compat/extend/string"
require "compat/gpg" require "compat/gpg"
require "compat/dependable"

View File

@ -0,0 +1,5 @@
module Dependable
def run?
tags.include? :run
end
end

View File

@ -33,7 +33,6 @@ class DependencyCollector
output_deprecation(spec) output_deprecation(spec)
Dependency.new(spec.to_s, tags) Dependency.new(spec.to_s, tags)
when :libltdl when :libltdl
tags << :run
output_deprecation("libtool") output_deprecation("libtool")
Dependency.new("libtool", tags) Dependency.new("libtool", tags)
when :apr when :apr
@ -68,7 +67,7 @@ class DependencyCollector
private private
def autotools_dep(spec, tags) def autotools_dep(spec, tags)
tags << :build unless tags.include? :run tags << :build
Dependency.new(spec.to_s, tags) Dependency.new(spec.to_s, tags)
end end

View File

@ -1,6 +1,8 @@
require "options" require "options"
module Dependable module Dependable
# :run and :linked are no longer used but keep them here to avoid them being
# misused in future.
RESERVED_TAGS = [:build, :optional, :recommended, :run, :test, :linked].freeze RESERVED_TAGS = [:build, :optional, :recommended, :run, :test, :linked].freeze
def build? def build?
@ -15,10 +17,6 @@ module Dependable
tags.include? :recommended tags.include? :recommended
end end
def run?
tags.include? :run
end
def test? def test?
tags.include? :test tags.include? :test
end end

View File

@ -163,13 +163,9 @@ class Dependency
end end
def merge_temporality(deps) def merge_temporality(deps)
if deps.all?(&:build?) # Means both build and runtime dependency.
[:build] return [] unless deps.all?(&:build?)
elsif deps.all?(&:run?) [:build]
[:run]
else
[] # Means both build and runtime dependency.
end
end end
end end
end end

View File

@ -172,23 +172,6 @@ class FormulaAuditor
attr_reader :formula, :text, :problems attr_reader :formula, :text, :problems
BUILD_TIME_DEPS = %w[
autoconf
automake
boost-build
bsdmake
cmake
godep
imake
intltool
libtool
pkg-config
scons
smake
sphinx-doc
swig
].freeze
def initialize(formula, options = {}) def initialize(formula, options = {})
@formula = formula @formula = formula
@new_formula = options[:new_formula] @new_formula = options[:new_formula]
@ -377,17 +360,12 @@ class FormulaAuditor
problem "Dependency #{dep} does not define option #{opt.name.inspect}" problem "Dependency #{dep} does not define option #{opt.name.inspect}"
end end
case dep.name if dep.name == "git"
when "git"
problem "Don't use git as a dependency (it's always available)" problem "Don't use git as a dependency (it's always available)"
when *BUILD_TIME_DEPS end
next if dep.build? || dep.run?
problem <<~EOS if dep.tags.include?(:run)
#{dep} dependency should be problem "Dependency '#{dep.name}' is marked as :run. Remove :run; it is a no-op."
depends_on "#{dep}" => :build
Or if it is indeed a runtime dependency
depends_on "#{dep}" => :run
EOS
end end
end end
end end

View File

@ -48,7 +48,7 @@ module RuboCop
def sort_dependencies_by_type(dependency_nodes) def sort_dependencies_by_type(dependency_nodes)
ordered = [] ordered = []
ordered.concat(dependency_nodes.select { |dep| buildtime_dependency? dep }.to_a) ordered.concat(dependency_nodes.select { |dep| buildtime_dependency? dep }.to_a)
ordered.concat(dependency_nodes.select { |dep| runtime_dependency? dep }.to_a) ordered.concat(dependency_nodes.select { |dep| test_dependency? dep }.to_a)
ordered.concat(dependency_nodes.reject { |dep| negate_normal_dependency? dep }.to_a) ordered.concat(dependency_nodes.reject { |dep| negate_normal_dependency? dep }.to_a)
ordered.concat(dependency_nodes.select { |dep| recommended_dependency? dep }.to_a) ordered.concat(dependency_nodes.select { |dep| recommended_dependency? dep }.to_a)
ordered.concat(dependency_nodes.select { |dep| optional_dependency? dep }.to_a) ordered.concat(dependency_nodes.select { |dep| optional_dependency? dep }.to_a)
@ -106,11 +106,11 @@ module RuboCop
def_node_search :recommended_dependency?, "(sym :recommended)" def_node_search :recommended_dependency?, "(sym :recommended)"
def_node_search :runtime_dependency?, "(sym :run)" def_node_search :test_dependency?, "(sym :test)"
def_node_search :optional_dependency?, "(sym :optional)" def_node_search :optional_dependency?, "(sym :optional)"
def_node_search :negate_normal_dependency?, "(sym {:build :recommended :run :optional})" def_node_search :negate_normal_dependency?, "(sym {:build :recommended :test :optional})"
# Node pattern method to extract `name` in `depends_on :name` # Node pattern method to extract `name` in `depends_on :name`
def_node_search :dependency_name_node, <<~EOS def_node_search :dependency_name_node, <<~EOS

View File

@ -171,7 +171,7 @@ module RuboCop
when :required when :required
type_match = required_dependency?(node) type_match = required_dependency?(node)
name_match ||= required_dependency_name?(node, name) if type_match name_match ||= required_dependency_name?(node, name) if type_match
when :build, :optional, :recommended, :run when :build, :test, :optional, :recommended
type_match = dependency_type_hash_match?(node, type) type_match = dependency_type_hash_match?(node, type)
name_match ||= dependency_name_hash_match?(node, name) if type_match name_match ||= dependency_name_hash_match?(node, name) if type_match
when :any when :any

View File

@ -2,7 +2,6 @@ require "dependency"
describe Dependency do describe Dependency do
alias_matcher :be_a_build_dependency, :be_build alias_matcher :be_a_build_dependency, :be_build
alias_matcher :be_a_runtime_dependency, :be_run
describe "::new" do describe "::new" do
it "accepts a single tag" do it "accepts a single tag" do
@ -73,22 +72,10 @@ describe Dependency do
it "merges temporality tags" do it "merges temporality tags" do
normal_dep = described_class.new("foo") normal_dep = described_class.new("foo")
build_dep = described_class.new("foo", [:build]) build_dep = described_class.new("foo", [:build])
run_dep = described_class.new("foo", [:run])
deps = described_class.merge_repeats([normal_dep, build_dep]) deps = described_class.merge_repeats([normal_dep, build_dep])
expect(deps.count).to eq(1) expect(deps.count).to eq(1)
expect(deps.first).not_to be_a_build_dependency expect(deps.first).not_to be_a_build_dependency
expect(deps.first).not_to be_a_runtime_dependency
deps = described_class.merge_repeats([normal_dep, run_dep])
expect(deps.count).to eq(1)
expect(deps.first).not_to be_a_build_dependency
expect(deps.first).not_to be_a_runtime_dependency
deps = described_class.merge_repeats([build_dep, run_dep])
expect(deps.count).to eq(1)
expect(deps.first).not_to be_a_build_dependency
expect(deps.first).not_to be_a_runtime_dependency
end end
end end

View File

@ -130,7 +130,7 @@ to favour finding `keg_only` formulae first.
```ruby ```ruby
class Foo < Formula class Foo < Formula
depends_on "pkg-config" => :run depends_on "pkg-config"
depends_on "jpeg" depends_on "jpeg"
depends_on "readline" => :recommended depends_on "readline" => :recommended
depends_on "gtk+" => :optional depends_on "gtk+" => :optional
@ -144,16 +144,13 @@ A Symbol (e.g. `:x11`) specifies a [`Requirement`](http://www.rubydoc.info/githu
A Hash (e.g. `=>`) specifies a formula dependency with some additional information. Given a single string key, the value can take several forms: A Hash (e.g. `=>`) specifies a formula dependency with some additional information. Given a single string key, the value can take several forms:
* a Symbol (currently one of `:build`, `:optional`, `:run` or `:recommended`) * a Symbol (currently one of `:build`, `:test`, `:optional` or `:recommended`)
- `:build` means that dependency is a build-time only dependency so it can - `:build` means that dependency is a build-time only dependency so it can
be skipped when installing from a bottle or when listing missing be skipped when installing from a bottle or when listing missing
dependencies using `brew missing`. dependencies using `brew missing`.
- `:test` means that dependency is only required when running `brew test`.
- `:optional` generates an implicit `with-foo` option for the formula. - `:optional` generates an implicit `with-foo` option for the formula.
This means that, given `depends_on "foo" => :optional`, the user must pass `--with-foo` in order to use the dependency. This means that, given `depends_on "foo" => :optional`, the user must pass `--with-foo` in order to use the dependency.
- `:run` can mean the dependency is only required at runtime, or it can be used
to declare build dependencies such as `pkg-config` that are needed at
runtime as well, which will silence the audit warning. `:run` dependencies
are currently available at build time.
- `:recommended` generates an implicit `without-foo` option, meaning that - `:recommended` generates an implicit `without-foo` option, meaning that
the dependency is enabled by default and the user must pass the dependency is enabled by default and the user must pass
`--without-foo` to disable this dependency. The default `--without-foo` to disable this dependency. The default