Merge pull request #17 from LedgerHQ/clean/legacymannifest

Removing references to LegacyManifest

.github/workflows/fast-checks.yml

@@ -15,9 +15,9 @@ jobs:
     steps:
     - name: Clone
       uses: actions/checkout@v4
-    - run: pip install flake8
+    - run: pip install flake8 flake8-pyproject
     - name: Flake8 lint Python code
-      run: find src/ -type f -name '*.py' -exec flake8 --max-line-length=100 '{}' '+'
+      run: flake8 src/
 
   yapf:
     name: Formatting

CHANGELOG.md

@@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+
+## [0.5.0] - 2024-??-??
+
+### Removed
+
+- BREAKING: removing references to `LegacyManifest` and `RepoManifest`. Only `Manifest` is to be
+  used from now on.
+
+
 ## [0.4.1] - 2024-02-22
 
 ### Fixed
@@ -24,6 +33,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - BREAKING: moving the `utils/manifest.py` module into its own `manifest/` subpackage.
 
+
 ## [0.3.0] - 2023-10-30
 
 ### Added
@@ -36,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - `utils/manifest.py`: LegacyManifest now has a `.from_path` method which mimics its initializer
   previous behavior. The initializer signature has changed.
 
+
 ## [0.2.1] - 2023-10-20
 
 ### Fixed

doc/manifest.md

@@ -183,7 +183,6 @@ positional arguments:
 options:
   -h, --help            show this help message and exit
   -v, --verbose
-  -l, --legacy          Specifies if the 'ledger_app.toml' file is a legacy one (with 'rust-app' section)
   -c CHECK, --check CHECK
                         Check the manifest content against the provided directory.
   -os, --output-sdk     outputs the SDK type
