package api: drop wildcard re-export (#48760)

* package api: drop wildcard re-export

To ensure package repos are forward/backward compatibility with Spack,
we should explicitly export all symbols we want to expose in the public
package API, and drop `from spack.something import *` because
removal/addition to the public API will go unnoticed.

Also `llnl.util.filesystem` has some methods that shouldn't be exposed
in the package API, so better to enumerate a subset explicitly.

* remove flatten_dependencies / install_dependency_symlinks

lib/spack/spack/package.py

@@ -3,29 +3,52 @@
 # SPDX-License-Identifier: (Apache-2.0 OR MIT)
 
 # flake8: noqa: F401
-"""spack.util.package is a set of useful build tools and directives for packages.
+"""spack.package defines the public API for Spack packages, by re-exporting useful symbols from
+other modules. Packages should import this module, instead of importing from spack.* directly
+to ensure forward compatibility with future versions of Spack."""
 
-Everything in this module is automatically imported into Spack package files.
-"""
 from os import chdir, environ, getcwd, makedirs, mkdir, remove, removedirs
 from shutil import move, rmtree
 
-from spack.error import InstallError, NoHeadersError, NoLibrariesError
-
-# Emulate some shell commands for convenience
-env = environ
-cd = chdir
-pwd = getcwd
-
 # import most common types used in packages
 from typing import Dict, List, Optional
 
-import llnl.util.filesystem
-from llnl.util.filesystem import *
+from llnl.util.filesystem import (
+    FileFilter,
+    FileList,
+    HeaderList,
+    LibraryList,
+    ancestor,
+    can_access,
+    change_sed_delimiter,
+    copy,
+    copy_tree,
+    filter_file,
+    find,
+    find_all_headers,
+    find_first,
+    find_headers,
+    find_libraries,
+    find_system_libraries,
+    force_remove,
+    force_symlink,
+    install,
+    install_tree,
+    is_exe,
+    join_path,
+    keep_modification_time,
+    library_extensions,
+    mkdirp,
+    remove_directory_contents,
+    remove_linked_tree,
+    rename,
+    set_executable,
+    set_install_permissions,
+    touch,
+    working_dir,
+)
 from llnl.util.symlink import symlink
 
-import spack.util.executable
-
 # These props will be overridden when the build env is set up.
 from spack.build_environment import MakeExecutable
 from spack.build_systems.aspell_dict import AspellDictPackage
@@ -76,7 +99,24 @@
 from spack.builder import BaseBuilder
 from spack.config import determine_number_of_jobs
 from spack.deptypes import ALL_TYPES as all_deptypes
-from spack.directives import *
+from spack.directives import (
+    build_system,
+    can_splice,
+    conditional,
+    conflicts,
+    depends_on,
+    extends,
+    license,
+    maintainers,
+    patch,
+    provides,
+    redistribute,
+    requires,
+    resource,
+    variant,
+    version,
+)
+from spack.error import InstallError, NoHeadersError, NoLibrariesError
 from spack.install_test import (
     SkipTest,
     cache_extra_test_sources,
@@ -86,26 +126,26 @@
     install_test_root,
     test_part,
 )
-from spack.installer import ExternalPackageError, InstallLockError, UpstreamPackageError
 from spack.mixins import filter_compiler_wrappers
 from spack.multimethod import default_args, when
-from spack.package_base import (
-    DependencyConflictError,
-    build_system_flags,
-    env_flags,
-    flatten_dependencies,
-    inject_flags,
-    install_dependency_symlinks,
-    on_package_attributes,
+from spack.package_base import build_system_flags, env_flags, inject_flags, on_package_attributes
+from spack.package_completions import (
+    bash_completion_path,
+    fish_completion_path,
+    zsh_completion_path,
 )
-from spack.package_completions import *
 from spack.phase_callbacks import run_after, run_before
-from spack.spec import InvalidSpecDetected, Spec
-from spack.util.executable import *
+from spack.spec import Spec
+from spack.util.executable import Executable, ProcessError, which, which_string
 from spack.util.filesystem import fix_darwin_install_name
 from spack.variant import any_combination_of, auto_or_any_combination_of, disjoint_sets
 from spack.version import Version, ver
 
+# Emulate some shell commands for convenience
+env = environ
+cd = chdir
+pwd = getcwd
+
 # These are just here for editor support; they may be set when the build env is set up.
 configure: Executable
 make_jobs: int

lib/spack/spack/package_base.py

@@ -30,7 +30,6 @@
 import llnl.util.filesystem as fsys
 import llnl.util.tty as tty
 from llnl.util.lang import classproperty, memoized
-from llnl.util.link_tree import LinkTree
 
 import spack.compilers
 import spack.config
@@ -2292,19 +2291,6 @@ def rpath_args(self):
 build_system_flags = PackageBase.build_system_flags
 
 
-def install_dependency_symlinks(pkg, spec, prefix):
-    """
-    Execute a dummy install and flatten dependencies.
-
-    This routine can be used in a ``package.py`` definition by setting
-    ``install = install_dependency_symlinks``.
-
-    This feature comes in handy for creating a common location for the
-    the installation of third-party libraries.
-    """
-    flatten_dependencies(spec, prefix)
-
-
 def use_cray_compiler_names():
     """Compiler names for builds that rely on cray compiler names."""
     os.environ["CC"] = "cc"
@@ -2313,23 +2299,6 @@ def use_cray_compiler_names():
     os.environ["F77"] = "ftn"
 
 
-def flatten_dependencies(spec, flat_dir):
-    """Make each dependency of spec present in dir via symlink."""
-    for dep in spec.traverse(root=False):
-        name = dep.name
-
-        dep_path = spack.store.STORE.layout.path_for_spec(dep)
-        dep_files = LinkTree(dep_path)
-
-        os.mkdir(flat_dir + "/" + name)
-
-        conflict = dep_files.find_conflict(flat_dir + "/" + name)
-        if conflict:
-            raise DependencyConflictError(conflict)
-
-        dep_files.merge(flat_dir + "/" + name)
-
-
 def possible_dependencies(
     *pkg_or_spec: Union[str, spack.spec.Spec, typing.Type[PackageBase]],
     transitive: bool = True,

lib/spack/spack/test/cmd/ci.py

@@ -329,14 +329,14 @@ def test_ci_generate_pkg_with_deps(ci_generate_test, tmp_path, ci_base_environme
         f"""\
 spack:
   specs:
-    - flatten-deps
+    - dependent-install
   mirrors:
     buildcache-destination: {tmp_path / 'ci-mirror'}
   ci:
     pipeline-gen:
     - submapping:
       - match:
-          - flatten-deps
+          - dependent-install
         build-job:
           tags:
             - donotcare
@@ -355,12 +355,12 @@ def test_ci_generate_pkg_with_deps(ci_generate_test, tmp_path, ci_base_environme
             assert "stage" in ci_obj
             assert ci_obj["stage"] == "stage-0"
             found.append("dependency-install")
-        if "flatten-deps" in ci_key:
+        if "dependent-install" in ci_key:
             assert "stage" in ci_obj
             assert ci_obj["stage"] == "stage-1"
-            found.append("flatten-deps")
+            found.append("dependent-install")
 
-    assert "flatten-deps" in found
+    assert "dependent-install" in found
     assert "dependency-install" in found
 
 
@@ -372,14 +372,14 @@ def test_ci_generate_for_pr_pipeline(ci_generate_test, tmp_path, monkeypatch):
         f"""\
 spack:
   specs:
-    - flatten-deps
+    - dependent-install
   mirrors:
     buildcache-destination: {tmp_path / 'ci-mirror'}
   ci:
     pipeline-gen:
     - submapping:
       - match:
-          - flatten-deps
+          - dependent-install
         build-job:
           tags:
             - donotcare
@@ -899,7 +899,7 @@ def test_ci_generate_override_runner_attrs(
         f"""\
 spack:
   specs:
-    - flatten-deps
+    - dependent-install
     - pkg-a
   mirrors:
     buildcache-destination: {tmp_path / "ci-mirror"}
@@ -908,7 +908,7 @@ def test_ci_generate_override_runner_attrs(
     - match_behavior: {match_behavior}
       submapping:
         - match:
-            - flatten-deps
+            - dependent-install
           build-job:
             tags:
               - specific-one
@@ -1006,8 +1006,8 @@ def test_ci_generate_override_runner_attrs(
             assert the_elt["script"][0] == "main step"
             assert len(the_elt["after_script"]) == 1
             assert the_elt["after_script"][0] == "post step one"
-        if "flatten-deps" in ci_key:
-            # The flatten-deps match specifies that we keep the two
+        if "dependent-install" in ci_key:
+            # The dependent-install match specifies that we keep the two
             # top level variables, but add a third specifc one.  It
             # also adds a custom tag which should be combined with
             # the top-level tag.
@@ -1182,12 +1182,12 @@ def test_ci_generate_read_broken_specs_url(
     spec_a = spack.concretize.concretize_one("pkg-a")
     a_dag_hash = spec_a.dag_hash()
 
-    spec_flattendeps = spack.concretize.concretize_one("flatten-deps")
+    spec_flattendeps = spack.concretize.concretize_one("dependent-install")
     flattendeps_dag_hash = spec_flattendeps.dag_hash()
 
     broken_specs_url = tmp_path.as_uri()
 
-    # Mark 'a' as broken (but not 'flatten-deps')
+    # Mark 'a' as broken (but not 'dependent-install')
     broken_spec_a_url = "{0}/{1}".format(broken_specs_url, a_dag_hash)
     job_stack = "job_stack"
     a_job_url = "a_job_url"
@@ -1201,7 +1201,7 @@ def test_ci_generate_read_broken_specs_url(
             f"""\
 spack:
   specs:
-    - flatten-deps
+    - dependent-install
     - pkg-a
   mirrors:
     buildcache-destination: {(tmp_path / "ci-mirror").as_uri()}
@@ -1211,7 +1211,7 @@ def test_ci_generate_read_broken_specs_url(
     - submapping:
       - match:
           - pkg-a
-          - flatten-deps
+          - dependent-install
           - pkg-b
           - dependency-install
         build-job:
@@ -1234,7 +1234,7 @@ def test_ci_generate_read_broken_specs_url(
             )
             assert expected in output
 
-            not_expected = f"flatten-deps/{flattendeps_dag_hash[:7]} (in stack"
+            not_expected = f"dependent-install/{flattendeps_dag_hash[:7]} (in stack"
             assert not_expected not in output
 
 
@@ -1447,7 +1447,7 @@ def test_gitlab_config_scopes(ci_generate_test, tmp_path):
   include: [{configs_path}]
   view: false
   specs:
-    - flatten-deps
+    - dependent-install
   mirrors:
     buildcache-destination: {tmp_path / "ci-mirror"}
   ci:

lib/spack/spack/test/install.py

@@ -198,17 +198,6 @@ def test_installed_dependency_request_conflicts(install_mockery, mock_fetch, mut
         spack.concretize.concretize_one(dependent)
 
 
-def test_install_dependency_symlinks_pkg(install_mockery, mock_fetch, mutable_mock_repo):
-    """Test dependency flattening/symlinks mock package."""
-    spec = spack.concretize.concretize_one("flatten-deps")
-    pkg = spec.package
-    PackageInstaller([pkg], explicit=True).install()
-
-    # Ensure dependency directory exists after the installation.
-    dependency_dir = os.path.join(pkg.prefix, "dependency-install")
-    assert os.path.isdir(dependency_dir)
-
-
 def test_install_times(install_mockery, mock_fetch, mutable_mock_repo):
     """Test install times added."""
     spec = spack.concretize.concretize_one("dev-build-test-install-phases")
@@ -228,26 +217,6 @@ def test_install_times(install_mockery, mock_fetch, mutable_mock_repo):
     assert all(isinstance(x["seconds"], float) for x in times["phases"])
 
 
-def test_flatten_deps(install_mockery, mock_fetch, mutable_mock_repo):
-    """Explicitly test the flattening code for coverage purposes."""
-    # Unfortunately, executing the 'flatten-deps' spec's installation does
-    # not affect code coverage results, so be explicit here.
-    spec = spack.concretize.concretize_one("dependent-install")
-    pkg = spec.package
-    PackageInstaller([pkg], explicit=True).install()
-
-    # Demonstrate that the directory does not appear under the spec
-    # prior to the flatten operation.
-    dependency_name = "dependency-install"
-    assert dependency_name not in os.listdir(pkg.prefix)
-
-    # Flatten the dependencies and ensure the dependency directory is there.
-    spack.package_base.flatten_dependencies(spec, pkg.prefix)
-
-    dependency_dir = os.path.join(pkg.prefix, dependency_name)
-    assert os.path.isdir(dependency_dir)
-
-
 @pytest.fixture()
 def install_upstream(tmpdir_factory, gen_mock_layout, install_mockery):
     """Provides a function that installs a specified set of specs to an

var/spack/repos/builtin.mock/packages/flatten-deps/package.py

@@ -1,18 +0,0 @@
-# Copyright Spack Project Developers. See COPYRIGHT file for details.
-#
-# SPDX-License-Identifier: (Apache-2.0 OR MIT)
-
-from spack.package import *
-
-
-class FlattenDeps(Package):
-    """Example install that flattens dependencies."""
-
-    homepage = "http://www.example.com"
-    url = "http://www.example.com/a-1.0.tar.gz"
-
-    version("1.0", md5="0123456789abcdef0123456789abcdef")
-
-    depends_on("dependency-install")
-
-    install = install_dependency_symlinks