From a87acba58ce72a25673644070ca0886acfcb5537 Mon Sep 17 00:00:00 2001 From: Katherine Whitlock Date: Fri, 17 Jan 2025 17:55:46 -0500 Subject: [PATCH] Change platformio_libraries to a dict, mark non-repository form as deprecated --- esphome/build_gen/platformio.py | 3 +- esphome/core/__init__.py | 108 +++++++++++++++++++------------- esphome/cpp_generator.py | 8 +++ tests/unit_tests/test_core.py | 55 ++++++++++++++++ 4 files changed, 128 insertions(+), 46 deletions(-) diff --git a/esphome/build_gen/platformio.py b/esphome/build_gen/platformio.py index 45b9c43e11..d2ddb919d7 100644 --- a/esphome/build_gen/platformio.py +++ b/esphome/build_gen/platformio.py @@ -39,7 +39,8 @@ def format_ini(data: dict[str, Union[str, list[str]]]) -> str: def get_ini_content(): CORE.add_platformio_option( "lib_deps", - [x.as_lib_dep for x in CORE.platformio_libraries] + ["${common.lib_deps}"], + [x.as_lib_dep for x in CORE.platformio_libraries.values()] + + ["${common.lib_deps}"], ) # Sort to avoid changing build flags order CORE.add_platformio_option("build_flags", sorted(CORE.build_flags)) diff --git a/esphome/core/__init__.py b/esphome/core/__init__.py index af041809b0..0e78960a00 100644 --- a/esphome/core/__init__.py +++ b/esphome/core/__init__.py @@ -467,6 +467,57 @@ class Library: return self.as_tuple == other.as_tuple return NotImplemented + def reconcile_with(self, other): + """Merge two libraries, reconciling any conflicts.""" + + if self.name != other.name: + # Different libraries, no reconciliation possible + raise ValueError( + f"Cannot reconcile libraries with different names: {self.name} and {other.name}" + ) + + # repository specificity takes precedence over version specificity + match (self.repository, other.repository): + case (None, None): + pass # No repositories, no conflict, continue on + + case (None, _): + # incoming library has a repository, use it + self.repository = other.repository + self.version = other.version + return self + + case (_, None): + return self # use the repository/version already present + + case (this_repo, other_repo): + if this_repo != other_repo: + raise ValueError( + f"Reconciliation failed! Libraries {self} and {other} requested with conflicting repositories!" + ) + pass # Repositories match, continue on + + match (self.version, other.version): + case (None, None): + return self # Arduino library reconciled against another Arduino library, current is acceptable + + case (None, _): + # incoming library has a version, use it + self.version = other.version + return self + + case (_, None): + return self # incoming library has no version, current is acceptable + + # Same versions, current library is acceptable + case (this_version, other_version): + if this_version != other_version: + raise ValueError( + f"Version pinning failed! Libraries {other} and {self} " + "requested with conflicting versions!" + ) + return self + # pylint: disable=too-many-public-methods class EsphomeCore: @@ -503,8 +554,8 @@ class EsphomeCore: self.main_statements: list[Statement] = [] # A list of statements to insert in the global block (includes and global variables) self.global_statements: list[Statement] = [] - # A set of platformio libraries to add to the project - self.platformio_libraries: list[Library] = [] + # A map of platformio libraries to add to the project (shortname: (name, version, repository)) + self.platformio_libraries: dict[str, Library] = {} # A set of build flags to set in the platformio project self.build_flags: set[str] = set() # A set of defines to set for the compile process in esphome/core/defines.h @@ -705,54 +756,21 @@ class EsphomeCore: _LOGGER.debug("Adding global: %s", expression) return expression - def add_library(self, library): + def add_library(self, library: Library): if not isinstance(library, Library): - raise ValueError( + raise TypeError( f"Library {library} must be instance of Library, not {type(library)}" ) - for other in self.platformio_libraries[:]: - if other.name is None or library.name is None: - continue - library_name = ( - library.name if "/" not in library.name else library.name.split("/")[1] - ) - other_name = ( - other.name if "/" not in other.name else other.name.split("/")[1] - ) - if other_name != library_name: - continue - if other.repository is not None: - if library.repository is None or other.repository == library.repository: - # Other is using a/the same repository, takes precedence - break - raise ValueError( - f"Adding named Library with repository failed! Libraries {library} and {other} " - "requested with conflicting repositories!" - ) + short_name = ( + library.name if "/" not in library.name else library.name.split("/")[-1] + ) - if library.repository is not None: - # This is more specific since its using a repository - self.platformio_libraries.remove(other) - continue - - if library.version is None: - # Other requirement is more specific - break - if other.version is None: - # Found more specific version requirement - self.platformio_libraries.remove(other) - continue - if other.version == library.version: - break - - raise ValueError( - f"Version pinning failed! Libraries {library} and {other} " - "requested with conflicting versions!" - ) - else: + if short_name not in self.platformio_libraries: _LOGGER.debug("Adding library: %s", library) - self.platformio_libraries.append(library) - return library + self.platformio_libraries[short_name] = library + return library + + self.platformio_libraries[short_name].reconcile_with(library) def add_build_flag(self, build_flag): self.build_flags.add(build_flag) diff --git a/esphome/cpp_generator.py b/esphome/cpp_generator.py index 4e283868e1..a194f7bbf5 100644 --- a/esphome/cpp_generator.py +++ b/esphome/cpp_generator.py @@ -4,6 +4,7 @@ import inspect import math import re from typing import Any, Callable, Optional, Union +import warnings from esphome.core import ( CORE, @@ -600,6 +601,13 @@ def add_library(name: str, version: Optional[str], repository: Optional[str] = N :param version: The version of the library, may be None. :param repository: The repository for the library """ + if repository is None: + warnings.warn( + "Auto-resolution of the repository of a PlatformIO libraries is deprecated, " + "please use add_library() with an explicit repository instead.", + DeprecationWarning, + stacklevel=2, + ) CORE.add_library(Library(name, version, repository)) diff --git a/tests/unit_tests/test_core.py b/tests/unit_tests/test_core.py index 4f2a6453b4..f7dda9fb95 100644 --- a/tests/unit_tests/test_core.py +++ b/tests/unit_tests/test_core.py @@ -473,6 +473,61 @@ class TestLibrary: assert actual == expected + @pytest.mark.parametrize( + "target, other, result, exception", + ( + (core.Library("libfoo", None), core.Library("libfoo", None), True, None), + ( + core.Library("libfoo", "1.2.3"), + core.Library("libfoo", "1.2.3"), + True, # target is unchanged + None, + ), + ( + core.Library("libfoo", None), + core.Library("libfoo", "1.2.3"), + False, # Use version from other + None, + ), + ( + core.Library("libfoo", "1.2.3"), + core.Library("libfoo", "1.2.4"), + False, + ValueError, # Version mismatch + ), + ( + core.Library("libfoo", "1.2.3"), + core.Library("libbar", "1.2.3"), + False, + ValueError, # Name mismatch + ), + ( + core.Library( + "libfoo", "1.2.4", "https://github.com/esphome/ESPAsyncWebServer" + ), + core.Library("libfoo", "1.2.3"), + True, # target is unchanged due to having a repository + None, + ), + ( + core.Library("libfoo", "1.2.3"), + core.Library( + "libfoo", "1.2.4", "https://github.com/esphome/ESPAsyncWebServer" + ), + False, # use other due to having a repository + None, + ), + ), + ) + def test_reconcile(self, target, other, result, exception): + if exception is not None: + with pytest.raises(exception): + target.reconcile_with(other) + else: + expected = target if result else other + actual = target.reconcile_with(other) + assert actual == expected + class TestEsphomeCore: @pytest.fixture