PageRenderTime 42ms CodeModel.GetById 19ms RepoModel.GetById 0ms app.codeStats 0ms

/Library/Homebrew/cmd/audit.rb

https://bitbucket.org/mgrimes/homebrew
Ruby | 547 lines | 449 code | 75 blank | 23 comment | 67 complexity | 21923df9467d59ae7bf216ab20513c30 MD5 | raw file
  1. require 'formula'
  2. require 'utils'
  3. require 'superenv'
  4. module Homebrew extend self
  5. def audit
  6. formula_count = 0
  7. problem_count = 0
  8. ENV.setup_build_environment
  9. ff = if ARGV.named.empty?
  10. Formula
  11. else
  12. ARGV.formulae
  13. end
  14. ff.each do |f|
  15. fa = FormulaAuditor.new f
  16. fa.audit
  17. unless fa.problems.empty?
  18. puts "#{f.name}:"
  19. fa.problems.each { |p| puts " * #{p}" }
  20. puts
  21. formula_count += 1
  22. problem_count += fa.problems.size
  23. end
  24. end
  25. unless problem_count.zero?
  26. ofail "#{problem_count} problems in #{formula_count} formulae"
  27. end
  28. end
  29. end
  30. class Module
  31. def redefine_const(name, value)
  32. __send__(:remove_const, name) if const_defined?(name)
  33. const_set(name, value)
  34. end
  35. end
  36. # Formula extensions for auditing
  37. class Formula
  38. def head_only?
  39. @head and @stable.nil?
  40. end
  41. def text
  42. @text ||= FormulaText.new(@path)
  43. end
  44. end
  45. class FormulaText
  46. def initialize path
  47. @text = path.open('r') { |f| f.read }
  48. end
  49. def without_patch
  50. @text.split("__END__")[0].strip()
  51. end
  52. def has_DATA?
  53. /\bDATA\b/ =~ @text
  54. end
  55. def has_END?
  56. /^__END__$/ =~ @text
  57. end
  58. def has_trailing_newline?
  59. /\Z\n/ =~ @text
  60. end
  61. end
  62. class FormulaAuditor
  63. attr_reader :f, :text, :problems
  64. BUILD_TIME_DEPS = %W[
  65. autoconf
  66. automake
  67. boost-build
  68. bsdmake
  69. cmake
  70. imake
  71. intltool
  72. libtool
  73. pkg-config
  74. scons
  75. smake
  76. swig
  77. ]
  78. def initialize f
  79. @f = f
  80. @problems = []
  81. @text = f.text.without_patch
  82. @specs = %w{stable devel head}.map { |s| f.send(s) }.compact
  83. # We need to do this in case the formula defines a patch that uses DATA.
  84. f.class.redefine_const :DATA, ""
  85. end
  86. def audit_file
  87. unless f.path.stat.mode.to_s(8) == "100644"
  88. problem "Incorrect file permissions: chmod 644 #{f.path}"
  89. end
  90. if f.text.has_DATA? and not f.text.has_END?
  91. problem "'DATA' was found, but no '__END__'"
  92. end
  93. if f.text.has_END? and not f.text.has_DATA?
  94. problem "'__END__' was found, but 'DATA' is not used"
  95. end
  96. unless f.text.has_trailing_newline?
  97. problem "File should end with a newline"
  98. end
  99. end
  100. def audit_deps
  101. # Don't depend_on aliases; use full name
  102. @@aliases ||= Formula.aliases
  103. f.deps.select { |d| @@aliases.include? d.name }.each do |d|
  104. problem "Dependency #{d} is an alias; use the canonical name."
  105. end
  106. # Check for things we don't like to depend on.
  107. # We allow non-Homebrew installs whenever possible.
  108. f.deps.each do |dep|
  109. begin
  110. dep_f = dep.to_formula
  111. rescue FormulaUnavailableError
  112. problem "Can't find dependency #{dep.name.inspect}."
  113. end
  114. dep.options.reject do |opt|
  115. dep_f.build.has_option?(opt.name)
  116. end.each do |opt|
  117. problem "Dependency #{dep} does not define option #{opt.name.inspect}"
  118. end
  119. case dep.name
  120. when *BUILD_TIME_DEPS
  121. # Build deps should be tagged
  122. problem <<-EOS.undent unless dep.tags.any? || f.name =~ /automake/ && dep.name == 'autoconf'
  123. #{dep} dependency should be "depends_on '#{dep}' => :build"
  124. EOS
  125. when "git", "ruby", "emacs", "mercurial"
  126. problem <<-EOS.undent
  127. Don't use #{dep} as a dependency. We allow non-Homebrew
  128. #{dep} installations.
  129. EOS
  130. when 'python', 'python2', 'python3'
  131. problem <<-EOS.undent
  132. Don't use #{dep} as a dependency (string).
  133. We have special `depends_on :python` (or :python2 or :python3 )
  134. that works with brewed and system Python and allows us to support
  135. bindings for 2.x and 3.x in parallel and much more.
  136. EOS
  137. when 'gfortran'
  138. problem "Use ENV.fortran during install instead of depends_on 'gfortran'"
  139. when 'open-mpi', 'mpich2'
  140. problem <<-EOS.undent
  141. There are multiple conflicting ways to install MPI. Use an MPIDependency:
  142. depends_on MPIDependency.new(<lang list>)
  143. Where <lang list> is a comma delimited list that can include:
  144. :cc, :cxx, :f90, :f77
  145. EOS
  146. end
  147. end
  148. end
  149. def audit_conflicts
  150. f.conflicts.each do |c|
  151. begin
  152. Formula.factory(c.name)
  153. rescue FormulaUnavailableError
  154. problem "Can't find conflicting formula #{c.name.inspect}."
  155. end
  156. end
  157. end
  158. def audit_urls
  159. unless f.homepage =~ %r[^https?://]
  160. problem "The homepage should start with http or https (url is #{f.homepage})."
  161. end
  162. # Check for http:// GitHub homepage urls, https:// is preferred.
  163. # Note: only check homepages that are repo pages, not *.github.com hosts
  164. if f.homepage =~ %r[^http://github\.com/]
  165. problem "Use https:// URLs for homepages on GitHub (url is #{f.homepage})."
  166. end
  167. # Google Code homepages should end in a slash
  168. if f.homepage =~ %r[^https?://code\.google\.com/p/[^/]+[^/]$]
  169. problem "Google Code homepage should end with a slash (url is #{f.homepage})."
  170. end
  171. if f.homepage =~ %r[^http://.*\.github\.com/]
  172. problem "GitHub pages should use the github.io domain (url is #{f.homepage})"
  173. end
  174. urls = @specs.map(&:url)
  175. # Check GNU urls; doesn't apply to mirrors
  176. urls.grep(%r[^(?:https?|ftp)://(?!alpha).+/gnu/]) do |u|
  177. problem "\"ftpmirror.gnu.org\" is preferred for GNU software (url is #{u})."
  178. end
  179. # the rest of the checks apply to mirrors as well
  180. urls.concat(@specs.map(&:mirrors).flatten)
  181. # Check SourceForge urls
  182. urls.each do |p|
  183. # Is it a filedownload (instead of svnroot)
  184. next if p =~ %r[/svnroot/]
  185. next if p =~ %r[svn\.sourceforge]
  186. # Is it a sourceforge http(s) URL?
  187. next unless p =~ %r[^https?://.*\bsourceforge\.]
  188. if p =~ /(\?|&)use_mirror=/
  189. problem "Don't use #{$1}use_mirror in SourceForge urls (url is #{p})."
  190. end
  191. if p =~ /\/download$/
  192. problem "Don't use /download in SourceForge urls (url is #{p})."
  193. end
  194. if p =~ %r[^http://prdownloads\.]
  195. problem "Don't use prdownloads in SourceForge urls (url is #{p}).\n" +
  196. "\tSee: http://librelist.com/browser/homebrew/2011/1/12/prdownloads-is-bad/"
  197. end
  198. if p =~ %r[^http://\w+\.dl\.]
  199. problem "Don't use specific dl mirrors in SourceForge urls (url is #{p})."
  200. end
  201. end
  202. # Check for git:// GitHub repo urls, https:// is preferred.
  203. urls.grep(%r[^git://[^/]*github\.com/]) do |u|
  204. problem "Use https:// URLs for accessing GitHub repositories (url is #{u})."
  205. end
  206. # Check for http:// GitHub repo urls, https:// is preferred.
  207. urls.grep(%r[^http://github\.com/.*\.git$]) do |u|
  208. problem "Use https:// URLs for accessing GitHub repositories (url is #{u})."
  209. end
  210. # Use new-style archive downloads
  211. urls.select { |u| u =~ %r[https://.*/(?:tar|zip)ball/] and not u =~ %r[\.git$] }.each do |u|
  212. problem "Use /archive/ URLs for GitHub tarballs (url is #{u})."
  213. end
  214. if urls.any? { |u| u =~ /\.xz/ } && !f.deps.any? { |d| d.name == "xz" }
  215. problem "Missing a build-time dependency on 'xz'"
  216. end
  217. end
  218. def audit_specs
  219. problem "Head-only (no stable download)" if f.head_only?
  220. [:stable, :devel].each do |spec|
  221. s = f.send(spec)
  222. next if s.nil?
  223. if s.version.to_s.empty?
  224. problem "Invalid or missing #{spec} version"
  225. else
  226. version_text = s.version unless s.version.detected_from_url?
  227. version_url = Version.parse(s.url)
  228. if version_url.to_s == version_text.to_s && s.version.instance_of?(Version)
  229. problem "#{spec} version #{version_text} is redundant with version scanned from URL"
  230. end
  231. end
  232. cksum = s.checksum
  233. next if cksum.nil?
  234. len = case cksum.hash_type
  235. when :sha1 then 40
  236. when :sha256 then 64
  237. end
  238. if cksum.empty?
  239. problem "#{cksum.hash_type} is empty"
  240. else
  241. problem "#{cksum.hash_type} should be #{len} characters" unless cksum.hexdigest.length == len
  242. problem "#{cksum.hash_type} contains invalid characters" unless cksum.hexdigest =~ /^[a-fA-F0-9]+$/
  243. problem "#{cksum.hash_type} should be lowercase" unless cksum.hexdigest == cksum.hexdigest.downcase
  244. end
  245. end
  246. # Check for :using that is already detected from the url
  247. @specs.each do |s|
  248. next if s.using.nil?
  249. url_strategy = DownloadStrategyDetector.detect(s.url)
  250. using_strategy = DownloadStrategyDetector.detect('', s.using)
  251. problem "redundant :using specification in url or head" if url_strategy == using_strategy
  252. end
  253. end
  254. def audit_patches
  255. Patches.new(f.patches).select(&:external?).each do |p|
  256. case p.url
  257. when %r[raw\.github\.com], %r[gist\.github\.com/raw]
  258. unless p.url =~ /[a-fA-F0-9]{40}/
  259. problem "GitHub/Gist patches should specify a revision:\n#{p.url}"
  260. end
  261. when %r[macports/trunk]
  262. problem "MacPorts patches should specify a revision instead of trunk:\n#{p.url}"
  263. end
  264. end
  265. end
  266. def audit_text
  267. if text =~ /<(Formula|AmazonWebServicesFormula|ScriptFileFormula|GithubGistFormula)/
  268. problem "Use a space in class inheritance: class Foo < #{$1}"
  269. end
  270. # Commented-out cmake support from default template
  271. if (text =~ /# system "cmake/)
  272. problem "Commented cmake call found"
  273. end
  274. # FileUtils is included in Formula
  275. if text =~ /FileUtils\.(\w+)/
  276. problem "Don't need 'FileUtils.' before #{$1}."
  277. end
  278. # Check for long inreplace block vars
  279. if text =~ /inreplace .* do \|(.{2,})\|/
  280. problem "\"inreplace <filenames> do |s|\" is preferred over \"|#{$1}|\"."
  281. end
  282. # Check for string interpolation of single values.
  283. if text =~ /(system|inreplace|gsub!|change_make_var!).*[ ,]"#\{([\w.]+)\}"/
  284. problem "Don't need to interpolate \"#{$2}\" with #{$1}"
  285. end
  286. # Check for string concatenation; prefer interpolation
  287. if text =~ /(#\{\w+\s*\+\s*['"][^}]+\})/
  288. problem "Try not to concatenate paths in string interpolation:\n #{$1}"
  289. end
  290. # Prefer formula path shortcuts in Pathname+
  291. if text =~ %r{\(\s*(prefix\s*\+\s*(['"])(bin|include|libexec|lib|sbin|share)[/'"])}
  292. problem "\"(#{$1}...#{$2})\" should be \"(#{$3}+...)\""
  293. end
  294. if text =~ %r[((man)\s*\+\s*(['"])(man[1-8])(['"]))]
  295. problem "\"#{$1}\" should be \"#{$4}\""
  296. end
  297. # Prefer formula path shortcuts in strings
  298. if text =~ %r[(\#\{prefix\}/(bin|include|libexec|lib|sbin|share))]
  299. problem "\"#{$1}\" should be \"\#{#{$2}}\""
  300. end
  301. if text =~ %r[((\#\{prefix\}/share/man/|\#\{man\}/)(man[1-8]))]
  302. problem "\"#{$1}\" should be \"\#{#{$3}}\""
  303. end
  304. if text =~ %r[((\#\{share\}/(man)))[/'"]]
  305. problem "\"#{$1}\" should be \"\#{#{$3}}\""
  306. end
  307. if text =~ %r[(\#\{prefix\}/share/(info|man))]
  308. problem "\"#{$1}\" should be \"\#{#{$2}}\""
  309. end
  310. # Commented-out depends_on
  311. if text =~ /#\s*depends_on\s+(.+)\s*$/
  312. problem "Commented-out dep #{$1}"
  313. end
  314. # No trailing whitespace, please
  315. if text =~ /[\t ]+$/
  316. problem "Trailing whitespace was found"
  317. end
  318. if text =~ /if\s+ARGV\.include\?\s+'--(HEAD|devel)'/
  319. problem "Use \"if ARGV.build_#{$1.downcase}?\" instead"
  320. end
  321. if text =~ /make && make/
  322. problem "Use separate make calls"
  323. end
  324. if text =~ /^[ ]*\t/
  325. problem "Use spaces instead of tabs for indentation"
  326. end
  327. # xcodebuild should specify SYMROOT
  328. if text =~ /system\s+['"]xcodebuild/ and not text =~ /SYMROOT=/
  329. problem "xcodebuild should be passed an explicit \"SYMROOT\""
  330. end
  331. if text =~ /ENV\.x11/
  332. problem "Use \"depends_on :x11\" instead of \"ENV.x11\""
  333. end
  334. # Avoid hard-coding compilers
  335. if text =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?(gcc|llvm-gcc|clang)['" ]}
  336. problem "Use \"\#{ENV.cc}\" instead of hard-coding \"#{$3}\""
  337. end
  338. if text =~ %r{(system|ENV\[.+\]\s?=)\s?['"](/usr/bin/)?((g|llvm-g|clang)\+\+)['" ]}
  339. problem "Use \"\#{ENV.cxx}\" instead of hard-coding \"#{$3}\""
  340. end
  341. if text =~ /system\s+['"](env|export)/
  342. problem "Use ENV instead of invoking '#{$1}' to modify the environment"
  343. end
  344. if text =~ /version == ['"]HEAD['"]/
  345. problem "Use 'build.head?' instead of inspecting 'version'"
  346. end
  347. if text =~ /build\.include\?\s+['"]\-\-(.*)['"]/
  348. problem "Reference '#{$1}' without dashes"
  349. end
  350. if text =~ /build\.with\?\s+['"]-?-?with-(.*)['"]/
  351. problem "No double 'with': Use `build.with? '#{$1}'` to check for \"--with-#{$1}\""
  352. end
  353. if text =~ /build\.without\?\s+['"]-?-?without-(.*)['"]/
  354. problem "No double 'without': Use `build.without? '#{$1}'` to check for \"--without-#{$1}\""
  355. end
  356. if text =~ /ARGV\.(?!(debug\?|verbose\?|find[\(\s]))/
  357. problem "Use build instead of ARGV to check options"
  358. end
  359. if text =~ /def options/
  360. problem "Use new-style option definitions"
  361. end
  362. if text =~ /MACOS_VERSION/
  363. problem "Use MacOS.version instead of MACOS_VERSION"
  364. end
  365. cats = %w{leopard snow_leopard lion mountain_lion}.join("|")
  366. if text =~ /MacOS\.(?:#{cats})\?/
  367. problem "\"#{$&}\" is deprecated, use a comparison to MacOS.version instead"
  368. end
  369. if text =~ /skip_clean\s+:all/
  370. problem "`skip_clean :all` is deprecated; brew no longer strips symbols"
  371. end
  372. if text =~ /depends_on [A-Z][\w:]+\.new$/
  373. problem "`depends_on` can take requirement classes instead of instances"
  374. end
  375. if text =~ /^def (\w+).*$/
  376. problem "Define method #{$1.inspect} in the class body, not at the top-level"
  377. end
  378. end
  379. def audit_python
  380. if text =~ /(def\s*)?which_python/
  381. problem "Replace `which_python` by `python.xy`, which returns e.g. 'python2.7'."
  382. end
  383. if text =~ /which\(?["']python/
  384. problem "Don't locate python with `which 'python'`, use `python.binary` instead"
  385. end
  386. # Checks that apply only to code in def install
  387. if text =~ /(\s*)def\s+install((.*\n)*?)(\1end)/
  388. install_body = $2
  389. if install_body =~ /system\(?\s*['"]python/
  390. problem "Instead of `system 'python', ...`, call `system python, ...`."
  391. end
  392. if text =~ /system\(?\s*python\.binary/
  393. problem "Instead of `system python.binary, ...`, call `system python, ...`."
  394. end
  395. end
  396. # Checks that apply only to code in def caveats
  397. if text =~ /(\s*)def\s+caveats((.*\n)*?)(\1end)/ || /(\s*)def\s+caveats;(.*?)end/
  398. caveats_body = $2
  399. if caveats_body =~ /[ \{=](python[23]?)\.(.*\w)/
  400. # So if in the body of caveats there is a `python.whatever` called,
  401. # check that there is a guard like `if python` or similiar:
  402. python = $1
  403. method = $2
  404. unless caveats_body =~ /(if python[23]?)|(if build\.with\?\s?\(?['"]python)|(unless build.without\?\s?\(?['"]python)/
  405. problem "Please guard `#{python}.#{method}` like so `#{python}.#{method} if #{python}`"
  406. end
  407. end
  408. end
  409. if f.requirements.any?{ |r| r.kind_of?(PythonInstalled) }
  410. # Don't check this for all formulae, because some are allowed to set the
  411. # PYTHONPATH. E.g. python.rb itself needs to set it.
  412. if text =~ /ENV\.append.*PYTHONPATH/ || text =~ /ENV\[['"]PYTHONPATH['"]\]\s*=[^=]/
  413. problem "Don't set the PYTHONPATH, instead declare `depends_on :python`."
  414. end
  415. else
  416. # So if there is no PythonInstalled requirement, we can check if the
  417. # formula still uses python and should add a `depends_on :python`
  418. unless f.name.to_s =~ /(pypy[0-9]*)|(python[0-9]*)/
  419. if text =~ /system.["' ]?python([0-9"'])?/
  420. problem "If the formula uses Python, it should declare so by `depends_on :python#{$1}`"
  421. end
  422. if text =~ /setup\.py/
  423. problem <<-EOS.undent
  424. If the formula installs Python bindings you should declare `depends_on :python[3]`"
  425. EOS
  426. end
  427. end
  428. end
  429. # Todo:
  430. # The python do ... end block is possibly executed twice. Once for
  431. # python 2.x and once for 3.x. So if a `system 'make'` is called, a
  432. # `system 'make clean'` should also be called at the end of the block.
  433. end
  434. def audit
  435. audit_file
  436. audit_specs
  437. audit_urls
  438. audit_deps
  439. audit_conflicts
  440. audit_patches
  441. audit_text
  442. audit_python
  443. end
  444. private
  445. def problem p
  446. @problems << p
  447. end
  448. end