Skip to content
This repository

requirements should be required by default #12794

Closed
wants to merge 1 commit into from

5 participants

camillol Jack Nagel Adam Vandenberg Max Howell Mike McQuaid
camillol

It doesn't make sense to have something called a "Requirement" that is not
actually required unless you explicitly set a "yes-I-really-mean-it" flag.
It's better to make the intuitive case the default (Requirement means it's
required), and have non-required requirements as the exception.

It turns out that most requirements used in formulas are in fact required,
so the intuitive case is also the common one.

It also turns out that most non-fatal requirements are that way because we
are not sure if they really are required or not, so for now we just display
a vague message saying things may work or not. Other times, the message says
the requirement is required, but the code left it non-fatal for unclear
reasons (possibly no reason). For these reasons, a non-fatal requirement is
likely to be a "code smell", and it's useful to make it explicit, so we can
grep for it and improve the affected formulas.

camillol requirements are now fatal by default
It doesn't make sense to have something called a "Requirement" that is not
actually required, unless you explicitly set a "yes-I-really-mean-it" flag.
It's better to make the sensible case the default (Requirement means it's
required), and have non-required requirements as the exception.

It turns out that most requirements used in formulas are in fact required,
so the sensible case is also the common one.

It also turns out that most non-fatal requirements are that way because we
are not sure if they really are required or not, so for now we just display
a vague message saying things may work or not. Other times, the message says
the requirement is required, but the code left it non-fatal for unclear
reasons (possibly no reason). For these reasons, a non-fatal requirement is
likely to be a "code smell", and it's useful to make it explicit, so we can
grep for it and improve the affected formulas.
5eeb6cd
camillol

There will probably be an objection that the Requirement interface has already been "published" and so we can't change it for the sake of backwards compatibility. However, as it is now it clashes with intuition, and forces formula authors to write more boilerplate code than necessary. I say we should instead change it ASAP.

Jack Nagel
Owner

At least git-hg and signing-party are non-fatal because you don't actually need the stuff installed for the build to succeed. It's about flexibility. I haven't looked through the others.

Adam Vandenberg
Owner
adamv commented June 12, 2012

Rejecting this on principle.

Would accept a commit that made Requirement.fatal? throw an exception, forcing all Requirements to explicitly define whether they were fatal or not.

camillol

Rejecting this on principle.

What principle, exactly?

Anyway, how about we deprecate the entire Requirement class, then? In terms of DSL, it's ugly and verbose. It would be far less code to have an optional preflight method on Formula which can do the tests and print messages are needed, and halts the build if it raises an exception (or if it returns false, if you prefer). It would require less boilerplate to specify requirements, and the execution flow would also be clearer.

I had to try installing a formula and see what happened to figure out what the heck a non-fatal Requirement was supposed to accomplish: a preflight method with a puts would accomplish the same thing with less code and more clarity.

camillol

At least git-hg and signing-party are non-fatal because you don't actually need the stuff installed for the build to succeed. It's about flexibility. I haven't looked through the others.

But you'll need it later to actually run the program. So, even if we flipped the default for fatal? and forgot to mark those two as def fatal?;false;end explicitly (and we didn't forget), it would cause no actual harm.

By the way, aren't we already using => :optional to mark optional dependencies? Why invent a completely different way for non-formula dependencies? Why couldn't we add a => :runtime_only as a counterpart to => :build?

I don't care if @mxcl came up with this, it's just not a very good design.

Jack Nagel
Owner

The "DSL" for Requirements is just an intermediate step; eventually common things can be factored out and specified with symbols or something.

But you'll need it later to actually run the program. So, even if we flipped the default for fatal? and forgot to mark those two as def fatal?;false;end explicitly (and we didn't forget), it would cause no actual harm.

That's not a good enough reason.

Your argument seems to be "I wouldn't have done it this way", which is fine, but others (myself included) disagree.

I don't see any real value in this change.

camillol

The "DSL" for Requirements is just an intermediate step; eventually common things can be factored out and specified with symbols or something.

Which happens to be what I did with depends_on :x11. Indeed, that would be the only reason to have Requirement subclasses (and yet, a good reason): sharing them between formulas. But we're not doing that. And after looking through all the Requirements in formulas, I can see why: those few that occur more than once tend to have different messages depending on the formula (and it's not just a matter of having a $FORMULA_NAME variable in the text). In other words, of the three parts of a Requirement:

  • message cannot be shared;
  • fatal? should not be shared (the same component may be required in one formula and optional in another: this should be in depends_on, not inside the requirement!)
  • satisfied? is the only part that would be shared, but it's better done as a simple method somewhere.

For example, in ghc.rb we might replace this:

