Brew: audit: suggest full formula name only for other taps

Created on 17 Jul 2016  路  12Comments  路  Source: Homebrew/brew

Since getdp is in homebrew/science the :linked below should not be printed as full names.

http://bot.brew.sh/job/Homebrew%20Science%20Pull%20Requests/5157/version=mavericks/console

==> FAILED
Error: 1 problem in 1 formula
homebrew/science/getdp:
  * Formulae are required to declare all linked dependencies.
    Please add all linked dependencies to the formula with:
      depends_on "fftw" => :linked
      depends_on "homebrew/science/hwloc" => :linked
      depends_on "homebrew/science/netcdf" => :linked
      depends_on "homebrew/science/parmetis" => :linked
      depends_on "homebrew/science/scalapack" => :linked
      depends_on "homebrew/science/suite-sparse" => :linked
      depends_on "homebrew/science/sundials" => :linked
      depends_on "homebrew/science/superlu_dist" => :linked

CC @xu-cheng

bug

All 12 comments

we could probably do this:

diff --git a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb
index dd21559..de61fb5 100644
--- a/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb
+++ b/Library/Homebrew/extend/os/mac/formula_cellar_checks.rb
@@ -72,10 +72,11 @@ module FormulaCellarChecks
     end

     if checker.undeclared_deps?
+      undeclared_deps = checker.undeclared_deps.map { |d| d.strip_prefix("#{formula.tap}/") }
       audit_check_output <<-EOS.undent
         Formulae are required to declare all linked dependencies.
         Please add all linked dependencies to the formula with:
-          #{checker.undeclared_deps.map { |d| "depends_on \"#{d}\" => :linked"} * "\n          "}
+          #{undeclared_deps.map { |d| "depends_on \"#{d}\" => :linked"} * "\n          "}
       EOS
     end
   end

Should work but seems a tad indirect :)

I think it's fine as-is but don't feel strongly either way.

@xu-cheng I'd ship it with the change you suggested. Contributors are already spontaneously manually intervening and editing out the tap prefixes, which is less than ideal CX (contributor experience).

As a reference point, in homebrew science, there's currently exactly one:

Josephs-MacBook-Pro:homebrew-science joe$ ag 'homebrew/science'|grep dep
gmtk.rb:13:  depends_on "homebrew/science/hdf5" => :optional

I don't think it makes sense to foist a new convention on them unless it's actually a new policy we're applying everywhere for all dependencies not in core.

I one of the confused contributors :) I would have preferred if brew audit would not have suggested full paths. It looked odd to me but since I did not know all the consequences, I choose to just follow the advice of audit.

@ilovezfs Would do you mind to send a PR? I may not have too much free time recently to work on this.

@xu-cheng Sure. Are you still happy with your proposal above?

Are you still happy with your proposal above?

:+1:

I should fix gmtk. I personally feel that dependencies that are either in core or in the same tap shouldn't have to be fully qualified but I may be missing something.

We dropped :linked.

Was this page helpful?
0 / 5 - 0 ratings