From e0e5918cd7b144a15710420c3829c33eafa898bf Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 4 Aug 2026 11:20:59 +0200 Subject: [PATCH 1/2] Handle uninitialized submodule and commit streams RootModule.update could swallow setup failures with keep_going and then iterate sms before assignment. Start with an empty submodule list so the handled failure safely skips updates. Commit iteration could likewise read stream before assignment for processes without stdout or unsupported inputs. Raise explicit input errors instead. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/objects/commit.py | 7 +++++-- git/objects/submodule/root.py | 5 +++-- test/test_commit.py | 5 +++++ test/test_submodule.py | 3 +++ 4 files changed, 16 insertions(+), 4 deletions(-) diff --git a/git/objects/commit.py b/git/objects/commit.py index 3e435453d..45843eac2 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -575,11 +575,14 @@ def _iter_from_process_or_stream(cls, repo: "Repo", proc_or_stream: Union[Popen, if hasattr(proc_or_stream, "wait"): proc_or_stream = cast(Popen, proc_or_stream) - if proc_or_stream.stdout is not None: - stream = proc_or_stream.stdout + stream = proc_or_stream.stdout + if stream is None: + raise ValueError("Process has no stdout stream") elif hasattr(proc_or_stream, "readline"): proc_or_stream = cast(IO, proc_or_stream) # type: ignore[redundant-cast] stream = proc_or_stream + else: + raise TypeError("Expected a process or stream") readline = stream.readline while True: diff --git a/git/objects/submodule/root.py b/git/objects/submodule/root.py index d93193fa3..d068049c1 100644 --- a/git/objects/submodule/root.py +++ b/git/objects/submodule/root.py @@ -7,6 +7,7 @@ import git from git.exc import InvalidGitRepositoryError +from git.util import IterableList from .base import Submodule, UpdateProgress from .util import find_first_remote_branch @@ -19,7 +20,6 @@ if TYPE_CHECKING: from git.repo import Repo - from git.util import IterableList # ---------------------------------------------------------------------------- @@ -162,6 +162,7 @@ def update( # type: ignore[override] prefix = "DRY-RUN: " repo = self.repo + sms: "IterableList[Submodule]" = IterableList("name") try: # SETUP BASE COMMIT @@ -182,7 +183,7 @@ def update( # type: ignore[override] # END handle previous commit psms: "IterableList[Submodule]" = self.list_items(repo, parent_commit=previous_commit) - sms: "IterableList[Submodule]" = self.list_items(repo) + sms = self.list_items(repo) spsms = set(psms) ssms = set(sms) diff --git a/test/test_commit.py b/test/test_commit.py index b9ceecf07..431269b29 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -327,6 +327,11 @@ def test_rev_list_bisect_all(self): for sha1, commit in zip(expected_ids, commits): self.assertEqual(sha1, commit.hexsha) + def test_iter_from_invalid_process_or_stream(self): + for source, error in ((Mock(wait=Mock(), stdout=None), ValueError), (object(), TypeError)): + with self.assertRaises(error): + list(Commit._iter_from_process_or_stream(self.rorepo, source)) + @with_rw_directory def test_ambiguous_arg_iteration(self, rw_dir): rw_repo = Repo.init(osp.join(rw_dir, "test_ambiguous_arg")) diff --git a/test/test_submodule.py b/test/test_submodule.py index 0e7164641..287986059 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -511,6 +511,9 @@ def test_root_module(self, rwrepo): # Cannot set the parent commit as root module's path didn't exist. self.assertRaises(ValueError, rm.set_parent_commit, "HEAD") + with mock.patch.object(RootModule, "list_items", side_effect=ValueError("boom")): + rm.update(keep_going=True) + # TEST UPDATE ############# # Set up a commit that removes existing, adds new and modifies existing From afe3ac98d8b125d5d9ef02cdbb3e18b444fbb11c Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 4 Aug 2026 11:23:58 +0200 Subject: [PATCH 2/2] Remove possibly-unbound type suppressions Make established repository invariants explicit, initialize loop-only locals, and move assignments ahead of exception handling so basedpyright can follow the existing control flow. Regenerate the baseline to remove all 19 reportPossiblyUnboundVariable suppressions, including the two runtime fixes from the preceding commit. Validation: basedpyright --warnings; unbaselined possibly-unbound count 0; eight focused subsystem tests passed. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- .basedpyright/baseline.json | 138 ---------------------------------- git/index/base.py | 11 ++- git/objects/submodule/base.py | 2 + git/refs/log.py | 1 + git/repo/base.py | 18 +++-- test/test_repo.py | 6 ++ 6 files changed, 28 insertions(+), 148 deletions(-) diff --git a/.basedpyright/baseline.json b/.basedpyright/baseline.json index 7066ccd48..14bf1024e 100644 --- a/.basedpyright/baseline.json +++ b/.basedpyright/baseline.json @@ -183,14 +183,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 52, - "endColumn": 59, - "lineCount": 1 - } - }, { "code": "reportSelfClsParameterName", "range": { @@ -207,14 +199,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 41, - "endColumn": 44, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { @@ -393,14 +377,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 19, - "endColumn": 25, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { @@ -467,30 +443,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 23, - "endColumn": 34, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 32, - "endColumn": 43, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 23, - "endColumn": 34, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { @@ -499,14 +451,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 28, - "endColumn": 32, - "lineCount": 1 - } - }, { "code": "reportAttributeAccessIssue", "range": { @@ -588,16 +532,6 @@ } } ], - "./git/objects/submodule/root.py": [ - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 18, - "endColumn": 21, - "lineCount": 1 - } - } - ], "./git/objects/tag.py": [ { "code": "reportIncompatibleVariableOverride", @@ -701,14 +635,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 41, - "endColumn": 45, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { @@ -855,38 +781,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 26, - "endColumn": 34, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 32, - "endColumn": 40, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 18, - "endColumn": 26, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 22, - "endColumn": 30, - "lineCount": 1 - } - }, { "code": "reportReturnType", "range": { @@ -967,22 +861,6 @@ "lineCount": 1 } }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 39, - "endColumn": 47, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 43, - "endColumn": 51, - "lineCount": 1 - } - }, { "code": "reportArgumentType", "range": { @@ -998,22 +876,6 @@ "endColumn": 38, "lineCount": 1 } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 26, - "endColumn": 42, - "lineCount": 1 - } - }, - { - "code": "reportPossiblyUnboundVariable", - "range": { - "startColumn": 18, - "endColumn": 34, - "lineCount": 1 - } } ], "./git/repo/fun.py": [ diff --git a/git/index/base.py b/git/index/base.py index 248a7f10a..dd5c1a905 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -730,14 +730,17 @@ def _entries_for_paths( ) -> List[BaseIndexEntry]: entries_added: List[BaseIndexEntry] = [] if path_rewriter: + working_tree_dir = self.repo.working_tree_dir + if working_tree_dir is None: + raise InvalidGitRepositoryError("Cannot rewrite paths without a working tree") + working_tree_dir = str(working_tree_dir) for path in paths: if osp.isabs(path): abspath = path - gitrelative_path = path[len(str(self.repo.working_tree_dir)) + 1 :] + gitrelative_path = path[len(working_tree_dir) + 1 :] else: gitrelative_path = path - if self.repo.working_tree_dir: - abspath = osp.join(self.repo.working_tree_dir, gitrelative_path) + abspath = osp.join(working_tree_dir, gitrelative_path) # END obtain relative and absolute paths blob = Blob( @@ -1467,8 +1470,8 @@ def reset( nie = new_inst.entries for path in paths: path = self._to_relative_path(path) + key = entry_key(path, 0) try: - key = entry_key(path, 0) self.entries[key] = nie[key] except KeyError: # If key is not in theirs, it mustn't be in ours. diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 3797bdc90..da0e09af4 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -877,6 +877,7 @@ def fetch_remotes(module_repo: "Repo") -> None: ############################# binsha = self.binsha hexsha = self.hexsha + is_detached = False if mrepo is not None: # mrepo is only set if we are not in dry-run mode or if the module # existed. @@ -1221,6 +1222,7 @@ def remove( for remote in mod.remotes: num_branches_with_new_commits = 0 rrefs = remote.refs + rref = None for rref in rrefs: num_branches_with_new_commits += len(mod.git.cherry(rref)) != 0 # END for each remote ref diff --git a/git/refs/log.py b/git/refs/log.py index fbbe66b22..0681943bf 100644 --- a/git/refs/log.py +++ b/git/refs/log.py @@ -269,6 +269,7 @@ def entry_at(cls, filepath: PathLike, index: int) -> "RefLogEntry": return RefLogEntry.from_line(fp.readlines()[index].strip()) # Read until index is reached. + line = b"" for i in range(index + 1): line = fp.readline() if not line: diff --git a/git/repo/base.py b/git/repo/base.py index dfd361747..e4d5e92c3 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -933,13 +933,17 @@ def is_valid_object(self, sha: str, object_type: Union[str, None] = None) -> boo return False def _get_daemon_export(self) -> bool: - if self.git_dir: - filename = osp.join(self.git_dir, self.DAEMON_EXPORT_FILE) + git_dir = getattr(self, "git_dir", None) + if git_dir is None: + return False + filename = osp.join(git_dir, self.DAEMON_EXPORT_FILE) return osp.exists(filename) def _set_daemon_export(self, value: object) -> None: - if self.git_dir: - filename = osp.join(self.git_dir, self.DAEMON_EXPORT_FILE) + git_dir = getattr(self, "git_dir", None) + if git_dir is None: + return + filename = osp.join(git_dir, self.DAEMON_EXPORT_FILE) fileexists = osp.exists(filename) if value and not fileexists: touch(filename) @@ -1279,6 +1283,7 @@ class InfoTD(TypedDict, total=False): keepends = True for line_bytes in data.splitlines(keepends): + line_str = "" try: line_str = line_bytes.rstrip().decode(defenc) except UnicodeDecodeError: @@ -1737,8 +1742,9 @@ def currently_rebasing_on(self) -> Commit | None: ``None`` if we are not currently rebasing. """ - if self.git_dir: - rebase_head_file = osp.join(self.git_dir, "REBASE_HEAD") + if not self.git_dir: + return None + rebase_head_file = osp.join(self.git_dir, "REBASE_HEAD") if not osp.isfile(rebase_head_file): return None with open(rebase_head_file, "rt") as f: diff --git a/test/test_repo.py b/test/test_repo.py index 84336a39b..5c4b416ff 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -337,6 +337,12 @@ def test_daemon_export(self): self.rorepo.daemon_export = orig_val self.assertEqual(self.rorepo.daemon_export, orig_val) + def test_daemon_export_without_git_dir(self): + repo = Repo.__new__(Repo) + repo.git_dir = None + self.assertFalse(repo._get_daemon_export()) + repo._set_daemon_export(True) + def test_alternates(self): cur_alternates = self.rorepo.alternates try: