Readthedocs.org: gitpython sometimes fails when checking for submodules

[BronsonQuick]

Details

I noticed the other week that our builds have actually been failing for a few months but I'm not sure how to debug it. Locally the docs generate as expected when I run sphinx-build -b dirhtml . _build/dirhtml

• Read the Docs project URL: https://readthedocs.org/projects/chassis/
• Build URL (if applicable): https://readthedocs.org/projects/chassis/builds/7462561/
• Read the Docs username (if applicable): BronsonQuick

Expected Result

A successful build.

Actual Result

There was a problem with Read the Docs while building your documentation. Please report this to us with your build id (7462561).

Thanks very much for all your work on this project!

[humitos]

I didn't dig too much on this issue, but at least I can tell that your build is failing when we check for the submodules of your repo:

In [1]: import git

In [2]: repo = git.Repo('/home/humitos/rtfd/repos/Chassis')

In [3]: repo.submodules()
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-3-332b59b5243e> in <module>()
----> 1 repo.submodules()

~/.pyenv/versions/3.6.6/envs/readthedocs.org/lib/python3.6/site-packages/git/repo/base.py in submodules(self)
    322         :return: git.IterableList(Submodule, ...) of direct submodules
    323             available from the current head"""
--> 324         return Submodule.list_items(self)
    325 
    326     def submodule(self, name):

~/.pyenv/versions/3.6.6/envs/readthedocs.org/lib/python3.6/site-packages/git/util.py in list_items(cls, repo, *args, **kwargs)
    932         :return:list(Item,...) list of item instances"""
    933         out_list = IterableList(cls._id_attribute_)
--> 934         out_list.extend(cls.iter_items(repo, *args, **kwargs))
    935         return out_list
    936 

~/.pyenv/versions/3.6.6/envs/readthedocs.org/lib/python3.6/site-packages/git/objects/submodule/base.py in iter_items(cls, repo, parent_commit)
   1191 
   1192             # fill in remaining info - saves time as it doesn't have to be parsed again
-> 1193             sm._name = n
   1194             if pc != repo.commit():
   1195                 sm._parent_commit = pc

AttributeError: 'Tree' object has no attribute '_name'

In [4]:

This seems to be a bug in gitpython module.

I added a flag to your project to skip submodule checkouts and triggered a new build for latest. It built successfully: https://readthedocs.org/projects/chassis/builds/7492481/

Please, let me know if your docs _require_ to checkout the submodules and I will re-enable it.

I changed the title of the issue to make reference to gitpython's bug

[humitos]

Sentry issue: READTHEDOCS-ORG-BDT

[stsewd]

This is because you don't have this submodule https://github.com/Chassis/Chassis/blob/master/.gitmodules#L10 https://github.com/Chassis/Chassis/tree/master/wp/wp-content/themes. That's why gitpython fails. You need to remove/update/fix that.

[humitos]

@stsewd it's true that there is a problem with that submodule in particular, but git doesn't fail if you clone it with git clone --depth 1 --recursive [email protected]:Chassis/Chassis.git. So, our clone step shouldn't fail.

I understand that submodule is not cloned by git command because the path to be checked out already exists in the repository.

We should take some action here:

1. handle this kind of exception and inform the user about an issue with the project's submodules
2. report the bug upstream so gitpython handles it in a better way (I suppose that it should behaves similar to git submodule regular command)

$ git submodule 
 3afcc85932586aaa36913b131ef6662166f3155e puppet/modules/apt (2.3.0-41-g3afcc85)
 9442272f5654b10d1ce33559ebb73d45ff23a128 puppet/modules/mysql (3.10.0-20-g9442272)
 b3c533a838f6e29537fff65a6201832b75451245 puppet/modules/stdlib (4.19.0)
 b8c915d4e052dac4282ba1e9b1e0cce3409fe271 puppet/modules/wp (heads/master)

[stsewd]

@humitos this is already reported in gitpython https://github.com/gitpython-developers/GitPython/issues/279

[stsewd]

I guess we can check for the exception here https://github.com/rtfd/readthedocs.org/blob/09f19a4b722b872278a08d55863e31c6beee43a1/readthedocs/vcs_support/backends/git.py#L79-L87 and raise https://github.com/rtfd/readthedocs.org/blob/09f19a4b722b872278a08d55863e31c6beee43a1/readthedocs/vcs_support/backends/git.py#L217-L217 (or maybe a more descriptive message)

[BronsonQuick]

@humitos Thank you so much for adding the flag to skip the submodules so the docs are building again.

Please, let me know if your docs require to checkout the submodules and I will re-enable it.

We don't need the submodules for the docs thankfully!

@stsewd https://github.com/rtfd/readthedocs.org/issues/4371#issuecomment-405273247 was a great find too. I'm amazed we missed removing that so I'll do a pull request in our project to fix that up.

Thanks again to you both for fixing it, finding the error and talking through a way to possibly handle the exception. Normally I'd close an issue like this seeing our docs are building again but seeing you've been discussing a way to handle the exception I'll leave it to you folks if you want to close this issue.

[humitos]

@stsewd I agree with your proposed solution in https://github.com/rtfd/readthedocs.org/issues/4371#issuecomment-405374327 . I'd just add a TODO comment explaining the situation and linking the original issue in gitpython so we can remove that chunk of code later.

Another possibility that needs some QA, is to use repo.iter_submodules() as they mention in the upstream issue which seems to not be broken. Although, I did a quick test and it returns a generator properly but it fails when iterating over it.

Also, I'm commenting the upstream issue to know if they are planning to include it in the roadmap. The issue is from 2015, though :(

[stsewd]

I'll be implementing the solution after https://github.com/rtfd/readthedocs.org/pull/4493 is merged

[stsewd]

The error happens inside the generator repo.submodules, so we can't just catch the exception, we would lose the other submodules. I'll try to fix this on upstream this weekend.

[stsewd]

Just updating that I have an open to fix this https://github.com/gitpython-developers/GitPython/pull/818