github/esphome
2
0
Fork 0
mirror of https://github.com/esphome/esphome.git synced 2026-08-22 22:26:21 +00:00
Code Issues Packages Projects Releases Wiki Activity

[git] Fix submodule update failure when cloning libraries on the esp-idf toolchain (#17862)

Browse Source
This commit is contained in:
J. Nick Koston
2026-07-27 19:42:21 +12:00
committed by GitHub
parent 8f321bf346
commit 43526a815e
5 changed files with 614 additions and 141 deletions
Download Patch File Download Diff File Expand all files Collapse all files
esphome/espidf/framework.py
+8 -18
View File
@@ -397,9 +397,10 @@ def _clone_idf_with_submodules(
handles branches, tags, and SHAs uniformly (mirrors the approach in
``esphome.git.clone_or_update``).
"""
from esphome.git import run_git_command
from esphome.git import run_git_command, update_submodules
_LOGGER.info("Cloning ESP-IDF from %s%s", git_url, f"@{ref}" if ref else "")
key = f"{git_url}@{ref}" if ref else git_url
_LOGGER.info("Cloning ESP-IDF from %s", key)
run_git_command(["git", "clone", "--depth=1", "--", git_url, str(framework_path)])
if ref:
run_git_command(
@@ -410,25 +411,14 @@ def _clone_idf_with_submodules(
["git", "reset", "--hard", "FETCH_HEAD"],
git_dir=framework_path,
)
run_git_command(
[
"git",
"submodule",
"update",
"--init",
"--recursive",
"--depth=1",
],
git_dir=framework_path,
)
update_submodules(framework_path, key)
# 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``.
# Sanity-check the resulting tree: a clone can exit 0 yet produce no
# usable ESP-IDF checkout, which 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}"
f"Clone of {key} produced no usable ESP-IDF tree at {framework_path}"
)
esphome/git.py
+132 -62
View File
@@ -2,6 +2,7 @@ from collections.abc import Callable
from dataclasses import dataclass
import hashlib
import logging
import os
from pathlib import Path
import re
import subprocess
@@ -11,7 +12,7 @@ import urllib.parse
import esphome.config_validation as cv
from esphome.core import CORE, EsphomeError, TimePeriodSeconds
from esphome.helpers import rmtree, write_file
from esphome.helpers import add_git_ceiling_directory, rmtree, write_file
_LOGGER = logging.getLogger(__name__)
@@ -26,6 +27,24 @@ NEVER_REFRESH = TimePeriodSeconds(seconds=-1)
# it does not pollute the worktree.
_CLONE_COMPLETE_MARKER = "esphome_clone_complete"
# Environment variables that scope git to a specific repository. Git hooks and
# some CI wrappers export these; if they leak into the git commands run here,
# git binds to the caller's repository instead of the one being managed. The
# effects range from loud (`git clone` producing a bare-style directory with
# no working tree) to silent (an ambient GIT_INDEX_FILE makes
# `git submodule update --init` exit 0 without initializing anything).
_GIT_REPO_SCOPING_ENV = frozenset(
{
"GIT_DIR",
"GIT_WORK_TREE",
"GIT_INDEX_FILE",
"GIT_OBJECT_DIRECTORY",
"GIT_ALTERNATE_OBJECT_DIRECTORIES",
"GIT_COMMON_DIR",
"GIT_NAMESPACE",
}
)
class GitException(cv.Invalid):
"""Base exception for git-related errors."""
@@ -43,32 +62,61 @@ class GitRepositoryError(GitException):
"""Exception raised when a git repository is in an invalid state."""
def run_git_command(cmd: list[str], git_dir: Path | None = None) -> str:
if git_dir is not None:
_LOGGER.debug(
"Running git command with repository isolation: %s (git_dir=%s)",
" ".join(cmd),
git_dir,
)
else:
_LOGGER.debug("Running git command: %s", " ".join(cmd))
def _redact_url_credentials(text: str) -> str:
"""Mask userinfo in any URLs embedded in ``text``.
# Set up environment for repository isolation if git_dir is provided
# Force git to only operate on this specific repository by setting
# GIT_DIR and GIT_WORK_TREE. This prevents git from walking up the
# directory tree to find parent repositories when the target repo's
# .git directory is corrupt. Without this, commands like 'git stash'
# could accidentally operate on parent repositories (e.g., the main
# ESPHome repo) instead of failing, causing data loss.
env: dict[str, str] | None = None
cwd: str | None = None
Users can put credentials directly in a git URL, and log output is
routinely pasted into public issues.
"""
return re.sub(r"://[^/@\s]+@", "://***@", text)
def run_git_command(
cmd: list[str], git_dir: Path | None = None, *, cwd: Path | None = None
) -> str:
"""Run a git command and return its stdout.
The repository-scoping environment variables in ``_GIT_REPO_SCOPING_ENV``
are always stripped. ``git_dir`` additionally pins GIT_DIR/GIT_WORK_TREE
to that repository and runs the command there; ``cwd`` alone runs the
command in that directory with GIT_CEILING_DIRECTORIES capping repository
discovery at its parent.
"""
# Every invocation starts from an environment with the repository-scoping
# variables stripped (see _GIT_REPO_SCOPING_ENV) so a git hook or CI
# wrapper invoking ESPHome can never redirect these commands to its own
# repository or index.
#
# ``git_dir`` then re-adds GIT_DIR and GIT_WORK_TREE pointing at the
# managed repository. This prevents git from walking up the directory
# tree to find parent repositories when the target repo's .git directory
# is corrupt. Without this, commands like 'git stash' could accidentally
# operate on parent repositories (e.g., the main ESPHome repo) instead of
# failing, causing data loss.
#
# ``cwd`` (without ``git_dir``) runs the command in that directory
# without GIT_DIR/GIT_WORK_TREE. The ``git submodule`` porcelain needs
# this: on some installations (e.g. Windows setups where a shim hands
# git untranslated paths) it refuses to run when GIT_DIR/GIT_WORK_TREE
# are set, failing with "cannot be used without a working tree".
# GIT_CEILING_DIRECTORIES (which git only honors as an absolute path)
# keeps the parent-repo-walk protection instead: if the repo's .git is
# missing or corrupt, git fails rather than discovering an enclosing
# repository.
env = {k: v for k, v in os.environ.items() if k not in _GIT_REPO_SCOPING_ENV}
if git_dir is not None:
env = {
**subprocess.os.environ,
"GIT_DIR": str(Path(git_dir) / ".git"),
"GIT_WORK_TREE": str(git_dir),
}
cwd = str(git_dir)
env["GIT_DIR"] = str(Path(git_dir) / ".git")
env["GIT_WORK_TREE"] = str(git_dir)
cwd = git_dir
elif cwd is not None:
add_git_ceiling_directory(env, Path(cwd).absolute().parent)
_LOGGER.debug(
"Running git command: %s (cwd=%s, isolated=%s)",
_redact_url_credentials(" ".join(cmd)),
cwd,
git_dir is not None,
)
try:
ret = subprocess.run(
@@ -86,12 +134,17 @@ def run_git_command(cmd: list[str], git_dir: Path | None = None) -> str:
"for installation instructions."
) from err
if ret.returncode != 0 and ret.stderr:
err_str = ret.stderr.decode("utf-8")
lines = [x.strip() for x in err_str.splitlines()]
if lines[-1].startswith("fatal:"):
raise GitCommandError(lines[-1][len("fatal: ") :])
raise GitCommandError(err_str)
if ret.returncode != 0:
if ret.stderr:
err_str = ret.stderr.decode("utf-8")
lines = [x.strip() for x in err_str.splitlines()]
if lines[-1].startswith("fatal:"):
raise GitCommandError(lines[-1][len("fatal: ") :])
raise GitCommandError(err_str)
raise GitCommandError(
f"git exited with code {ret.returncode}: "
f"{_redact_url_credentials(' '.join(cmd))}"
)
return ret.stdout.decode("utf-8").strip()
@@ -123,6 +176,27 @@ def _remove_repo_dir(repo_dir: Path) -> None:
rmtree(repo_dir)
def update_submodules(repo_dir: Path, key: str) -> None:
"""Initialize/update every submodule the repository declares, recursively,
matching how PlatformIO clones libraries.
Most repositories declare no submodules, so this does nothing when there
is no ``.gitmodules`` file. Which submodules get populated is git's own
policy (``update = none``, ``submodule.active``, sparse checkouts);
git's exit code is the error signal.
Runs with plain ``cwd`` rather than ``git_dir`` isolation, which the
``git submodule`` porcelain does not tolerate (see ``run_git_command``).
"""
if not (repo_dir / ".gitmodules").is_file():
return
_LOGGER.info("Updating submodules for %s", _redact_url_credentials(key))
run_git_command(
["git", "submodule", "update", "--init", "--recursive", "--depth=1"],
cwd=repo_dir,
)
def resolve_symlink_stub(repo_dir: Path, file_path: Path) -> Path | None:
"""Return the symlink target if ``file_path`` is a Windows-checked-out symlink stub.
@@ -217,12 +291,19 @@ def clone_or_update(
domain: str,
username: str = None,
password: str = None,
submodules: list[str] | None = None,
init_submodules: bool = False,
subpath: Path | None = None,
_recover_broken: bool = True,
) -> tuple[Path, Callable[[], None] | None]:
key = f"{url}@{ref}"
# The user may have embedded credentials in the URL itself; log this
# instead of key.
safe_key = _redact_url_credentials(key)
# Keep the caller's URL for the recovery re-clone below: rewriting the
# rewritten URL would double the userinfo, and the recursive call must
# compute the same cache key as this one.
original_url = url
if username is not None and password is not None:
url = url.replace(
"://", f"://{urllib.parse.quote(username)}:{urllib.parse.quote(password)}@"
@@ -238,12 +319,12 @@ def clone_or_update(
# predates the marker; either way it cannot be trusted, especially
# with NEVER_REFRESH where it would otherwise be reused forever.
_LOGGER.warning(
"Removing incomplete clone of %s at %s, will re-clone", key, repo_dir
"Removing incomplete clone of %s at %s, will re-clone", safe_key, repo_dir
)
_remove_repo_dir(repo_dir)
if not repo_dir.is_dir():
_LOGGER.info("Cloning %s", key)
_LOGGER.info("Cloning %s", safe_key)
_LOGGER.debug("Location: %s", repo_dir)
try:
cmd = ["git", "clone", "--depth=1"]
@@ -262,15 +343,8 @@ def clone_or_update(
["git", "reset", "--hard", "FETCH_HEAD"], git_dir=repo_dir
)
if submodules is not None:
_LOGGER.info(
"Initializing submodules (%s) for %s", ", ".join(submodules), key
)
run_git_command(
["git", "submodule", "update", "--init", "--depth=1", "--"]
+ submodules,
git_dir=repo_dir,
)
if init_submodules:
update_submodules(repo_dir, key)
except GitException:
# Remove incomplete clone to prevent stale state. Without this,
@@ -290,12 +364,12 @@ def clone_or_update(
)
except EsphomeError as err:
_LOGGER.warning(
"Could not write clone completion marker for %s: %s", key, err
"Could not write clone completion marker for %s: %s", safe_key, err
)
else:
if refresh == NEVER_REFRESH or CORE.skip_external_update:
_LOGGER.debug("Skipping update for %s (refresh disabled)", key)
_LOGGER.debug("Skipping update for %s (refresh disabled)", safe_key)
return repo_dir, None
file_timestamp = Path(repo_dir / ".git" / "FETCH_HEAD")
@@ -319,7 +393,7 @@ def clone_or_update(
["git", "rev-parse", "HEAD"], git_dir=repo_dir
)
_LOGGER.info("Updating %s", key)
_LOGGER.info("Updating %s", safe_key)
_LOGGER.debug("Location: %s", repo_dir)
# Stash local changes (if any)
@@ -345,19 +419,25 @@ def clone_or_update(
["git", "reset", "--hard", "FETCH_HEAD"],
git_dir=repo_dir,
)
# Inside the try so a submodule failure routes through the
# recovery re-clone below instead of leaving a repo that the
# refresh window would silently accept on the next run.
if init_submodules:
update_submodules(repo_dir, key)
except GitException as err:
# Repository is in a broken state or update failed
# Only attempt recovery once to prevent infinite recursion
if not _recover_broken:
_LOGGER.error(
"Repository %s recovery failed, cannot retry (already attempted once)",
key,
safe_key,
)
raise
_LOGGER.warning(
"Repository %s has issues (%s), attempting recovery",
key,
safe_key,
err,
)
_LOGGER.info("Removing broken repository at %s", repo_dir)
@@ -367,31 +447,21 @@ def clone_or_update(
# Recursively call clone_or_update to re-clone
# Set _recover_broken=False to prevent infinite recursion
result = clone_or_update(
url=url,
url=original_url,
ref=ref,
refresh=refresh,
domain=domain,
username=username,
password=password,
submodules=submodules,
init_submodules=init_submodules,
subpath=subpath,
_recover_broken=False,
)
_LOGGER.info("Repository %s successfully recovered", key)
_LOGGER.info("Repository %s successfully recovered", safe_key)
return result
if submodules is not None:
_LOGGER.info(
"Updating submodules (%s) for %s", ", ".join(submodules), key
)
run_git_command(
["git", "submodule", "update", "--init", "--depth=1", "--"]
+ submodules,
git_dir=repo_dir,
)
def revert():
_LOGGER.info("Reverting changes to %s -> %s", key, old_sha)
_LOGGER.info("Reverting changes to %s -> %s", safe_key, old_sha)
run_git_command(["git", "reset", "--hard", old_sha], git_dir=repo_dir)
return repo_dir, revert
esphome/platformio/library.py
+1 -1
View File
@@ -134,7 +134,7 @@ class GitSource(Source):
ref=self.ref,
refresh=git.NEVER_REFRESH if not force else None,
domain=domain,
submodules=[],
init_submodules=True,
subpath=Path(dir_suffix),
)
return path
tests/unit_tests/test_espidf_framework.py
+31 -2
View File
@@ -137,10 +137,17 @@ 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."""
def _make_idf_tree(framework_path: Path, *, gitmodules: bool = True) -> None:
"""Create the minimum tree _clone_idf_with_submodules sanity-checks for.
``gitmodules=False`` simulates a fork that vendors components in-tree
instead of declaring submodules; update_submodules skips the git call
when that file is missing.
"""
(framework_path / "tools").mkdir(parents=True)
(framework_path / "tools" / "idf_tools.py").write_text("# stub\n")
if gitmodules:
(framework_path / ".gitmodules").write_text("# stub\n")
def test_clone_idf_with_submodules_without_ref(tmp_path: Path) -> None:
@@ -214,6 +221,28 @@ def test_clone_idf_with_submodules_raises_when_tree_missing(
)
def test_clone_idf_accepts_flattened_fork_without_gitmodules(
tmp_path: Path,
) -> None:
"""A fork that vendors components in-tree instead of as submodules is valid.
No .gitmodules means the submodule step is skipped entirely.
"""
framework_path = tmp_path / "idf"
framework_path.mkdir()
_make_idf_tree(framework_path, gitmodules=False)
with patch("esphome.git.run_git_command", return_value="") as run_git_command_mock:
_clone_idf_with_submodules(
framework_path,
"https://github.com/example/flattened-esp-idf.git",
None,
)
calls = [c.args[0] for c in run_git_command_mock.call_args_list]
assert not any(c[1] == "submodule" for c in calls)
# ---------------------------------------------------------------------------
# Helpers for _tar_extract_all hard-link prefix-stripping tests
# ---------------------------------------------------------------------------
tests/unit_tests/test_git.py
+442 -58
View File
@@ -1,8 +1,10 @@
"""Tests for git.py module."""
from collections.abc import Callable
import logging
import os
from pathlib import Path
import subprocess
import time
from typing import Any
from unittest.mock import Mock, patch
@@ -71,19 +73,40 @@ def _simulate_cloned_repo(repo_dir: Path) -> None:
(repo_dir / ".git").mkdir(exist_ok=True)
def _make_clone_side_effect(repo_dir: Path) -> Callable[..., str]:
"""Return a run_git_command side effect whose clone creates the repo dir."""
def _make_clone_side_effect(
repo_dir: Path, gitmodules: bool = False
) -> Callable[..., str]:
"""Return a run_git_command side effect whose clone creates the repo dir.
With ``gitmodules`` the cloned repo also declares submodules.
"""
def git_command_side_effect(
cmd: list[str], cwd: str | None = None, **kwargs: Any
) -> str:
if _get_git_command_type(cmd) == "clone":
_simulate_cloned_repo(repo_dir)
if gitmodules:
(repo_dir / ".gitmodules").write_text("test")
return ""
return git_command_side_effect
def _submodule_calls(mock: Mock) -> list[Any]:
"""Return the mock's `git submodule` calls."""
return [
c for c in mock.call_args_list if _get_git_command_type(c[0][0]) == "submodule"
]
def _assert_submodule_runs_without_isolation(call: Any, repo_dir: Path) -> None:
"""Assert a git submodule call ran with plain cwd, not GIT_DIR/GIT_WORK_TREE
isolation, which breaks the submodule porcelain on some installations."""
assert call.kwargs.get("git_dir") is None
assert call.kwargs.get("cwd") == repo_dir
def test_run_git_command_success(tmp_path: Path) -> None:
"""Test that run_git_command returns output on success."""
# Create a simple git repo to test with
@@ -100,6 +123,22 @@ def test_run_git_command_success(tmp_path: Path) -> None:
assert isinstance(result, str)
def test_run_git_command_debug_log_redacts_credentials(
tmp_path: Path, mock_subprocess_run: Mock, caplog: pytest.LogCaptureFixture
) -> None:
"""Embedded URL credentials never reach the debug log; -v output is
routinely pasted into public issues. subprocess is mocked so no real
git ever sees the URL (the path is not creatable on Windows)."""
mock_subprocess_run.return_value = Mock(returncode=0, stdout=b"", stderr=b"")
with caplog.at_level(logging.DEBUG, logger="esphome.git"):
git.run_git_command(
["git", "clone", "https://user:hunter2@github.com/test/repo"],
cwd=tmp_path,
)
assert "hunter2" not in caplog.text
assert "://***@github.com/test/repo" in caplog.text
def test_run_git_command_with_git_dir_isolation(
tmp_path: Path, mock_subprocess_run: Mock
) -> None:
@@ -116,10 +155,17 @@ def test_run_git_command_with_git_dir_isolation(
stderr=b"",
)
result = git.run_git_command(
["git", "rev-parse", "HEAD"],
git_dir=repo_dir,
)
# Ambient repo-scoping vars simulate a git hook invoking ESPHome; an
# ambient GIT_INDEX_FILE surviving into a git_dir invocation fails
# silently (git operates on the caller's index and exits 0).
with patch.dict(
os.environ,
{"GIT_INDEX_FILE": "/caller/index", "GIT_OBJECT_DIRECTORY": "/caller/objects"},
):
result = git.run_git_command(
["git", "rev-parse", "HEAD"],
git_dir=repo_dir,
)
# Verify subprocess.run was called
assert mock_subprocess_run.called
@@ -131,6 +177,9 @@ def test_run_git_command_with_git_dir_isolation(
assert "GIT_WORK_TREE" in env
assert env["GIT_DIR"] == str(repo_dir / ".git")
assert env["GIT_WORK_TREE"] == str(repo_dir)
# The ambient scoping vars must be stripped, not passed through.
assert "GIT_INDEX_FILE" not in env
assert "GIT_OBJECT_DIRECTORY" not in env
assert result == "test output"
@@ -216,6 +265,89 @@ def test_run_git_command_without_git_dir(mock_subprocess_run: Mock) -> None:
assert result == "Cloning into 'test_repo'..."
@pytest.mark.parametrize("relative", [False, True], ids=["absolute", "relative"])
def test_run_git_command_with_cwd_runs_in_dir_without_isolation(
tmp_path: Path,
mock_subprocess_run: Mock,
monkeypatch: pytest.MonkeyPatch,
relative: bool,
) -> None:
"""The cwd parameter sets the working directory without GIT_DIR/GIT_WORK_TREE.
Ambient GIT_DIR/GIT_WORK_TREE (e.g. from a git hook or CI wrapper) must be
stripped too, and GIT_CEILING_DIRECTORIES must stop git from walking up to
an enclosing repository if the target repo's .git is missing or corrupt.
Git silently ignores a relative ceiling entry, so the variable must come
out absolute even when the given cwd is relative.
"""
repo_dir = tmp_path / "test_repo"
repo_dir.mkdir()
if relative:
monkeypatch.chdir(tmp_path)
cwd_arg = Path("test_repo")
else:
cwd_arg = repo_dir
mock_subprocess_run.return_value = Mock(
returncode=0,
stdout=b"test output",
stderr=b"",
)
with patch.dict(
os.environ,
{
"GIT_DIR": "/ambient/.git",
"GIT_WORK_TREE": "/ambient",
"GIT_INDEX_FILE": "/ambient/.git/index",
},
):
result = git.run_git_command(["git", "submodule", "update"], cwd=cwd_arg)
call_args = mock_subprocess_run.call_args
env = call_args[1]["env"]
assert "GIT_DIR" not in env
assert "GIT_WORK_TREE" not in env
assert "GIT_INDEX_FILE" not in env
ceiling = Path(env["GIT_CEILING_DIRECTORIES"])
assert ceiling.is_absolute()
assert ceiling.samefile(tmp_path)
assert call_args[1]["cwd"] == cwd_arg
assert result == "test output"
def test_run_git_command_raises_on_nonfatal_stderr(
tmp_path: Path, mock_subprocess_run: Mock
) -> None:
"""Nonzero exit with stderr lacking a fatal: prefix raises with full stderr."""
mock_subprocess_run.return_value = Mock(
returncode=1,
stdout=b"",
stderr=b"error: pathspec 'nope' did not match any file(s)\n",
)
with pytest.raises(GitCommandError, match="did not match"):
git.run_git_command(["git", "checkout", "nope"], git_dir=tmp_path)
def test_run_git_command_raises_on_nonzero_exit_without_stderr(
tmp_path: Path, mock_subprocess_run: Mock
) -> None:
"""A nonzero exit must raise even when git printed nothing to stderr.
Silent nonzero exits were previously treated as success, which is how
broken checkouts could be cached as complete.
"""
mock_subprocess_run.return_value = Mock(
returncode=1,
stdout=b"",
stderr=b"",
)
with pytest.raises(GitCommandError, match="exited with code 1"):
git.run_git_command(["git", "submodule", "update"], cwd=tmp_path)
def test_run_git_command_without_git_dir_raises_error(
mock_subprocess_run: Mock,
) -> None:
@@ -1156,46 +1288,6 @@ def test_clone_with_ref_uses_shallow_fetch(
assert ref in fetch_calls[0][0][0]
def test_clone_with_submodules_uses_shallow_submodule_update(
tmp_path: Path, mock_run_git_command: Mock
) -> None:
"""Submodule init on a fresh clone should use --depth=1."""
CORE.config_path = tmp_path / "test.yaml"
url = "https://github.com/test/repo"
domain = "test"
repo_dir = _compute_repo_dir(url, None, domain)
def git_command_side_effect(
cmd: list[str], cwd: str | None = None, **kwargs: Any
) -> str:
if _get_git_command_type(cmd) == "clone":
repo_dir.mkdir(parents=True, exist_ok=True)
(repo_dir / ".git").mkdir(exist_ok=True)
return ""
mock_run_git_command.side_effect = git_command_side_effect
git.clone_or_update(
url=url,
ref=None,
refresh=None,
domain=domain,
submodules=["components/foo"],
)
submodule_calls = [
c for c in mock_run_git_command.call_args_list if "submodule" in c[0][0]
]
assert len(submodule_calls) == 1
cmd = submodule_calls[0][0][0]
assert "--depth=1" in cmd
assert "components/foo" in cmd
# The `--` terminator must precede the submodule paths so a path
# beginning with `-` cannot be parsed as an option.
assert cmd.index("--") < cmd.index("components/foo")
def test_refresh_fetch_is_shallow(tmp_path: Path, mock_run_git_command: Mock) -> None:
"""The refresh-path fetch should use --depth=1."""
CORE.config_path = tmp_path / "test.yaml"
@@ -1220,10 +1312,91 @@ def test_refresh_fetch_is_shallow(tmp_path: Path, mock_run_git_command: Mock) ->
assert cmd[-1] == ref
def test_refresh_submodule_update_is_shallow(
@pytest.mark.parametrize(
"refresh", [None, TimePeriodSeconds(days=1)], ids=["clone", "refresh"]
)
def test_all_submodules_skipped_without_gitmodules(
tmp_path: Path, mock_run_git_command: Mock, refresh: TimePeriodSeconds | None
) -> None:
"""init_submodules is a no-op for repos with no .gitmodules.
This is the esp-idf toolchain library scenario from issue #17860: the
PlatformIO library converter requests "all submodules" for every git
library, and most libraries declare none. The git submodule porcelain
must not run at all in that case — it fails outright on some git
installations.
"""
CORE.config_path = tmp_path / "test.yaml"
url = "https://github.com/test/repo"
domain = "test"
repo_dir = _compute_repo_dir(url, None, domain)
if refresh is None:
mock_run_git_command.side_effect = _make_clone_side_effect(repo_dir)
else:
_setup_old_repo(repo_dir)
mock_run_git_command.return_value = "abc123"
git.clone_or_update(
url=url,
ref=None,
refresh=refresh,
domain=domain,
init_submodules=True,
)
assert not _submodule_calls(mock_run_git_command)
@pytest.mark.parametrize(
"refresh", [None, TimePeriodSeconds(days=1)], ids=["clone", "refresh"]
)
def test_all_submodules_updated_with_gitmodules(
tmp_path: Path, mock_run_git_command: Mock, refresh: TimePeriodSeconds | None
) -> None:
"""init_submodules initializes all submodules when .gitmodules exists."""
CORE.config_path = tmp_path / "test.yaml"
url = "https://github.com/test/repo"
domain = "test"
repo_dir = _compute_repo_dir(url, None, domain)
if refresh is None:
mock_run_git_command.side_effect = _make_clone_side_effect(
repo_dir, gitmodules=True
)
else:
_setup_old_repo(repo_dir)
(repo_dir / ".gitmodules").write_text("test")
mock_run_git_command.return_value = "abc123"
git.clone_or_update(
url=url,
ref=None,
refresh=refresh,
domain=domain,
init_submodules=True,
)
submodule_calls = _submodule_calls(mock_run_git_command)
# Which submodules get populated is git's own policy, so no status
# verification follows the update.
assert len(submodule_calls) == 1
cmd = submodule_calls[0][0][0]
assert cmd[2] == "update"
assert "--depth=1" in cmd
# Recursive, mirroring PlatformIO's recursive library clones.
assert "--recursive" in cmd
_assert_submodule_runs_without_isolation(submodule_calls[0], repo_dir)
def test_recovery_reclone_keeps_credentials_and_cache_key(
tmp_path: Path, mock_run_git_command: Mock
) -> None:
"""The refresh-path submodule update should use --depth=1."""
"""The recovery re-clone must not re-apply credentials to the already
rewritten URL (no doubled userinfo) and must land in the same cache
directory, or a credentialed private repo re-clones on every run."""
CORE.config_path = tmp_path / "test.yaml"
url = "https://github.com/test/repo"
@@ -1231,24 +1404,235 @@ def test_refresh_submodule_update_is_shallow(
repo_dir = _compute_repo_dir(url, None, domain)
_setup_old_repo(repo_dir)
mock_run_git_command.return_value = "abc123"
(repo_dir / ".gitmodules").write_text("test")
git.clone_or_update(
calls = {"submodule": 0}
def git_command_side_effect(
cmd: list[str], cwd: str | None = None, **kwargs: Any
) -> str:
if _get_git_command_type(cmd) == "clone":
_simulate_cloned_repo(repo_dir)
if _get_git_command_type(cmd) == "submodule":
calls["submodule"] += 1
if calls["submodule"] == 1:
raise git.GitCommandError("git submodule update exited with code 1")
return ""
mock_run_git_command.side_effect = git_command_side_effect
recovered_dir, _ = git.clone_or_update(
url=url,
ref=None,
refresh=TimePeriodSeconds(days=1),
domain=domain,
submodules=["components/foo"],
username="user",
password="hunter2",
init_submodules=True,
)
submodule_calls = [
c for c in mock_run_git_command.call_args_list if "submodule" in c[0][0]
assert recovered_dir == repo_dir
clone_cmds = [
c[0][0]
for c in mock_run_git_command.call_args_list
if _get_git_command_type(c[0][0]) == "clone"
]
assert len(submodule_calls) == 1
cmd = submodule_calls[0][0][0]
assert "--depth=1" in cmd
assert "components/foo" in cmd
assert cmd.index("--") < cmd.index("components/foo")
assert clone_cmds
clone_url = clone_cmds[0][-2]
assert clone_url == "https://user:hunter2@github.com/test/repo"
assert clone_url.count("@") == 1
def test_refresh_submodule_failure_recovers_then_raises(
tmp_path: Path, mock_run_git_command: Mock
) -> None:
"""A refresh-path submodule failure routes through the recovery re-clone.
The broken repo is removed and re-cloned; when the submodule update fails
again on the fresh clone the cache entry is removed and the error
propagates, instead of leaving behind a repo the refresh window would
silently accept on the next run.
"""
CORE.config_path = tmp_path / "test.yaml"
url = "https://github.com/test/repo"
domain = "test"
repo_dir = _compute_repo_dir(url, None, domain)
_setup_old_repo(repo_dir)
(repo_dir / ".gitmodules").write_text("test")
def git_command_side_effect(
cmd: list[str], cwd: str | None = None, **kwargs: Any
) -> str:
if _get_git_command_type(cmd) == "clone":
_simulate_cloned_repo(repo_dir)
(repo_dir / ".gitmodules").write_text("test")
if _get_git_command_type(cmd) == "submodule":
raise git.GitCommandError("git submodule update exited with code 1")
return ""
mock_run_git_command.side_effect = git_command_side_effect
with pytest.raises(git.GitCommandError, match="exited with code 1"):
git.clone_or_update(
url=url,
ref=None,
refresh=TimePeriodSeconds(days=1),
domain=domain,
init_submodules=True,
)
assert not repo_dir.is_dir()
# Recovery removed the repo and re-cloned before failing again.
assert any(
_get_git_command_type(c[0][0]) == "clone"
for c in mock_run_git_command.call_args_list
)
def _real_git(*args: str, cwd: Path) -> None:
"""Run real git to build a test fixture repository."""
subprocess.run(
[
"git",
"-c",
"user.email=test@test.invalid",
"-c",
"user.name=test",
"-c",
"commit.gpgsign=false",
"-c",
"protocol.file.allow=always",
*args,
],
cwd=cwd,
check=True,
capture_output=True,
)
# Git blocks file-protocol submodules by default (CVE-2022-39253); the e2e
# tests allow them via GIT_CONFIG_* environment variables, which reach the
# child git processes through run_git_command's filtered environment.
_ALLOW_FILE_PROTOCOL_ENV = {
"GIT_CONFIG_COUNT": "1",
"GIT_CONFIG_KEY_0": "protocol.file.allow",
"GIT_CONFIG_VALUE_0": "always",
}
def _make_real_repo(path: Path, filename: str) -> None:
"""Create a real git repository containing one committed file."""
path.mkdir()
_real_git("init", "-q", cwd=path)
(path / filename).write_text("content")
_real_git("add", filename, cwd=path)
_real_git("commit", "-q", "-m", "init", cwd=path)
def _add_submodule(
repo: Path, url: Path, path: str, *, update_none: bool = False
) -> None:
"""Add ``url`` as a submodule of ``repo`` at ``path`` and commit it."""
_real_git("submodule", "add", str(url), path, cwd=repo)
if update_none:
_real_git(
"config", "-f", ".gitmodules", f"submodule.{path}.update", "none", cwd=repo
)
_real_git("add", ".gitmodules", cwd=repo)
_real_git("commit", "-q", "-m", f"add submodule {path}", cwd=repo)
def test_clone_or_update_real_git_without_submodules(tmp_path: Path) -> None:
"""End-to-end with real git: a repo with no .gitmodules clones cleanly.
This is the issue #17860 scenario: requesting "all submodules" on a
submodule-less repository must not invoke the git submodule porcelain
and must produce a usable checkout.
"""
CORE.config_path = tmp_path / "test.yaml"
upstream = tmp_path / "upstream"
_make_real_repo(upstream, "README.md")
repo_dir, _ = git.clone_or_update(
url=str(upstream),
ref=None,
refresh=None,
domain="test_e2e",
init_submodules=True,
)
assert (repo_dir / "README.md").is_file()
def test_clone_or_update_real_git_initializes_submodules(tmp_path: Path) -> None:
"""End-to-end with real git: submodules are actually checked out.
Exercises the real `git submodule update` invocation, including the
env handling in run_git_command that the mocked tests cannot cover.
"""
CORE.config_path = tmp_path / "test.yaml"
sub_repo = tmp_path / "sub"
_make_real_repo(sub_repo, "sub_file.txt")
upstream = tmp_path / "upstream"
_make_real_repo(upstream, "README.md")
_add_submodule(upstream, sub_repo, "vendor/sub")
with patch.dict(os.environ, _ALLOW_FILE_PROTOCOL_ENV):
repo_dir, _ = git.clone_or_update(
url=str(upstream),
ref=None,
refresh=None,
domain="test_e2e",
init_submodules=True,
)
assert (repo_dir / "vendor" / "sub" / "sub_file.txt").is_file()
def test_clone_or_update_real_git_honors_update_none_submodule(
tmp_path: Path,
) -> None:
"""End-to-end with real git: submodules declared `update = none` stay skipped.
Shows git itself skipping the declared paths at both nesting levels
(and exiting 0) while the regular submodules check out.
"""
CORE.config_path = tmp_path / "test.yaml"
sub_repo = tmp_path / "sub"
_make_real_repo(sub_repo, "sub_file.txt")
# Intermediate submodule that itself declares a skipped nested submodule.
mid_repo = tmp_path / "mid"
_make_real_repo(mid_repo, "mid_file.txt")
_add_submodule(mid_repo, sub_repo, "vendor/leaf", update_none=True)
upstream = tmp_path / "upstream"
_make_real_repo(upstream, "README.md")
_add_submodule(upstream, sub_repo, "vendor/sub")
_add_submodule(upstream, sub_repo, "vendor/skipped", update_none=True)
_add_submodule(upstream, mid_repo, "vendor/mid")
with patch.dict(os.environ, _ALLOW_FILE_PROTOCOL_ENV):
repo_dir, _ = git.clone_or_update(
url=str(upstream),
ref=None,
refresh=None,
domain="test_e2e",
init_submodules=True,
)
assert (repo_dir / "vendor" / "sub" / "sub_file.txt").is_file()
assert not (repo_dir / "vendor" / "skipped" / "sub_file.txt").exists()
assert (repo_dir / "vendor" / "mid" / "mid_file.txt").is_file()
assert not (
repo_dir / "vendor" / "mid" / "vendor" / "leaf" / "sub_file.txt"
).exists()
def test_refresh_picks_up_new_remote_commits(