class NeedsSnowLeopard < Requirement
  def satisfied?
    MacOS.snow_leopard?
  end

  def message; <<-EOS.undent
    GHC requires OS X 10.6 or newer. The binary releases no longer work on
    Leopard. See the following issue for details:
        http://hackage.haskell.org/trac/ghc/ticket/6009
    EOS
  end

  def fatal?; false; end
end
...
  depends_on NeedsSnowLeopard.new

with this:

def preflight
  puts(<<-EOS.undent) unless MacOS.snow_leopard?
    GHC requires OS X 10.6 or newer. The binary releases no longer work on
    Leopard. See the following issue for details:
        http://hackage.haskell.org/trac/ghc/ticket/6009
  EOS
end

and other formulas can reuse MacOS.snow_leopard?.

Your argument seems to be "I wouldn't have done it this way", which is fine, but others (myself included) disagree.

My original arguments were detailed in the commit message:

  • formula dependencies are required by default, while external dependencies are optional by default, even though they are called Requirements: this is simply counterintuitive;
  • it also requires us to write more boilerplate than necessary: a required requirement is the usual case;
  • we have some formulas where the message says something is categorically required, yet it's non-fatal (maybe the formula author simply forgot to make fatal? explicitly true? see argument 1); in other formulas, the message vaguely says "this may or may not work", which means we should review the formula and figure out why it does not work and when; therefore, making non-fatal the exception (and thus the one that has to be written explicit) is more useful for finding formulas to review.

All three of these arguments point towards having required requirements as the default and optional requirements as the exception.

I have also added more arguments in the course of this conversation.

I'd like to hear your arguments in favor of the current unintuitive, inconsistent design, other than "this is what we ended up with and we're stuck with it".

I don't see any real value in this change.

Consistency, least surprise, DRY, reduced boilerplate.

Jack Nagel
Owner

Okay, so maybe the word "Requirement" is being abused here. Rename it to "AribtraryPredicate" or something in your head.

Indeed, that would be the only reason to have Requirement subclasses (and yet, a good reason): sharing them between formulas. But we're not doing that.

Yet. It was rolled out slowly in a deliberate attempt to not marry us to DSL syntax that we regret having to support later. What's the rush?

From a design standpoint, I don't like your preflight method idea. We'd have two totally uncoupled ways of specifying dependencies? To save a few lines of code here and there? No. Ick.

camillol

Yet. It was rolled out slowly in a deliberate attempt to not marry us to DSL syntax that we regret having to support later. What's the rush?

Then there should be no objection to making fatal? true by default.

From a design standpoint, I don't like your preflight method idea. We'd have two totally uncoupled ways of specifying dependencies? To save a few lines of code here and there? No. Ick.

But these are not dependencies: like you said, they're really just arbitrary predicates. In fact, in many formulas we have things like "is this Python a universal build?" (boost.rb), "is camlp5 built in transitional mode?" (coq.rb; and this is separate from the actual dependency on camlp5!), "was Poppler built with Qt4?" (diffpdf.rb, again separate from the actual dependency on poppler), "is BerkeleyDB not installed?" (dsniff.rb: an antidependency!!), "is Homebrew installed in /usr/local?" (miton.rb). They really are arbitrary predicates!
Look at how ugly subversion.rb is, too. depends_on 'neon', and then depends_on UniversalNeon.new, and again the same thing for sqlite.

Of course, the natural way to express a predicate in Ruby would be simply as a boolean expression. If you want to print a message if an expression is false, the natural Ruby way is using an unless and a puts. But here we have wrapped an expression and a string into a class which is only used once to do depends_on CheckSomeArbitraryPredicate.new. This is the ugliest wart yet in Homebrew.

Anyway. You think you're avoiding the creation of a different way to specify dependencies (...by still creating a different way to specify dependencies...), but in fact what you're doing is creating a new way to specify an if/unless statement. In fact, the normal way to check a predicate and print a message was already in use in formulas: inside the caveats method.
Unfortunately, caveats are displayed after installation, and we have quite a few formulas that make you go through the install to then say "hey look, maybe you should have passed this option instead". So it would be useful to have a way to display messages before building: considering this, and the fact that non-fatal Requirements are literally just a "print this message if this condition" (the exact same thing we do in caveats in so many formulas), the simplest solution would have been to have a method that runs before building and can check conditions and print messages, i.e. preflight.

But let's take a step back. We don't need to eliminate Requirement (I'm using it myself in the XQuartz patch, remember?), just avoid turning every if into a nail just because we have the Requirement hammer. And this pull request isn't even about that: it's just about improving Requirement. So, let's at least rip out the fatal? method and replace it with the existing options on depends_on. You said you're against creating completely different mechanisms to do the same thing, right?

Jack Nagel
Owner

"is camlp5 built in transitional mode?" (coq.rb; and this is separate from the actual dependency on camlp5!), "was Poppler built with Qt4?" (diffpdf.rb, again separate from the actual dependency on poppler)

These are things that will eventually be solved by passing options to dependencies, but for now Requirements are an easy way to check them.

Of course, the natural way to express a predicate in Ruby would be simply as a boolean expression. If you want to print a message if an expression is false, the natural Ruby way is using an unless and a puts. But here we have wrapped an expression and a string into a class which is only used once to do depends_on CheckSomeArbitraryPredicate.new.

You're taking a very narrow application of requirement deps and assuming they will never be used for anything other than that narrow application. Requirements were added in order to remove the plethora of conditional blocks in formulae, with the added bonus that because they are now real dependency objects, we can track them as part of the formula's metadata and use them to (simple example) display OS version requirements in brew info or something.

This is the ugliest wart yet in Homebrew.

It's a hell of a lot nicer than the random assortment of conditionals done a dozen different ways that we had before.

but in fact what you're doing is creating a new way to specify an if/unless statement.

See above.

In fact, the normal way to check a predicate and print a message was already in use in formulas: it's the caveats predicate.

Says who? Using the caveats to do this has always been a hack, and it's certainly inferior to real dependencies.

the simplest solution would have been to have a method that runs before building and can check conditions and print messages, i.e. preflight.

No, it wouldn't have been, because you can't track that information without creating some kind of object, and a system to do so already exists in Requirements.

But let's take a step back. This pull request is not about eliminating Requirement entirely, just improving it. Let's at least rip out the fatal? method and replace it with the existing options on depends_on. You said you're against creating completely different mechanisms to do the same thing, right?

The existing depends_on options don't do anything, and their exact semantics have never been specified.

camillol

You're taking a very narrow application of requirement deps and assuming they will never be used for anything other than that narrow application. Requirements were added in order to remove the plethora of conditional blocks in formulae, with the added bonus that because they are now real dependency objects, we can track them as part of the formula's metadata and use them to (simple example) display OS version requirements in brew info or something.

No we cannot, because they're all ad-hoc classes that exist only inside a specific formula. The only things homebrew code can do with them are checking if they are satisfied, and printing the message if they're not. You can't even describe the requirement if it's satisfied (unless you think printing the class name is acceptable...).

The idea of non-formula dependency objects is useful, but their current application is terrible. The only good one - and the only shared one, uncoincidentally - is the X11Requirement (which is not even in Homebrew yet).

Says who? Using the caveats to do this has always been a hack, and it's certainly inferior to real dependencies.

Which you don't have yet.

No, it wouldn't have been, because you can't track that information without creating some kind of object, and a system to do so already exists in Requirements.

And, as it is now, it enables zero additional functionality over a simple caveats-like method.

The existing depends_on options don't do anything, and their exact semantics have never been specified.

Which is ridiculous. And it also suggests that all the grand plans you have for Requirements will never come to fruition, and all you'll be left with will be a verbose, overcomplicated syntax to replace an if.

Anyway, if we can stop dodging the question and agree that attributes such as :optional should be specified uniformly for both formula and non-formula dependencies, maybe we can make some progress on that front. Yes or no?

camillol

Also, please assume that I'm not as belligerent as I sound. I'd like to tone down the polemic, but I'm going to sleep now, so please apply that filter on the receiving end.

Jack Nagel
Owner

No we cannot, because they're all ad-hoc classes that exist only inside a specific formula. The only things homebrew code can do with them are checking if they are satisfied, and printing the message if they're not. You can't even describe the requirement if it's satisfied (unless you think printing the class name is acceptable...).

I have said repeatedly that eventually common requirements will be factored out. When they are, we will absolutely be able to do things with them.

And, as it is now, it enables zero additional functionality over a simple caveats-like method.

Maybe we should just make every formula a standalone procedural script that doesn't require any other parts of Homebrew to work? Come on, these things happen incrementally, not all at once. To reject the feature altogether because it isn't finished is just silly.

Anyway, if we can stop dodging the question and agree that attributes such as :optional should be specified uniformly for both formula and non-formula dependencies, maybe we can make some progress on that front. Yes or no?

I don't necessarily agree that an "optional" dependency and a "non-fatal" requirement are always the same thing, no.

Adam Vandenberg
Owner
adamv commented June 12, 2012

Also, please assume that I'm not as belligerent as I sound.

I appreciate the passion you're showing on this issue. We all want to make this stuff better.

Max Howell
Owner
mxcl commented August 06, 2012

I would have preferred, depends_on :x11, behind the scenes this could easily have been converted to X11Requirement.new or whatever.

Regarding the main point. We can break backwards compat, like whatever. I agree it's weird to call them requirements when they by default are not required, unintuitive.

Apologies, I should have reviewed this stuff more earlier. I guess I should get some time and do a better comment.

Mike McQuaid
Owner

@mxcl We do have that syntax now I think.

Adam Vandenberg
Owner

I would accept a patch that made Requirement.fatal? raise NotImplemented and force every concrete Requirement to implement fatal.

Other than that, Requirements are being used for more useful things that just caveats, and they are being integrated into the depends_on syntax.

We'll also accept patches to fix smelly Requirements on a case-by-case basis.

Adam Vandenberg adamv closed this August 09, 2012
camillol

@mxcl

I would have preferred, depends_on :x11, behind the scenes this could easily have been converted to X11Requirement.new or whatever.

That's exactly how I wrote the X11 requirement. Everywhere else we still have depends_on Something.new, though (at least for now).

@adamv, here is a very tiny proposal for improvement:

  • Get rid of fatal? entirely.
  • Formulas which want to trigger a requirement but proceed even if it is not satisfied should use the :optional option on depends_on.

This way, we have the same syntax and the same semantics for dependencies on formulas and dependencies on non-formula requirements (both are strictly required by default, both can be made optional by using :optional). The same dependency/requirement may be strict for one formula and optional for another, so the right place to specify it is on the dependency relationship. This also improves reusability of Requirements.

@mxcl said we can break backwards compatibility in this case, so there is no reason not to go ahead with this.

Adam Vandenberg
Owner

I am in favor of removing "fatal" and using the :optional syntax.

Thought in that case it should not auto-create "with/without" options per Jack's new branch.

Jack Nagel
Owner

Replacing fatal with => :optional precludes using it to make depends_on :x11 => :optional generate a "with-x" option. That part isn't yet implemented in my branch, but I've already had to roll back a few changes that assumed it will work that way.

Adam Vandenberg
Owner

If Ruby didn't already use require, I would suggest using requires foo instead of depends_on, so we can have different semantics.

camillol

@jacknagel I haven't followed the latest developments, but I don't see why we couldn't still allow special handling for depends_on :x11 => :optional. Or are you planning to turn all optional dependencies into corresponding with-something options? I don't think there is a one-to-one mapping there.

Mike McQuaid

optionally_depends_on ? Excited by that @jacknagel; it'll be neat when we can generate options that easily.

Jack Nagel
Owner

I don't really want :optional to mean two things; in my deps branch it currently means "off by default, but generate a with-foo option to turn it on".

Jack Nagel
Owner

Non-fatal requirements are advisory, so how about advise :requirement?

Mike McQuaid

That sounds good to me @jacknagel. Presumably required is the same but on by default with an option to turn off?

Jack Nagel
Owner
# "fatal" requirements would continue to use 'depends_on'.
depends_on :xcode
depends_on :x11

# that way they can eventually have access to optional/recommended,
# and the automatic option generation that comes with that.
depends_on :x11 => :optional # generates "with-x" option

# The "conditional aliases" of the x11 dep can do the same
depends_on :freetype => :recommended # "generates "without-x" option

# "non-fatal", or "advisory", requirements would use a seperate DSL
# element, which avoids overloading the `:optional` tag for normal dependencies.
# Requirements passed to "advise" would never be fatal to the build
advise FooRequirement.new
advise :foosymbol
Mike McQuaid

Looks amazing. The only tweak I'd say would be :freetype should be without-freetype (particularly as we always use the Homebrew version on Mountain Lion and above).

Jack Nagel
Owner

Right, typo.

camillol

So the idea is to have a shorthand for declaring an option that turns a specific dependency on or off, right? And in more complex situations we keep things as they are? For instance, ruby.rb currently has depends_on :x11 if build.include? 'with-tcltk', and poppler.rb has:

depends_on 'glib' if build.include? 'with-glib'
depends_on 'cairo' if build.include? 'with-glib' # Needs a newer Cairo build than OS X 10.6.7 provides

Would those stay the same?

Jack Nagel
Owner

Right, it is to reduce things like

depends_on :foo if build.include? 'with-foo'

option 'with-foo', 'Build with foo support'

to

depends_on 'foo' => :optional

:recommended is similar, but is on-by-default and generates a "without-foo" option.

It will be possible to shorten your second example to

depends_on 'glib' => :optional
depends_on 'cairo' if build.with? 'glib'

which itself is only an incremental improvement, but it does eliminate the explicit option declaration as well.

These auto-generated options will show up in brew info and brew options, of course.

camillol

Still looking at various usage cases. In transmission.rb, I see the following:

depends_on 'intltool' => :optional
depends_on 'gettext' => :optional # need gettext if intltool is used

So here we'd want a single option, perhaps with-nls.

In imagemagick.rb, there's a bunch of things like:

depends_on 'libtiff' => :optional if build.include? 'use-tiff'

These should not use :optional and if at the same time, right? If the user asks for tiff support, libtiff should be required. But what if the user says nothing, and libtiff is already installed? We should let configure pick it up, right?

Mike McQuaid

I think we want to try and make options named vaguely consistently and if that means --with-intltool helps then we should do that. With imagemagick that will go away once @jacknagel's stuff hits. If a user has libtiff I don't think, personally, we should be passing it to configure unless they specifically request it (mostly for bug prevention/resolution).

camillol

@mikemcquaid that complicates things a bit, though, because then it's no longer just a matter of turning a formula dependency on or off from Homebrew's point of view: we'd need to add special machinery to pass the right options to configure to prevent it from using some libraries that the user has lying around but has not explicitly requested.

Jack Nagel
Owner

These should not use :optional and if at the same time, right?

The new optional/recommended stuff is not merged yet, so a bunch of things got merged that way citing "future-compatibleness" with the new semantics, but I don't really understand why as we'll want to remove the conditional to de-uglify it at that point anyway.

That said, something like

depends_on 'foo' => :optional if build.include? 'with-foo'

actually does work OK, it's just redundant.

If the user asks for tiff support, libtiff should be required. But what if the user says nothing, and libtiff is already installed? We should let configure pick it up, right?

If libtiff is installed, configure scripts will find it unless they are braindead. Whether the user requests it or not doesn't affect that.

Jack Nagel
Owner

Most of the existing uses of optional/recommended were in place long (years?) before these tags meant anything, so don't let them cloud the issue.

camillol

If libtiff is installed, configure scripts will find it unless they are braindead. Whether the user requests it or not doesn't affect that.

Right, but I think @mikemcquaid is leaning towards the idea of guaranteeing a consistent build given a set of flags: e.g., if you only pass --with-jpeg and not --with-tiff, the resulting product is guaranteed not to link with libtiff, even if the user has installed it anyway. But while I can see the attraction for some use cases, I don't think we should make that a goal.

camillol

@jacknagel that's true. And many optional packages should really be :recommended. For instance, for imagemagick we'll probably want to support as many image formats as possible by default.

Jack Nagel
Owner

Yeah, it's tilting at windmills; there is little to no consistency in how configure options are named across projects so formulae would have to carry a mapping of formula options to configure options.

Jack Nagel
Owner

As far as things that are coupled like intltool/gettext, @adamv proposed a further enhancement: #14315 which would cover those situations.

Mike McQuaid

Yeh, I don't mind whether we force things off or just let configure pick them up. There's advantages both ways but I guess another of forcing things off unless requested is that brew missing will actually show if you remove libtiff if it's explicit.

I don't think most things should be recommended; we should have a minimal set of useful dependencies (like I did recently for imagemagick) that provide the 99% (or perhaps even 90%) use case as that's what will be used for bottles. I don't think most imagemagick users are using e.g. tiff support and if they are they can complain and we can change the options around later (as opposed to forcing 20 dependencies when most users only need 5).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Jun 12, 2012
camillol requirements are now fatal by default
It doesn't make sense to have something called a "Requirement" that is not
actually required, unless you explicitly set a "yes-I-really-mean-it" flag.
It's better to make the sensible case the default (Requirement means it's
required), and have non-required requirements as the exception.

