Tighten up graphviz package (explicitly disable unused languages, etc...) (#4408)

* Tighten up graphviz package

The fun started when configure discovered a broken/partial
installation of `swig` in `/usr/local`, then auto-discovered my
system's python and ruby packages.

- SpackException doesn't seem to exist.  Convert it to a SpackError
  and call `.format(...)` on the error string to fill in the
  placeholder.

- Pull swig out of the list of languages.  It's something that can be
  asked for explicitly and that is needed if *any* of the langagues
  are enabled.  It's disabled by default.

- Explicitly disable the languages that are in "untested_bindings"
  list lest the configure script pick up things from the system.

* Touch up variant description string

* Clean up conditional statement

* Use InstallError, not SpackError

* Drop the swig variant

Get rid of the swig variant and drive that bit based on whether any
languages are enabled.

* Move perl to the untested list

That's not strictly accurate.  I tested it and it doesn't work.

There's a missing depends_on().  When you add that you'll discover
that the language binding bit can't find Perl's 'EXTERN.h'.  Then
you'll discover that graphviz's `configure` script doesn't have a good
way to include the paths to Perl's bits (looks like I'll have to
gather them for each language and then use them to build `CFLAGS` and
`CXXFLAGS` and `LDFLAGS`).  While pondering that, you'll discover that
EXTERN.h is buried down here:

```
opt/spack/linux-centos7-x86_64/gcc-4.8.5/perl-5.24.1-35ejv4426dmzreum4ekdibu3ddmhquvi/lib/5.24.1/x86_64-linux/CORE/EXTERN.h
```

and decide that you wish you had never thought to actually test
`graphviz+perl`.

I could find that directory with a snippet like so:

```
perl -MConfig -e 'print "$Config{archlib}\n"'
```

but at this point I'm much, much further down this rabbit hole then I
ever wanted to go.

* Convince python that tested_bindings is a list

When I removed `+perl` and made `tested_bindings` a list of one
thing, I ended up with this:

```
==> Error: cannot concatenate 'str' and 'tuple' objects
```

* Flake8 cleanup

* Don't convert a string to a string

* rm unused () and clarify variable name

Feedback from @adamjstewart

- Get rid of some unnecessary parens.
- Clearer variable name and use.

* Further cleanup of language enabling loop

Now we don't need that pesky temporary variable.

1 files changed, 30 insertions, 19 deletions

diff --git a/var/spack/repos/builtin/packages/graphviz/package.py b/var/spack/repos/builtin/packages/graphviz/package.py
index 18b69b9f2b..89e7ffc635 100644
--- a/var/spack/repos/builtin/packages/graphviz/package.py
+++ b/var/spack/repos/builtin/packages/graphviz/package.py
@@ -36,9 +36,6 @@ class Graphviz(AutotoolsPackage):
 
     # We try to leave language bindings enabled if they don't cause
     # build issues or add dependencies.
-    variant('swig', default=False,
-            description='Enable for optional swig language bindings'
-            ' (not yet functional)')
     variant('sharp', default=False,
             description='Enable for optional sharp language bindings'
             ' (not yet functional)')
@@ -79,7 +76,20 @@ class Graphviz(AutotoolsPackage):
 
     parallel = False
 
-    depends_on('swig', when='+swig')
+    # These language bindings have been tested, we know they work.
+    tested_bindings = ('+java', )
+
+    # These language bindings have not yet been tested.  They
+    # likely need additional dependencies to get working.
+    untested_bindings = (
+        '+perl',
+        '+sharp', '+go', '+guile', '+io',
+        '+lua', '+ocaml', '+php',
+        '+python', '+r', '+ruby', '+tcl')
+
+    for b in tested_bindings + untested_bindings:
+        depends_on('swig', when=b)
+
     depends_on('ghostscript')
     depends_on('freetype')
     depends_on('expat')
@@ -93,19 +103,11 @@ class Graphviz(AutotoolsPackage):
         spec = self.spec
         options = []
 
-        # These language bindings have been tested, we know they work.
-        tested_bindings = ('+java', '+perl')
+        need_swig = False
 
-        # These language bindings have not yet been tested.  They
-        # likely need additional dependencies to get working.
-        untested_bindings = (
-            '+swig', '+sharp', '+go', '+guile', '+io',
-            '+lua', '+ocaml', '+php',
-            '+python', '+r', '+ruby', '+tcl')
-
-        for var in untested_bindings:
+        for var in self.untested_bindings:
             if var in spec:
-                raise SpackException(
+                raise InstallError(
                     "The variant {0} for language bindings has not been "
                     "tested.  It might or might not work.  To try it "
                     "out, run `spack edit graphviz`, and then move '{0}' "
@@ -113,11 +115,20 @@ class Graphviz(AutotoolsPackage):
                     "`tested_bindings` list.  Be prepared to add "
                     "required dependencies.  "
                     "Please then submit a pull request to "
-                    "http://github.com/llnl/spack")
+                    "http://github.com/llnl/spack".format(var))
+            options.append('--disable-%s' % var[1:])
+
+        for var in self.tested_bindings:
+            if var in spec:
+                need_swig = True
+                options.append('--enable-{0}'.format(var[1:]))
+            else:
+                options.append('--disable-{0}'.format(var[1:]))
 
-        for var in tested_bindings:
-            enable = 'enable' if (var in spec) else 'disable'
-            options.append('--%s-%s' % (enable, var[1:]))
+        if need_swig:
+            options.append('--enable-swig=yes')
+        else:
+            options.append('--enable-swig=no')
 
         # On OSX fix the compiler error:
         # In file included from tkStubLib.c:15: