chore: address PR feedback

Signed-off-by: behnazh-w <[email protected]>

Author: behnazh-w

src/macaron/build_spec_generator/build_command_patcher.py

@@ -59,7 +59,7 @@ def _patch_commands(
             cli_command = effective_cli_parser.parse(cmds)
         except CommandLineParseError as error:
             logger.error(
-                "Failed to parse the cli command %s. Error %s.",
+                "Failed to patch the cli command %s. Error %s.",
                 " ".join(cmds),
                 error,
             )
@@ -77,7 +77,7 @@ def _patch_commands(
             )
         except PatchBuildCommandError as error:
             logger.error(
-                "Failed to patch the mvn command %s. Error %s.",
+                "Failed to patch the build command %s. Error %s.",
                 " ".join(cmds),
                 error,
             )

src/macaron/build_spec_generator/build_spec_generator.py

@@ -78,9 +78,12 @@ def gen_build_spec_for_purl(
     purl: PackageURL
         The package URL to generate build spec for.
     database_path: str
-        The path to the Macaron database.
+        The path to the Macaron SQLite database file. This database will be accessed in read-only mode,
+        ensuring that no modifications can be made during operations.
     build_spec_format: BuildSpecFormat
         The format of the final build spec content.
+    output_path: str
+        The path to the output directory.
 
     Returns
     -------
@@ -89,10 +92,10 @@ def gen_build_spec_for_purl(
         buildspec file cannot be created in the local filesystem, ``os.EX_DATAERR`` if there was an
         error generating the content for the buildspec file.
     """
-    db_engine = create_engine(f"sqlite+pysqlite:///{database_path}", echo=False)
+    db_engine = create_engine(f"sqlite+pysqlite:///file:{database_path}?mode=ro&uri=true", echo=False)
+    build_spec_content = None
 
     with Session(db_engine) as session, session.begin():
-        build_spec_content = None
         match build_spec_format:
             case BuildSpecFormat.REPRODUCIBLE_CENTRAL:
                 build_spec_content = gen_reproducible_central_build_spec(
@@ -101,42 +104,42 @@ def gen_build_spec_for_purl(
                     patches=CLI_COMMAND_PATCHES,
                 )
 
-        if not build_spec_content:
-            logger.error("Error while generating reproducible central build spec.")
-            return os.EX_DATAERR
-
-        logger.debug("Build spec content: \n%s", build_spec_content)
-
-        build_spec_filepath = os.path.join(
-            output_path,
-            "buildspec",
-            get_purl_based_dir(
-                purl_name=purl.name,
-                purl_namespace=purl.namespace,
-                purl_type=purl.type,
-            ),
-            "macaron.buildspec",
+    if not build_spec_content:
+        logger.error("Error while generating the build spec.")
+        return os.EX_DATAERR
+
+    logger.debug("Build spec content: \n%s", build_spec_content)
+
+    build_spec_filepath = os.path.join(
+        output_path,
+        "buildspec",
+        get_purl_based_dir(
+            purl_name=purl.name,
+            purl_namespace=purl.namespace,
+            purl_type=purl.type,
+        ),
+        "macaron.buildspec",
+    )
+
+    os.makedirs(
+        name=os.path.dirname(build_spec_filepath),
+        exist_ok=True,
+    )
+
+    logger.info(
+        "Generating the %s format build spec to %s.",
+        build_spec_format.value,
+        os.path.relpath(build_spec_filepath, os.getcwd()),
+    )
+    try:
+        with open(build_spec_filepath, mode="w", encoding="utf-8") as file:
+            file.write(build_spec_content)
+    except OSError as error:
+        logger.error(
+            "Could not create the build spec at %s. Error: %s",
+            os.path.relpath(build_spec_filepath, os.getcwd()),
+            error,
         )
+        return os.EX_OSERR
 
-        os.makedirs(
-            name=os.path.dirname(build_spec_filepath),
-            exist_ok=True,
-        )
-
-        try:
-            with open(build_spec_filepath, mode="w", encoding="utf-8") as file:
-                logger.info(
-                    "Generating the %s format build spec to %s.",
-                    build_spec_format.value,
-                    os.path.relpath(build_spec_filepath, os.getcwd()),
-                )
-                file.write(build_spec_content)
-        except OSError as error:
-            logger.error(
-                "Could not create the build spec at %s. Error: %s",
-                os.path.relpath(build_spec_filepath, os.getcwd()),
-                error,
-            )
-            return os.EX_OSERR
-
-        return os.EX_OK
+    return os.EX_OK

src/macaron/build_spec_generator/cli_command_parser/__init__.py

@@ -67,10 +67,10 @@ def patch_mapping(
 
 @dataclass
 class OptionDef(Generic[P]):
-    """This class represent a definition of a CLI option for argparse.ArgumentParser.
+    """This class represents a definition of a CLI option for argparse.ArgumentParser.
 
     This class also contains the information for validating a patch value.
-    The generic type T is the patch expected type (if it's not None).
+    The generic type P is the patch expected type (if it's not None).
     """
 
     # e.g. `--long-option-name`
@@ -83,7 +83,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[P]:
         raise NotImplementedError()
 
     @abstractmethod
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         raise NotImplementedError()
 

src/macaron/build_spec_generator/cli_command_parser/gradle_cli_parser.py

@@ -50,7 +50,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[bool]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return isinstance(patch, bool)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         kwargs: dict[str, Any] = {}
 
@@ -90,7 +90,7 @@ def get_negated_long_name(long_name: str) -> str:
         """Return the negated version of a long option name."""
         return f"--no-{long_name.removeprefix('--')}"
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         # We allow providing both the normal and negated form.
         negated_long_name = self.get_negated_long_name(self.long_name)
@@ -127,7 +127,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[str]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return isinstance(patch, str)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         if self.short_name:
             arg_parse.add_argument(
@@ -161,7 +161,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[dict[str, str | None]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_dict_of_str_to_str_or_none(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         arg_parse.add_argument(
             *(self.short_name, self.long_name),
@@ -184,7 +184,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[list[str]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_list_of_strs(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         # Doesn't require to allow cases like "gradle --help".
         arg_parse.add_argument(
@@ -212,7 +212,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[list[str]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_list_of_strs(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         arg_parse.add_argument(
             *(self.short_name, self.long_name),
@@ -464,7 +464,7 @@ def __init__(self) -> None:
         self.option_defs: dict[str, OptionDef] = {}
 
         for opt_def in GRADLE_OPTION_DEF:
-            opt_def.add_itself_to_arg_parser(self.arg_parser)
+            opt_def.add_to_arg_parser(self.arg_parser)
 
             self.option_defs[opt_def.long_name] = opt_def
 

src/macaron/build_spec_generator/cli_command_parser/maven_cli_parser.py

@@ -46,7 +46,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[bool]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return isinstance(patch, bool)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         if self.dest:
             arg_parse.add_argument(
@@ -80,7 +80,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[str]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return isinstance(patch, str)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         arg_parse.add_argument(
             *(self.short_name, self.long_name),
@@ -111,7 +111,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[list[str]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_list_of_strs(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         arg_parse.add_argument(
             *(self.short_name, self.long_name),
@@ -141,7 +141,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[dict[str, str | None]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_dict_of_str_to_str_or_none(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         arg_parse.add_argument(
             *(self.short_name, self.long_name),
@@ -164,7 +164,7 @@ def is_valid_patch_option(self, patch: Any) -> TypeGuard[list[str]]:
         """Return True if the provided patch value is compatible with the internal type of this option."""
         return is_list_of_strs(patch)
 
-    def add_itself_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
+    def add_to_arg_parser(self, arg_parse: argparse.ArgumentParser) -> None:
         """Add a new argument to argparser.ArgumentParser representing this option."""
         # Doesn't require to allow cases like "mvn --help".
         arg_parse.add_argument(
@@ -364,7 +364,7 @@ def __init__(self) -> None:
         self.option_defs: dict[str, OptionDef] = {}
 
         for opt_def in MAVEN_OPTION_DEF:
-            opt_def.add_itself_to_arg_parser(self.arg_parser)
+            opt_def.add_to_arg_parser(self.arg_parser)
 
             self.option_defs[opt_def.long_name] = opt_def
 

src/macaron/build_spec_generator/reproducible_central/reproducible_central.py

@@ -339,6 +339,15 @@ def gen_reproducible_central_build_spec(
         for this PURL, 3. Failed to patch the build commands using the provided ``patches``, 4. The database from
         ``session`` doesn't contain enough information.
     """
+    if purl.type != "maven":
+        logger.error(
+            "Reproducible Central build specification only supports PURLs of type 'maven'. "
+            "Received PURL type: '%s' (%s). Please provide a valid Maven package URL.",
+            purl.type,
+            purl.to_string(),
+        )
+        return None
+
     logger.debug(
         "Generating build spec for %s with command patches:\n%s",
         purl,