It turns out that most requirements used in formulas are in fact required,
so the sensible case is also the common one.

It also turns out that most non-fatal requirements are that way because we
are not sure if they really are required or not, so for now we just display
a vague message saying things may work or not. Other times, the message says
the requirement is required, but the code left it non-fatal for unclear
reasons (possibly no reason). For these reasons, a non-fatal requirement is
likely to be a "code smell", and it's useful to make it explicit, so we can
grep for it and improve the affected formulas.
5eeb6cd
This page is out of date. Refresh to see the latest.

Showing 36 changed files with 23 additions and 72 deletions. Show diff stats Hide diff stats

  1. 3  Library/Formula/asymptote.rb
  2. 3  Library/Formula/auctex.rb
  3. 1  Library/Formula/boost.rb
  4. 1  Library/Formula/cmake.rb
  5. 2  Library/Formula/cmu-sphinxbase.rb
  6. 3  Library/Formula/coq.rb
  7. 4  Library/Formula/cvs2svn.rb
  8. 3  Library/Formula/dbslayer.rb
  9. 4  Library/Formula/diffpdf.rb
  10. 1  Library/Formula/dsniff.rb
  11. 4  Library/Formula/elixir.rb
  12. 1  Library/Formula/exim.rb
  13. 2  Library/Formula/ghc.rb
  14. 1  Library/Formula/git-hg.rb
  15. 2  Library/Formula/google-js-test.rb
  16. 2  Library/Formula/jstalk.rb
  17. 3  Library/Formula/lilypond.rb
  18. 1  Library/Formula/mlton.rb
  19. 3  Library/Formula/mu.rb
  20. 4  Library/Formula/mupdf.rb
  21. 3  Library/Formula/mydumper.rb
  22. 3  Library/Formula/mysql-proxy.rb
  23. 3  Library/Formula/nu.rb
  24. 1  Library/Formula/open-babel.rb
  25. 3  Library/Formula/osm2pgsql.rb
  26. 4  Library/Formula/pdf2svg.rb
  27. 3  Library/Formula/pg_top.rb
  28. 3  Library/Formula/pgpool-ii.rb
  29. 3  Library/Formula/pgtap.rb
  30. 2  Library/Formula/phantomjs.rb
  31. 3  Library/Formula/saga-core.rb
  32. 2  Library/Formula/signing-party.rb
  33. 3  Library/Formula/skytools.rb
  34. 1  Library/Formula/squid.rb
  35. 2  Library/Formula/subversion.rb
  36. 8  Library/Homebrew/dependencies.rb
