Merge pull request #105 from igoropaniuk/feat/pyproject_ruff_mypy_fixes

Introduce pyproject.toml, ruff and mypy for linting and type checking

Author: lumag

.github/workflows/build.yml

@@ -14,9 +14,14 @@ jobs:
         with:
           fetch-depth: 0
 
+      - name: Install linters
+        run: |
+          sudo snap install ruff
+          sudo apt-get update
+          sudo apt-get install -y mypy
+
       - name: Build all platforms and run tests
         run: |
-          sudo apt install pycodestyle
           make lint
           make all integration
 

.gitignore

@@ -1,2 +1,9 @@
 platforms/**/*.xml
 platforms/**/*.bin
+
+# Python
+__pycache__/
+*.pyc
+.mypy_cache/
+.ruff_cache/
+.venv/

Makefile

@@ -25,12 +25,8 @@ all: $(PLATFORMS) $(PARTITIONS_XML) $(CONTENTS_XML)
 	$(TOPDIR)/gen_contents.py -p $< -t $@.in -o $@ $${BUILD_ID:+ -b $(BUILD_ID)}
 
 lint:
-	# W605: invalid escape sequence
-	pycodestyle --select=W605 *.py
-
-	# gen_contents.py is nearly perfect except E501: line too long.
-	# Ensure there are no regressions.
-	pycodestyle --ignore=E501 gen_contents.py
+	ruff check *.py
+	mypy *.py
 
 integration: all
 	# make sure generated output has created expected files

README.md

@@ -7,6 +7,48 @@ qcom-ptool contains various device partitioning utilities like ptool.py, gen_par
 
 # Development
 
+## Dependencies
+
+The scripts use only the Python standard library (Python 3.8+), so no
+runtime dependencies need to be installed.
+
+For development, `make lint` invokes `ruff` and `mypy` directly from the
+command line. On Debian/Ubuntu, install them as follows (ruff is not
+packaged in apt on all releases/architectures, so we install it from
+snap):
+
+```sh
+sudo snap install ruff
+sudo apt install mypy
+```
+
+## Makefile targets
+
+| Target        | Description                                                |
+|---------------|------------------------------------------------------------|
+| `all`         | Generate partition XML and GPT binaries for all platforms  |
+| `lint`        | Run ruff (linter) and mypy (type checker) on all scripts   |
+| `integration` | Build all platforms and verify generated files are present |
+| `check`       | Run both `lint` and `integration`                          |
+| `install`     | Install scripts to `$(DESTDIR)$(PREFIX)/bin`               |
+| `clean`       | Remove generated XML and binary files from platforms/      |
+
+### Quick start
+
+```sh
+# install linters (Debian/Ubuntu)
+sudo snap install ruff
+sudo apt install mypy
+
+# run linters
+make lint
+
+# build all platforms and run tests
+make check
+```
+
+## Code contributions
+
 See [CONTRIBUTING.md file](CONTRIBUTING.md) for instructions on how to send
 code contributions to this project. You can also [report an issue on
 GitHub](../../issues).

gen_contents.py

@@ -33,13 +33,13 @@ def ParseXML(XMLFile):
 
 def UpdateMetaData(TemplateRoot, PartitionRoot, BuildId):
     ChipIdList = TemplateRoot.findall("product_info/chipid")
-    DefaultStorageType = None
+    DefaultStorageType: str = ""
     for ChipId in ChipIdList:
         Flavor = ChipId.get("flavor")
         StorageType = ChipId.get("storage_type")
         print(f"Chipid Flavor: {Flavor} Storage Type: {StorageType}")
         if Flavor == "default":
-            DefaultStorageType = ChipId.get("storage_type")
+            DefaultStorageType = ChipId.get("storage_type", "")
 
     PhyPartition = PartitionRoot.findall("physical_partition")
     Partitions = []
@@ -67,6 +67,8 @@ def _add_file_elements(parent_element, pathname, file_path_flavor=None):
     builds = TemplateRoot.findall("builds_flat/build")
     for build in builds:
         Name = build.find("name")
+        if Name is None or Name.text is None:
+            continue
         print(f"Build Name: {Name.text}")
         new_build_id = ET.SubElement(build, "build_id")
         new_build_id.text = BuildId

gen_partition.py

@@ -32,10 +32,11 @@
 import sys
 import xml.etree.ElementTree as ET
 from collections import OrderedDict
+from typing import NoReturn
 from xml.dom import minidom
 
 
-def usage():
+def usage() -> NoReturn:
     print(
         "\n\tUsage: %s -i <input> -o <output> -m [partition_name1=image_filename1,partition_name2=image_filename2,...]\n\tVersion 1.0\n"
         % (sys.argv[0])
@@ -72,7 +73,7 @@ def usage():
 disk_entry = None
 partition_entries = []
 # store partition image map passed from command line
-partition_image_map = {}
+partition_image_map: dict[str, str] = {}
 input_file = None
 output_xml = None
 
@@ -92,19 +93,23 @@ def disk_options(argv):
             disk_params["GROW_LAST_PARTITION_TO_FILL_DISK"] = "true"
         elif opt in ["--align-partitions"]:
             disk_params["ALIGN_PARTITIONS_TO_PERFORMANCE_BOUNDARY"] = "true"
-            disk_params["PERFORMANCE_BOUNDARY_IN_KB"] = int(arg) // 1024
+            disk_params["PERFORMANCE_BOUNDARY_IN_KB"] = str(int(arg) // 1024)
     return disk_params
 
 
 def partition_size_in_kb(size):
     if not re.search("[a-zA-Z]+", size):
         return int(size) // 1024
-    if re.search("([0-9])*(?=([Kk]([Bb])*))", size):
-        return int(re.search("([0-9])*(?=([Kk]([Bb])*))", size).group(0))
-    if re.search("([0-9])*(?=([Mm]([Bb])*))", size):
-        return int(re.search("([0-9])*(?=([Mm]([Bb])*))", size).group(0)) * 1024
-    if re.search("([0-9])*(?=([Gg]([Bb])*))", size):
-        return int(re.search("([0-9])*(?=([Gg]([Bb])*))", size).group(0)) * 1024 * 1024
+    m = re.search("([0-9]+)(?=[Kk][Bb]?)", size)
+    if m:
+        return int(m.group(0))
+    m = re.search("([0-9]+)(?=[Mm][Bb]?)", size)
+    if m:
+        return int(m.group(0)) * 1024
+    m = re.search("([0-9]+)(?=[Gg][Bb]?)", size)
+    if m:
+        return int(m.group(0)) * 1024 * 1024
+    raise ValueError("Unrecognized size format: '%s'" % size)
 
 
 def partition_options(argv):
@@ -134,19 +139,19 @@ def partition_options(argv):
             partition_entry["filename"] = arg
         elif opt in ["--sparse"]:
             partition_entry["sparse"] = arg
-        if partition_entry["label"] in partition_image_map.keys():
+        if partition_entry["label"] in partition_image_map:
             partition_entry["filename"] = partition_image_map[partition_entry["label"]]
     return phys_part, partition_entry
 
 
 def parse_partition_entries(partition_entries):
-    partitions_params = {}
+    partitions_params: dict[int, list[dict[str, str]]] = {}
 
     for partition_entry in partition_entries:
         opts_list = list(partition_entry.split(" "))
         if opts_list[0] == "--partition":
             try:
-                options, remainders = getopt.gnu_getopt(
+                options, _remainders = getopt.gnu_getopt(
                     opts_list[1:],
                     "",
                     [
@@ -173,7 +178,7 @@ def parse_disk_entry(disk_entry):
     opts_list = list(disk_entry.split(" "))
     if opts_list[0] == "--disk":
         try:
-            options, remainders = getopt.gnu_getopt(
+            options, _remainders = getopt.gnu_getopt(
                 opts_list[1:],
                 "",
                 [
@@ -196,12 +201,12 @@ def generate_multi_lun_xml(disk_params, partitions, output_xml):
     parser_instruction_text = ""
 
     for key, value in disk_params.items():
-        if not key == "size" and not key == "type":
+        if key != "size" and key != "type":
             parser_instruction_text += "\n\t" + str(key) + "=" + str(value) + "\n\t"
 
     ET.SubElement(root, "parser_instructions").text = parser_instruction_text
 
-    for phys_part, entries in sorted(partitions.items(), key=lambda item: int(item[0])):
+    for _phys_part, entries in sorted(partitions.items(), key=lambda item: int(item[0])):
         found = False
         for part_entry in entries:
 
@@ -257,6 +262,8 @@ def generate_partition_xml(disk_params, partitions, output_xml):
     except Exception as argerr:
         print(str(argerr))
         usage()
+    if input_file is None or output_xml is None:
+        usage()
     f = open(input_file)
     line = f.readline()
     while line: