Merge branch 'fix/inject_optional_dependencies' into 'main'

fix: inject optional requirements Closes PACMAN-360 See merge request espressif/idf-component-manager!129
espressif · May 30, 2022 · 0d5022b · 0d5022b
2 parents 916f256 + ed31743
commit 0d5022b
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Fix expansion of environment variables in manifest for `rules`
+- Fix inject optional dependencies even if they are excluded
 
 ## [1.1.0] 2022-05-19
 

diff --git a/idf_component_manager/version_solver/version_solver.py b/idf_component_manager/version_solver/version_solver.py
@@ -24,8 +24,6 @@ def __init__(
         self._target = None
         self._overriders = set()  # type: set[str]
 
-        self._missing_optional_dependencies = set()  # type: set[str]
-
     def solve(self):  # type: () -> SolvedManifest
         for manifest in self.requirements.manifests:
             self.solve_manifest(manifest)
@@ -46,15 +44,8 @@ def solve(self):  # type: () -> SolvedManifest
 
     def solve_manifest(self, manifest):
         for requirement in manifest.dependencies:  # type: ComponentRequirement
-            if requirement.name in self._missing_optional_dependencies:
-                continue
-
-            if requirement.meet_optional_dependencies:
-                self._source.root_dep(Package(requirement.name, requirement.source), requirement.version_spec)
-                self.solve_component(requirement)
-            else:
-                print('Skipping optional dependency: {}'.format(requirement.name))
-                self._missing_optional_dependencies.add(requirement.name)
+            self._source.root_dep(Package(requirement.name, requirement.source), requirement.version_spec)
+            self.solve_component(requirement)
 
     def solve_component(self, requirement):  # type: (ComponentRequirement) -> None
         cmp_with_versions = requirement.source.versions(

diff --git a/idf_component_tools/manifest/manifest.py b/idf_component_tools/manifest/manifest.py
@@ -114,7 +114,10 @@ def fromdict(cls, manifest_tree, name):  # type: (dict, str) -> Manifest
                 public=details.get('public'),
                 if_clauses=details.get('rules'),
             )
-            manifest._dependencies.append(component)
+            if component.meet_optional_dependencies:
+                manifest._dependencies.append(component)
+            else:
+                print('Skipping optional dependency: {}'.format(name))
 
         return manifest
 

diff --git a/integration_tests/test_integration.py b/integration_tests/test_integration.py
@@ -589,3 +589,32 @@ def test_build_pure_cmake(project):
     assert 'Generating done' in res
     res = live_print_call(['cmake', '--build', build_dir])
     assert 'FAILED' not in res
+
+
+@pytest.mark.parametrize(
+    'project', [
+        {
+            'components': {
+                'main': {
+                    'dependencies': {
+                        'cmp': {
+                            'version': '*',
+                            'path': fixtures_path('components', 'cmp'),
+                            'include': 'cmp.h',
+                            'rules': [
+                                {
+                                    'if': 'idf_version < 3.0'
+                                },
+                            ]
+                        },
+                    }
+                }
+            }
+        },
+    ],
+    indirect=True)
+def test_inject_requirements_with_optional_dependency(project):
+    res = project_action(project, 'reconfigure')
+    assert 'Skipping optional dependency: cmp' in res
+    assert '[1/1] idf' in res
+    assert 'cmake failed with exit code 1' not in res
diff --git a/tests/manifest/test_solved_manifest.py b/tests/manifest/test_solved_manifest.py
@@ -49,9 +49,6 @@ def test_solve_optional_dependency(self, monkeypatch, release_component_path):
             }
         }
         manifest = manifest_manager.load()
-        assert len(manifest.dependencies) == 3
-        for dependency in manifest.dependencies:
-            if dependency.name == 'espressif/foo':
-                assert not dependency.meet_optional_dependencies
-            else:
-                assert dependency.meet_optional_dependencies
+        assert len(manifest.dependencies) == 2
+        assert manifest.dependencies[0].name == 'espressif/pest'
+        assert manifest.dependencies[1].name == 'espressif/test'