3  Library/Formula/asymptote.rb
@@ -10,9 +10,6 @@ def message; <<-EOS.undent
10 10
   def satisfied?
11 11
     which 'latex'
12 12
   end
13  
-  def fatal?
14  
-    true
15  
-  end
16 13
 end
17 14
 
18 15
 class Asymptote < Formula
3  Library/Formula/auctex.rb
@@ -10,9 +10,6 @@ def message; <<-EOS.undent
10 10
   def satisfied?
11 11
     which 'latex'
12 12
   end
13  
-  def fatal?
14  
-    true
15  
-  end
16 13
 end
17 14
 
18 15
 class Auctex < Formula
1  Library/Formula/boost.rb
@@ -15,6 +15,7 @@ def message; <<-EOS.undent
15 15
   def satisfied?
16 16
     archs_for_command("python").universal?
17 17
   end
  18
+  def fatal?; false; end	# "likely" fail? is this required or not??
18 19
 end
19 20
 
20 21
 class Boost < Formula
1  Library/Formula/cmake.rb
@@ -13,6 +13,7 @@ def message; <<-EOS.undent
13 13
   def satisfied?
14 14
     not File.exist? "/Library/Frameworks/expat.framework"
15 15
   end
  16
+  def fatal?; false; end	# shouldn't we check for an actual arch mismatch?
16 17
 end
