Skip to content

Commit

Permalink
audit: make audit_revision_and_version_scheme faster.
Browse files Browse the repository at this point in the history 
This is really, really slow at the moment for a few reasons:
- it goes through the list of revisions twice
- it checks many more revisions than it needs to

Even after these improvements it's still by far the slowest audit so
am also making it a strict or new-formula audit.
  • Loading branch information
@MikeMcQuaid
MikeMcQuaid committed Jun 3, 2020
1 parent 5fb158f commit bc57e04
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 124 deletions.
112 changes: 50 additions & 62 deletions Library/Homebrew/dev-cmd/audit.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -709,86 +709,74 @@ def audit_specs
end

def audit_revision_and_version_scheme
return if @new_formula
return unless @strict
return unless formula.tap # skip formula not from core or any taps
return unless formula.tap.git? # git log is required
return if @new_formula
return if formula.stable.blank?

fv = FormulaVersions.new(formula)

previous_version_and_checksum = fv.previous_version_and_checksum("origin/master")
[:stable, :devel].each do |spec_sym|
next unless spec = formula.send(spec_sym)
next unless previous_version_and_checksum[spec_sym][:version] == spec.version
next if previous_version_and_checksum[spec_sym][:checksum] == spec.checksum
current_version = formula.stable.version
current_version_scheme = formula.version_scheme
current_revision = formula.revision

previous_version = nil
previous_checksum = nil
previous_version_scheme = nil
previous_revision = nil

fv.rev_list("origin/master") do |rev|
fv.formula_at_revision(rev) do |f|
stable = f.stable
next if stable.blank?

previous_version = stable.version
previous_checksum = stable.checksum

if (version_scheme = f.version_scheme) && current_version_scheme != version_scheme
previous_version_scheme = version_scheme
end

if (revision = f.revision) && current_revision != revision
previous_revision = revision
end
end

break if previous_version && previous_checksum
end

if previous_version &&
(stable = formula.stable) &&
previous_version == stable.version &&
previous_checksum != stable.checksum
problem(
"#{spec_sym}: sha256 changed without the version also changing; " \
"stable sha256 changed without the version also changing; " \
"please create an issue upstream to rule out malicious " \
"circumstances and to find out why the file changed.",
)
end

attributes = [:revision, :version_scheme]
attributes_map = fv.version_attributes_map(attributes, "origin/master")

current_version_scheme = formula.version_scheme
[:stable, :devel].each do |spec|
spec_version_scheme_map = attributes_map[:version_scheme][spec]
next if spec_version_scheme_map.empty?

version_schemes = spec_version_scheme_map.values.flatten
max_version_scheme = version_schemes.max
max_version = spec_version_scheme_map.select do |_, version_scheme|
version_scheme.first == max_version_scheme
end.keys.max

if max_version_scheme && current_version_scheme < max_version_scheme
problem "version_scheme should not decrease (from #{max_version_scheme} to #{current_version_scheme})"
end

if max_version_scheme && current_version_scheme >= max_version_scheme &&
current_version_scheme > 1 &&
!version_schemes.include?(current_version_scheme - 1)
if previous_version_scheme
if current_version_scheme < previous_version_scheme
problem "version_scheme should not decrease (from #{previous_version_scheme} to #{current_version_scheme})"
elsif current_version_scheme > (previous_version_scheme + 1)
problem "version_schemes should only increment by 1"
end
end

formula_spec = formula.send(spec)
next unless formula_spec

spec_version = formula_spec.version
next unless max_version
next if spec_version >= max_version

above_max_version_scheme = current_version_scheme > max_version_scheme
map_includes_version = spec_version_scheme_map.key?(spec_version)
next if !current_version_scheme.zero? &&
(above_max_version_scheme || map_includes_version)

problem "#{spec} version should not decrease (from #{max_version} to #{spec_version})"
if previous_version && current_version < previous_version
problem "stable version should not decrease (from #{previous_version} to #{current_version})"
end

current_revision = formula.revision
revision_map = attributes_map[:revision][:stable]
if formula.stable && !revision_map.empty?
stable_revisions = revision_map[formula.stable.version]
stable_revisions ||= []
max_revision = stable_revisions.max || 0

if current_revision < max_revision
problem "revision should not decrease (from #{max_revision} to #{current_revision})"
end
return unless previous_revision

stable_revisions -= [formula.revision]
if !current_revision.zero? && stable_revisions.empty? &&
revision_map.keys.length > 1
problem "'revision #{formula.revision}' should be removed"
elsif current_revision > 1 &&
current_revision != max_revision &&
!stable_revisions.include?(current_revision - 1)
problem "revisions should only increment by 1"
end
elsif !current_revision.zero? # head/devel-only formula
problem "'revision #{current_revision}' should be removed"
if !current_revision.zero? && current_version != previous_version
problem "'revision #{formula.revision}' should be removed"
elsif !current_revision.zero? && current_revision < previous_revision
problem "revision should not decrease (from #{previous_revision} to #{current_revision})"
elsif current_revision > (previous_revision + 1)
problem "revisions should only increment by 1"
end
end

Expand Down
62 changes: 0 additions & 62 deletions Library/Homebrew/formula_versions.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,66 +65,4 @@ def bottle_version_map(branch)
end
map
end

def previous_version_and_checksum(branch)
map = {}

rev_list(branch) do |rev|
formula_at_revision(rev) do |f|
[:stable, :devel].each do |spec_sym|
next unless spec = f.send(spec_sym)

map[spec_sym] ||= { version: spec.version, checksum: spec.checksum }
end
end

break if map[:stable] || map[:devel]
end

map[:stable] ||= {}
map[:devel] ||= {}

map
end

def version_attributes_map(attributes, branch)
attributes_map = {}
return attributes_map if attributes.empty?

attributes.each do |attribute|
attributes_map[attribute] ||= {
stable: {},
devel: {},
}
end

stable_versions_seen = 0
rev_list(branch) do |rev|
formula_at_revision(rev) do |f|
attributes.each do |attribute|
map = attributes_map[attribute]
set_attribute_map(map, f, attribute)

stable_keys_length = (map[:stable].keys + [f.version]).uniq.length
stable_versions_seen = [stable_versions_seen, stable_keys_length].max
end
end
break if stable_versions_seen > MAX_VERSIONS_DEPTH
end

attributes_map
end

private

def set_attribute_map(map, f, attribute)
if f.stable
map[:stable][f.stable.version] ||= []
map[:stable][f.stable.version] << f.send(attribute)
end
return unless f.devel

map[:devel][f.devel.version] ||= []
map[:devel][f.devel.version] << f.send(attribute)
end
end

0 comments on commit bc57e04

Please sign in to comment.