Error in `repo.active_branch` if branch is in DETACHED HEAD state

[cgi1]

Error in repo.active_branch if branch is in DETACHED HEAD state:

{TypeError}HEAD is a detached symbolic reference as it points to '0fa1f2a7d9dd9fb63fa775d2d97ad72f56f1d819'

The branch is None in def _get_ref_info(cls, repo, ref_path): because of

# its a commit
        if repo.re_hexsha_only.match(tokens[0]):
            return (tokens[0], None)

def active_branch(self) could return HEAD-{commit-hash} instead like:

    try:
        active_branch = repo.active_branch
    except:
        active_branch = 'DETACHED_' + repo.head.object.hexsha

[cgi1]

Looks similar to #629

[Byron]

I am not sure it would be right to return a commit instead. The function is called active_branch, and I would assume people expect it to return a branch, never a commit. If HEAD is detached, there is no branch to return. What do you think?

[cgi1]

Hm good point, I don´t have a pefect answer.. To return no branch seems to be ok.

[Byron]

I think returning no branch would break existing code which expects an exception. On the other hand, the exception is not documented, so in a way it's running under the radar anyway.

Maybe it could be adjusted to return None if there is no active branch instead, which would make the calling code a little less cumbersome and safer, as it could start to expect that.

Would you like to contribute a respective PR?

[peterbe]

I stumbled on this too. With submodules. My solution wasn't to try to rely on a try: except: ... but I made it always check out at least some branch first before it started switching to the branch I ultimately wanted to. I.e. always start with repo.heads[config["master_branch"]].checkout()

[stas00]

3 years later and this is still a problem :(

[Byron]

I think returning no branch would break existing code which expects an exception. On the other hand, the exception is not documented, so in a way it's running under the radar anyway.

Maybe it could be adjusted to return None if there is no active branch instead, which would make the calling code a little less cumbersome and safer, as it could start to expect that.

Would you like to contribute a respective PR?

I believe this comment still holds.

[stas00]

Thank you for this follow up, @Byron

My suggestion for the best solution would be this: In order to keep backward compatibility (exception that is) adding a new argument that allows for what OP suggested:

git.Repo(search_parent_directories=True, active_branch_flexible=False)

and then:

if active_branch_flexible:
    try:
        active_branch = repo.active_branch
    except:
        active_branch = 'DETACHED_' + repo.head.object.hexsha

I haven't looked at the code, so this is totally a pseudocode based on what I saw in this issue.

Of course, if that suggestion resonates, you'd know better what to call it, active_branch_flexible is just a quick idea ;)

Thank you.

[Byron]

I think at this point it's clear that the methods behaviour is surprising, so having alternatives seems valuable.

Thanks @stas00 for the suggestion, which certainly works as to remove the exception that keeps tripping people up. Making the returned branch a string as proposed above could be just as dangerous though, as the type it will return has never been a string. Also I would hope to keep things more local and avoid adding another, very specialized configuration flag to the repository.

My proposition is to add a new method or property along the lines of active_branch_or_none with documentation explaining under which circumstances None would be expected.

[stas00]

Surely at the very least active_branch_or_none will remove the element of surprise.

Thank you for your thoughtful commentary, @Byron.

[Byron]

Indeed, ideally there is a way to not enforce any branching, but I don't believe that exists since there either is a branch, or there is none. With a method name that makes this clear there should be no surprise, but I invite you to experiment with styles and names and submit a PR with what works best for you.

It really would be great if you could be the one to put an end to this issue :), so imagine me cheering you on from the sidelines :).

[stas00]

Thank you for this invitation, but I will pass at the moment. I have never used gitpython and it was in someone else's code totally unrelated to what I was doing, so I had to remove that obstacle to proceed. That's how I ended up here.

Should I start using the library and get to learn how it works, I'd be happy to submit a PR.

[Eoin-McMahon]

3.5 years later and this is still a problem :(

[Byron]

This comment might help.