Factor PlatformIO buildgen out of writer.py (#9378)

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Jesse Hills <3060199+jesserockz@users.noreply.github.com>

esphome/__main__.py

@@ -34,6 +34,7 @@ from esphome.const import (
     CONF_PORT,
     CONF_SUBSTITUTIONS,
     CONF_TOPIC,
+    ENV_NOGITIGNORE,
     PLATFORM_ESP32,
     PLATFORM_ESP8266,
     PLATFORM_RP2040,
@@ -209,6 +210,9 @@ def wrap_to_code(name, comp):
 
 
 def write_cpp(config):
+    if not get_bool_env(ENV_NOGITIGNORE):
+        writer.write_gitignore()
+
     generate_cpp_contents(config)
     return write_cpp_file()
 
@@ -225,10 +229,13 @@ def generate_cpp_contents(config):
 
 
 def write_cpp_file():
-    writer.write_platformio_project()
-
     code_s = indent(CORE.cpp_main_section)
     writer.write_cpp(code_s)
+
+    from esphome.build_gen import platformio
+
+    platformio.write_project()
+
     return 0
 
 

esphome/build_gen/platformio.py

@@ -0,0 +1,102 @@
+import os
+
+from esphome.const import __version__
+from esphome.core import CORE
+from esphome.helpers import mkdir_p, read_file, write_file_if_changed
+from esphome.writer import find_begin_end, update_storage_json
+
+INI_AUTO_GENERATE_BEGIN = "; ========== AUTO GENERATED CODE BEGIN ==========="
+INI_AUTO_GENERATE_END = "; =========== AUTO GENERATED CODE END ============"
+
+INI_BASE_FORMAT = (
+    """; Auto generated code by esphome
+
+[common]
+lib_deps =
+build_flags =
+upload_flags =
+
+""",
+    """
+
+""",
+)
+
+
+def format_ini(data: dict[str, str | list[str]]) -> str:
+    content = ""
+    for key, value in sorted(data.items()):
+        if isinstance(value, list):
+            content += f"{key} =\n"
+            for x in value:
+                content += f"    {x}\n"
+        else:
+            content += f"{key} = {value}\n"
+    return content
+
+
+def get_ini_content():
+    CORE.add_platformio_option(
+        "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))
+
+    # Sort to avoid changing build unflags order
+    CORE.add_platformio_option("build_unflags", sorted(CORE.build_unflags))
+
+    # Add extra script for C++ flags
+    CORE.add_platformio_option("extra_scripts", [f"pre:{CXX_FLAGS_FILE_NAME}"])
+
+    content = "[platformio]\n"
+    content += f"description = ESPHome {__version__}\n"
+
+    content += f"[env:{CORE.name}]\n"
+    content += format_ini(CORE.platformio_options)
+
+    return content
+
+
+def write_ini(content):
+    update_storage_json()
+    path = CORE.relative_build_path("platformio.ini")
+
+    if os.path.isfile(path):
+        text = read_file(path)
+        content_format = find_begin_end(
+            text, INI_AUTO_GENERATE_BEGIN, INI_AUTO_GENERATE_END
+        )
+    else:
+        content_format = INI_BASE_FORMAT
+    full_file = f"{content_format[0] + INI_AUTO_GENERATE_BEGIN}\n{content}"
+    full_file += INI_AUTO_GENERATE_END + content_format[1]
+    write_file_if_changed(path, full_file)
+
+
+def write_project():
+    mkdir_p(CORE.build_path)
+
+    content = get_ini_content()
+    write_ini(content)
+
+    # Write extra script for C++ specific flags
+    write_cxx_flags_script()
+
+
+CXX_FLAGS_FILE_NAME = "cxx_flags.py"
+CXX_FLAGS_FILE_CONTENTS = """# Auto-generated ESPHome script for C++ specific compiler flags
+Import("env")
+
+# Add C++ specific flags
+"""
+
+
+def write_cxx_flags_script() -> None:
+    path = CORE.relative_build_path(CXX_FLAGS_FILE_NAME)
+    contents = CXX_FLAGS_FILE_CONTENTS
+    if not CORE.is_host:
+        contents += 'env.Append(CXXFLAGS=["-Wno-volatile"])'
+        contents += "\n"
+    write_file_if_changed(path, contents)

esphome/core/__init__.py

@@ -470,6 +470,52 @@ 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
+        if self.repository is None and other.repository is None:
+            pass  # No repositories, no conflict, continue on
+
+        elif self.repository is None:
+            # incoming library has a repository, use it
+            self.repository = other.repository
+            self.version = other.version
+            return self
+
+        elif other.repository is None:
+            return self  # use the repository/version already present
+
+        elif self.repository != other.repository:
+            raise ValueError(
+                f"Reconciliation failed! Libraries {self} and {other} requested with conflicting repositories!"
+            )
+
+        if self.version is None and other.version is None:
+            return self  # Arduino library reconciled against another Arduino library, current is acceptable
+
+        if self.version is None:
+            # incoming library has a version, use it
+            self.version = other.version
+            return self
+
+        if other.version is None:
+            return self  # incoming library has no version, current is acceptable
+
+            # Same versions, current library is acceptable
+        if self.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:
@@ -505,8 +551,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
+        # A map of platformio libraries to add to the project (shortname: (name, version, repository))
-        self.libraries: list[Library] = []
+        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 build unflags to set in the platformio project
@@ -550,7 +596,7 @@ class EsphomeCore:
         self.variables = {}
         self.main_statements = []
         self.global_statements = []
-        self.libraries = []
+        self.platformio_libraries = {}
         self.build_flags = set()
         self.build_unflags = set()
         self.defines = set()
@@ -738,54 +784,22 @@ 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.libraries[:]:
+        short_name = (
-            if other.name is None or library.name is None:
+            library.name if "/" not in library.name else library.name.split("/")[-1]
-                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!"
-                )
 
-            if library.repository is not None:
+        if short_name not in self.platformio_libraries:
-                # This is more specific since its using a repository
-                self.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.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:
             _LOGGER.debug("Adding library: %s", library)
-            self.libraries.append(library)
+            self.platformio_libraries[short_name] = library
-        return library
+            return library
+
+        self.platformio_libraries[short_name].reconcile_with(library)
+        return self.platformio_libraries[short_name]
 
     def add_build_flag(self, build_flag: str) -> str:
         self.build_flags.add(build_flag)

esphome/writer.py

@@ -7,7 +7,6 @@ import re
 from esphome import loader
 from esphome.config import iter_component_configs, iter_components
 from esphome.const import (
-    ENV_NOGITIGNORE,
     HEADER_FILE_EXTENSIONS,
     PLATFORM_ESP32,
     SOURCE_FILE_EXTENSIONS,
@@ -16,8 +15,6 @@ from esphome.const import (
 from esphome.core import CORE, EsphomeError
 from esphome.helpers import (
     copy_file_if_changed,
-    get_bool_env,
-    mkdir_p,
     read_file,
     walk_files,
     write_file_if_changed,
@@ -30,8 +27,6 @@ CPP_AUTO_GENERATE_BEGIN = "// ========== AUTO GENERATED CODE BEGIN ==========="
 CPP_AUTO_GENERATE_END = "// =========== AUTO GENERATED CODE END ============"
 CPP_INCLUDE_BEGIN = "// ========== AUTO GENERATED INCLUDE BLOCK BEGIN ==========="
 CPP_INCLUDE_END = "// ========== AUTO GENERATED INCLUDE BLOCK END ==========="
-INI_AUTO_GENERATE_BEGIN = "; ========== AUTO GENERATED CODE BEGIN ==========="
-INI_AUTO_GENERATE_END = "; =========== AUTO GENERATED CODE END ============"
 
 CPP_BASE_FORMAT = (
     """// Auto generated code by esphome
@@ -50,20 +45,6 @@ void loop() {
 """,
 )
 
-INI_BASE_FORMAT = (
-    """; Auto generated code by esphome
-
-[common]
-lib_deps =
-build_flags =
-upload_flags =
-
-""",
-    """
-
-""",
-)
-
 UPLOAD_SPEED_OVERRIDE = {
     "esp210": 57600,
 }
@@ -140,40 +121,6 @@ def update_storage_json():
     new.save(path)
 
 
-def format_ini(data: dict[str, str | list[str]]) -> str:
-    content = ""
-    for key, value in sorted(data.items()):
-        if isinstance(value, list):
-            content += f"{key} =\n"
-            for x in value:
-                content += f"    {x}\n"
-        else:
-            content += f"{key} = {value}\n"
-    return content
-
-
-def get_ini_content():
-    CORE.add_platformio_option(
-        "lib_deps", [x.as_lib_dep for x in CORE.libraries] + ["${common.lib_deps}"]
-    )
-    # Sort to avoid changing build flags order
-    CORE.add_platformio_option("build_flags", sorted(CORE.build_flags))
-
-    # Sort to avoid changing build unflags order
-    CORE.add_platformio_option("build_unflags", sorted(CORE.build_unflags))
-
-    # Add extra script for C++ flags
-    CORE.add_platformio_option("extra_scripts", [f"pre:{CXX_FLAGS_FILE_NAME}"])
-
-    content = "[platformio]\n"
-    content += f"description = ESPHome {__version__}\n"
-
-    content += f"[env:{CORE.name}]\n"
-    content += format_ini(CORE.platformio_options)
-
-    return content
-
-
 def find_begin_end(text, begin_s, end_s):
     begin_index = text.find(begin_s)
     if begin_index == -1:
@@ -201,34 +148,6 @@ def find_begin_end(text, begin_s, end_s):
     return text[:begin_index], text[(end_index + len(end_s)) :]
 
 
-def write_platformio_ini(content):
-    update_storage_json()
-    path = CORE.relative_build_path("platformio.ini")
-
-    if os.path.isfile(path):
-        text = read_file(path)
-        content_format = find_begin_end(
-            text, INI_AUTO_GENERATE_BEGIN, INI_AUTO_GENERATE_END
-        )
-    else:
-        content_format = INI_BASE_FORMAT
-    full_file = f"{content_format[0] + INI_AUTO_GENERATE_BEGIN}\n{content}"
-    full_file += INI_AUTO_GENERATE_END + content_format[1]
-    write_file_if_changed(path, full_file)
-
-
-def write_platformio_project():
-    mkdir_p(CORE.build_path)
-
-    content = get_ini_content()
-    if not get_bool_env(ENV_NOGITIGNORE):
-        write_gitignore()
-    write_platformio_ini(content)
-
-    # Write extra script for C++ specific flags
-    write_cxx_flags_script()
-
-
 DEFINES_H_FORMAT = ESPHOME_H_FORMAT = """\
 #pragma once
 #include "esphome/core/macros.h"
@@ -400,20 +319,3 @@ def write_gitignore():
     if not os.path.isfile(path):
         with open(file=path, mode="w", encoding="utf-8") as f:
             f.write(GITIGNORE_CONTENT)
-
-
-CXX_FLAGS_FILE_NAME = "cxx_flags.py"
-CXX_FLAGS_FILE_CONTENTS = """# Auto-generated ESPHome script for C++ specific compiler flags
-Import("env")
-
-# Add C++ specific flags
-"""
-
-
-def write_cxx_flags_script() -> None:
-    path = CORE.relative_build_path(CXX_FLAGS_FILE_NAME)
-    contents = CXX_FLAGS_FILE_CONTENTS
-    if not CORE.is_host:
-        contents += 'env.Append(CXXFLAGS=["-Wno-volatile"])'
-        contents += "\n"
-    write_file_if_changed(path, contents)

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