Minor improvements to reccmp project create (#48)

* Minor improvements to reccmp project create

* Require subcommand for argparse subparser

reccmp/project/create.py

@@ -1,5 +1,4 @@
 import enum
-import itertools
 import logging
 from pathlib import Path
 import shutil
@@ -19,12 +18,17 @@
 from .common import RECCMP_PROJECT_CONFIG, RECCMP_USER_CONFIG, RECCMP_BUILD_CONFIG
 from .detect import RecCmpProject, RecCmpTarget
 from .error import RecCmpProjectException
-from .util import get_path_sha256, path_to_id
+from .util import get_path_sha256, unique_targets
 
 
 logger = logging.getLogger(__name__)
 
 
+class RecCmpProjectAlreadyExistsError(RecCmpProjectException):
+    def __init__(self, *_, path: Path | str | None = None, **__):
+        super().__init__(f"Cannot overwrite existing project {path or ''}")
+
+
 class TargetType(enum.Enum):
     SHARED_LIBRARY = "SHARED_LIBRARY"
     EXECUTABLE = "EXECUTABLE"
@@ -41,6 +45,7 @@ def executable_or_library(path: Path) -> TargetType:
 
 
 def get_default_cmakelists_txt(project_name: str, targets: dict[str, Path]) -> str:
+    """Generate template CMakeLists.txt file contents to build each target."""
     result = textwrap.dedent(
         f"""\
         cmake_minimum_required(VERSION 3.20)
@@ -89,6 +94,7 @@ def get_default_cmakelists_txt(project_name: str, targets: dict[str, Path]) -> s
 
 
 def get_default_main_hpp(target_id: str) -> str:
+    """Generate template C++ header for the given target."""
     return textwrap.dedent(
         f"""\
         #ifndef {target_id.upper()}_HPP
@@ -107,6 +113,8 @@ class SomeClass {{
 
 
 def get_default_main_cpp(target_id: str, original_path: Path, hpp_path: Path) -> str:
+    """Generate a template C++ source file for the given target, depending on
+    whether its file path is DLL or EXE. Includes sample reccmp annotations."""
     target_type = executable_or_library(original_path)
     match target_type:
         case TargetType.EXECUTABLE:
@@ -160,66 +168,88 @@ def get_default_main_cpp(target_id: str, original_path: Path, hpp_path: Path) ->
 
 
 def create_project(
-    project_directory: Path, original_paths: list[Path], scm: bool, cmake: bool
+    project_directory: Path,
+    original_paths: list[Path],
+    scm: bool = False,
+    cmake: bool = False,
 ) -> RecCmpProject:
+    """Generates project.yml and user.yml files in the given project directory.
+    Requires a list of paths to original binaries that will be the focus of the decomp project.
+    If `scm` is enabled, update an existing .gitignore to skip user.yml and build.yml files.
+    If `cmake` is enabled, create CMakeLists.txt and generate sample source files to help get started.
+    """
+
+    # Intended reccmp-project.yml location
+    project_config_path = project_directory / RECCMP_PROJECT_CONFIG
+
+    # Don't overwrite an existing project
+    if project_config_path.exists():
+        raise RecCmpProjectAlreadyExistsError(path=project_config_path)
+
     if not original_paths:
         raise RecCmpProjectException("Need at least one original binary")
-    id_path: dict[str, Path] = {}
-    project_config_data = ProjectFile(targets={})
-    project_config_path = project_directory / RECCMP_PROJECT_CONFIG
-    user_config_path = project_directory / RECCMP_USER_CONFIG
-    project = RecCmpProject(project_config_path=project_config_path)
+
     for original_path in original_paths:
         if not original_path.is_file():
-            raise RecCmpProjectException(
-                f"Original binary ({original_path}) is not a file"
-            )
-        target_id = path_to_id(original_path)
+            raise FileNotFoundError(f"Original binary ({original_path}) is not a file")
+
+    # reccmp-user.yml location
+    user_config_path = project_directory / RECCMP_USER_CONFIG
+
+    # Use the base name for each original binary to create a unique ID.
+    # If any base names are non-unique, add a number.
+    targets = dict(unique_targets(original_paths))
+
+    # Object to serialize to YAML
+    project_config_data = ProjectFile(targets={})
+
+    # Return object for user
+    project = RecCmpProject(project_config_path=project_config_path)
+
+    # Populate targets for each project object
+    for target_id, original_path in targets.items():
+        # Calculate SHA256 checksum of the original binary. reccmp will verify this
+        # at startup to make sure each contributor is working with the same file.
         hash_sha256 = get_path_sha256(original_path)
+
+        # The project file uses the base filename only. The path to the binary file
+        # is in the user file because it is different for each contributor.
         target_filename = original_path.name
-        target_data = ProjectFileTarget(
+
+        project_config_data.targets[target_id] = ProjectFileTarget(
             filename=target_filename,
             source_root=project_directory,
             hash=Hash(sha256=hash_sha256),
         )
-        if target_id in project_config_data.targets:
-            for suffix_nb in itertools.count(start=0, step=1):
-                new_target_id = f"{target_id}_{suffix_nb}"
-                if new_target_id not in project_config_data.targets:
-                    target_id = new_target_id
-                    break
-        project_config_data.targets[target_id] = target_data
-        id_path[target_id] = original_path
+
         project.targets[target_id] = RecCmpTarget(
             target_id=target_id,
             filename=target_filename,
             source_root=project_directory,
             ghidra_config=GhidraConfig.default(),
         )
 
-    if project_config_path.exists():
-        raise RecCmpProjectException(
-            f"Failed to create a new reccmp project: there already exists one: {project_config_path}"
-        )
-
-    project_name = path_to_id(original_paths[0])
+    # Write project YAML file
     logger.debug("Creating %s...", project_config_path)
     with project_config_path.open("w") as f:
         yaml = ruamel.yaml.YAML()
         yaml.dump(data=project_config_data.model_dump(mode="json"), stream=f)
 
+    # The user YAML file has the path to the original binary for each target
     user_config_data = UserFile(
         targets={
-            uid: UserFileTarget(path=path.resolve()) for uid, path in id_path.items()
+            uid: UserFileTarget(path=path.resolve()) for uid, path in targets.items()
         }
     )
 
+    # Write user YAML file
     logger.debug("Creating %s...", user_config_data)
     with user_config_path.open("w") as f:
         yaml = ruamel.yaml.YAML()
         yaml.dump(data=user_config_data.model_dump(mode="json"), stream=f)
 
     if scm:
+        # Update existing .gitignore to skip build.yml and user.yml.
         gitignore_path = project_directory / ".gitignore"
         if gitignore_path.exists():
             ignore_rules = gitignore_path.read_text().splitlines()
@@ -233,23 +263,31 @@ def create_project(
                     f.write(f"{RECCMP_BUILD_CONFIG}\n")
 
     if cmake:
+        # Generate tempalte files so you can start building each target with CMake.
         project_cmake_dir = project_directory / "cmake"
         project_cmake_dir.mkdir(exist_ok=True)
-        logger.debug("Copying %s...", "cmake/reccmp.py")
+
+        # Copy template CMake script that generates build.yml
+        logger.debug("Copying %s...", "cmake/reccmp.cmake")
         shutil.copy(
             get_asset_file("cmake/reccmp.cmake"),
             project_directory / "cmake/reccmp.cmake",
         )
 
+        # Use first target ID as cmake project name
+        project_name = next(iter(targets.keys()), "NEW_DECOMP_PROJECT")
         cmakelists_txt = get_default_cmakelists_txt(
-            project_name=project_name, targets=id_path
+            project_name=project_name, targets=targets
         )
+
+        # Create CMakeLists.txt
         cmakelists_path = project_directory / "CMakeLists.txt"
         logger.debug("Creating %s...", cmakelists_path)
         with cmakelists_path.open("w") as f:
             f.write(cmakelists_txt)
 
-        for target_id, original_path in id_path.items():
+        # Create template C++ source and header file for each target.
+        for target_id, original_path in targets.items():
             main_cpp_path = project_directory / f"main_{target_id}.cpp"
             main_hpp_path = project_directory / f"main_{target_id}.hpp"
             main_cpp = get_default_main_cpp(
@@ -263,4 +301,5 @@ def create_project(
             logger.debug("Creating %s...", main_hpp_path)
             with main_hpp_path.open("w") as f:
                 f.write(main_hpp)
+
     return project

reccmp/project/util.py

@@ -1,6 +1,8 @@
 import hashlib
 from pathlib import Path
 import re
+import itertools
+from typing import Iterable, Iterator
 
 
 def get_path_sha256(p: Path) -> str:
@@ -11,3 +13,20 @@ def get_path_sha256(p: Path) -> str:
 
 def path_to_id(path: Path) -> str:
     return re.sub("[^0-9a-zA-Z_]", "", path.stem.upper())
+
+
+def unique_targets(paths: Iterable[Path]) -> Iterator[tuple[str, Path]]:
+    """Create a unique ID for each path, starting with the base filename."""
+    seen_targets = set()
+    for path in paths:
+        target = path_to_id(path)
+        if target in seen_targets:
+            for new_target in (
+                f"{target}_{suffix}" for suffix in itertools.count(start=0, step=1)
+            ):
+                if new_target not in seen_targets:
+                    target = new_target
+                    break
+
+        seen_targets.add(target)
+        yield (target, path)

reccmp/tools/project.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 import argparse
 import logging
+import enum
 from pathlib import Path
 
 import reccmp
@@ -17,11 +18,16 @@
 logger = logging.getLogger(__name__)
 
 
+class ProjectSubcommand(enum.Enum):
+    CREATE = enum.auto()
+    DETECT = enum.auto()
+
+
 def main():
     parser = argparse.ArgumentParser(
         description="Project management", allow_abbrev=False
     )
-    parser.set_defaults(action=None)
+    parser.set_defaults(subcommand=None)
     parser.add_argument(
         "--version", action="version", version=f"%(prog)s {reccmp.VERSION}"
     )
@@ -33,10 +39,10 @@ def main():
         default=Path.cwd(),
         help="Run as if %(prog)s was started in %(metavar)s",
     )
-    subparsers = parser.add_subparsers()
+    subparsers = parser.add_subparsers(required=True)
 
     create_parser = subparsers.add_parser("create")
-    create_parser.set_defaults(action="CREATE")
+    create_parser.set_defaults(subcommand=ProjectSubcommand.CREATE)
     create_parser.add_argument(
         "--originals",
         type=Path,
@@ -68,7 +74,7 @@ def main():
     )
 
     detect_parser = subparsers.add_parser("detect")
-    detect_parser.set_defaults(action="DETECT")
+    detect_parser.set_defaults(subcommand=ProjectSubcommand.DETECT)
     detect_parser.add_argument(
         "--search-path",
         nargs="+",
@@ -93,7 +99,7 @@ def main():
 
     argparse_parse_logging(args=args)
 
-    if args.action == "CREATE":  # FIXME: use enum or callback function
+    if args.subcommand == ProjectSubcommand.CREATE:
         try:
             # pylint: disable=unused-argument
             project = create_project(
@@ -106,7 +112,7 @@ def main():
         except RecCmpProjectException as e:
             logger.error("Project creation failed: %s", e.args[0])
 
-    if args.action == "DETECT":  # FIXME: use enum or callback function
+    elif args.subcommand == ProjectSubcommand.DETECT:
         project = RecCmpProject.from_directory(Path.cwd())
         if not project:
             parser.error(
@@ -123,13 +129,6 @@ def main():
         except RecCmpProjectException as e:
             logger.error("Project detection failed: %s", e.args[0])
 
-    parser.error("Missing command: create/detect")
-
-    # try:
-    #     project = RecCmpBuiltProject.from_directory(Path.cwd())
-    # except RecCmpProjectException as e:
-    #     parser.error(e.args[0])
-
     return 1
 
 

tests/test_project.py

@@ -1,8 +1,17 @@
 from pathlib import Path
 import textwrap
+import pytest
 
-from reccmp.project.create import create_project, RecCmpProject
+from reccmp.project.common import RECCMP_PROJECT_CONFIG
+from reccmp.project.create import (
+    create_project,
+    RecCmpProject,
+    RecCmpProjectAlreadyExistsError,
+)
 from reccmp.project.detect import detect_project, DetectWhat, RecCmpBuiltProject
+from reccmp.project.error import (
+    RecCmpProjectException,
+)
 from reccmp.isledecomp.formats import PEImage
 from .conftest import LEGO1_SHA256
 
@@ -104,3 +113,32 @@ def test_project_creation(tmp_path_factory, binfile: PEImage):
     assert not (project_root / ".gitignore").is_file()
     assert (project_root / "CMakeLists.txt").is_file()
     assert (project_root / "cmake/reccmp.cmake").is_file()
+
+
+def test_create_overwrite_project_file(tmp_path_factory):
+    """Do not overwrite an existing reccmp-project.yml file"""
+    project_root = tmp_path_factory.mktemp("project")
+    with (project_root / RECCMP_PROJECT_CONFIG).open("w+") as f:
+        f.write("test")
+
+    with pytest.raises(RecCmpProjectAlreadyExistsError):
+        create_project(project_directory=project_root, original_paths=[])
+
+
+def test_create_require_original_paths(tmp_path_factory):
+    """Cannot create reccmp project without at least one original binary."""
+    project_root = tmp_path_factory.mktemp("project")
+
+    with pytest.raises(RecCmpProjectException):
+        create_project(project_directory=project_root, original_paths=[])
+
+
+def test_create_original_path_must_exist(tmp_path_factory):
+    """Fail if any original binaries do not exist"""
+    project_root = tmp_path_factory.mktemp("project")
+    temp_dir = tmp_path_factory.mktemp("temp")
+
+    with pytest.raises(FileNotFoundError):
+        create_project(
+            project_directory=project_root, original_paths=[temp_dir / "nonexist.dll"]
+        )