From f96def28cb8b1263670dcf7294e948ba880cf64a Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Thu, 15 May 2025 18:16:23 +0200 Subject: [PATCH] spack repo migrate: add missing imports (#50491) --- lib/spack/spack/cmd/repo.py | 210 +++++++++++++++++++++++++++++- lib/spack/spack/test/cmd/repo.py | 63 +++++++++ share/spack/spack-completion.bash | 11 +- share/spack/spack-completion.fish | 7 + 4 files changed, 289 insertions(+), 2 deletions(-) diff --git a/lib/spack/spack/cmd/repo.py b/lib/spack/spack/cmd/repo.py index 0f14190c029..934b3a74443 100644 --- a/lib/spack/spack/cmd/repo.py +++ b/lib/spack/spack/cmd/repo.py @@ -2,12 +2,14 @@ # # SPDX-License-Identifier: (Apache-2.0 OR MIT) +import ast import os import sys -from typing import List +from typing import Any, Dict, List, Optional, Set import llnl.util.tty as tty +import spack import spack.config import spack.repo import spack.util.path @@ -65,6 +67,12 @@ def setup_parser(subparser): help="configuration scope to modify", ) + # Migrate + migrate_parser = sp.add_parser("migrate", help=repo_migrate.__doc__) + migrate_parser.add_argument( + "namespace_or_path", help="path to a Spack package repository directory" + ) + def repo_create(args): """create a new package repository""" @@ -155,6 +163,205 @@ def repo_list(args): print(f"{repo.namespace:<{max_ns_len + 4}}{repo.package_api_str:<8}{repo.root}") +def _get_repo(name_or_path: str) -> Optional[spack.repo.Repo]: + try: + return spack.repo.from_path(name_or_path) + except spack.repo.RepoError: + pass + + for repo in spack.config.get("repos"): + try: + r = spack.repo.from_path(spack.util.path.canonicalize_path(repo)) + except spack.repo.RepoError: + continue + if r.namespace == name_or_path or os.path.samefile(r.root, name_or_path): + return r + + return None + + +def repo_migrate(args: Any) -> None: + """migrate a package repository to the latest Package API""" + repo = _get_repo(args.namespace_or_path) + + if repo is None: + tty.die(f"No such repository: {args.namespace_or_path}") + + if repo.package_api < (2, 0): + tty.die("Migration from Spack repo API < 2.0 is not supported yet") + + symbol_to_module = { + "AspellDictPackage": "spack.build_systems.aspell_dict", + "AutotoolsPackage": "spack.build_systems.autotools", + "BundlePackage": "spack.build_systems.bundle", + "CachedCMakePackage": "spack.build_systems.cached_cmake", + "cmake_cache_filepath": "spack.build_systems.cached_cmake", + "cmake_cache_option": "spack.build_systems.cached_cmake", + "cmake_cache_path": "spack.build_systems.cached_cmake", + "cmake_cache_string": "spack.build_systems.cached_cmake", + "CargoPackage": "spack.build_systems.cargo", + "CMakePackage": "spack.build_systems.cmake", + "generator": "spack.build_systems.cmake", + "CompilerPackage": "spack.build_systems.compiler", + "CudaPackage": "spack.build_systems.cuda", + "Package": "spack.build_systems.generic", + "GNUMirrorPackage": "spack.build_systems.gnu", + "GoPackage": "spack.build_systems.go", + "IntelPackage": "spack.build_systems.intel", + "LuaPackage": "spack.build_systems.lua", + "MakefilePackage": "spack.build_systems.makefile", + "MavenPackage": "spack.build_systems.maven", + "MesonPackage": "spack.build_systems.meson", + "MSBuildPackage": "spack.build_systems.msbuild", + "NMakePackage": "spack.build_systems.nmake", + "OctavePackage": "spack.build_systems.octave", + "INTEL_MATH_LIBRARIES": "spack.build_systems.oneapi", + "IntelOneApiLibraryPackage": "spack.build_systems.oneapi", + "IntelOneApiLibraryPackageWithSdk": "spack.build_systems.oneapi", + "IntelOneApiPackage": "spack.build_systems.oneapi", + "IntelOneApiStaticLibraryList": "spack.build_systems.oneapi", + "PerlPackage": "spack.build_systems.perl", + "PythonExtension": "spack.build_systems.python", + "PythonPackage": "spack.build_systems.python", + "QMakePackage": "spack.build_systems.qmake", + "RPackage": "spack.build_systems.r", + "RacketPackage": "spack.build_systems.racket", + "ROCmPackage": "spack.build_systems.rocm", + "RubyPackage": "spack.build_systems.ruby", + "SConsPackage": "spack.build_systems.scons", + "SIPPackage": "spack.build_systems.sip", + "SourceforgePackage": "spack.build_systems.sourceforge", + "SourcewarePackage": "spack.build_systems.sourceware", + "WafPackage": "spack.build_systems.waf", + "XorgPackage": "spack.build_systems.xorg", + } + + for f in os.scandir(repo.packages_path): + pkg_path = os.path.join(f.path, "package.py") + try: + if f.name in ("__init__.py", "__pycache__") or not f.is_dir(): + continue + with open(pkg_path, "rb") as file: + tree = ast.parse(file.read()) + except (OSError, SyntaxError) as e: + print(f"Skipping {pkg_path}: {e}", file=sys.stderr) + continue + + #: Symbols that are referenced in the package and may need to be imported. + referenced_symbols: Set[str] = set() + + #: Set of symbols of interest that are already defined through imports, assignments, or + #: function definitions. + defined_symbols: Set[str] = set() + + best_line: Optional[int] = None + + seen_import = False + + for node in ast.walk(tree): + # Get the last import statement from the first block of top-level imports + if isinstance(node, ast.Module): + for child in ast.iter_child_nodes(node): + # if we never encounter an import statement, the best line to add is right + # before the first node under the module + if best_line is None and isinstance(child, ast.stmt): + best_line = child.lineno + + # prefer adding right before `from spack.package import ...` + if isinstance(child, ast.ImportFrom) and child.module == "spack.package": + seen_import = True + best_line = child.lineno # add it right before spack.package + break + + # otherwise put it right after the last import statement + is_import = isinstance(child, (ast.Import, ast.ImportFrom)) + + if is_import: + if isinstance(child, (ast.stmt, ast.expr)): + best_line = (child.end_lineno or child.lineno) + 1 + + if not seen_import and is_import: + seen_import = True + elif seen_import and not is_import: + break + + # Function definitions or assignments to variables whose name is a symbol of interest + # are considered as redefinitions, so we skip them. + elif isinstance(node, ast.FunctionDef): + if node.name in symbol_to_module: + print( + f"{pkg_path}:{node.lineno}: redefinition of `{node.name}` skipped", + file=sys.stderr, + ) + defined_symbols.add(node.name) + elif isinstance(node, ast.Assign): + for target in node.targets: + if isinstance(target, ast.Name) and target.id in symbol_to_module: + print( + f"{pkg_path}:{target.lineno}: redefinition of `{target.id}` skipped", + file=sys.stderr, + ) + defined_symbols.add(target.id) + + # Register symbols that are not imported. + elif isinstance(node, ast.Name) and node.id in symbol_to_module: + referenced_symbols.add(node.id) + + # Register imported symbols to make this operation idempotent + elif isinstance(node, ast.ImportFrom): + for alias in node.names: + if alias.name in symbol_to_module: + defined_symbols.add(alias.name) + if node.module == "spack.package": + print( + f"{pkg_path}:{node.lineno}: `{alias.name}` is imported from " + "`spack.package`, which no longer provides this symbol", + file=sys.stderr, + ) + + if alias.asname and alias.asname in symbol_to_module: + defined_symbols.add(alias.asname) + + # Remove imported symbols from the referenced symbols + referenced_symbols.difference_update(defined_symbols) + + if not referenced_symbols: + continue + + if best_line is None: + print(f"{pkg_path}: failed to update imports", file=sys.stderr) + continue + + # Add the missing imports right after the last import statement + with open(pkg_path, "r", encoding="utf-8") as file: + lines = file.readlines() + + # Group missing symbols by their module + missing_imports_by_module: Dict[str, list] = {} + for symbol in referenced_symbols: + module = symbol_to_module[symbol] + if module not in missing_imports_by_module: + missing_imports_by_module[module] = [] + missing_imports_by_module[module].append(symbol) + + new_lines = [ + f"from {module} import {', '.join(sorted(symbols))}\n" + for module, symbols in sorted(missing_imports_by_module.items()) + ] + + if not seen_import: + new_lines.extend(("\n", "\n")) + + lines[best_line - 1 : best_line - 1] = new_lines + + tmp_file = pkg_path + ".tmp" + + with open(tmp_file, "w", encoding="utf-8") as file: + file.writelines(lines) + + os.replace(tmp_file, pkg_path) + + def repo(parser, args): action = { "create": repo_create, @@ -162,5 +369,6 @@ def repo(parser, args): "add": repo_add, "remove": repo_remove, "rm": repo_remove, + "migrate": repo_migrate, } action[args.repo_command](args) diff --git a/lib/spack/spack/test/cmd/repo.py b/lib/spack/spack/test/cmd/repo.py index 56abb8d668d..218c94ccb0d 100644 --- a/lib/spack/spack/test/cmd/repo.py +++ b/lib/spack/spack/test/cmd/repo.py @@ -9,6 +9,7 @@ import spack.config import spack.environment as ev import spack.main +import spack.repo from spack.main import SpackCommand repo = spack.main.SpackCommand("repo") @@ -68,3 +69,65 @@ def test_env_repo_path_vars_substitution( with ev.read("test") as newenv: repos_specs = spack.config.get("repos", default={}, scope=newenv.scope_name) assert current_dir in repos_specs + + +def test_repo_migrate(tmp_path: pathlib.Path, config): + path, _ = spack.repo.create_repo(str(tmp_path), "mockrepo", package_api=(2, 0)) + pkgs_path = spack.repo.from_path(path).packages_path + + pkg1 = pathlib.Path(os.path.join(pkgs_path, "foo", "package.py")) + pkg2 = pathlib.Path(os.path.join(pkgs_path, "bar", "package.py")) + + pkg1.parent.mkdir(parents=True) + pkg2.parent.mkdir(parents=True) + + pkg1.write_text( + """\ +# some comment + +from spack.package import * + +class Foo(Package): + pass +""", + encoding="utf-8", + ) + pkg2.write_text( + """\ +# some comment + +from spack.package import * + +class Bar(CMakePackage): + generator("ninja") +""", + encoding="utf-8", + ) + + repo("migrate", path) + + assert ( + pkg1.read_text(encoding="utf-8") + == """\ +# some comment + +from spack.build_systems.generic import Package +from spack.package import * + +class Foo(Package): + pass +""" + ) + + assert ( + pkg2.read_text(encoding="utf-8") + == """\ +# some comment + +from spack.build_systems.cmake import CMakePackage, generator +from spack.package import * + +class Bar(CMakePackage): + generator("ninja") +""" + ) diff --git a/share/spack/spack-completion.bash b/share/spack/spack-completion.bash index ef2328868b2..876afdf687d 100644 --- a/share/spack/spack-completion.bash +++ b/share/spack/spack-completion.bash @@ -1784,7 +1784,7 @@ _spack_repo() { then SPACK_COMPREPLY="-h --help" else - SPACK_COMPREPLY="create list add remove rm" + SPACK_COMPREPLY="create list add remove rm migrate" fi } @@ -1828,6 +1828,15 @@ _spack_repo_rm() { fi } +_spack_repo_migrate() { + if $list_options + then + SPACK_COMPREPLY="-h --help" + else + _repos + fi +} + _spack_resource() { if $list_options then diff --git a/share/spack/spack-completion.fish b/share/spack/spack-completion.fish index c38356a5bdb..03e04797b44 100644 --- a/share/spack/spack-completion.fish +++ b/share/spack/spack-completion.fish @@ -2749,6 +2749,7 @@ complete -c spack -n '__fish_spack_using_command_pos 0 repo' -f -a list -d 'show complete -c spack -n '__fish_spack_using_command_pos 0 repo' -f -a add -d 'add a package source to Spack'"'"'s configuration' complete -c spack -n '__fish_spack_using_command_pos 0 repo' -f -a remove -d 'remove a repository from Spack'"'"'s configuration' complete -c spack -n '__fish_spack_using_command_pos 0 repo' -f -a rm -d 'remove a repository from Spack'"'"'s configuration' +complete -c spack -n '__fish_spack_using_command_pos 0 repo' -f -a migrate -d 'migrate a package repository to the latest Package API' complete -c spack -n '__fish_spack_using_command repo' -s h -l help -f -a help complete -c spack -n '__fish_spack_using_command repo' -s h -l help -d 'show this help message and exit' @@ -2791,6 +2792,12 @@ complete -c spack -n '__fish_spack_using_command repo rm' -s h -l help -d 'show complete -c spack -n '__fish_spack_using_command repo rm' -l scope -r -f -a '_builtin defaults system site user command_line' complete -c spack -n '__fish_spack_using_command repo rm' -l scope -r -d 'configuration scope to modify' +# spack repo migrate +set -g __fish_spack_optspecs_spack_repo_migrate h/help +complete -c spack -n '__fish_spack_using_command_pos 0 repo migrate' $__fish_spack_force_files -a '(__fish_spack_repos)' +complete -c spack -n '__fish_spack_using_command repo migrate' -s h -l help -f -a help +complete -c spack -n '__fish_spack_using_command repo migrate' -s h -l help -d 'show this help message and exit' + # spack resource set -g __fish_spack_optspecs_spack_resource h/help complete -c spack -n '__fish_spack_using_command_pos 0 resource' -f -a list -d 'list all resources known to spack (currently just patches)'