Skip to content

Commit 058dad6

Browse files
authored
Merge pull request Homebrew#3948 from MikeMcQuaid/audit-run
Audit use of :run dependencies.
2 parents 08db242 + d2c23bd commit 058dad6

File tree

10 files changed

+24
-63
lines changed

10 files changed

+24
-63
lines changed

Library/Homebrew/compat.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,3 +29,4 @@
2929
require "compat/utils/shell"
3030
require "compat/extend/string"
3131
require "compat/gpg"
32+
require "compat/dependable"

Library/Homebrew/compat/dependable.rb

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
module Dependable
2+
def run?
3+
tags.include? :run
4+
end
5+
end

Library/Homebrew/compat/dependency_collector.rb

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ def parse_symbol_spec(spec, tags)
3333
output_deprecation(spec)
3434
Dependency.new(spec.to_s, tags)
3535
when :libltdl
36-
tags << :run
3736
output_deprecation("libtool")
3837
Dependency.new("libtool", tags)
3938
when :apr
@@ -68,7 +67,7 @@ def parse_symbol_spec(spec, tags)
6867
private
6968

7069
def autotools_dep(spec, tags)
71-
tags << :build unless tags.include? :run
70+
tags << :build
7271
Dependency.new(spec.to_s, tags)
7372
end
7473

Library/Homebrew/dependable.rb

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
require "options"
22

33
module Dependable
4+
# :run and :linked are no longer used but keep them here to avoid them being
5+
# misused in future.
46
RESERVED_TAGS = [:build, :optional, :recommended, :run, :test, :linked].freeze
57

68
def build?
@@ -15,10 +17,6 @@ def recommended?
1517
tags.include? :recommended
1618
end
1719

18-
def run?
19-
tags.include? :run
20-
end
21-
2220
def test?
2321
tags.include? :test
2422
end

Library/Homebrew/dependency.rb

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -163,13 +163,9 @@ def merge_necessity(deps)
163163
end
164164

165165
def merge_temporality(deps)
166-
if deps.all?(&:build?)
167-
[:build]
168-
elsif deps.all?(&:run?)
169-
[:run]
170-
else
171-
[] # Means both build and runtime dependency.
172-
end
166+
# Means both build and runtime dependency.
167+
return [] unless deps.all?(&:build?)
168+
[:build]
173169
end
174170
end
175171
end

Library/Homebrew/dev-cmd/audit.rb

Lines changed: 5 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -172,23 +172,6 @@ class FormulaAuditor
172172

173173
attr_reader :formula, :text, :problems
174174

175-
BUILD_TIME_DEPS = %w[
176-
autoconf
177-
automake
178-
boost-build
179-
bsdmake
180-
cmake
181-
godep
182-
imake
183-
intltool
184-
libtool
185-
pkg-config
186-
scons
187-
smake
188-
sphinx-doc
189-
swig
190-
].freeze
191-
192175
def initialize(formula, options = {})
193176
@formula = formula
194177
@new_formula = options[:new_formula]
@@ -377,17 +360,12 @@ def audit_deps
377360
problem "Dependency #{dep} does not define option #{opt.name.inspect}"
378361
end
379362

380-
case dep.name
381-
when "git"
363+
if dep.name == "git"
382364
problem "Don't use git as a dependency (it's always available)"
383-
when *BUILD_TIME_DEPS
384-
next if dep.build? || dep.run?
385-
problem <<~EOS
386-
#{dep} dependency should be
387-
depends_on "#{dep}" => :build
388-
Or if it is indeed a runtime dependency
389-
depends_on "#{dep}" => :run
390-
EOS
365+
end
366+
367+
if dep.tags.include?(:run)
368+
problem "Dependency '#{dep.name}' is marked as :run. Remove :run; it is a no-op."
391369
end
392370
end
393371
end

Library/Homebrew/rubocops/dependency_order_cop.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ def sort_by_dependency_name(a, b)
4848
def sort_dependencies_by_type(dependency_nodes)
4949
ordered = []
5050
ordered.concat(dependency_nodes.select { |dep| buildtime_dependency? dep }.to_a)
51-
ordered.concat(dependency_nodes.select { |dep| runtime_dependency? dep }.to_a)
51+
ordered.concat(dependency_nodes.select { |dep| test_dependency? dep }.to_a)
5252
ordered.concat(dependency_nodes.reject { |dep| negate_normal_dependency? dep }.to_a)
5353
ordered.concat(dependency_nodes.select { |dep| recommended_dependency? dep }.to_a)
5454
ordered.concat(dependency_nodes.select { |dep| optional_dependency? dep }.to_a)
@@ -106,11 +106,11 @@ def verify_order_in_source(ordered)
106106