17 18
 
18 19
 
2  Library/Formula/cmu-sphinxbase.rb
@@ -12,7 +12,7 @@ def message; <<-EOS.undent
12 12
   def satisfied?
13 13
     Formula.factory('python').installed?
14 14
   end
15  
-  def fatal?
  15
+  def fatal?	# TODO: figure this out
16 16
     false
17 17
   end
18 18
 end
3  Library/Formula/coq.rb
@@ -12,9 +12,6 @@ def satisfied?
12 12
     # If installed, make sure it is transitional instead of strict.
13 13
     `camlp5 -pmode 2>&1`.chomp == 'transitional'
14 14
   end
15  
-  def fatal?
16  
-    true
17  
-  end
18 15
 end
19 16
 
20 17
 class Coq < Formula
4  Library/Formula/cvs2svn.rb
@@ -14,10 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     quiet_system "python", "-c", "import gdbm"
16 16
   end
17  
-
18  
-  def fatal?
19  
-    true
20  
-  end
21 17
 end
22 18
 
23 19
 class Cvs2svn < Formula
3  Library/Formula/dbslayer.rb
@@ -18,9 +18,6 @@ def message; <<-EOS.undent
18 18
   def satisfied?
19 19
     which 'mysql_config'
20 20
   end
21  
-  def fatal?
22  
-    true
23  
-  end
24 21
 end
25 22
 
26 23
 class Dbslayer < Formula
4  Library/Formula/diffpdf.rb
@@ -6,10 +6,6 @@ def satisfied?
6 6
     poppler.installed_with? '--with-qt4'
7 7
   end
8 8
 
9  
-  def fatal?
10  
-    true
11  
-  end
12  
-
13 9
   def message; <<-EOS.undent
14 10
     DiffPDF requires the Poppler-Qt4 bindings but Poppler was not installed
15 11
     with support for Qt. Please reinstall Poppler using the `--with-qt4`
1  Library/Formula/dsniff.rb
@@ -13,6 +13,7 @@ def satisfied?
13 13
     f = Formula.factory("berkeley-db")
