Skip to content

References.get() and Branches.get() raise InvalidSpecError where the docs promise None #1489

Description

@rawsun007

References.get() and Branches.get() are documented to return None when the lookup fails, but they only catch KeyError, and libgit2 reports two different codes for a failed lookup.

docs/references.rst:

>>> head = repo.references.get('refs/heads/master')  # Returns None if not found

docs/branches.rst:

>>> other_branch = repo.branches.get('does-not-exist')  # Returns None

errors.py maps GIT_ENOTFOUND to NotFoundError, which subclasses KeyError, but GIT_EINVALIDSPEC to InvalidSpecError, which subclasses ValueError. git_reference_lookup and git_branch_lookup both document GIT_EINVALIDSPEC alongside GIT_ENOTFOUND and both say the name is checked for validity, so only one of the two documented failures reaches the except KeyError.

On pygit2 1.20.0 with libgit2 1.9.7, in a fresh repository with one commit:

repo.references.get('master')     -> InvalidSpecError: the given reference name 'master' is not valid
'master' in repo.references       -> InvalidSpecError
repo.branches.get('my branch')    -> InvalidSpecError: cannot locate local branch 'my branch'
'my branch' in repo.branches      -> InvalidSpecError

Documented result for all four is None, or False for the membership tests. repo.references.get('refs/heads/x') and repo.branches.get('nope') do return None, so the contract holds for a well-formed name that is absent and breaks for a malformed one.

Two things make this worth more than the docstring mismatch. __contains__ is built on get in both classes and inherits it, so if name in repo.references raises ValueError on an arbitrary string - a membership test that cannot be used to validate untrusted input, which is the obvious reason to reach for it. And for references the trigger is the ordinary mistake rather than an exotic name: 'master' is a valid branch name but not a valid reference name, since references must be fully qualified, so the unqualified form raises where the docs promise None.

libgit2's own wording suggests it means not-found here: cannot locate local branch 'my branch'.

This is the same shape as #1487, where GIT_EEXISTS from git_submodule_lookup still meant "no submodule by that name" and SubmoduleCollection.get now catches (KeyError, AlreadyExistsError). Index.__contains__ and ConflictCollection.__contains__ both check GIT_ENOTFOUND explicitly and return False, so References.get and Branches.get look like the two remaining places that were not updated.

No existing test pins the current behaviour - every references.get / branches.get call in test_refs.py and test_branch.py passes a valid name.

I would be glad to open a PR catching InvalidSpecError in both get methods, with tests, if you want it. I will wait to be assigned rather than sending one unprompted, since the scope is arguably yours to choose: Branches.get also lets GitError and InvalidError through the with_commit filter when the filter commit is missing or is not a commit, and swallowing that may be the wrong call, as "the filter commit does not exist" is a different mistake from "no such branch".

Disclosure: this was found and written by Claude Opus 5 running in Claude Code, through my account and under my direction. The reproduction above was run against a local build; I have not independently reviewed every line of the analysis myself.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions