diff --git a/gitfourchette/porcelain.py b/gitfourchette/porcelain.py index 464231e5..fb361c8c 100644 --- a/gitfourchette/porcelain.py +++ b/gitfourchette/porcelain.py @@ -758,10 +758,10 @@ def in_gitdir(self, path: str, common: bool = True) -> str: assert not _isabs(path) parent = self.commondir if common else self.path - + parent_resolved = _Path(parent).resolve() p = _Path(parent, path).resolve() - if not p.is_relative_to(parent): + if not p.is_relative_to(parent_resolved): raise ValueError("Won't resolve absolute path outside gitdir") return str(p) @@ -903,13 +903,13 @@ def listall_remote_branches(self, value_style: _typing.Literal["strip", "shortha for remote in self.remotes: names[remote.name] = [] - for refname in self.listall_references(): - prefix, shorthand = RefPrefix.split(refname) + for ref in self.references.iterator(): + prefix, shorthand = RefPrefix.split(ref.name) if prefix != RefPrefix.REMOTES: continue - if refname.endswith("/HEAD"): + if ref.name.endswith("/HEAD"): # Skip refs/remotes/*/HEAD (the remote's default branch). # The ref file (.git/refs/remotes/*/HEAD) is created ONCE when first cloning the repository, # and it's never updated again automatically, even if the default branch has changed on the remote. @@ -918,13 +918,25 @@ def listall_remote_branches(self, value_style: _typing.Literal["strip", "shortha # See: https://stackoverflow.com/questions/8839958 continue + # Skip symbolic refs (e.g., repo tool's refs/remotes/m/master -> origin/master). + # We only show direct refs in real remotes; resolving would duplicate the target + # (e.g. origin/master would appear twice in the upstream context menu). + if ref.type == ReferenceType.SYMBOLIC: + continue + remote_name, branch_name = split_remote_branch_shorthand(shorthand) + # Skip references without a branch name (e.g., refs/remotes/git-svn from git svn clone) + if not branch_name: + continue + # Skip references that don't match any known remote (e.g., stale refs from deleted remotes) + if remote_name not in names: + continue if value_style == "strip": value = branch_name elif value_style == "shorthand": value = shorthand elif value_style == "refname": - value = refname + value = ref.name else: raise NotImplementedError(f"unsupported value_style {value_style}") names[remote_name].append(value) diff --git a/test/test_porcelain.py b/test/test_porcelain.py new file mode 100644 index 00000000..6b0c75d3 --- /dev/null +++ b/test/test_porcelain.py @@ -0,0 +1,132 @@ +# ----------------------------------------------------------------------------- +# Copyright (C) 2026 Iliyas Jorio. +# This file is part of GitFourchette, distributed under the GNU GPL v3. +# For full terms, see the included LICENSE file. +# ----------------------------------------------------------------------------- + +""" +Unit tests for gitfourchette.porcelain (Repo, listall_remote_branches, in_gitdir). +""" + +from pathlib import Path + +import pytest + +from .util import unpackRepo, RepoContext, WINDOWS + + +def testListallRemoteBranchesWithSymbolicRef(tempDir): + """ + Repo.listall_remote_branches() must not raise when the repo contains + symbolic refs (e.g. from the "repo" tool: refs/remotes/m/master -> refs/remotes/origin/master). + Right-clicking such a branch in the sidebar would previously cause a stack trace. + """ + wd = unpackRepo(tempDir) + # Create a symbolic ref like repo tool: refs/remotes// + # pointing at refs/remotes/origin/. "m" is not a configured remote. + refs_m_dir = Path(wd.rstrip("/")) / ".git" / "refs" / "remotes" / "m" + refs_m_dir.mkdir(parents=True, exist_ok=True) + (refs_m_dir / "master").write_text("ref: refs/remotes/origin/master\n") + + with RepoContext(wd) as repo: + # Must not raise KeyError or similar + result = repo.listall_remote_branches() + assert "origin" in result + assert "master" in result["origin"] + + +def testListallRemoteBranchesSymbolicRefToOriginMasterNoDuplicate(tempDir): + """ + A symbolic ref pointing at origin/master (e.g. m/master -> origin/master) + must not cause origin/master to appear twice in the results. + """ + wd = unpackRepo(tempDir) + refs_m_dir = Path(wd.rstrip("/")) / ".git" / "refs" / "remotes" / "m" + refs_m_dir.mkdir(parents=True, exist_ok=True) + (refs_m_dir / "master").write_text("ref: refs/remotes/origin/master\n") + + with RepoContext(wd) as repo: + result = repo.listall_remote_branches() + result_shorthand = repo.listall_remote_branches(value_style="shorthand") + assert result["origin"].count("master") == 1 + assert result_shorthand["origin"].count("origin/master") == 1 + + +def testListallRemoteBranchesWithStaleSymbolicRef(tempDir): + """ + Stale symbolic refs (pointing to a ref that no longer exists) must be + skipped without raising. + """ + wd = unpackRepo(tempDir) + refs_m_dir = Path(wd.rstrip("/")) / ".git" / "refs" / "remotes" / "m" + refs_m_dir.mkdir(parents=True, exist_ok=True) + (refs_m_dir / "master").write_text("ref: refs/remotes/origin/nonexistent\n") + + with RepoContext(wd) as repo: + result = repo.listall_remote_branches() + # Should still have origin's branches; stale symref is skipped + assert set(result.keys()) == {"origin"} + assert "master" in result["origin"] + + +def testListallRemoteBranchesSkipsRefWithoutBranchName(tempDir): + """ + Refs without a branch name (e.g. refs/remotes/git-svn from git svn clone) + must be skipped. split_remote_branch_shorthand yields empty branch_name + for such refs. + """ + wd = unpackRepo(tempDir) + git_dir = Path(wd.rstrip("/")) / ".git" + with RepoContext(wd) as repo: + oid = str(repo.head_commit_id) + # refs/remotes/git-svn has no "/branch" part -> branch_name is "" + (git_dir / "refs" / "remotes" / "git-svn").write_text(oid + "\n") + + with RepoContext(wd) as repo: + result = repo.listall_remote_branches() + # git-svn is not a known remote; even if it were, it has no branch name. + # We must still see origin's branches and must not include git-svn. + assert "origin" in result + assert "git-svn" not in result + assert "master" in result["origin"] + + +def testListallRemoteBranchesSkipsStaleRefsFromDeletedRemote(tempDir): + """ + Refs that belong to a remote no longer in the config (e.g. after + git remote remove) must be skipped so we don't show stale data. + """ + wd = unpackRepo(tempDir) + git_dir = Path(wd.rstrip("/")) / ".git" + with RepoContext(wd) as repo: + oid = str(repo.head_commit_id) + # Create refs/remotes/deletedremote/master but do not add "deletedremote" as a remote + deleted_dir = git_dir / "refs" / "remotes" / "deletedremote" + deleted_dir.mkdir(parents=True, exist_ok=True) + (deleted_dir / "master").write_text(oid + "\n") + + with RepoContext(wd) as repo: + result = repo.listall_remote_branches() + # Only configured remotes (origin) appear; deletedremote is skipped + assert set(result.keys()) == {"origin"} + assert "deletedremote" not in result + assert "master" in result["origin"] + + +@pytest.mark.skipif(WINDOWS, reason="symlinks are flaky on Windows") +def testInGitdirWithSymlinkedRepo(tempDir): + """ + Repo.in_gitdir() must not raise ValueError when the repo path is a symlink. + Previously is_relative_to(parent) failed because the resolved path was + compared against the unresolved (symlink) parent. + """ + wd = unpackRepo(tempDir) + real_path = Path(wd.rstrip("/")).resolve() + link_path = Path(tempDir.name) / "repo-link" + link_path.symlink_to(real_path) + repo_path = str(link_path) + "/" + + with RepoContext(repo_path) as repo: + # Must not raise ValueError("Won't resolve absolute path outside gitdir") + config_path = repo.in_gitdir("config", common=True) + assert config_path.endswith("config")