From fc0a4e22011fc4e852dc05b1433fbbc398434db7 Mon Sep 17 00:00:00 2001 From: Jonathan Swoboda <154711427+swoboda1337@users.noreply.github.com> Date: Mon, 25 May 2026 17:07:35 -0400 Subject: [PATCH] [espidf] Support github:// and https://github.com/.../.git framework sources (#16639) --- esphome/espidf/framework.py | 115 +++++++++++++--- esphome/git.py | 5 +- tests/unit_tests/test_espidf_framework.py | 156 ++++++++++++++++++++++ 3 files changed, 256 insertions(+), 20 deletions(-) create mode 100644 tests/unit_tests/test_espidf_framework.py diff --git a/esphome/espidf/framework.py b/esphome/espidf/framework.py index 079c97cc98..fb53066edb 100644 --- a/esphome/espidf/framework.py +++ b/esphome/espidf/framework.py @@ -8,6 +8,7 @@ import logging import os from pathlib import Path import platform +import re import shutil import subprocess import sys @@ -784,6 +785,77 @@ def download_from_mirrors( return None +_GITHUB_SHORTHAND_RE = re.compile( + r"^github://([a-zA-Z0-9\-]+)/([a-zA-Z0-9\-\._]+?)(?:@([a-zA-Z0-9\-_.\./]+))?$" +) +_GITHUB_HTTPS_RE = re.compile( + r"^(https://github\.com/[a-zA-Z0-9\-]+/[a-zA-Z0-9\-\._]+?\.git)(?:@([a-zA-Z0-9\-_.\./]+))?$" +) + + +def _parse_git_source(source_url: str) -> tuple[str, str | None] | None: + """Return ``(url, ref)`` for ``github://owner/repo[@ref]`` or + ``https://github.com/owner/repo.git[@ref]``, else ``None``.""" + if m := _GITHUB_SHORTHAND_RE.match(source_url): + owner, repo, ref = m.group(1), m.group(2), m.group(3) + # Tolerate a trailing ".git" on the shorthand repo so the + # github://owner/repo.git form doesn't silently become repo.git.git. + repo = repo.removesuffix(".git") + return f"https://github.com/{owner}/{repo}.git", ref + if m := _GITHUB_HTTPS_RE.match(source_url): + return m.group(1), m.group(2) + return None + + +def _clone_idf_with_submodules( + framework_path: Path, git_url: str, ref: str | None +) -> None: + """Shallow-clone ESP-IDF with submodules into ``framework_path``. + + GitHub's archive zip strips submodules, so vendored components + (mbedtls, openthread, esptool, ...) come down empty and CMake fails. + + Uses clone + ``fetch FETCH_HEAD`` + ``reset --hard`` instead of + ``--branch``: ``--branch`` only accepts branch or tag names, but a + user can also point at a commit SHA. The fetch-then-reset pattern + handles branches, tags, and SHAs uniformly (mirrors the approach in + ``esphome.git.clone_or_update``). + """ + from esphome.git import run_git_command + + _LOGGER.info("Cloning ESP-IDF from %s%s", git_url, f"@{ref}" if ref else "") + run_git_command(["git", "clone", "--depth=1", "--", git_url, str(framework_path)]) + if ref: + run_git_command( + ["git", "fetch", "--depth=1", "--", "origin", ref], + git_dir=framework_path, + ) + run_git_command( + ["git", "reset", "--hard", "FETCH_HEAD"], + git_dir=framework_path, + ) + run_git_command( + [ + "git", + "submodule", + "update", + "--init", + "--recursive", + "--depth=1", + ], + git_dir=framework_path, + ) + + # Sanity-check the resulting tree. run_git_command only raises when + # stderr is non-empty, so a clone that silently produces no working + # tree would otherwise be marked extracted and stuck until + # ``esphome clean``. + if not (framework_path / "tools" / "idf_tools.py").is_file(): + raise RuntimeError( + f"Clone of {git_url} produced no usable ESP-IDF tree at {framework_path}" + ) + + def _write_idf_version_txt(framework_path: Path, version: str) -> None: """Write /version.txt if missing. @@ -939,27 +1011,34 @@ def _check_esphome_idf_framework_install( if install: rmdir(framework_path, msg=f"Clean up ESP-IDF {version} framework") - # Download in temporary file - with tempfile.NamedTemporaryFile() as tmp: - _LOGGER.info("Downloading ESP-IDF %s framework ...", version) + git_source = _parse_git_source(source_url) if source_url else None + if git_source is not None: + git_url, ref = git_source + _clone_idf_with_submodules(framework_path, git_url, ref) + else: + # Download in temporary file + with tempfile.NamedTemporaryFile() as tmp: + _LOGGER.info("Downloading ESP-IDF %s framework ...", version) - # Create substitutions for the URLs - substitutions = {"VERSION": version} - try: - ver = Version.parse(version) - substitutions["MAJOR"] = str(ver.major) - substitutions["MINOR"] = str(ver.minor) - substitutions["PATCH"] = str(ver.patch) - substitutions["EXTRA"] = ver.extra - except ValueError: - pass + # Create substitutions for the URLs + substitutions = {"VERSION": version} + try: + ver = Version.parse(version) + substitutions["MAJOR"] = str(ver.major) + substitutions["MINOR"] = str(ver.minor) + substitutions["PATCH"] = str(ver.patch) + substitutions["EXTRA"] = ver.extra + except ValueError: + pass - mirrors = [source_url] if source_url else ESPHOME_IDF_FRAMEWORK_MIRRORS - download_from_mirrors(mirrors, substitutions, tmp.file) + mirrors = [source_url] if source_url else ESPHOME_IDF_FRAMEWORK_MIRRORS + download_from_mirrors(mirrors, substitutions, tmp.file) - _LOGGER.info("Extracting ESP-IDF %s framework ...", version) - archive_extract_all(tmp.file, framework_path, progress_header="Extracting") - extracted_marker.touch() + _LOGGER.info("Extracting ESP-IDF %s framework ...", version) + archive_extract_all( + tmp.file, framework_path, progress_header="Extracting" + ) + extracted_marker.touch() # Idempotent post-extract patch: written every invocation so a build # dir extracted before this fix gets the file too, without forcing a diff --git a/esphome/git.py b/esphome/git.py index 0106f24845..f36bd559ef 100644 --- a/esphome/git.py +++ b/esphome/git.py @@ -72,8 +72,9 @@ def run_git_command(cmd: list[str], git_dir: Path | None = None) -> str: ) except FileNotFoundError as err: raise GitNotInstalledError( - "git is not installed but required for external_components.\n" - "Please see https://git-scm.com/book/en/v2/Getting-Started-Installing-Git for installing git" + "git is not installed. See " + "https://git-scm.com/book/en/v2/Getting-Started-Installing-Git " + "for installation instructions." ) from err if ret.returncode != 0 and ret.stderr: diff --git a/tests/unit_tests/test_espidf_framework.py b/tests/unit_tests/test_espidf_framework.py new file mode 100644 index 0000000000..9f4e4fcca8 --- /dev/null +++ b/tests/unit_tests/test_espidf_framework.py @@ -0,0 +1,156 @@ +"""Tests for esphome.espidf.framework helpers.""" + +# pylint: disable=protected-access + +from pathlib import Path +from unittest.mock import patch + +import pytest + +from esphome.espidf.framework import _clone_idf_with_submodules, _parse_git_source + + +@pytest.mark.parametrize( + ("source", "expected"), + [ + # github:// shorthand + ( + "github://espressif/esp-idf", + ("https://github.com/espressif/esp-idf.git", None), + ), + ( + "github://espressif/esp-idf@master", + ("https://github.com/espressif/esp-idf.git", "master"), + ), + ( + "github://espressif/esp-idf@release/v6.0", + ("https://github.com/espressif/esp-idf.git", "release/v6.0"), + ), + # explicit https://github.com/...git URL + ( + "https://github.com/espressif/esp-idf.git", + ("https://github.com/espressif/esp-idf.git", None), + ), + ( + "https://github.com/espressif/esp-idf.git@master", + ("https://github.com/espressif/esp-idf.git", "master"), + ), + ( + "https://github.com/espressif/esp-idf.git@v6.0.1", + ("https://github.com/espressif/esp-idf.git", "v6.0.1"), + ), + # Tolerate a trailing ".git" on the shorthand so the user doesn't + # silently end up with a doubled "...esp-idf.git.git" URL. + ( + "github://espressif/esp-idf.git", + ("https://github.com/espressif/esp-idf.git", None), + ), + ( + "github://espressif/esp-idf.git@master", + ("https://github.com/espressif/esp-idf.git", "master"), + ), + ], +) +def test_parse_git_source_recognized( + source: str, expected: tuple[str, str | None] +) -> None: + assert _parse_git_source(source) == expected + + +@pytest.mark.parametrize( + "source", + [ + # archive URLs fall through to the existing download path + "https://github.com/espressif/esp-idf/archive/refs/heads/master.zip", + "https://dl.espressif.com/dl/esp-idf/v6.0.1/esp-idf-v6.0.1.zip", + "https://github.com/esphome-libs/esp-idf/releases/download/v5.5.4/esp-idf-v5.5.4.tar.xz", + # SSH and other git protocols are intentionally rejected — match + # external_components, which only recognizes github:// + structured + # dicts for these. + "git@github.com:espressif/esp-idf.git", + "ssh://git@github.com/espressif/esp-idf.git", + "git://github.com/espressif/esp-idf.git", + # non-GitHub .git URLs are intentionally rejected for the same reason + "https://gitlab.com/foo/bar.git", + "https://github.example.com/foo/bar.git", + ], +) +def test_parse_git_source_rejected(source: str) -> None: + assert _parse_git_source(source) is None + + +def _make_idf_tree(framework_path: Path) -> None: + """Create the minimum tree _clone_idf_with_submodules sanity-checks for.""" + (framework_path / "tools").mkdir(parents=True) + (framework_path / "tools" / "idf_tools.py").write_text("# stub\n") + + +def test_clone_idf_with_submodules_without_ref(tmp_path: Path) -> None: + framework_path = tmp_path / "idf" + framework_path.mkdir() + _make_idf_tree(framework_path) + + with patch("esphome.git.run_git_command", return_value="") as run_git_command_mock: + _clone_idf_with_submodules( + framework_path, "https://github.com/espressif/esp-idf.git", None + ) + + # No ref -> just clone + submodule update, no fetch/reset. + calls = [c.args[0] for c in run_git_command_mock.call_args_list] + assert calls[0] == [ + "git", + "clone", + "--depth=1", + "--", + "https://github.com/espressif/esp-idf.git", + str(framework_path), + ] + assert calls[-1][:5] == ["git", "submodule", "update", "--init", "--recursive"] + assert not any(c[1] == "fetch" for c in calls) + assert not any(c[1] == "reset" for c in calls) + + +def test_clone_idf_with_submodules_with_ref(tmp_path: Path) -> None: + framework_path = tmp_path / "idf" + framework_path.mkdir() + _make_idf_tree(framework_path) + + with patch("esphome.git.run_git_command", return_value="") as run_git_command_mock: + _clone_idf_with_submodules( + framework_path, + "https://github.com/espressif/esp-idf.git", + "master", + ) + + calls = [c.args[0] for c in run_git_command_mock.call_args_list] + # clone, fetch ref, reset hard, submodule update + assert calls[0][:2] == ["git", "clone"] + assert calls[1] == [ + "git", + "fetch", + "--depth=1", + "--", + "origin", + "master", + ] + assert calls[2] == ["git", "reset", "--hard", "FETCH_HEAD"] + assert calls[3][:5] == ["git", "submodule", "update", "--init", "--recursive"] + + +def test_clone_idf_with_submodules_raises_when_tree_missing( + tmp_path: Path, +) -> None: + framework_path = tmp_path / "idf" + framework_path.mkdir() + # Deliberately do NOT call _make_idf_tree — simulate a clone that + # returned 0 but produced no tools/idf_tools.py. + + with ( + patch("esphome.git.run_git_command", return_value=""), + pytest.raises(RuntimeError, match="no usable ESP-IDF tree"), + ): + _clone_idf_with_submodules( + framework_path, + "https://github.com/espressif/esp-idf.git", + None, + )