@@ -213,8 +212,8 @@ This manifest had this format:
 manifest-path = "rust-app/Cargo.toml"
 ```
 
-This format is now considered legacy and won't be supported soon. It should be changed to the new
-manifest format.
+This format is considered legacy since October 2023 and is no longer supported since February 2024.
+It should be changed to fit the new manifest format.
 
 In this case, the new manifest should be:
 

pyproject.toml

@@ -62,3 +62,6 @@ exclude_lines = [
 
 [tool.bandit]
 skips = ["B101"]
+
+[tool.flake8]
+max-line-length = 100

src/ledgered/manifest/__init__.py

@@ -1,9 +1,6 @@
 from .app import AppConfig
 from .constants import EXISTING_DEVICES, MANIFEST_FILE_NAME
-from .manifest import LegacyManifest, Manifest
+from .manifest import Manifest
 from .tests import TestsConfig
 
-__all__ = [
-    "AppConfig", "EXISTING_DEVICES", "LegacyManifest", "Manifest", "MANIFEST_FILE_NAME",
-    "TestsConfig"
-]
+__all__ = ["AppConfig", "EXISTING_DEVICES", "Manifest", "MANIFEST_FILE_NAME", "TestsConfig"]

src/ledgered/manifest/cli.py

@@ -7,7 +7,7 @@
 from typing import Dict
 
 from .constants import MANIFEST_FILE_NAME
-from .manifest import Manifest, LegacyManifest
+from .manifest import Manifest
 from .utils import getLogger
 
 
@@ -42,13 +42,6 @@ def set_parser() -> ArgumentParser:
 
     # generic options
     parser.add_argument('-v', '--verbose', action='count', default=0)
-    parser.add_argument("-l",
-                        "--legacy",
-                        required=False,
-                        action="store_true",
-                        default=False,
-                        help="Specifies if the 'ledger_app.toml' file is a legacy one (with "
-                        "'rust-app' section)")
     parser.add_argument("-c",
                         "--check",
                         required=False,
@@ -127,19 +120,8 @@ def main():  # pragma: no cover
     elif args.verbose > 1:
         logger.setLevel(logging.DEBUG)
 
-    # compatibility check: legacy manifest cannot display sdk, devices, unit/pytest directory
-    if args.legacy and (args.output_sdk or args.output_devices or args.output_devices
-                        or args.output_unit_directory or args.output_pytest_directory):
-        raise ValueError("'-l' option is not compatible with '-os', '-od', 'ou' or 'op'")
-
-    # parsing the manifest
-    if args.legacy:
-        logger.info("Expecting a legacy manifest")
-        manifest_cls = LegacyManifest
-    else:
-        logger.info("Expecting a classic manifest")
-        manifest_cls = Manifest
-    repo_manifest = manifest_cls.from_path(manifest)
+    logger.info("Loading the manifest")
+    repo_manifest = Manifest.from_path(manifest)
 
     # check directory path against manifest data
     if args.check is not None:
@@ -151,14 +133,9 @@ def main():  # pragma: no cover
     logger.info("Displaying manifest info")
     display_content = defaultdict(dict)
 
-    # build_directory can be 'deduced' from legacy manifest
     if args.output_build_directory:
-        if args.legacy:
-            display_content["build_directory"] = str(repo_manifest.manifest_path.parent)
-        else:
-            display_content["build_directory"] = str(repo_manifest.app.build_directory)
+        display_content["build_directory"] = str(repo_manifest.app.build_directory)
 
-    # unlike build_directory, other field can not be deduced from legacy manifest
     if args.output_sdk:
         display_content["sdk"] = repo_manifest.app.sdk
     if args.output_devices:

src/ledgered/manifest/manifest.py

@@ -1,6 +1,5 @@
 import logging
 import toml
-from abc import ABC, abstractmethod
 from dataclasses import dataclass
 from pathlib import Path
 from typing import Dict, IO, Optional, Union
@@ -12,36 +11,8 @@
 from .use_cases import UseCasesConfig
 
 
-class RepoManifest(ABC, Jsonable):
-
-    @abstractmethod
-    def check(self, directory: Union[str, Path]) -> None:
-        raise NotImplementedError
-
-    @staticmethod
-    def __from_file(filee: Union[Path, IO]) -> "RepoManifest":
-        manifest = toml.load(filee)
-        cls: type
-        if "rust-app" in manifest:
-            cls = LegacyManifest
-        else:
-            cls = Manifest
-        return cls(**manifest)
-
-    @classmethod
-    def from_io(cls, manifest_io: IO) -> "RepoManifest":
-        return cls.__from_file(manifest_io)
-
-    @classmethod
-    def from_path(cls, path: Path) -> "RepoManifest":
-        if path.is_dir():
-            path = path / MANIFEST_FILE_NAME
-        assert path.is_file(), f"'{path.resolve()}' is not a manifest file."
-        return cls.__from_file(path)
-
-
 @dataclass
-class Manifest(RepoManifest):
+class Manifest(Jsonable):
     app: AppConfig
     tests: Optional[TestsConfig]
     use_cases: Optional[UseCasesConfig]
@@ -77,30 +48,3 @@ def check(self, base_directory: Union[str, Path]) -> None:
         logging.info("Checking existence of file %s", build_file)
         assert build_file.is_file(), f"No file '{build_file}' (from the given base directory " \
             f"'{base_directory}' + the manifest path '{self.app.build_directory}') was found"
-
-
-class LegacyManifest(RepoManifest):
-
-    def __init__(self, **kwargs) -> None:
-        try:
-            self.manifest_path = Path(kwargs["rust-app"]["manifest-path"])
-        except KeyError as e:
-            msg = "Given manifest is not a legacy manifest (does not contain 'rust-app' section)"
-            logging.error(msg)
-            raise ValueError(msg) from e
-
-    def check(self, base_directory: Union[str, Path]) -> None:
-        base_directory = Path(base_directory)
-        assert base_directory.is_dir(), f"Given '{base_directory}' must be an existing directory"
-        cargo_toml = base_directory / self.manifest_path
-        logging.info("Checking existence of file %s", cargo_toml)
-        assert cargo_toml.is_file(), f"No file '{cargo_toml}' (from the given base directory " \
-            f"'{base_directory}' + the manifest path '{self.manifest_path}') was found"
-
-    @classmethod
-    def from_path(cls, path: Union[str, Path]) -> "LegacyManifest":
-        path = Path(path)
-        if path.is_dir():
-            path = path / MANIFEST_FILE_NAME
-        assert path.is_file(), f"'{path.resolve()}' is not a manifest file."
-        return cls(**toml.load(path))

tests/functional/manifest/test_cli.py

@@ -89,7 +89,6 @@ class TestCLIMain(TestCase):
     def setUp(self):
         self.args = Namespace(
             manifest=TEST_MANIFEST_DIRECTORY / "full_correct.toml",
-            legacy=False,
             check=None,
             verbose=0,
             output_build_directory=False,

tests/unit/manifest/test_manifest.py

@@ -1,37 +1,11 @@
 from pathlib import Path
 from unittest import TestCase
 
-from ledgered.manifest.manifest import LegacyManifest, Manifest, RepoManifest, MANIFEST_FILE_NAME
+from ledgered.manifest.manifest import Manifest, MANIFEST_FILE_NAME
 
 from .. import TEST_MANIFEST_DIRECTORY
 
 
-class DummyRepoManifest(RepoManifest):
-    def check(self, directory) -> None:
-        pass
-
-
-class TestRepoManifest(TestCase):
-
-    def test_from_path(self):
-        manifest = TEST_MANIFEST_DIRECTORY
-        self.assertIsInstance(DummyRepoManifest.from_path(manifest), Manifest)
-
-        manifest = TEST_MANIFEST_DIRECTORY / "ledger_app.toml"
-        self.assertIsInstance(DummyRepoManifest.from_path(manifest), Manifest)
-
-        legacy = TEST_MANIFEST_DIRECTORY / "legacy" / "ledger_app.toml"
-        self.assertIsInstance(DummyRepoManifest.from_path(legacy), LegacyManifest)
-
-    def test_from_io(self):
-        with (TEST_MANIFEST_DIRECTORY / "ledger_app.toml").open() as io:
-            self.assertIsInstance(DummyRepoManifest.from_io(io), Manifest)
-
-    def test_from_path_nok(self):
-        with self.assertRaises(AssertionError):
-            RepoManifest.from_path(Path("/not/existing/path"))
-
-
 class TestManifest(TestCase):
 
     def check_ledger_app_toml(self, manifest: Manifest) -> None:
@@ -71,32 +45,3 @@ def test_check_ok(self):
     def test_check_nok(self):
         with self.assertRaises(AssertionError):
             Manifest.from_path(TEST_MANIFEST_DIRECTORY).check("wrong_directory")
-
-
-class TestLegacyManifest(TestCase):
-
-    def test___init___ok(self):
-        expected = "some expected path"
-        manifest = LegacyManifest(**{"rust-app": {"manifest-path": expected}})
-        self.assertEqual(manifest.manifest_path, Path(expected))
-
-    def test___init___nok(self):
-        with self.assertRaises(ValueError):
-            LegacyManifest(wrong_key=4)
-        with self.assertRaises(ValueError):
-            LegacyManifest(**{"rust-app": {"wrong subkey": 4}})
-
-    def test_from_path_ok(self):
-        manifest = LegacyManifest.from_path(TEST_MANIFEST_DIRECTORY / "legacy" / "ledger_app.toml")
-        self.assertEqual(manifest.manifest_path, Path("Cargo.toml"))
-
-    def test_from_path_ok2(self):
-        manifest = LegacyManifest.from_path(TEST_MANIFEST_DIRECTORY / "legacy")
-        self.assertEqual(manifest.manifest_path, Path("Cargo.toml"))
-
-    def test_check_ok(self):
-        LegacyManifest.from_path(TEST_MANIFEST_DIRECTORY / "legacy").check(TEST_MANIFEST_DIRECTORY / "legacy")
-
-    def test_check_nok(self):
-        with self.assertRaises(AssertionError):
-            LegacyManifest.from_path(TEST_MANIFEST_DIRECTORY / "legacy").check("wrong_directory")