Qiskit / qiskit_sphinx_theme

A Sphinx theme and documentation guidelines for Qiskit and Qiskit Ecosystem projects
https://qisk.it/docs-guide
Apache License 2.0
15 stars 29 forks source link

Wrong links generated for previous versions #551

Closed eddybrando closed 1 year ago

eddybrando commented 1 year ago

We had some 404 issues in the documentation pages are are investigating them as part of https://github.com/Qiskit/qiskit.org/issues/3498.

From the investigation in https://github.com/Qiskit/qiskit.org/issues/3500:

Bug in Links to Previous Versions: There seems to be a bug in the way we generate the links to previous versions of documentation pages, potentially originating from the Qiskit Sphinx Theme repository.

This issue is to fix those generated links to construct the URLs correctly.

Eric-Arellano commented 1 year ago

This is only broken on Qiskit version 0.43 https://qiskit.org/documentation/stable/0.43/index.html. It works everywhere else, including https://qiskit.org/documentation/stable/0.44/index.html. To reproduce, you go to the stable version and then try clicking any other release in the Previous Releases left tab.

There are three builds to compare:

--

Between 0.43 and 0.44, the major difference is that 0.43 sets the option docs_url_prefix to documentation/stable/<version> whereas 0.44 always keeps it to documentation/:

https://github.com/Qiskit/qiskit-metapackage/blob/2b423d04dfb799650c1459b0e0d8a70dc1d095f3/tools/deploy_documentation_tag.sh#L36-L36

https://github.com/Qiskit/qiskit/blob/39b0cb82ab2f3bce5aee663ec1f1e0817aca71db/docs/conf.py#L34-L36

Jake was correct when he moved the docs config to always use documentation/. That is a bug to set it to include /stable, and it explains why the URL has /stable twice:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/d1a353dea1ca367b7dfc03e3c381aa97633319f3/src/qiskit_sphinx_theme/previous_releases.py#L39

--

So, then the only remaining question is why did this work in 0.42 and prior versions? In 0.42 and earlier, the option was called content_prefix:

https://github.com/Qiskit/qiskit-metapackage/blob/96f760475cb6ed9969c574dd6d79eec49165e930/tools/deploy_documentation_tag.sh#L36-L36

At that point, our previous versions logic ignored content_prefix and the code solely worked for Qiskit Terra:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/73f4d2cd6bf523b44cd285a532229d92ce079f1b/qiskit_sphinx_theme/stable_versions.html#L5-L7

We used content_prefix for translations only:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/73f4d2cd6bf523b44cd285a532229d92ce079f1b/qiskit_sphinx_theme/versionutils.py#L107-L115

https://github.com/Qiskit/qiskit_sphinx_theme/blob/73f4d2cd6bf523b44cd285a532229d92ce079f1b/qiskit_sphinx_theme/versionutils.py#L71-L73

Note that we don't deploy translations to versions, though! So the config was always bogus. If you click the language drop down on https://qiskit.org/documentation/stable/0.42/locale/bn_BN/index.html, it 404s, even though we had those translations provided in 0.42.

https://github.com/Qiskit/qiskit_sphinx_theme/pull/321 replaced content_prefix with docs_url_prefix to fix Ecosystem projects using Previous Releases, which previously did not work. The implementation was sound in https://github.com/Qiskit/qiskit_sphinx_theme/pull/321, but it was not correct to keep setting content_prefix=documentation/stable/<version> and to rename that to docs_url_prefix.

Eric-Arellano commented 1 year ago

This is now fixed via a manual redeploy of the 0.43 docs with keeping docs_url_prefix only set to documentation/. https://qiskit.org/documentation/stable/0.43/tutorials.html