Skip to content

Commit 0ea97cd

Browse files
committed
refactor: consolidate git submodule handling into git_submodule.py
Submodule-related logic was spread across three places: GitLocalRepo methods in git.py (submodules(), _get_submodule_urls(), _filter_submodules_by_src(), _move_src_folder_up() and friends), the standalone git_gitlinks.py (orphan-gitlink stripping added for #1380), and the Submodule dataclass in git_types.py. Move all of it into one dfetch/vcs/git_submodule.py: - Submodule dataclass (moved from git_types.py; CheckoutOptions stays, it isn't submodule-specific) - everything from git_gitlinks.py (gitlink_paths, declared_submodule_paths, drop_orphan_gitlinks) - submodule url/branch resolution (get_submodule_urls, ensure_abs_url) - src/ignore filtering and promotion of a fetched submodule tree (apply_src_and_ignore, filter_submodules_by_src, remove_empty_parents, move_src_folder_up and its helpers) None of the moved code touched GitLocalRepo instance state, so it translates directly into free functions. GitLocalRepo.submodules() stays in git.py as a thin orchestrator: it's the one piece that genuinely needs GitRemote/GitLocalRepo (to resolve a submodule's branch from its own remote or local history), so keeping it there avoids a circular import between the two modules while git.py imports git_submodule normally for everything else. Test files follow: tests/test_git_gitlinks.py is folded into a new tests/test_git_submodule.py along with the filter/move tests that used to live in test_git_vcs.py, mirroring the module split. No behavioral change: full pytest (705) and non-SVN behave suites pass identically before and after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DziAso23sxMgeTSUmzkus8
1 parent 8caa158 commit 0ea97cd

8 files changed

Lines changed: 573 additions & 546 deletions

File tree

dfetch/vcs/git.py

Lines changed: 10 additions & 198 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import contextlib
44
import functools
5-
import glob
65
import os
76
import re
87
import shutil
@@ -13,18 +12,11 @@
1312

1413
from dfetch.log import get_logger
1514
from dfetch.util.cmdline import SubprocessCommandError, run_on_cmdline
16-
from dfetch.util.license import is_license_file
1715
from dfetch.util.ssh import InvalidSshCommandError, sanitize_ssh_cmd
18-
from dfetch.util.util import (
19-
glob_within_root,
20-
in_directory,
21-
move_directory_contents,
22-
safe_rm,
23-
strip_glob_prefix,
24-
unique_parent_dirs,
25-
)
26-
from dfetch.vcs import git_gitlinks
27-
from dfetch.vcs.git_types import CheckoutOptions, Submodule
16+
from dfetch.util.util import in_directory, safe_rm
17+
from dfetch.vcs import git_submodule
18+
from dfetch.vcs.git_submodule import Submodule
19+
from dfetch.vcs.git_types import CheckoutOptions
2820
from dfetch.vcs.patch import Patch, PatchType
2921

3022
__all__ = ["CheckoutOptions", "GitLocalRepo", "GitRemote", "Submodule"]
@@ -525,7 +517,7 @@ def checkout_version(
525517
if options.eol is not None:
526518
self._renormalize_eol()
527519

528-
git_gitlinks.drop_orphan_gitlinks()
520+
git_submodule.drop_orphan_gitlinks()
529521

530522
run_on_cmdline(
531523
logger,
@@ -541,153 +533,12 @@ def checkout_version(
541533
.strip()
542534
)
543535

544-
submodules = self._apply_src_and_ignore(
536+
submodules = git_submodule.apply_src_and_ignore(
545537
options.remote, options.src, options.ignore, submodules
546538
)
547539

548540
return str(current_sha), submodules
549541

550-
def _apply_src_and_ignore(
551-
self,
552-
remote: str,
553-
src: str | None,
554-
ignore: Sequence[str] | None,
555-
submodules: list[Submodule],
556-
) -> list[Submodule]:
557-
"""Apply src filter and ignore patterns, returning surviving submodules."""
558-
if src:
559-
submodules = self._filter_submodules_by_src(remote, src, submodules)
560-
561-
for ignore_path in ignore or []:
562-
paths = [
563-
p
564-
for p in glob.glob(ignore_path)
565-
if not (os.path.isfile(p) and is_license_file(os.path.basename(p)))
566-
]
567-
safe_rm(paths, within=".")
568-
569-
return [s for s in submodules if os.path.exists(s.path)]
570-
571-
def _filter_submodules_by_src(
572-
self, remote: str, src: str, submodules: list[Submodule]
573-
) -> list[Submodule]:
574-
"""Keep only submodules within *src*, remove others, then promote *src* to root."""
575-
within_src = []
576-
to_remove: set[str] = set()
577-
for submodule in submodules:
578-
if submodule.path == src:
579-
# Submodule IS the src directory itself; keep it in-scope without
580-
# altering its path and let _move_src_folder_up handle promotion.
581-
within_src.append(submodule)
582-
continue
583-
new_path = strip_glob_prefix(submodule.path, src)
584-
if new_path != submodule.path:
585-
submodule.path = new_path
586-
within_src.append(submodule)
587-
else:
588-
if Path(src).is_relative_to(Path(submodule.path)):
589-
continue
590-
to_remove.add(submodule.path)
591-
for path in to_remove:
592-
safe_rm(path, within=".")
593-
GitLocalRepo._remove_empty_parents(to_remove)
594-
self._move_src_folder_up(remote, src)
595-
return within_src
596-
597-
@staticmethod
598-
def _remove_empty_parents(paths: set[str]) -> None:
599-
"""Remove empty ancestor directories left after removing out-of-scope submodule dirs.
600-
601-
git submodule update may create a parent directory for a submodule even when
602-
sparse-checkout excludes it; after safe_rm removes the exact submodule path the
603-
parent can be left as an empty directory. os.rmdir is used because it is atomic
604-
and raises OSError when the directory is not empty, which stops the upward walk.
605-
"""
606-
for path in paths:
607-
parent = Path(path).parent
608-
while parent != Path("."):
609-
try:
610-
parent.rmdir()
611-
except OSError:
612-
break
613-
parent = parent.parent
614-
615-
@staticmethod
616-
def _collect_safe_paths(src: str, repo_root: Path, remote: str) -> list[str]:
617-
"""Return glob-matched paths for *src* that are within *repo_root*.
618-
619-
Paths that escape the repo root are skipped with a warning.
620-
"""
621-
safe_matched, escaped = glob_within_root(src, repo_root)
622-
for p in escaped:
623-
logger.warning(
624-
f"The 'src:' filter '{src}' matched '{p}' outside the repo root"
625-
f" for '{remote}'; skipping"
626-
)
627-
return safe_matched
628-
629-
@staticmethod
630-
def _apply_move(chosen: Path, repo_root: Path, remote: str) -> None:
631-
"""Move the contents of *chosen* to the repo root and remove the empty parent."""
632-
# Pre-remove git metadata at the root of *chosen* before promoting its contents.
633-
# When *chosen* is itself a cloned submodule it contains a .git file that would
634-
# collide with the parent repo's .git directory; the caller cleans these up
635-
# recursively after checkout anyway.
636-
for name in (GitLocalRepo.METADATA_DIR, GitLocalRepo.GIT_MODULES_FILE):
637-
safe_rm(chosen / name, within=chosen)
638-
try:
639-
move_directory_contents(str(chosen), ".")
640-
except FileNotFoundError:
641-
logger.warning(
642-
f"The 'src:' filter '{chosen}' didn't match any files from '{remote}'"
643-
)
644-
return
645-
parts = chosen.relative_to(repo_root).parts
646-
if parts:
647-
try:
648-
safe_rm(repo_root / parts[0], within=repo_root)
649-
except FileNotFoundError:
650-
logger.debug(
651-
f"Nothing left to remove at '{repo_root / parts[0]}' after moving '{chosen}' for '{remote}'"
652-
)
653-
654-
@staticmethod
655-
def _move_src_folder_up(remote: str, src: str) -> None:
656-
"""Move the files from the src folder into the root of the project.
657-
658-
Args:
659-
remote (str): Name of the root
660-
src (str): Src folder to move up
661-
"""
662-
if os.path.isabs(src):
663-
logger.warning(
664-
f"The 'src:' filter '{src}' is an absolute path; skipping for '{remote}'"
665-
)
666-
return
667-
668-
repo_root = Path(os.getcwd()).resolve()
669-
safe_matched = GitLocalRepo._collect_safe_paths(src, repo_root, remote)
670-
671-
if not safe_matched:
672-
logger.warning(
673-
f"The 'src:' filter '{src}' didn't match any files from '{remote}'"
674-
)
675-
return
676-
677-
# Resolve to canonical absolute paths so downstream steps use stable paths
678-
# regardless of any '..' components in the original glob results.
679-
resolved_dirs = [Path(d).resolve() for d in unique_parent_dirs(safe_matched)]
680-
681-
if len(resolved_dirs) > 1:
682-
display = resolved_dirs[0].relative_to(repo_root)
683-
logger.warning(
684-
f"The 'src:' filter '{src}' matches multiple directories from '{remote}'. "
685-
f"Only considering files in '{display}'."
686-
)
687-
688-
if resolved_dirs:
689-
GitLocalRepo._apply_move(resolved_dirs[0], repo_root, remote)
690-
691542
@staticmethod
692543
def _determine_ignore_paths(
693544
src: str | None, ignore: Sequence[str]
@@ -846,7 +697,7 @@ def untracked_files_patch(self, ignore: Sequence[str] | None = None) -> Patch:
846697
@staticmethod
847698
def submodules() -> list[Submodule]:
848699
"""Get a list of submodules in the current directory."""
849-
git_gitlinks.drop_orphan_gitlinks()
700+
git_submodule.drop_orphan_gitlinks()
850701

851702
result = run_on_cmdline(
852703
logger,
@@ -864,7 +715,9 @@ def submodules() -> list[Submodule]:
864715
for line in result.stdout.decode().split("\n"):
865716
if line:
866717
name, sm_path, sha, toplevel = line.split("\0")
867-
urls = urls or GitLocalRepo._get_submodule_urls(toplevel)
718+
urls = urls or git_submodule.get_submodule_urls(
719+
toplevel, GitLocalRepo.get_remote_url()
720+
)
868721
url = urls[name]
869722
branch, tag = GitRemote(url).find_branch_tip_or_tag_from_sha(sha)
870723

@@ -894,47 +747,6 @@ def submodules() -> list[Submodule]:
894747

895748
return submodules
896749

897-
@staticmethod
898-
def _get_submodule_urls(toplevel: str) -> dict[str, str]:
899-
result = run_on_cmdline(
900-
logger,
901-
[
902-
"git",
903-
"config",
904-
"--file",
905-
toplevel + "/.gitmodules",
906-
"--get-regexp",
907-
"url",
908-
],
909-
)
910-
911-
origin_url = GitLocalRepo.get_remote_url()
912-
return {
913-
str(match.group(1)): GitLocalRepo._ensure_abs_url(
914-
origin_url, str(match.group(2))
915-
)
916-
for match in re.finditer(
917-
r"submodule\.(.*)\.url\s+(.*)", result.stdout.decode()
918-
)
919-
}
920-
921-
@staticmethod
922-
def _ensure_abs_url(root_url: str, rel_url: str) -> str:
923-
"""Make sure the given url is an absolute url."""
924-
if not rel_url.startswith("../"):
925-
return rel_url
926-
927-
new_root_url = root_url.split("/")
928-
new_rel_url = rel_url.split("/")
929-
for elt in new_rel_url.copy():
930-
if elt != "..":
931-
break
932-
933-
new_root_url.pop()
934-
new_rel_url.pop(0)
935-
936-
return "/".join(new_root_url + new_rel_url)
937-
938750
def find_branch_containing_sha(self, sha: str) -> str:
939751
"""Try to find the branch that contains the given sha."""
940752
if not os.path.isdir(os.path.join(self._path, GitLocalRepo.METADATA_DIR)):

dfetch/vcs/git_gitlinks.py

Lines changed: 0 additions & 92 deletions
This file was deleted.

0 commit comments

Comments
 (0)