From 54d741ba5448951ea689f3b8b91e3b1552d0a4d1 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Wed, 5 Jan 2022 09:32:27 -0800 Subject: unparser: handle package-level loops, if statements, and with blocks Many packages implement logic at the class level to handle complex dependencies and conflicts. Others have started using `with when("@1.0"):` blocks since we added that capability. The loops and other control logic can cause some pure directive logic not to be removed by our package hashing logic -- and in many cases that's a lot of code that will cause unnecessary rebuilds. This commit changes the unparser so that it will descend into these blocks. Specifically: 1. Descend into loops, if statements, and with blocks at the class level. 2. Don't look inside function definitions (in or outside a class). 3. Don't look at nested class definitions (they don't have directives) 4. Add logic to *remove* empty loops/with blocks/if statements if all directives in them were removed. This allows our package hash to ignore a lot of pure metadata that it was not ignoring before, and makes it less sensitive. In addition, we add `maintainers` and `tags` to the list of metadata attributes that Spack should remove from packages when constructing canonoical source for a package hash. - [x] Make unparser handle if/for/while/with at class level. - [x] Add tests for control logic removal. - [x] Add a test to ensure that all packages are not only unparseable, but also that their canonical source is still compilable. This is a test for our control logic removal. - [x] Add another unparse test package that has complex logic. --- lib/spack/spack/package.py | 13 +- lib/spack/spack/test/data/unparse/trilinos.txt | 786 +++++++++++++++++++++++++ lib/spack/spack/test/package_sanity.py | 19 +- lib/spack/spack/test/util/package_hash.py | 166 +++++- lib/spack/spack/util/package_hash.py | 129 ++-- 5 files changed, 1039 insertions(+), 74 deletions(-) create mode 100644 lib/spack/spack/test/data/unparse/trilinos.txt diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 5c49ccc00c..eb3ff79a29 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -676,8 +676,17 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)): maintainers = [] # type: List[str] #: List of attributes to be excluded from a package's hash. - metadata_attrs = ['homepage', 'url', 'urls', 'list_url', 'extendable', - 'parallel', 'make_jobs'] + metadata_attrs = [ + "homepage", + "url", + "urls", + "list_url", + "extendable", + "parallel", + "make_jobs", + "maintainers", + "tags", + ] #: Boolean. If set to ``True``, the smoke/install test requires a compiler. #: This is currently used by smoke tests to ensure a compiler is available diff --git a/lib/spack/spack/test/data/unparse/trilinos.txt b/lib/spack/spack/test/data/unparse/trilinos.txt new file mode 100644 index 0000000000..ae60c2088c --- /dev/null +++ b/lib/spack/spack/test/data/unparse/trilinos.txt @@ -0,0 +1,786 @@ + # -*- python -*- +# Copyright 2013-2021 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) +"""This is an unparser test package. + +``trilinos`` was chosen because it's one of the most complex packages in Spack, because +it has a lot of nested ``with when():`` blocks, and because it has loops and nested +logic at the package level. + +""" + +import os +import sys + +from spack import * +from spack.build_environment import dso_suffix +from spack.error import NoHeadersError +from spack.operating_systems.mac_os import macos_version +from spack.pkg.builtin.kokkos import Kokkos + +# Trilinos is complicated to build, as an inspiration a couple of links to +# other repositories which build it: +# https://github.com/hpcugent/easybuild-easyblocks/blob/master/easybuild/easyblocks/t/trilinos.py#L111 +# https://github.com/koecher/candi/blob/master/deal.II-toolchain/packages/trilinos.package +# https://gitlab.com/configurations/cluster-config/blob/master/trilinos.sh +# https://github.com/Homebrew/homebrew-science/blob/master/trilinos.rb and some +# relevant documentation/examples: +# https://github.com/trilinos/Trilinos/issues/175 + + +class Trilinos(CMakePackage, CudaPackage): + """The Trilinos Project is an effort to develop algorithms and enabling + technologies within an object-oriented software framework for the solution + of large-scale, complex multi-physics engineering and scientific problems. + A unique design feature of Trilinos is its focus on packages. + """ + homepage = "https://trilinos.org/" + url = "https://github.com/trilinos/Trilinos/archive/trilinos-release-12-12-1.tar.gz" + git = "https://github.com/trilinos/Trilinos.git" + + maintainers = ['keitat', 'sethrj', 'kuberry'] + + tags = ['e4s'] + + # ###################### Versions ########################## + + version('master', branch='master') + version('develop', branch='develop') + version('13.2.0', commit='4a5f7906a6420ee2f9450367e9cc95b28c00d744') # tag trilinos-release-13-2-0 + version('13.0.1', commit='4796b92fb0644ba8c531dd9953e7a4878b05c62d', preferred=True) # tag trilinos-release-13-0-1 + version('13.0.0', commit='9fec35276d846a667bc668ff4cbdfd8be0dfea08') # tag trilinos-release-13-0-0 + version('12.18.1', commit='55a75997332636a28afc9db1aee4ae46fe8d93e7') # tag trilinos-release-12-8-1 + version('12.14.1', sha256='52a4406cca2241f5eea8e166c2950471dd9478ad6741cbb2a7fc8225814616f0') + version('12.12.1', sha256='5474c5329c6309224a7e1726cf6f0d855025b2042959e4e2be2748bd6bb49e18') + version('12.10.1', sha256='ab81d917196ffbc21c4927d42df079dd94c83c1a08bda43fef2dd34d0c1a5512') + version('12.8.1', sha256='d20fe60e31e3ba1ef36edecd88226240a518f50a4d6edcc195b88ee9dda5b4a1') + version('12.6.4', sha256='1c7104ba60ee8cc4ec0458a1c4f6a26130616bae7580a7b15f2771a955818b73') + version('12.6.3', sha256='4d28298bb4074eef522db6cd1626f1a934e3d80f292caf669b8846c0a458fe81') + version('12.6.2', sha256='8be7e3e1166cc05aea7f856cc8033182e8114aeb8f87184cb38873bfb2061779') + version('12.6.1', sha256='4b38ede471bed0036dcb81a116fba8194f7bf1a9330da4e29c3eb507d2db18db') + version('12.4.2', sha256='fd2c12e87a7cedc058bcb8357107ffa2474997aa7b17b8e37225a1f7c32e6f0e') + version('12.2.1', sha256='088f303e0dc00fb4072b895c6ecb4e2a3ad9a2687b9c62153de05832cf242098') + version('12.0.1', sha256='eee7c19ca108538fa1c77a6651b084e06f59d7c3307dae77144136639ab55980') + version('11.14.3', sha256='e37fa5f69103576c89300e14d43ba77ad75998a54731008b25890d39892e6e60') + version('11.14.2', sha256='f22b2b0df7b88e28b992e19044ba72b845292b93cbbb3a948488199647381119') + version('11.14.1', sha256='f10fc0a496bf49427eb6871c80816d6e26822a39177d850cc62cf1484e4eec07') + + # ###################### Variants ########################## + + # Build options + variant('complex', default=False, description='Enable complex numbers in Trilinos') + variant('cuda_rdc', default=False, description='turn on RDC for CUDA build') + variant('cxxstd', default='14', values=['11', '14', '17'], multi=False) + variant('debug', default=False, description='Enable runtime safety and debug checks') + variant('explicit_template_instantiation', default=True, description='Enable explicit template instantiation (ETI)') + variant('float', default=False, description='Enable single precision (float) numbers in Trilinos') + variant('fortran', default=True, description='Compile with Fortran support') + variant('gotype', default='long_long', + values=('int', 'long', 'long_long', 'all'), + multi=False, + description='global ordinal type for Tpetra') + variant('openmp', default=False, description='Enable OpenMP') + variant('python', default=False, description='Build PyTrilinos wrappers') + variant('shared', default=True, description='Enables the build of shared libraries') + variant('wrapper', default=False, description="Use nvcc-wrapper for CUDA build") + + # TPLs (alphabet order) + variant('adios2', default=False, description='Enable ADIOS2') + variant('boost', default=False, description='Compile with Boost') + variant('hdf5', default=False, description='Compile with HDF5') + variant('hypre', default=False, description='Compile with Hypre preconditioner') + variant('mpi', default=True, description='Compile with MPI parallelism') + variant('mumps', default=False, description='Compile with support for MUMPS solvers') + variant('suite-sparse', default=False, description='Compile with SuiteSparse solvers') + variant('superlu-dist', default=False, description='Compile with SuperluDist solvers') + variant('superlu', default=False, description='Compile with SuperLU solvers') + variant('strumpack', default=False, description='Compile with STRUMPACK solvers') + variant('x11', default=False, description='Compile with X11 when +exodus') + + # Package options (alphabet order) + variant('amesos', default=True, description='Compile with Amesos') + variant('amesos2', default=True, description='Compile with Amesos2') + variant('anasazi', default=True, description='Compile with Anasazi') + variant('aztec', default=True, description='Compile with Aztec') + variant('belos', default=True, description='Compile with Belos') + variant('chaco', default=False, description='Compile with Chaco from SEACAS') + variant('epetra', default=True, description='Compile with Epetra') + variant('epetraext', default=True, description='Compile with EpetraExt') + variant('exodus', default=False, description='Compile with Exodus from SEACAS') + variant('ifpack', default=True, description='Compile with Ifpack') + variant('ifpack2', default=True, description='Compile with Ifpack2') + variant('intrepid', default=False, description='Enable Intrepid') + variant('intrepid2', default=False, description='Enable Intrepid2') + variant('isorropia', default=False, description='Compile with Isorropia') + variant('gtest', default=False, description='Build vendored Googletest') + variant('kokkos', default=True, description='Compile with Kokkos') + variant('ml', default=True, description='Compile with ML') + variant('minitensor', default=False, description='Compile with MiniTensor') + variant('muelu', default=True, description='Compile with Muelu') + variant('nox', default=False, description='Compile with NOX') + variant('piro', default=False, description='Compile with Piro') + variant('phalanx', default=False, description='Compile with Phalanx') + variant('rol', default=False, description='Compile with ROL') + variant('rythmos', default=False, description='Compile with Rythmos') + variant('sacado', default=True, description='Compile with Sacado') + variant('stk', default=False, description='Compile with STK') + variant('shards', default=False, description='Compile with Shards') + variant('shylu', default=False, description='Compile with ShyLU') + variant('stokhos', default=False, description='Compile with Stokhos') + variant('stratimikos', default=False, description='Compile with Stratimikos') + variant('teko', default=False, description='Compile with Teko') + variant('tempus', default=False, description='Compile with Tempus') + variant('tpetra', default=True, description='Compile with Tpetra') + variant('trilinoscouplings', default=False, description='Compile with TrilinosCouplings') + variant('zoltan', default=False, description='Compile with Zoltan') + variant('zoltan2', default=False, description='Compile with Zoltan2') + + # Internal package options (alphabetical order) + variant('basker', default=False, description='Compile with the Basker solver in Amesos2') + variant('epetraextbtf', default=False, description='Compile with BTF in EpetraExt') + variant('epetraextexperimental', default=False, description='Compile with experimental in EpetraExt') + variant('epetraextgraphreorderings', default=False, description='Compile with graph reorderings in EpetraExt') + + # External package options + variant('dtk', default=False, description='Enable DataTransferKit (deprecated)') + variant('scorec', default=False, description='Enable SCOREC') + variant('mesquite', default=False, description='Enable Mesquite (deprecated)') + + resource(name='dtk', + git='https://github.com/ornl-cees/DataTransferKit.git', + commit='4fe4d9d56cfd4f8a61f392b81d8efd0e389ee764', # branch dtk-3.0 + placement='DataTransferKit', + when='+dtk @12.14.0:12.14') + resource(name='dtk', + git='https://github.com/ornl-cees/DataTransferKit.git', + commit='edfa050cd46e2274ab0a0b7558caca0079c2e4ca', # tag 3.1-rc1 + placement='DataTransferKit', + submodules=True, + when='+dtk @12.18.0:12.18') + resource(name='scorec', + git='https://github.com/SCOREC/core.git', + commit='73c16eae073b179e45ec625a5abe4915bc589af2', # tag v2.2.5 + placement='SCOREC', + when='+scorec') + resource(name='mesquite', + url='https://github.com/trilinos/mesquite/archive/trilinos-release-12-12-1.tar.gz', + sha256='e0d09b0939dbd461822477449dca611417316e8e8d8268fd795debb068edcbb5', + placement='packages/mesquite', + when='+mesquite @12.12.1:12.16') + resource(name='mesquite', + git='https://github.com/trilinos/mesquite.git', + commit='20a679679b5cdf15bf573d66c5dc2b016e8b9ca1', # branch trilinos-release-12-12-1 + placement='packages/mesquite', + when='+mesquite @12.18.1:12.18') + resource(name='mesquite', + git='https://github.com/trilinos/mesquite.git', + tag='develop', + placement='packages/mesquite', + when='+mesquite @master') + + # ###################### Conflicts ########################## + + # Epetra packages + with when('~epetra'): + conflicts('+amesos') + conflicts('+aztec') + conflicts('+epetraext') + conflicts('+ifpack') + conflicts('+isorropia') + conflicts('+ml', when='@13.2:') + with when('~epetraext'): + conflicts('+isorropia') + conflicts('+teko') + conflicts('+epetraextbtf') + conflicts('+epetraextexperimental') + conflicts('+epetraextgraphreorderings') + + # Tpetra packages + with when('~kokkos'): + conflicts('+cuda') + conflicts('+tpetra') + conflicts('+intrepid2') + conflicts('+phalanx') + with when('~tpetra'): + conflicts('+amesos2') + conflicts('+dtk') + conflicts('+ifpack2') + conflicts('+muelu') + conflicts('+teko') + conflicts('+zoltan2') + + with when('+teko'): + conflicts('~amesos') + conflicts('~anasazi') + conflicts('~aztec') + conflicts('~ifpack') + conflicts('~ml') + conflicts('~stratimikos') + conflicts('@:12 gotype=long') + + # Known requirements from tribits dependencies + conflicts('+aztec', when='~fortran') + conflicts('+basker', when='~amesos2') + conflicts('+minitensor', when='~boost') + conflicts('+ifpack2', when='~belos') + conflicts('+intrepid', when='~sacado') + conflicts('+intrepid', when='~shards') + conflicts('+intrepid2', when='~shards') + conflicts('+isorropia', when='~zoltan') + conflicts('+phalanx', when='~sacado') + conflicts('+scorec', when='~mpi') + conflicts('+scorec', when='~shards') + conflicts('+scorec', when='~stk') + conflicts('+scorec', when='~zoltan') + conflicts('+tempus', when='~nox') + conflicts('+zoltan2', when='~zoltan') + + # Only allow DTK with Trilinos 12.14, 12.18 + conflicts('+dtk', when='~boost') + conflicts('+dtk', when='~intrepid2') + conflicts('+dtk', when='@:12.12,13:') + + # Installed FindTrilinos are broken in SEACAS if Fortran is disabled + # see https://github.com/trilinos/Trilinos/issues/3346 + conflicts('+exodus', when='@:13.0.1 ~fortran') + # Only allow Mesquite with Trilinos 12.12 and up, and master + conflicts('+mesquite', when='@:12.10,master') + # Strumpack is only available as of mid-2021 + conflicts('+strumpack', when='@:13.0') + # Can only use one type of SuperLU + conflicts('+superlu-dist', when='+superlu') + # For Trilinos v11 we need to force SuperLUDist=OFF, since only the + # deprecated SuperLUDist v3.3 together with an Amesos patch is working. + conflicts('+superlu-dist', when='@11.4.1:11.14.3') + # see https://github.com/trilinos/Trilinos/issues/3566 + conflicts('+superlu-dist', when='+float+amesos2+explicit_template_instantiation^superlu-dist@5.3.0:') + # Amesos, conflicting types of double and complex SLU_D + # see https://trilinos.org/pipermail/trilinos-users/2015-March/004731.html + # and https://trilinos.org/pipermail/trilinos-users/2015-March/004802.html + conflicts('+superlu-dist', when='+complex+amesos2') + # https://github.com/trilinos/Trilinos/issues/2994 + conflicts( + '+shared', when='+stk platform=darwin', + msg='Cannot build Trilinos with STK as a shared library on Darwin.' + ) + conflicts('+adios2', when='@:12.14.1') + conflicts('cxxstd=11', when='@master:') + conflicts('cxxstd=11', when='+wrapper ^cuda@6.5.14') + conflicts('cxxstd=14', when='+wrapper ^cuda@6.5.14:8.0.61') + conflicts('cxxstd=17', when='+wrapper ^cuda@6.5.14:10.2.89') + + # Multi-value gotype only applies to trilinos through 12.14 + conflicts('gotype=all', when='@12.15:') + + # CUDA without wrapper requires clang + for _compiler in spack.compilers.supported_compilers(): + if _compiler != 'clang': + conflicts('+cuda', when='~wrapper %' + _compiler, + msg='trilinos~wrapper+cuda can only be built with the ' + 'Clang compiler') + conflicts('+cuda_rdc', when='~cuda') + conflicts('+wrapper', when='~cuda') + conflicts('+wrapper', when='%clang') + + # Old trilinos fails with new CUDA (see #27180) + conflicts('@:13.0.1 +cuda', when='^cuda@11:') + + # stokhos fails on xl/xl_r + conflicts('+stokhos', when='%xl') + conflicts('+stokhos', when='%xl_r') + + # Fortran mangling fails on Apple M1 (see spack/spack#25900) + conflicts('@:13.0.1 +fortran', when='target=m1') + + # ###################### Dependencies ########################## + + depends_on('adios2', when='+adios2') + depends_on('blas') + depends_on('boost', when='+boost') + depends_on('cgns', when='+exodus') + depends_on('hdf5+hl', when='+hdf5') + depends_on('hypre~internal-superlu~int64', when='+hypre') + depends_on('kokkos-nvcc-wrapper', when='+wrapper') + depends_on('lapack') + # depends_on('perl', type=('build',)) # TriBITS finds but doesn't use... + depends_on('libx11', when='+x11') + depends_on('matio', when='+exodus') + depends_on('metis', when='+zoltan') + depends_on('mpi', when='+mpi') + depends_on('netcdf-c', when="+exodus") + depends_on('parallel-netcdf', when='+exodus+mpi') + depends_on('parmetis', when='+mpi +zoltan') + depends_on('parmetis', when='+scorec') + depends_on('py-mpi4py', when='+mpi+python', type=('build', 'run')) + depends_on('py-numpy', when='+python', type=('build', 'run')) + depends_on('python', when='+python') + depends_on('python', when='@13.2: +ifpack +hypre', type='build') + depends_on('python', when='@13.2: +ifpack2 +hypre', type='build') + depends_on('scalapack', when='+mumps') + depends_on('scalapack', when='+strumpack+mpi') + depends_on('strumpack+shared', when='+strumpack') + depends_on('suite-sparse', when='+suite-sparse') + depends_on('superlu-dist', when='+superlu-dist') + depends_on('superlu@4.3 +pic', when='+superlu') + depends_on('swig', when='+python') + depends_on('zlib', when='+zoltan') + + # Trilinos' Tribits config system is limited which makes it very tricky to + # link Amesos with static MUMPS, see + # https://trilinos.org/docs/dev/packages/amesos2/doc/html/classAmesos2_1_1MUMPS.html + # One could work it out by getting linking flags from mpif90 --showme:link + # (or alike) and adding results to -DTrilinos_EXTRA_LINK_FLAGS together + # with Blas and Lapack and ScaLAPACK and Blacs and -lgfortran and it may + # work at the end. But let's avoid all this by simply using shared libs + depends_on('mumps@5.0:+shared', when='+mumps') + + for _flag in ('~mpi', '+mpi'): + depends_on('hdf5' + _flag, when='+hdf5' + _flag) + depends_on('mumps' + _flag, when='+mumps' + _flag) + for _flag in ('~openmp', '+openmp'): + depends_on('mumps' + _flag, when='+mumps' + _flag) + + depends_on('hwloc', when='@13: +kokkos') + depends_on('hwloc+cuda', when='@13: +kokkos+cuda') + depends_on('hypre@develop', when='@master: +hypre') + depends_on('netcdf-c+mpi+parallel-netcdf', when="+exodus+mpi@12.12.1:") + depends_on('superlu-dist@4.4:5.3', when='@12.6.2:12.12.1+superlu-dist') + depends_on('superlu-dist@5.4:6.2.0', when='@12.12.2:13.0.0+superlu-dist') + depends_on('superlu-dist@6.3.0:', when='@13.0.1:99 +superlu-dist') + depends_on('superlu-dist@:4.3', when='@11.14.1:12.6.1+superlu-dist') + depends_on('superlu-dist@develop', when='@master: +superlu-dist') + + # ###################### Patches ########################## + + patch('umfpack_from_suitesparse.patch', when='@11.14.1:12.8.1') + for _compiler in ['xl', 'xl_r', 'clang']: + patch('xlf_seacas.patch', when='@12.10.1:12.12.1 %' + _compiler) + patch('xlf_tpetra.patch', when='@12.12.1 %' + _compiler) + patch('fix_clang_errors_12_18_1.patch', when='@12.18.1%clang') + patch('cray_secas_12_12_1.patch', when='@12.12.1%cce') + patch('cray_secas.patch', when='@12.14.1:%cce') + + # workaround an NVCC bug with c++14 (https://github.com/trilinos/Trilinos/issues/6954) + # avoid calling deprecated functions with CUDA-11 + patch('fix_cxx14_cuda11.patch', when='@13.0.0:13.0.1 cxxstd=14 ^cuda@11:') + # Allow building with +teko gotype=long + patch('https://github.com/trilinos/Trilinos/commit/b17f20a0b91e0b9fc5b1b0af3c8a34e2a4874f3f.patch', + sha256='dee6c55fe38eb7f6367e1896d6bc7483f6f9ab8fa252503050cc0c68c6340610', + when='@13.0.0:13.0.1 +teko gotype=long') + + def flag_handler(self, name, flags): + is_cce = self.spec.satisfies('%cce') + + if name == 'cxxflags': + spec = self.spec + if '+mumps' in spec: + # see https://github.com/trilinos/Trilinos/blob/master/packages/amesos/README-MUMPS + flags.append('-DMUMPS_5_0') + if '+stk platform=darwin' in spec: + flags.append('-DSTK_NO_BOOST_STACKTRACE') + if '+stk%intel' in spec: + # Workaround for Intel compiler segfaults with STK and IPO + flags.append('-no-ipo') + if '+wrapper' in spec: + flags.append('--expt-extended-lambda') + elif name == 'ldflags' and is_cce: + flags.append('-fuse-ld=gold') + + if is_cce: + return (None, None, flags) + return (flags, None, None) + + def url_for_version(self, version): + url = "https://github.com/trilinos/Trilinos/archive/trilinos-release-{0}.tar.gz" + return url.format(version.dashed) + + def setup_dependent_run_environment(self, env, dependent_spec): + if '+cuda' in self.spec: + # currently Trilinos doesn't perform the memory fence so + # it relies on blocking CUDA kernel launch. This is needed + # in case the dependent app also run a CUDA backend via Trilinos + env.set('CUDA_LAUNCH_BLOCKING', '1') + + def setup_dependent_package(self, module, dependent_spec): + if '+wrapper' in self.spec: + self.spec.kokkos_cxx = self.spec["kokkos-nvcc-wrapper"].kokkos_cxx + else: + self.spec.kokkos_cxx = spack_cxx + + def setup_build_environment(self, env): + spec = self.spec + if '+cuda' in spec and '+wrapper' in spec: + if '+mpi' in spec: + env.set('OMPI_CXX', spec["kokkos-nvcc-wrapper"].kokkos_cxx) + env.set('MPICH_CXX', spec["kokkos-nvcc-wrapper"].kokkos_cxx) + env.set('MPICXX_CXX', spec["kokkos-nvcc-wrapper"].kokkos_cxx) + else: + env.set('CXX', spec["kokkos-nvcc-wrapper"].kokkos_cxx) + + def cmake_args(self): + options = [] + + spec = self.spec + define = CMakePackage.define + define_from_variant = self.define_from_variant + + def _make_definer(prefix): + def define_enable(suffix, value=None): + key = prefix + suffix + if value is None: + # Default to lower-case spec + value = suffix.lower() + elif isinstance(value, bool): + # Explicit true/false + return define(key, value) + return define_from_variant(key, value) + return define_enable + + # Return "Trilinos_ENABLE_XXX" for spec "+xxx" or boolean value + define_trilinos_enable = _make_definer("Trilinos_ENABLE_") + # Same but for TPLs + define_tpl_enable = _make_definer("TPL_ENABLE_") + + # #################### Base Settings ####################### + + options.extend([ + define('Trilinos_VERBOSE_CONFIGURE', False), + define_from_variant('BUILD_SHARED_LIBS', 'shared'), + define_from_variant('CMAKE_CXX_STANDARD', 'cxxstd'), + define_trilinos_enable('ALL_OPTIONAL_PACKAGES', False), + define_trilinos_enable('ALL_PACKAGES', False), + define_trilinos_enable('CXX11', True), + define_trilinos_enable('DEBUG', 'debug'), + define_trilinos_enable('EXAMPLES', False), + define_trilinos_enable('SECONDARY_TESTED_CODE', True), + define_trilinos_enable('TESTS', False), + define_trilinos_enable('Fortran'), + define_trilinos_enable('OpenMP'), + define_trilinos_enable('EXPLICIT_INSTANTIATION', + 'explicit_template_instantiation') + ]) + + # ################## Trilinos Packages ##################### + + options.extend([ + define_trilinos_enable('Amesos'), + define_trilinos_enable('Amesos2'), + define_trilinos_enable('Anasazi'), + define_trilinos_enable('AztecOO', 'aztec'), + define_trilinos_enable('Belos'), + define_trilinos_enable('Epetra'), + define_trilinos_enable('EpetraExt'), + define_trilinos_enable('FEI', False), + define_trilinos_enable('Gtest'), + define_trilinos_enable('Ifpack'), + define_trilinos_enable('Ifpack2'), + define_trilinos_enable('Intrepid'), + define_trilinos_enable('Intrepid2'), + define_trilinos_enable('Isorropia'), + define_trilinos_enable('Kokkos'), + define_trilinos_enable('MiniTensor'), + define_trilinos_enable('Mesquite'), + define_trilinos_enable('ML'), + define_trilinos_enable('MueLu'), + define_trilinos_enable('NOX'), + define_trilinos_enable('Pamgen', False), + define_trilinos_enable('Panzer', False), + define_trilinos_enable('Pike', False), + define_trilinos_enable('Piro'), + define_trilinos_enable('Phalanx'), + define_trilinos_enable('PyTrilinos', 'python'), + define_trilinos_enable('ROL'), + define_trilinos_enable('Rythmos'), + define_trilinos_enable('Sacado'), + define_trilinos_enable('SCOREC'), + define_trilinos_enable('Shards'), + define_trilinos_enable('ShyLU'), + define_trilinos_enable('STK'), + define_trilinos_enable('Stokhos'), + define_trilinos_enable('Stratimikos'), + define_trilinos_enable('Teko'), + define_trilinos_enable('Tempus'), + define_trilinos_enable('Tpetra'), + define_trilinos_enable('TrilinosCouplings'), + define_trilinos_enable('Zoltan'), + define_trilinos_enable('Zoltan2'), + define_tpl_enable('Cholmod', False), + define_from_variant('EpetraExt_BUILD_BTF', 'epetraextbtf'), + define_from_variant('EpetraExt_BUILD_EXPERIMENTAL', + 'epetraextexperimental'), + define_from_variant('EpetraExt_BUILD_GRAPH_REORDERINGS', + 'epetraextgraphreorderings'), + define_from_variant('Amesos2_ENABLE_Basker', 'basker'), + ]) + + if '+dtk' in spec: + options.extend([ + define('Trilinos_EXTRA_REPOSITORIES', 'DataTransferKit'), + define_trilinos_enable('DataTransferKit', True), + ]) + + if '+exodus' in spec: + options.extend([ + define_trilinos_enable('SEACAS', True), + define_trilinos_enable('SEACASExodus', True), + define_trilinos_enable('SEACASIoss', True), + define_trilinos_enable('SEACASEpu', True), + define_trilinos_enable('SEACASExodiff', True), + define_trilinos_enable('SEACASNemspread', True), + define_trilinos_enable('SEACASNemslice', True), + ]) + else: + options.extend([ + define_trilinos_enable('SEACASExodus', False), + define_trilinos_enable('SEACASIoss', False), + ]) + + if '+chaco' in spec: + options.extend([ + define_trilinos_enable('SEACAS', True), + define_trilinos_enable('SEACASChaco', True), + ]) + else: + # don't disable SEACAS, could be needed elsewhere + options.extend([ + define_trilinos_enable('SEACASChaco', False), + define_trilinos_enable('SEACASNemslice', False) + ]) + + if '+stratimikos' in spec: + # Explicitly enable Thyra (ThyraCore is required). If you don't do + # this, then you get "NOT setting ${pkg}_ENABLE_Thyra=ON since + # Thyra is NOT enabled at this point!" leading to eventual build + # errors if using MueLu because `Xpetra_ENABLE_Thyra` is set to + # off. + options.append(define_trilinos_enable('Thyra', True)) + + # Add thyra adapters based on package enables + options.extend( + define_trilinos_enable('Thyra' + pkg + 'Adapters', pkg.lower()) + for pkg in ['Epetra', 'EpetraExt', 'Tpetra']) + + # ######################### TPLs ############################# + + def define_tpl(trilinos_name, spack_name, have_dep): + options.append(define('TPL_ENABLE_' + trilinos_name, have_dep)) + if not have_dep: + return + depspec = spec[spack_name] + libs = depspec.libs + try: + options.extend([ + define(trilinos_name + '_INCLUDE_DIRS', + depspec.headers.directories), + ]) + except NoHeadersError: + # Handle case were depspec does not have headers + pass + + options.extend([ + define(trilinos_name + '_ROOT', depspec.prefix), + define(trilinos_name + '_LIBRARY_NAMES', libs.names), + define(trilinos_name + '_LIBRARY_DIRS', libs.directories), + ]) + + # Enable these TPLs explicitly from variant options. + # Format is (TPL name, variant name, Spack spec name) + tpl_variant_map = [ + ('ADIOS2', 'adios2', 'adios2'), + ('Boost', 'boost', 'boost'), + ('CUDA', 'cuda', 'cuda'), + ('HDF5', 'hdf5', 'hdf5'), + ('HYPRE', 'hypre', 'hypre'), + ('MUMPS', 'mumps', 'mumps'), + ('UMFPACK', 'suite-sparse', 'suite-sparse'), + ('SuperLU', 'superlu', 'superlu'), + ('SuperLUDist', 'superlu-dist', 'superlu-dist'), + ('X11', 'x11', 'libx11'), + ] + if spec.satisfies('@13.0.2:'): + tpl_variant_map.append(('STRUMPACK', 'strumpack', 'strumpack')) + + for tpl_name, var_name, spec_name in tpl_variant_map: + define_tpl(tpl_name, spec_name, spec.variants[var_name].value) + + # Enable these TPLs based on whether they're in our spec; prefer to + # require this way so that packages/features disable availability + tpl_dep_map = [ + ('BLAS', 'blas'), + ('CGNS', 'cgns'), + ('LAPACK', 'lapack'), + ('Matio', 'matio'), + ('METIS', 'metis'), + ('Netcdf', 'netcdf-c'), + ('SCALAPACK', 'scalapack'), + ('Zlib', 'zlib'), + ] + if spec.satisfies('@12.12.1:'): + tpl_dep_map.append(('Pnetcdf', 'parallel-netcdf')) + if spec.satisfies('@13:'): + tpl_dep_map.append(('HWLOC', 'hwloc')) + + for tpl_name, dep_name in tpl_dep_map: + define_tpl(tpl_name, dep_name, dep_name in spec) + + # MPI settings + options.append(define_tpl_enable('MPI')) + if '+mpi' in spec: + # Force Trilinos to use the MPI wrappers instead of raw compilers + # to propagate library link flags for linkers that require fully + # resolved symbols in shared libs (such as macOS and some newer + # Ubuntu) + options.extend([ + define('CMAKE_C_COMPILER', spec['mpi'].mpicc), + define('CMAKE_CXX_COMPILER', spec['mpi'].mpicxx), + define('CMAKE_Fortran_COMPILER', spec['mpi'].mpifc), + define('MPI_BASE_DIR', spec['mpi'].prefix), + ]) + + # ParMETIS dependencies have to be transitive explicitly + have_parmetis = 'parmetis' in spec + options.append(define_tpl_enable('ParMETIS', have_parmetis)) + if have_parmetis: + options.extend([ + define('ParMETIS_LIBRARY_DIRS', [ + spec['parmetis'].prefix.lib, spec['metis'].prefix.lib + ]), + define('ParMETIS_LIBRARY_NAMES', ['parmetis', 'metis']), + define('TPL_ParMETIS_INCLUDE_DIRS', + spec['parmetis'].headers.directories + + spec['metis'].headers.directories), + ]) + + if spec.satisfies('^superlu-dist@4.0:'): + options.extend([ + define('HAVE_SUPERLUDIST_LUSTRUCTINIT_2ARG', True), + ]) + + if spec.satisfies('^parallel-netcdf'): + options.extend([ + define('TPL_Netcdf_Enables_Netcdf4', True), + define('TPL_Netcdf_PARALLEL', True), + define('PNetCDF_ROOT', spec['parallel-netcdf'].prefix), + ]) + + # ################# Explicit template instantiation ################# + + complex_s = spec.variants['complex'].value + float_s = spec.variants['float'].value + + options.extend([ + define('Teuchos_ENABLE_COMPLEX', complex_s), + define('Teuchos_ENABLE_FLOAT', float_s), + ]) + + if '+tpetra +explicit_template_instantiation' in spec: + options.append(define_from_variant('Tpetra_INST_OPENMP', 'openmp')) + options.extend([ + define('Tpetra_INST_DOUBLE', True), + define('Tpetra_INST_COMPLEX_DOUBLE', complex_s), + define('Tpetra_INST_COMPLEX_FLOAT', float_s and complex_s), + define('Tpetra_INST_FLOAT', float_s), + define('Tpetra_INST_SERIAL', True), + ]) + + gotype = spec.variants['gotype'].value + if gotype == 'all': + # default in older Trilinos versions to enable multiple GOs + options.extend([ + define('Tpetra_INST_INT_INT', True), + define('Tpetra_INST_INT_LONG', True), + define('Tpetra_INST_INT_LONG_LONG', True), + ]) + else: + options.extend([ + define('Tpetra_INST_INT_INT', gotype == 'int'), + define('Tpetra_INST_INT_LONG', gotype == 'long'), + define('Tpetra_INST_INT_LONG_LONG', gotype == 'long_long'), + ]) + + # ################# Kokkos ###################### + + if '+kokkos' in spec: + arch = Kokkos.get_microarch(spec.target) + if arch: + options.append(define("Kokkos_ARCH_" + arch.upper(), True)) + + define_kok_enable = _make_definer("Kokkos_ENABLE_") + options.extend([ + define_kok_enable('CUDA'), + define_kok_enable('OPENMP' if spec.version >= Version('13') + else 'OpenMP'), + ]) + if '+cuda' in spec: + options.extend([ + define_kok_enable('CUDA_UVM', True), + define_kok_enable('CUDA_LAMBDA', True), + define_kok_enable('CUDA_RELOCATABLE_DEVICE_CODE', 'cuda_rdc') + ]) + arch_map = Kokkos.spack_cuda_arch_map + options.extend( + define("Kokkos_ARCH_" + arch_map[arch].upper(), True) + for arch in spec.variants['cuda_arch'].value + ) + + # ################# System-specific ###################### + + # Fortran lib (assumes clang is built with gfortran!) + if ('+fortran' in spec + and spec.compiler.name in ['gcc', 'clang', 'apple-clang']): + fc = Executable(spec['mpi'].mpifc) if ( + '+mpi' in spec) else Executable(spack_fc) + libgfortran = fc('--print-file-name', + 'libgfortran.' + dso_suffix, + output=str).strip() + # if libgfortran is equal to "libgfortran." then + # print-file-name failed, use static library instead + if libgfortran == 'libgfortran.' + dso_suffix: + libgfortran = fc('--print-file-name', + 'libgfortran.a', + output=str).strip() + # -L -lgfortran required for OSX + # https://github.com/spack/spack/pull/25823#issuecomment-917231118 + options.append( + define('Trilinos_EXTRA_LINK_FLAGS', + '-L%s/ -lgfortran' % os.path.dirname(libgfortran))) + + if sys.platform == 'darwin' and macos_version() >= Version('10.12'): + # use @rpath on Sierra due to limit of dynamic loader + options.append(define('CMAKE_MACOSX_RPATH', True)) + else: + options.append(define('CMAKE_INSTALL_NAME_DIR', self.prefix.lib)) + + return options + + @run_after('install') + def filter_python(self): + # When trilinos is built with Python, libpytrilinos is included + # through cmake configure files. Namely, Trilinos_LIBRARIES in + # TrilinosConfig.cmake contains pytrilinos. This leads to a + # run-time error: Symbol not found: _PyBool_Type and prevents + # Trilinos to be used in any C++ code, which links executable + # against the libraries listed in Trilinos_LIBRARIES. See + # https://github.com/trilinos/Trilinos/issues/569 and + # https://github.com/trilinos/Trilinos/issues/866 + # A workaround is to remove PyTrilinos from the COMPONENTS_LIST + # and to remove -lpytrilonos from Makefile.export.Trilinos + if '+python' in self.spec: + filter_file(r'(SET\(COMPONENTS_LIST.*)(PyTrilinos;)(.*)', + (r'\1\3'), + '%s/cmake/Trilinos/TrilinosConfig.cmake' % + self.prefix.lib) + filter_file(r'-lpytrilinos', '', + '%s/Makefile.export.Trilinos' % + self.prefix.include) + + def setup_run_environment(self, env): + if '+exodus' in self.spec: + env.prepend_path('PYTHONPATH', self.prefix.lib) + + if '+cuda' in self.spec: + # currently Trilinos doesn't perform the memory fence so + # it relies on blocking CUDA kernel launch. + env.set('CUDA_LAUNCH_BLOCKING', '1') diff --git a/lib/spack/spack/test/package_sanity.py b/lib/spack/spack/test/package_sanity.py index 0ae3313c8a..b57c853e0e 100644 --- a/lib/spack/spack/test/package_sanity.py +++ b/lib/spack/spack/test/package_sanity.py @@ -3,6 +3,7 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) """This test does sanity checks on Spack's builtin package database.""" +import ast import os.path import pickle import re @@ -58,18 +59,30 @@ def test_packages_are_pickleable(): def test_packages_are_unparseable(): - failed_to_unparse = list() + """Ensure that all packages can unparse and that unparsed code is valid Python.""" + failed_to_unparse = [] + failed_to_compile = [] + for name in spack.repo.all_package_names(): try: - ph.canonical_source(name, filter_multimethods=False) + source = ph.canonical_source(name, filter_multimethods=False) except Exception: failed_to_unparse.append(name) + try: + compile(source, "internal", "exec", ast.PyCF_ONLY_AST) + except Exception: + failed_to_compile.append(name) + if failed_to_unparse: tty.msg('The following packages failed to unparse: ' + ', '.join(failed_to_unparse)) + assert False - assert len(failed_to_unparse) == 0 + if failed_to_compile: + tty.msg('The following unparsed packages failed to compile: ' + + ', '.join(failed_to_compile)) + assert False def test_repo_getpkg_names_and_classes(): diff --git a/lib/spack/spack/test/util/package_hash.py b/lib/spack/spack/test/util/package_hash.py index d410817303..db8ac1f5be 100644 --- a/lib/spack/spack/test/util/package_hash.py +++ b/lib/spack/spack/test/util/package_hash.py @@ -156,32 +156,28 @@ class ManyDocstrings: return "TWELVE" ''' +many_strings_no_docstrings = """\ +var = 'THREE' -def test_remove_docstrings(): - tree = ast.parse(many_strings) - tree = ph.RemoveDocstrings().visit(tree) +class ManyDocstrings: + x = 'SEVEN' - unparsed = unparse(tree) + def method1(): + print('NINE') + for i in range(10): + print(i) - # make sure the methods are preserved - assert "method1" in unparsed - assert "method2" in unparsed + def method2(): + return 'TWELVE' +""" - # all of these are unassigned and should be removed - assert "ONE" not in unparsed - assert "TWO" not in unparsed - assert "FOUR" not in unparsed - assert "FIVE" not in unparsed - assert "SIX" not in unparsed - assert "EIGHT" not in unparsed - assert "TEN" not in unparsed - assert "ELEVEN" not in unparsed - # these are used in legitimate expressions - assert "THREE" in unparsed - assert "SEVEN" in unparsed - assert "NINE" in unparsed - assert "TWELVE" in unparsed +def test_remove_docstrings(): + tree = ast.parse(many_strings) + tree = ph.RemoveDocstrings().visit(tree) + + unparsed = unparse(tree, py_ver_consistent=True) + assert unparsed == many_strings_no_docstrings many_directives = """\ @@ -199,31 +195,137 @@ class HasManyDirectives: )) -def test_remove_directives(): +def test_remove_all_directives(): """Ensure all directives are removed from packages before hashing.""" + for name in spack.directives.directive_names: + assert name in many_directives + tree = ast.parse(many_directives) spec = Spec("has-many-directives") tree = ph.RemoveDirectives(spec).visit(tree) - unparsed = unparse(tree) + unparsed = unparse(tree, py_ver_consistent=True) for name in spack.directives.directive_names: assert name not in unparsed +many_attributes = """\ +class HasManyMetadataAttributes: + homepage = "https://example.com" + url = "https://example.com/foo.tar.gz" + git = "https://example.com/foo/bar.git" + + maintainers = ["alice", "bob"] + tags = ["foo", "bar", "baz"] + + depends_on("foo") + conflicts("foo") +""" + + +many_attributes_canonical = """\ +class HasManyMetadataAttributes: + pass +""" + + +def test_remove_spack_attributes(): + tree = ast.parse(many_attributes) + spec = Spec("has-many-metadata-attributes") + tree = ph.RemoveDirectives(spec).visit(tree) + unparsed = unparse(tree, py_ver_consistent=True) + + assert unparsed == many_attributes_canonical + + +complex_package_logic = """\ +class ComplexPackageLogic: + for variant in ["+foo", "+bar", "+baz"]: + conflicts("quux" + variant) + + for variant in ["+foo", "+bar", "+baz"]: + # logic in the loop prevents our dumb analyzer from having it removed. This + # is uncommon so we don't (yet?) implement logic to detect that spec is unused. + print("oops can't remove this.") + conflicts("quux" + variant) + + # Hard to make a while loop that makes sense, so ignore the infinite loop here. + # Likely nobody uses while instead of for, but we test it just in case. + while x <= 10: + depends_on("garply@%d.0" % x) + + # all of these should go away, as they only contain directives + with when("@10.0"): + depends_on("foo") + with when("+bar"): + depends_on("bar") + with when("+baz"): + depends_on("baz") + + # this whole statement should disappear + if sys.platform == "linux": + conflicts("baz@9.0") + + # the else block here should disappear + if sys.platform == "linux": + print("foo") + else: + conflicts("foo@9.0") + + # both blocks of this statement should disappear + if sys.platform == "darwin": + conflicts("baz@10.0") + else: + conflicts("bar@10.0") + + # This one is complicated as the body goes away but the else block doesn't. + # Again, this could be optimized, but we're just testing removal logic here. + if sys.platform() == "darwin": + conflicts("baz@10.0") + else: + print("oops can't remove this.") + conflicts("bar@10.0") +""" + + +complex_package_logic_filtered = """\ +class ComplexPackageLogic: + for variant in ['+foo', '+bar', '+baz']: + print("oops can't remove this.") + if sys.platform == 'linux': + print('foo') + if sys.platform() == 'darwin': + pass + else: + print("oops can't remove this.") +""" + + +def test_remove_complex_package_logic_filtered(): + tree = ast.parse(complex_package_logic) + spec = Spec("has-many-metadata-attributes") + tree = ph.RemoveDirectives(spec).visit(tree) + unparsed = unparse(tree, py_ver_consistent=True) + + assert unparsed == complex_package_logic_filtered + + @pytest.mark.parametrize("package_spec,expected_hash", [ - ("amdfftw", "nfrk76xyu6wxs4xb4nyichm3om3kb7yp"), + ("amdfftw", "tivb752zddjgvfkogfs7cnnvp5olj6co"), ("grads", "rrlmwml3f2frdnqavmro3ias66h5b2ce"), - ("llvm", "ngact4ds3xwgsbn5bruxpfs6f4u4juba"), + ("llvm", "g3hoqf4rhprd3da7byp5nzco6tcwliiy"), # has @when("@4.1.0") and raw unicode literals - ("mfem", "65xryd5zxarwzqlh2pojq7ykohpod4xz"), - ("mfem@4.0.0", "65xryd5zxarwzqlh2pojq7ykohpod4xz"), - ("mfem@4.1.0", "2j655nix3oe57iwvs2mlgx2mresk7czl"), + ("mfem", "tiiv7uq7v2xtv24vdij5ptcv76dpazrw"), + ("mfem@4.0.0", "tiiv7uq7v2xtv24vdij5ptcv76dpazrw"), + ("mfem@4.1.0", "gxastq64to74qt4he4knpyjfdhh5auel"), # has @when("@1.5.0:") - ("py-torch", "lnwmqk4wadtlsc2badrt7foid5tl5vaw"), - ("py-torch@1.0", "lnwmqk4wadtlsc2badrt7foid5tl5vaw"), - ("py-torch@1.6", "5nwndnknxdfs5or5nrl4pecvw46xc5i2"), + ("py-torch", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"), + ("py-torch@1.0", "qs7djgqn7dy7r3ps4g7hv2pjvjk4qkhd"), + ("py-torch@1.6", "p4ine4hc6f2ik2f2wyuwieslqbozll5w"), # has a print with multiple arguments - ("legion", "ba4tleyb3g5mdhhsje6t6jyitqj3yfpz"), + ("legion", "zdpawm4avw3fllxcutvmqb5c3bj5twqt"), + # has nested `with when()` blocks and loops + ("trilinos", "vqrgscjrla4hi7bllink7v6v6dwxgc2p"), ]) def test_package_hash_consistency(package_spec, expected_hash): """Ensure that that package hash is consistent python version to version. diff --git a/lib/spack/spack/util/package_hash.py b/lib/spack/spack/util/package_hash.py index d2b9055579..e6ae29b067 100644 --- a/lib/spack/spack/util/package_hash.py +++ b/lib/spack/spack/util/package_hash.py @@ -34,51 +34,106 @@ class RemoveDocstrings(ast.NodeTransformer): self.generic_visit(node) return node - def visit_FunctionDef(self, node): # noqa + def visit_FunctionDef(self, node): return self.remove_docstring(node) - def visit_ClassDef(self, node): # noqa + def visit_ClassDef(self, node): return self.remove_docstring(node) - def visit_Module(self, node): # noqa + def visit_Module(self, node): return self.remove_docstring(node) class RemoveDirectives(ast.NodeTransformer): - """Remove Spack directives from a package AST.""" + """Remove Spack directives from a package AST. + + This removes Spack directives (e.g., ``depends_on``, ``conflicts``, etc.) and + metadata attributes (e.g., ``tags``, ``homepage``, ``url``) in a top-level class + definition within a ``package.py``, but it does not modify nested classes or + functions. + + If removing directives causes a ``for``, ``with``, or ``while`` statement to have an + empty body, we remove the entire statement. Similarly, If removing directives causes + an ``if`` statement to have an empty body or ``else`` block, we'll remove the block + (or replace the body with ``pass`` if there is an ``else`` block but no body). + + """ + def __init__(self, spec): + # list of URL attributes and metadata attributes + # these will be removed from packages. + self.metadata_attrs = [s.url_attr for s in spack.fetch_strategy.all_strategies] + self.metadata_attrs += spack.package.Package.metadata_attrs + self.spec = spec + self.in_classdef = False # used to avoid nested classdefs + + def visit_Expr(self, node): + # Directives are represented in the AST as named function call expressions (as + # opposed to function calls through a variable callback). We remove them. + # + # Note that changes to directives (e.g., a preferred version change or a hash + # chnage on an archive) are already represented in the spec *outside* the + # package hash. + return None if ( + node.value and isinstance(node.value, ast.Call) and + isinstance(node.value.func, ast.Name) and + node.value.func.id in spack.directives.directive_names + ) else node + + def visit_Assign(self, node): + # Remove assignments to metadata attributes, b/c they don't affect the build. + return None if ( + node.targets and isinstance(node.targets[0], ast.Name) and + node.targets[0].id in self.metadata_attrs + ) else node + + def visit_With(self, node): + self.generic_visit(node) # visit children + return node if node.body else None # remove with statement if it has no body + + def visit_For(self, node): + self.generic_visit(node) # visit children + return node if node.body else None # remove loop if it has no body + + def visit_While(self, node): + self.generic_visit(node) # visit children + return node if node.body else None # remove loop if it has no body + + def visit_If(self, node): + self.generic_visit(node) + + # an empty orelse is ignored by unparsing, but an empty body with a full orelse + # ends up unparsing as a syntax error, so we replace the empty body into `pass`. + if not node.body: + if node.orelse: + node.body = [ast.Pass()] + else: + return None + + # if the node has a body, it's valid python code with or without an orelse + return node + + def visit_FunctionDef(self, node): + # do not descend into function definitions + return node + + def visit_ClassDef(self, node): + # packages are always top-level, and we do not descend + # into nested class defs and their attributes + if self.in_classdef: + return node + + # guard against recrusive class definitions + self.in_classdef = True + self.generic_visit(node) + self.in_classdef = False + + # replace class definition with `pass` if it's empty (e.g., packages that only + # have directives b/c they subclass a build system class) + if not node.body: + node.body = [ast.Pass()] - def is_directive(self, node): - """Check to determine if the node is a valid directive - - Directives are assumed to be represented in the AST as a named function - call expression. This means that they will NOT be represented by a - named function call within a function call expression (e.g., as - callbacks are sometimes represented). - - Args: - node (ast.AST): the AST node being checked - - Returns: - bool: ``True`` if the node represents a known directive, - ``False`` otherwise - """ - return (isinstance(node, ast.Expr) and - node.value and isinstance(node.value, ast.Call) and - isinstance(node.value.func, ast.Name) and - node.value.func.id in spack.directives.directive_names) - - def is_spack_attr(self, node): - return (isinstance(node, ast.Assign) and - node.targets and isinstance(node.targets[0], ast.Name) and - node.targets[0].id in spack.package.Package.metadata_attrs) - - def visit_ClassDef(self, node): # noqa - if node.name == spack.util.naming.mod_to_class(self.spec.name): - node.body = [ - c for c in node.body - if (not self.is_directive(c) and not self.is_spack_attr(c))] return node @@ -89,7 +144,7 @@ class TagMultiMethods(ast.NodeVisitor): # map from function name to (implementation, condition_list) tuples self.methods = {} - def visit_FunctionDef(self, func): # noqa + def visit_FunctionDef(self, func): conditions = [] for dec in func.decorator_list: if isinstance(dec, ast.Call) and dec.func.id == 'when': @@ -207,7 +262,7 @@ class ResolveMultiMethods(ast.NodeTransformer): # if nothing was picked, the last definition wins. return result - def visit_FunctionDef(self, func): # noqa + def visit_FunctionDef(self, func): # if the function def wasn't visited on the first traversal there is a problem assert func.name in self.methods, "Inconsistent package traversal!" @@ -277,7 +332,7 @@ def package_ast(spec, filter_multimethods=True, source=None): # create an AST root = ast.parse(source) - # remove docstrings and directives from the package AST + # remove docstrings, comments, and directives from the package AST root = RemoveDocstrings().visit(root) root = RemoveDirectives(spec).visit(root) -- cgit v1.2.3-70-g09d2