From bfb45ba1cec521ea7ba25d6d248de1f913dde0c7 Mon Sep 17 00:00:00 2001 From: George Hartzell Date: Mon, 5 Jun 2017 11:02:39 -0700 Subject: 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. --- .../repos/builtin/packages/graphviz/package.py | 49 +++++++++++++--------- 1 file changed, 30 insertions(+), 19 deletions(-) (limited to 'var') 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: -- cgit v1.2.3-60-g2f50