14 14
     not f.installed?
15 15
   end
  16
+  def fatal?; false; end	# can fail? what is the problem, and why don't we fix it?
16 17
 end
17 18
 
18 19
 class Dsniff < Formula
4  Library/Formula/elixir.rb
@@ -15,10 +15,6 @@ def message; <<-EOS.undent
15 15
   def satisfied?
16 16
     which 'erl'
17 17
   end
18  
-
19  
-  def fatal?
20  
-    true
21  
-  end
22 18
 end
23 19
 
24 20
 class Elixir < Formula
1  Library/Formula/exim.rb
@@ -13,6 +13,7 @@ def satisfied?
13 13
     f = Formula.factory("berkeley-db")
14 14
     not f.installed?
15 15
   end
  16
+  def fatal?; false; end	# need to figure this out
16 17
 end
17 18
 
18 19
 class Exim < Formula
2  Library/Formula/ghc.rb
@@ -11,6 +11,8 @@ def message; <<-EOS.undent
11 11
         http://hackage.haskell.org/trac/ghc/ticket/6009
12 12
     EOS
13 13
   end
  14
+
  15
+  def fatal?; false; end	# why is this non-fatal? is 10.6 required or not?
14 16
 end
15 17
 
16 18
 class Ghc < Formula
1  Library/Formula/git-hg.rb
@@ -14,6 +14,7 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'hg'
16 16
   end
  17
+  def fatal?; false; end	# why is this non-fatal? is mercurial required or not?
17 18
 end
18 19
 
19 20
 class GitHg < Formula
2  Library/Formula/google-js-test.rb
@@ -8,6 +8,8 @@ def message
8 8
   def satisfied?
9 9
     MacOS.snow_leopard?
10 10
   end
  11
+
  12
+  def fatal?; false; end	# why is this non-fatal? is 10.6 required or not?
11 13
 end
12 14
 
13 15
 class GoogleJsTest < Formula
2  Library/Formula/jstalk.rb
@@ -8,6 +8,8 @@ def satisfied?
8 8
   def message
9 9
     "jstalk requires Mac OS X 10.6 or newer"
10 10
   end
  11
+
  12
+  def fatal?; false; end	# why is this non-fatal? is 10.6 required or not?
11 13
 end
12 14
 
13 15
 class Jstalk < Formula
3  Library/Formula/lilypond.rb
@@ -10,9 +10,6 @@ def message; <<-EOS.undent
10 10
   def satisfied?
11 11
     which 'mpost'
12 12
   end
13  
-  def fatal?
14  
-    true
15  
-  end
16 13
 end
17 14
 
18 15
 class Lilypond < Formula
1  Library/Formula/mlton.rb
@@ -16,6 +16,7 @@ def message; <<-EOS.undent
16 16
   def satisfied?
17 17
     HOMEBREW_PREFIX.to_s == "/usr/local"
18 18
   end
  19
+  def fatal?; false; end	# why is this non-fatal if it won't work without it?
19 20
 end
20 21
 
21 22
 class Mlton < Formula
3  Library/Formula/mu.rb
@@ -15,9 +15,6 @@ def satisfied?
15 15
     `emacs --version 2>/dev/null` =~ /^GNU Emacs (\d{2})/
16 16
     $1.to_i >= 23
17 17
   end
18  
-  def fatal?
19  
-    true
20  
-  end
21 18
 end
22 19
 
23 20
 class Mu < Formula
4  Library/Formula/mupdf.rb
@@ -5,10 +5,6 @@ def satisfied?
5 5
     MACOS_VERSION >= 10.6
6 6
   end
7 7
 
8  
-  def fatal?
9  
-    true
10  
-  end
11  
-
12 8
   def message; <<-EOS.undent
13 9
     The version of Freetype that comes with Leopard is too old to build MuPDF
14 10
     against. It is possible to get MuPDF working on Leopard using the Freetype
3  Library/Formula/mydumper.rb
@@ -18,9 +18,6 @@ def message; <<-EOS.undent
18 18
   def satisfied?
19 19
     which 'mysql_config'
20 20
   end
21  
-  def fatal?
22  
-    true
23  
-  end
24 21
 end
25 22
 
26 23
 class Mydumper < Formula
3  Library/Formula/mysql-proxy.rb
@@ -18,9 +18,6 @@ def message; <<-EOS.undent
18 18
   def satisfied?
19 19
     which 'mysql_config'
20 20
   end
21  
-  def fatal?
22  
-    true
23  
-  end
24 21
 end
25 22
 
26 23
 class MysqlProxy < Formula
3  Library/Formula/nu.rb
@@ -7,9 +7,6 @@ def satisfied?
7 7
   def message
