Remove support for the Sphinx 0.5 intersphinx_mapping format

[picnixz]

Extracted from #11706.

cc @jayaddison

[AA-Turner]

Have rebased this on the now-split intersphinx package.

A

[AA-Turner]

Looks good -- main high-level feedback is that we should now be stricter on rejecting invalid formats (and add appropriate tests that ConfigError is raised)

A

[jayaddison]

Do we need to update ENV_VERSION with this change?

[picnixz]

Do we need to update ENV_VERSION with this change?

I don't know but to be sure, let's do it :)

[AA-Turner]

Do we need to update ENV_VERSION with this change?

We shouldn't need to, as the resulting data that gets stored in the environment is unchanged -- we haven't changed how we do the normalisation.

A

[picnixz]

Actually, by doing so, it would at least invalidate the cache (which could contain old intersphinx references). But if you want, you can revert the last commit (just tell me whether it's you or me that need to work on the branch now; I don't want race issues)

[AA-Turner]

just tell me whether it's you or me that need to work on the branch now

I'm done for now, don't worry

[jayaddison]

@jayaddison

Do we need to update ENV_VERSION with this change?

@AA-Turner

We shouldn't need to, as the resulting data that gets stored in the environment is unchanged -- we haven't changed how we do the normalisation.

@picnixz

Actually, by doing so, it would at least invalidate the cache (which could contain old intersphinx references). But if you want, you can revert the last commit (just tell me whether it's you or me that need to work on the branch now; I don't want race issues)

Ok. Perhaps reverting it here, but adding it in #12087 could make sense? Reasoning: it could be unexpected to have resolved non-deterministic inventories, and yet not rewrite existing environment files affected by that after an upgrade to 8.x takes place.

[picnixz]

Ok. Perhaps reverting it here

Actually, by invalidating the environment, you somewhat fix the bugs in #12087. I prefer erring on the safe side at the cost of a single rebuild (also, it's kind of a major change where now we issue errors instead).

Also, we are just storing less information so technically I would say we should still create a new environment (from a typing PoV, it's no more possible to have a None name, now it should always be a string).

[AA-Turner]

@picnixz do you have any other changes, or is this ready to merge from your perspective?

A

[picnixz]

do you have any other changes, or is this ready to merge from your perspective?

I don't have anything else! I expect people to be surprised if they build very old projects but I think the test coverage is sufficient. I'm just unsure whether the typing definitions I put should be kept in shared or not but this is something we can decide later (for now, let's keep them like that because the other PR is based on this one and I don't want to again solve conflicts). By the way, thank you for the CHANGES entry.

[AA-Turner]

Thanks all!

A

[EwoutH]

Hi!

I just stumbled onto this PR as the root cause of our docs breaking. While I looked back and there was a deprecation warning cast by Sphinx 7.0 (which is nice, thanks!), the error message in 8.0 could be a bit clearer, pointing directly to some docs or migration guide on how to update it.

ERROR: Invalid value `None` in intersphinx_mapping['http://docs.python.org/']. Expected a two-element tuple or list.

For anyone also stumbling on this PR, the our fix was replacing:

intersphinx_mapping = {"http://docs.python.org/": None}

with

intersphinx_mapping = {'python': ('https://docs.python.org/3', None)}

As following these docs.

I was a bit amazed to see a 10 year old line of code breaking our docs configuration. Doesn't happen often!

[AA-Turner]

Thanks @EwoutH. Do you have any concrete suggestions for the error message? I'm happy to update it.

A