-
Notifications
You must be signed in to change notification settings - Fork 1
build warning: Git clone too shallow #115
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
base: main
Are you sure you want to change the base?
Conversation
|
sphinx-last-updated-by-git is suddenly pulled in transitively due to a newer sphinx_sitemap (upgraded from 2.6.0 to 2.7.1) - c'est la vie when we don't use lock files for dependencies... There are several ways to fix this as described in sphinx_last_updated_by_git.
@amotl Are aware to which extend it will have an effect to disable the warning? Or perhaps we should just do a fuller fetch to allow support for features instead of restricting them? It's still not clear for me what this new feature does (add last-updated date in sitemap.xml?) and if it actually has an effect on our site without other changes to support it? Are we actually hosting a sitemap.xml file for our docs? (I could only see the root sitemap.xml file - is it included there?) |
|
Didn't check yet if we can do either of the solutions just in this repo or it's just makefile rules that are inherited from here to all the projects. |
|
Hi. I don't know anything about relevant consequences, but any option can be explored. Another one, for example a quick one to restore the builds, would be to NOT upgrade to the offending @msbt is the expert in maintaining our sitemap.xml's, but if you would like I can also gather relevant information. I dearly hope other departments didn't disconnect the documentation sitemap again, which happened in the past, not just once. |
| @@ -1 +1,3 @@ | |||
| from crate.theme.rtd.conf.theme import * | |||
|
|
|||
| #suppress_warnings = ['git.too_shallow'] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one will just apply to the documentation of this tool repository in isolation, nothing else. If we want to roll out a Sphinx setting across all documentation projects, crate-docs-theme is the right place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think suppressing warnings should only happen as a very last resort, of course always depending on circumstances.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this, merge, and then spread it out to other repositories?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are still a few open questions:
- What depth is needed when fetching?
- Do we get any value out of the new sphinx-sitemap? I.e. dates on when a page was updated? Is that for end users or search?
- Can we disable this by configuration (assuming we don't need/want it). I remember seeing something, but don't recall exact config option right now...
- Will this be fixed in sphinx-sitemap itself?
Therefore itt seems very attractive to be able to fix this temporarily for all docs builds just one place, so I would suggest to pin sphinx-sitemap in crate-docs-theme instead for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Brian.
It seems very attractive to be able to fix this temporarily for all docs builds just one place, so I would suggest to crate/crate-docs-theme#611 instead for now.
That's absolutely way to go in such a situation, thank you so much.
| jobs: | ||
| post_checkout: | ||
| - git fetch --unshallow || true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the build is not failing on RTD (is it?), so we don't need this update here, just for GHA?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it also fails on RTD, so this is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh ok. Do you have a sample? Maybe an update of Git on their ends responsible for the regression?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://docs.readthedocs.com/platform/stable/build-customization.html#unshallow-git-clone describes this (in general). I doubt RTD can do anything about this except describe the issue in general?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://app.readthedocs.org/projects/crate-jdbc/builds/28608809/ shows an example of the same build problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://pypi.org/project/sphinx-last-updated-by-git/ describes how to handle this issue if the extension is used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, thanks.
|
Those tickets relay further information about the issue at hand. |
You can find more information on a non-public repository here. |
testing latest build issue that occurs in all docs repos suddenly:
The problem has been observed on those CI runs on GHA, and others: THEME-609, THEME-610.