107107
def_node_search :recommended_dependency?, "(sym :recommended)"
108108

109-
def_node_search :runtime_dependency?, "(sym :run)"
109+
def_node_search :test_dependency?, "(sym :test)"
110110

111111
def_node_search :optional_dependency?, "(sym :optional)"
112112

113-
def_node_search :negate_normal_dependency?, "(sym {:build :recommended :run :optional})"
113+
def_node_search :negate_normal_dependency?, "(sym {:build :recommended :test :optional})"
114114

115115
# Node pattern method to extract `name` in `depends_on :name`
116116
def_node_search :dependency_name_node, <<~EOS

Library/Homebrew/rubocops/extend/formula_cop.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ def depends_on_name_type?(node, name = nil, type = :required)
171171
when :required
172172
type_match = required_dependency?(node)
173173
name_match ||= required_dependency_name?(node, name) if type_match
174-
when :build, :optional, :recommended, :run
174+
when :build, :test, :optional, :recommended
175175
type_match = dependency_type_hash_match?(node, type)
176176
name_match ||= dependency_name_hash_match?(node, name) if type_match
177177
when :any

Library/Homebrew/test/dependency_spec.rb

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
describe Dependency do
44
alias_matcher :be_a_build_dependency, :be_build
5-
alias_matcher :be_a_runtime_dependency, :be_run
65

76
describe "::new" do
87
it "accepts a single tag" do
@@ -73,22 +72,10 @@
7372
it "merges temporality tags" do
7473
normal_dep = described_class.new("foo")
7574
build_dep = described_class.new("foo", [:build])
76-
run_dep = described_class.new("foo", [:run])
7775

7876
deps = described_class.merge_repeats([normal_dep, build_dep])
7977
expect(deps.count).to eq(1)
8078
expect(deps.first).not_to be_a_build_dependency
81-
expect(deps.first).not_to be_a_runtime_dependency
82-
83-
deps = described_class.merge_repeats([normal_dep, run_dep])
84-
expect(deps.count).to eq(1)
85-
expect(deps.first).not_to be_a_build_dependency
86-
expect(deps.first).not_to be_a_runtime_dependency
87-
88-
deps = described_class.merge_repeats([build_dep, run_dep])
89-
expect(deps.count).to eq(1)
90-
expect(deps.first).not_to be_a_build_dependency
91-
expect(deps.first).not_to be_a_runtime_dependency
9279
end
9380
end
9481

docs/Formula-Cookbook.md

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ to favour finding `keg_only` formulae first.
130130

131131
```ruby
132132
class Foo < Formula
133-
depends_on "pkg-config" => :run
133+
depends_on "pkg-config"
134134
depends_on "jpeg"
135135
depends_on "readline" => :recommended
136136
depends_on "gtk+" => :optional
@@ -144,16 +144,13 @@ A Symbol (e.g. `:x11`) specifies a [`Requirement`](http://www.rubydoc.info/githu
144144

145145
A Hash (e.g. `=>`) specifies a formula dependency with some additional information. Given a single string key, the value can take several forms:
146146

147-
* a Symbol (currently one of `:build`, `:optional`, `:run` or `:recommended`)
147+
* a Symbol (currently one of `:build`, `:test`, `:optional` or `:recommended`)
148148
- `:build` means that dependency is a build-time only dependency so it can
149149
be skipped when installing from a bottle or when listing missing
150150
dependencies using `brew missing`.
151+
- `:test` means that dependency is only required when running `brew test`.
151152
- `:optional` generates an implicit `with-foo` option for the formula.
152153
This means that, given `depends_on "foo" => :optional`, the user must pass `--with-foo` in order to use the dependency.
153-
- `:run` can mean the dependency is only required at runtime, or it can be used
154-
to declare build dependencies such as `pkg-config` that are needed at
155-
runtime as well, which will silence the audit warning. `:run` dependencies
156-
are currently available at build time.
157154
- `:recommended` generates an implicit `without-foo` option, meaning that
158155
the dependency is enabled by default and the user must pass
159156
`--without-foo` to disable this dependency. The default

0 commit comments

Comments
 (0)