Skip to content
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

Fix audit version_scheme and revision checks. #2109

Merged
merged 1 commit into from
Apr 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 52 additions & 28 deletions Library/Homebrew/dev-cmd/audit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -827,51 +827,71 @@ def audit_revision_and_version_scheme
return unless formula.tap.git? # git log is required
return if @new_formula

fv = FormulaVersions.new(formula, max_depth: 1)
fv = FormulaVersions.new(formula)
attributes = [:revision, :version_scheme]

attributes_map = fv.version_attributes_map(attributes, "origin/master")

attributes.each do |attribute|
stable_attribute_map = attributes_map[attribute][:stable]
next if stable_attribute_map.nil? || stable_attribute_map.empty?

attributes_for_version = stable_attribute_map[formula.version]
next if attributes_for_version.nil? || attributes_for_version.empty?

old_attribute = formula.send(attribute)
max_attribute = attributes_for_version.max
if max_attribute && old_attribute < max_attribute
problem "#{attribute} should not decrease (from #{max_attribute} to #{old_attribute})"
end
end

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.nil? || spec_version_scheme_map.empty?

max_version_scheme = spec_version_scheme_map.values.flatten.max
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

formula_spec = formula.send(spec)
next if formula_spec.nil?
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 && formula_spec.version < max_version
problem "#{spec} version should not decrease (from #{max_version} to #{formula_spec.version})"
if max_version_scheme && current_version_scheme >= max_version_scheme &&
current_version_scheme > 1 &&
!version_schemes.include?(current_version_scheme - 1)
problem "version_schemes should only increment by 1"
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.keys.include?(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})"
end

return if formula.revision.zero?
current_revision = formula.revision
if formula.stable
revision_map = attributes_map[:revision][:stable]
stable_revisions = revision_map[formula.stable.version] if revision_map
if !stable_revisions || stable_revisions.empty?
problem "'revision #{formula.revision}' should be removed"
if revision_map = attributes_map[:revision][:stable]
if !revision_map.nil? && !revision_map.empty?
stable_revisions = revision_map[formula.stable.version]
stable_revisions ||= []
current_revision = formula.revision
max_revision = stable_revisions.max || 0

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

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
end
end
else # head/devel-only formula
problem "'revision #{formula.revision}' should be removed"
elsif !current_revision.zero? # head/devel-only formula
problem "'revision #{current_revision}' should be removed"
end
end

Expand Down Expand Up @@ -1211,6 +1231,10 @@ def line_problems(line, _lineno)
end
end

if line =~ /((revision|version_scheme)\s+0)/
problem "'#{$1}' should be removed"
end

return unless @strict

problem "`#{$1}` in formulae is deprecated" if line =~ /(env :(std|userpaths))/
Expand Down
47 changes: 30 additions & 17 deletions Library/Homebrew/formula_versions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,22 @@ class FormulaVersions
ErrorDuringExecution, LoadError, MethodDeprecatedError
].freeze

MAX_VERSIONS_DEPTH = 2

attr_reader :name, :path, :repository, :entry_name

def initialize(formula, options = {})
def initialize(formula)
@name = formula.name
@path = formula.path
@repository = formula.tap.path
@entry_name = @path.relative_path_from(repository).to_s
@max_depth = options[:max_depth]
@current_formula = formula
end

def rev_list(branch)
repository.cd do
depth = 0
Utils.popen_read("git", "rev-list", "--abbrev-commit", "--remove-empty", branch, "--", entry_name) do |io|
yield io.readline.chomp until io.eof? || (@max_depth && (depth += 1) > @max_depth)
yield io.readline.chomp until io.eof?
end
end
end
Expand Down Expand Up @@ -49,11 +50,15 @@ def formula_at_revision(rev)

def bottle_version_map(branch)
map = Hash.new { |h, k| h[k] = [] }

versions_seen = 0
rev_list(branch) do |rev|
formula_at_revision(rev) do |f|
bottle = f.bottle_specification
map[f.pkg_version] << bottle.rebuild unless bottle.checksums.empty?
versions_seen = (map.keys + [f.pkg_version]).uniq.length
end
return map if versions_seen > MAX_VERSIONS_DEPTH
end
map
end
Expand All @@ -62,27 +67,35 @@ def version_attributes_map(attributes, branch)
attributes_map = {}
return attributes_map if attributes.empty?

attributes.each do |attribute|
attributes_map[attribute] ||= {}
end

stable_versions_seen = 0
rev_list(branch) do |rev|
formula_at_revision(rev) do |f|
attributes.each do |attribute|
attributes_map[attribute] ||= {}
map = attributes_map[attribute]
if f.stable
map[:stable] ||= {}
map[:stable][f.stable.version] ||= []
map[:stable][f.stable.version] << f.send(attribute)
end
next unless f.devel
map[:devel] ||= {}
map[:devel][f.devel.version] ||= []
map[:devel][f.devel.version] << f.send(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] ||= {}
map[:stable][f.stable.version] ||= []
map[:stable][f.stable.version] << f.send(attribute)
end
return unless f.devel
map[:devel] ||= {}
map[:devel][f.devel.version] ||= []
map[:devel][f.devel.version] << f.send(attribute)
end
end