Merge branch 'fix/pack_and_upload' into 'main'

Fix issues related to component uploads

Closes PACMAN-961, PACMAN-958, PACMAN-962, PACMAN-957, and PACMAN-956

See merge request espressif/idf-component-manager!423

Author: kumekay

idf_component_manager/cli/component.py

@@ -5,6 +5,7 @@
 
 from .constants import (
     get_dest_dir_option,
+    get_name_option,
     get_namespace_name_options,
     get_project_dir_option,
     get_project_options,
@@ -15,6 +16,7 @@
 def init_component():
     PROJECT_DIR_OPTION = get_project_dir_option()
     PROJECT_OPTIONS = get_project_options()
+    NAME_OPTION = get_name_option()
     NAMESPACE_NAME_OPTIONS = get_namespace_name_options()
     DEST_DIR_OPTION = get_dest_dir_option()
 
@@ -55,14 +57,13 @@ def component():
     @component.command()
     @add_options(
         PROJECT_DIR_OPTION
-        + NAMESPACE_NAME_OPTIONS
+        + NAME_OPTION
         + COMPONENT_VERSION_OPTION
         + DEST_DIR_OPTION
         + COMMIT_SHA_REPO_OPTION
     )
     def pack(
         manager,
-        namespace,
         name,
         version,
         dest_dir,

idf_component_tools/config.py

@@ -5,6 +5,7 @@
 
 import os
 import typing as t
+from pathlib import Path
 
 import yaml
 from pydantic import (
@@ -26,8 +27,6 @@
 
 from .build_system_tools import get_idf_version
 
-DEFAULT_CONFIG_DIR = os.path.join('~', '.espressif')
-
 RegistryUrlField = t.Union[
     Literal['default'],
     UrlField,
@@ -81,12 +80,12 @@ class Config(BaseModel):
     profiles: t.Dict[str, t.Optional[ProfileItem]] = {}
 
 
-def config_dir():
-    return os.environ.get('IDF_TOOLS_PATH') or os.path.expanduser(DEFAULT_CONFIG_DIR)
+def config_dir() -> Path:
+    return Path(os.environ.get('IDF_TOOLS_PATH') or Path.home() / '.espressif')
 
 
-def root_managed_components_dir():
-    return os.path.join(config_dir(), 'root_managed_components', f'idf{get_idf_version()}')
+def root_managed_components_dir() -> Path:
+    return config_dir() / 'root_managed_components' / f'idf{get_idf_version()}'
 
 
 class ConfigError(FatalError):
@@ -95,11 +94,11 @@ class ConfigError(FatalError):
 
 class ConfigManager:
     def __init__(self, path=None):
-        self.config_path = path or os.path.join(config_dir(), 'idf_component_manager.yml')
+        self.config_path = Path(path) if path else (config_dir() / 'idf_component_manager.yml')
 
     def load(self) -> Config:
         """Loads config from disk"""
-        if not os.path.isfile(self.config_path):
+        if not self.config_path.is_file():
             return Config()
 
         with open(self.config_path, encoding='utf-8') as f:
@@ -120,5 +119,9 @@ def validate(cls, data: t.Any) -> Config:
 
     def dump(self, config: Config) -> None:
         """Writes config to disk"""
+
+        # Make sure that directory exists
+        self.config_path.parent.mkdir(parents=True, exist_ok=True)
+
         with open(self.config_path, mode='w', encoding='utf-8') as f:
             yaml.dump(data=config.model_dump(), stream=f, encoding='utf-8', allow_unicode=True)

idf_component_tools/manager.py

@@ -3,6 +3,7 @@
 
 import os
 import typing as t
+from pathlib import Path
 
 import yaml
 
@@ -30,7 +31,9 @@ def __init__(
         commit_sha: t.Optional[str] = None,
         repository_path: t.Optional[str] = None,
     ) -> None:
-        self.path = os.path.join(path, MANIFEST_FILENAME) if os.path.isdir(path) else path
+        source_path = Path(path)
+        self.path: Path = source_path / MANIFEST_FILENAME if source_path.is_dir() else source_path
+
         self.name = name
 
         self._manifest: 'Manifest' = None  # type: ignore
@@ -48,30 +51,32 @@ def __init__(
         self._validation_errors: t.List[str] = None  # type: ignore
 
     def validate(self) -> 'ManifestManager':
-        from .manifest.models import Manifest, RepositoryInfoField  # avoid circular dependency
+        from .manifest.models import (
+            Manifest,
+            RepositoryInfoField,
+        )
+
+        # avoid circular dependency
         from .utils import ComponentVersion
 
         if self._manifest:
             return self
 
-        if not os.path.isfile(self.path):
-            self._validation_errors = []
-            self._manifest = Manifest(name=self.name, manifest_manager=self)
-            return self
-
+        if not self.path.exists():
+            manifest_dict: t.Dict[str, t.Any] = {}
         # validate manifest
-        try:
-            with open(self.path, 'r') as f:
-                d = yaml.safe_load(f) or {}
-        except yaml.YAMLError:
-            self._validation_errors = [
-                'Cannot parse the manifest file. Please check that\n'
-                '\t{}\n'
-                'is a valid YAML file\n'.format(self.path)
-            ]
-            return self
-
-        if not isinstance(d, dict):
+        else:
+            try:
+                manifest_dict = yaml.safe_load(self.path.read_text()) or {}
+            except yaml.YAMLError:
+                self._validation_errors = [
+                    'Cannot parse the manifest file. Please check that\n'
+                    '\t{}\n'
+                    'is a valid YAML file\n'.format(self.path)
+                ]
+                return self
+
+        if not isinstance(manifest_dict, dict):
             self._validation_errors = [
                 'Manifest file should be a dictionary. Please check that\n'
                 '\t{}\n'
@@ -80,15 +85,15 @@ def validate(self) -> 'ManifestManager':
             return self
 
         if self.name:
-            d['name'] = self.name
+            manifest_dict['name'] = self.name
 
         if self._version:
-            d['version'] = self._version
+            manifest_dict['version'] = self._version
 
-        d['manifest_manager'] = self
+        manifest_dict['manifest_manager'] = self
 
         self._validation_errors, self._manifest = Manifest.validate_manifest(  # type: ignore
-            d,
+            manifest_dict,
             upload_mode=self.upload_mode,
             return_with_object=True,
         )
@@ -150,7 +155,7 @@ def load(self) -> 'Manifest':
 
     def dump(
         self,
-        path: t.Optional[str] = None,
+        path: t.Optional[t.Union[str, Path]] = None,
     ) -> None:
         if path is None:
             path = self.path

idf_component_tools/manifest/models.py

@@ -18,7 +18,10 @@
 from pydantic_core.core_schema import SerializerFunctionWrapHandler
 from pyparsing import ParseException
 
-from idf_component_tools.build_system_tools import build_name, build_name_to_namespace_name
+from idf_component_tools.build_system_tools import (
+    build_name,
+    build_name_to_namespace_name,
+)
 from idf_component_tools.constants import (
     COMMIT_ID_RE,
     COMPILED_GIT_URL_RE,
@@ -386,7 +389,7 @@ def model_dump(
         self,
         **kwargs,
     ) -> t.Dict[str, t.Any]:
-        return super().model_dump(exclude=['name'])
+        return super().model_dump(exclude=['name'], exclude_unset=True)
 
     @field_validator('version')
     @classmethod
@@ -566,7 +569,7 @@ def real_name(self) -> str:
 
     @property
     def path(self) -> str:
-        return self._manifest_manager.path if self._manifest_manager else ''
+        return str(self._manifest_manager.path) if self._manifest_manager else ''
 
 
 class SolvedComponent(BaseModel):

tests/cli/test_compote.py

@@ -47,16 +47,17 @@ def test_raise_exception_on_warnings(monkeypatch):
     )
 
 
-def test_login_to_registry(monkeypatch, tmp_path, mock_registry, mock_token_information):
-    monkeypatch.setenv('IDF_TOOLS_PATH', str(tmp_path))
-
+def test_login_to_registry(tmp_path, mock_registry, mock_token_information):
     runner = CliRunner()
     cli = initialize_cli()
     output = runner.invoke(
         cli,
         ['registry', 'login', '--no-browser'],
         input='test_token',
-        env={'IDF_TOOLS_PATH': str(tmp_path)},
+        env={
+            #  non-existing path is to check PACMAN-961
+            'IDF_TOOLS_PATH': str(tmp_path / 'non-existing-path')
+        },
     )
 
     assert output.exit_code == 0

tests/core/test_add_dependency.py

@@ -0,0 +1,65 @@
+# SPDX-FileCopyrightText: 2022-2024 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: Apache-2.0
+"""Test Core commands"""
+
+import pytest
+import vcr
+
+from idf_component_manager.core import ComponentManager
+from idf_component_tools.constants import MANIFEST_FILENAME
+from idf_component_tools.errors import FatalError
+from idf_component_tools.manager import ManifestManager
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_init_project.yaml')
+def test_init_project(mock_registry, tmp_path):
+    (tmp_path / 'main').mkdir()
+    (tmp_path / 'components' / 'foo').mkdir(parents=True)
+    main_manifest_path = tmp_path / 'main' / MANIFEST_FILENAME
+    foo_manifest_path = tmp_path / 'components' / 'foo' / MANIFEST_FILENAME
+
+    manager = ComponentManager(path=str(tmp_path))
+    manager.create_manifest()
+    manager.create_manifest(component='foo')
+
+    for filepath in [main_manifest_path, foo_manifest_path]:
+        assert filepath.read_text().startswith('## IDF Component Manager')
+
+    manager.add_dependency('cmp==4.0.3')
+    manifest_manager = ManifestManager(str(main_manifest_path), 'main')
+    assert manifest_manager.manifest_tree['dependencies']['espressif/cmp'] == '==4.0.3'
+
+    manager.add_dependency('espressif/cmp==4.0.3', component='foo')
+    manifest_manager = ManifestManager(str(foo_manifest_path), 'foo')
+    assert manifest_manager.manifest_tree['dependencies']['espressif/cmp'] == '==4.0.3'
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_init_project_with_path.yaml')
+def test_init_project_with_path(mock_registry, tmp_path):
+    src_path = tmp_path / 'src'
+    src_path.mkdir(parents=True, exist_ok=True)
+    src_manifest_path = src_path / MANIFEST_FILENAME
+
+    outside_project_path = tmp_path.parent
+    outside_project_path_error_match = 'Directory ".*" is not under project directory!'
+    component_and_path_error_match = 'Cannot determine manifest directory.'
+
+    manager = ComponentManager(path=str(tmp_path))
+    manager.create_manifest(path=str(src_path))
+
+    with pytest.raises(FatalError, match=outside_project_path_error_match):
+        manager.create_manifest(path=str(outside_project_path))
+
+    with pytest.raises(FatalError, match=component_and_path_error_match):
+        manager.create_manifest(component='src', path=str(src_path))
+
+    manager.add_dependency('espressif/cmp==4.0.3', path=str(src_path))
+    manifest_manager = ManifestManager(str(src_manifest_path), 'src')
+
+    assert manifest_manager.manifest_tree['dependencies']['espressif/cmp'] == '==4.0.3'
+
+    with pytest.raises(FatalError, match=outside_project_path_error_match):
+        manager.create_manifest(path=str(outside_project_path))
+
+    with pytest.raises(FatalError, match=component_and_path_error_match):
+        manager.add_dependency('espressif/cmp==4.0.3', component='src', path=str(src_path))

tests/core/test_create_project_from_example.py

@@ -0,0 +1,62 @@
+# SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD
+# SPDX-License-Identifier: Apache-2.0
+import vcr
+from pytest import raises
+
+from idf_component_manager.core import ComponentManager
+from idf_component_tools.errors import FatalError
+
+
+def test_create_example_project_path_not_a_directory(tmp_path):
+    existing_file = tmp_path / 'example'
+    existing_file.write_text('test')
+
+    manager = ComponentManager(path=str(tmp_path))
+
+    with raises(FatalError, match='Your target path is not a directory*'):
+        manager.create_project_from_example('test:example')
+
+
+def test_create_example_project_path_not_empty(tmp_path):
+    example_dir = tmp_path / 'example'
+    example_dir.mkdir()
+    existing_file = example_dir / 'test'
+    existing_file.write_text('test')
+
+    manager = ComponentManager(path=str(tmp_path))
+
+    with raises(FatalError, match='To create an example you must*'):
+        manager.create_project_from_example('test:example')
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_component_not_exist.yaml')
+def test_create_example_component_not_exist(tmp_path):
+    manager = ComponentManager(path=str(tmp_path))
+    with raises(FatalError, match='Component "espressif/test" not found'):
+        manager.create_project_from_example('test:example')
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_not_exist.yaml')
+def test_create_example_version_not_exist(mock_registry, tmp_path):
+    manager = ComponentManager(path=str(tmp_path))
+    with raises(
+        FatalError,
+        match='Version of the component "test/cmp" satisfying the spec "=2.0.0" was not found.',
+    ):
+        manager.create_project_from_example('test/cmp=2.0.0:example')
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_not_exist.yaml')
+def test_create_example_not_exist(mock_registry, tmp_path):
+    manager = ComponentManager(path=str(tmp_path))
+    with raises(
+        FatalError,
+        match='Cannot find example "example" for "test/cmp" version "=1.0.1"',
+    ):
+        manager.create_project_from_example('test/cmp=1.0.1:example')
+
+
+@vcr.use_cassette('tests/fixtures/vcr_cassettes/test_create_example_success.yaml')
+def test_create_example_success(mock_registry, tmp_path):
+    manager = ComponentManager(path=str(tmp_path))
+    manager.create_project_from_example('test/cmp>=1.0.0:sample_project')