Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Shallow clone breaks build #5031

Closed
stephenfin opened this issue Dec 23, 2018 · 16 comments
Closed

Shallow clone breaks build #5031

stephenfin opened this issue Dec 23, 2018 · 16 comments
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required

Comments

@stephenfin
Copy link
Contributor

stephenfin commented Dec 23, 2018

Details

Expected Result

The build, which utilizes the reno.sphinxext module, should work.

Actual Result

The build fails due to missing tags. These are presumably missing due to the shallow cloning of repos introduced in #4939.

Additional Information

The original issue that #4939 purported to resolve was #1888, Optional use of shallow clone for git repos (emphasis mine). While shallow clone might be useful at large, it should be possible to disable it for projects that rely on git history to build their docs.

@stsewd
Copy link
Member

stsewd commented Dec 23, 2018

I'm guessing this is because the reno package needs the git history https://docs.openstack.org/reno/.

@stsewd stsewd added Improvement Minor improvement to code Needed: design decision A core team decision is required labels Dec 23, 2018
@stephenfin
Copy link
Contributor Author

I'm guessing this is because the reno package needs the git history docs.openstack.org/reno.

Indeed 😞

@humitos
Copy link
Member

humitos commented Dec 24, 2018

@stsewd I think we should add a Feature flag for these cases to omit shallowing.

@stsewd
Copy link
Member

stsewd commented Dec 24, 2018

We have to options here, feature flag (this can only be added by the core team), or an option in the web interface (this can be handled by the user).

@stephenfin
Copy link
Contributor Author

Yeah, I've been looking at this. I see both approaches were adopted for submodules, with bd36494 providing the feature flag approach and the later bfa6bdb providing a config file based approach. I'd been adding a git.shallow_clone setting using the latter approach based on the idea that pretty much everyone using reno or similar tools would be affected. I'm not sure how the config file ties into the web interface though and I don't see submodule configuration there. Is this a third option?

@stsewd
Copy link
Member

stsewd commented Dec 24, 2018

We can't have this option in the config file because the config file is parsed after the clone step. And we don't have the submodules option on the web because we are moving all per-version settings to the config only, all per-project settings will be available on the web.

@stephenfin
Copy link
Contributor Author

Oh, of course 🤦 I guess web UI is the way to go so, though I'm not sure where to nestle that in.

@humitos
Copy link
Member

humitos commented Dec 24, 2018

feature flag (this can only be added by the core team), or an option in the web interface (this can be handled by the user).

Feature flag is the way to go. Admin option is overkill I'd say. I don't expect to have this problem too often.

@stephenfin
Copy link
Contributor Author

See above. I've a pull request which should resolve this using the feature flag.

@humitos
Copy link
Member

humitos commented Dec 26, 2018

We can't have this option in the config file because the config file is parsed after the clone step

Weird idea: I think that in the future we could do a simple/plain first clone of the repo to get the config file and make more decisions after parsing the YAML file (like running another git command to get the full history of the repo). This way will help us to create a "more fully configuration YAML".

@stsewd
Copy link
Member

stsewd commented Dec 26, 2018

@humitos I was thinking that we can use the providers API to fetch only the yaml, I think Travis does that.

ericholscher added a commit that referenced this issue Dec 26, 2018
Add temporary method for disabling shallow cloning (#5031)
ericholscher pushed a commit that referenced this issue Dec 26, 2018
This adds a project feature that allows us to use a standard clone and
fetch rather than the shallow clone/fetch introduced in #4939.
Eventually we should move this to the web UI, but doing so requires some
work to make sure, for example, that git options are only show when
'Project.repo_type' is 'git'.

Signed-off-by: Stephen Finucane <stephen@that.guru>
@ericholscher
Copy link
Member

This is now deployed behind a feature flag, if anyone needs this functionality, just ask an admin.

@stephenfin
Copy link
Contributor Author

This is now deployed behind a feature flag, if anyone needs this functionality, just ask an admin.

How does one do this? I'd like this enabled for git-pw.readthedocs.io and patchwork.readthedocs.io, and I've been told by colleagues that infrared.readthedocs.io is also broken.

@humitos
Copy link
Member

humitos commented Dec 27, 2018

@stephenfin done for git-pw, patchwork and infrared. Please, if you have any issue open a new one to continue the discussion there. Thanks!

I triggered a build manually in git-pw and it worked ;)

@tylerbutler
Copy link

tylerbutler commented Jan 23, 2019

Feature flag is the way to go. Admin option is overkill I'd say. I don't expect to have this problem too often.

Is that the final decision? I think anyone who includes Git commit info in their docs using something like sphinx-git will hit this problem. Moreover, it's a time bomb. Builds will work fine for a while, then one day, once commits you are referencing have fallen out of the shallow clone, builds will start failing, complaining that commit hashes you're referring to can't be found.

I understand prioritizing other work - but this seems like it represents a lot of ongoing maintenance work for support, and also is confusing to users IMO.

@amercader
Copy link

For anyone stumbling onto this issue, you can now add extra steps in the .readthedocs.yaml file to get an unshallow clone:

https://docs.readthedocs.io/en/latest/build-customization.html#unshallow-git-clone

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code Needed: design decision A core team decision is required
Projects
None yet
Development

No branches or pull requests

6 participants