8 8
     "Nu requires Mac OS X 10.7 or newer"
9 9
   end
10  
-  def fatal?
11  
-    true
12  
-  end
13 10
 end
14 11
 
15 12
 class Nu < Formula
1  Library/Formula/open-babel.rb
@@ -11,6 +11,7 @@ def satisfied?
11 11
     args = %W{/usr/bin/env python -c import\ oasa}
12 12
     quiet_system *args
13 13
   end
  14
+  def fatal?; false; end	# eventually we should have depends_on ... => :optional
14 15
 end
15 16
 
16 17
 class OpenBabel < Formula
3  Library/Formula/osm2pgsql.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'pg_config'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 class Osm2pgsql < Formula
4  Library/Formula/pdf2svg.rb
@@ -6,10 +6,6 @@ def satisfied?
6 6
     poppler.installed_with? '--with-glib'
7 7
   end
8 8
 
9  
-  def fatal?
10  
-    true
11  
-  end
12  
-
13 9
   def message; <<-EOS.undent
14 10
     pdf2svg requires the Poppler-Glib bindings but Poppler was not installed
15 11
     with support for glib. Please reinstall Poppler using the `--with-glib`
3  Library/Formula/pg_top.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'pg_config'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 
3  Library/Formula/pgpool-ii.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'pg_config'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 class PgpoolIi < Formula
3  Library/Formula/pgtap.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'pg_config'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 class Pgtap < Formula
2  Library/Formula/phantomjs.rb
@@ -8,6 +8,8 @@ def satisfied?
8 8
   def message
9 9
     "PhantomJS requires Mac OS X 10.6 (Snow Leopard) or newer."
10 10
   end
  11
+
  12
+  def fatal?; false; end	# why is this non-fatal? is 10.6 required or not?
11 13
 end
12 14
 
13 15
 class Phantomjs < Formula
3  Library/Formula/saga-core.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'pg_config'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 class SagaCore < Formula
2  Library/Formula/signing-party.rb
@@ -17,7 +17,7 @@ def satisfied?
17 17
     which 'gpg' or which 'gpg2'
18 18
   end
19 19
 
20  
-  def fatal?
  20
+  def fatal?	# is it really useful to allow users to build this without installing the requirements?
21 21
     false
22 22
   end
23 23
 end
3  Library/Formula/skytools.rb
@@ -14,9 +14,6 @@ def message; <<-EOS.undent
14 14
   def satisfied?
15 15
     which 'postgres'
16 16
   end
17  
-  def fatal?
18  
-    true
19  
-  end
20 17
 end
21 18
 
22 19
 class Skytools < Formula
1  Library/Formula/squid.rb
@@ -13,6 +13,7 @@ def satisfied?
13 13
     f = Formula.factory("berkeley-db")
14 14
     not f.installed?
15 15
   end
  16
+  def fatal?; false; end	# figure out what the problem is
16 17
 end
17 18
 
18 19
 class Squid < Formula
2  Library/Formula/subversion.rb
@@ -16,6 +16,7 @@ def satisfied?
16 16
     f = Formula.factory('neon')
17 17
     !f.installed? || archs_for_command(f.lib+'libneon.dylib').universal?
18 18
   end
  19
+  def fatal?; false; end	# why is this non-fatal? is it required or not?
19 20
 end
20 21
 
21 22
 class UniversalSqlite < Requirement
@@ -28,6 +29,7 @@ def satisfied?
28 29
     f = Formula.factory('sqlite')
29 30
     !f.installed? || archs_for_command(f.lib+'libsqlite3.dylib').universal?
30 31
   end
  32
+  def fatal?; false; end	# why is this non-fatal? is it required or not?
31 33
 end
32 34
 
33 35
 class Subversion < Formula
8  Library/Homebrew/dependencies.rb
@@ -85,11 +85,11 @@ def options
85 85
 
86 86
 
87 87
 # A base class for non-formula requirements needed by formulae.
88  
-# A "fatal" requirement is one that will fail the build if it is not present.
89  
-# By default, Requirements are non-fatal.
  88
+# A "fatal" requirement is one that is required for the build to proceed.
  89
+# A requirement may be made non-fatal to simply display a message before the build.
90 90
 class Requirement
91 91
   def satisfied?; false; end
92  
-  def fatal?; false; end
  92
+  def fatal?; true; end
93 93
   def message; ""; end
94 94
 end
95 95
 
@@ -102,8 +102,6 @@ def initialize language, module_name, import_name=nil
102 102
     @import_name = import_name || module_name
103 103
   end
104 104
 
105  
-  def fatal?; true; end
106  
-
107 105
   def satisfied?
108 106
     quiet_system(*the_test)
109 107
   end
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.