Sphinx requires that labels within documents are unique across the
whole manual.  This is because the "create a hyperlink" directive
specifies only the name of the label, not a filename+label.  Some
Sphinx versions will warn about duplicate labels, but even if there
is no warning there is still an ambiguity and no guarantee that the
hyperlink will be created to the intended target.

For QEMU this is awkward, because we have various .rst.inc fragments
which we include into multiple .rst files.  If you define a label in
the .rst.inc file then it will be a duplicate label.  We have mostly
worked around this by not putting labels into those .rst.inc files,
or by adding "insert a label" functionality into the hxtool extension
(see commit 1eeb432a953b0 "doc/sphinx/hxtool.py: add optional label
argument to SRST directive"). However, we let one into the codebase
without initially noticing, in commit 7f6314427e ("docs/devel: add a
codebase section"), because not all versions of Sphinx warn about
the duplicate labels.

This patchset resolves the problem by implementing a small Sphinx
extension. The extension lets you write in a .rst.inc:

  .. uniquelabel:: mylabel

and it will be as if you had written:

  .. _foo/bar-mylabel

where foo/bar.rst is the top level document that includes the
.rst.inc file.

Patch 1 is the extension; patch 2 is the use of it to fix the
problem in qemu-block-drivers.rst.inc. (Concretely, the result is
that instead of an ambiguous "nbd" label, we now have separate
"system/images-nbd" and "system/qemu-block-drivers-nbd" labels.
We want to link to the former, because the latter is in the
manpage, not the proper HTML manual.)

This patchset is a bit RFC quality -- I have not tested it
super thoroughly, and the extension itself is written based on
our existing ones, because I'm neither a Python nor a Sphinx
expert. I figured I'd send it out to see if people agreed that
it was the right way to solve this problem.

(In theory we could remove the SRST(label) functionality from
the hxtool extension and have the .hx files use uniquelabel.
Not sure that's worthwhile at this point.)

PS: I find that our extensions are confused about whether they
should set "required_arguments = 1" or "required_argument = 1";
probably the latter are all bugs that happen to have no bad
side effects...

thanks
-- PMM

Peter Maydell (2):
  docs: Create a uniquelabel Sphinx extension
  docs: Use uniquelabel in qemu-block-drivers.rst.inc

 docs/conf.py                           |  1 +
 docs/devel/codebase.rst                |  2 +-
 docs/sphinx/uniquelabel.py             | 74 ++++++++++++++++++++++++++
 docs/system/qemu-block-drivers.rst.inc |  2 +-
 4 files changed, 77 insertions(+), 2 deletions(-)
 create mode 100644 docs/sphinx/uniquelabel.py

-- 
2.43.0

Ping? Any opinions on this?

In the interim we've applied commit 82707dd4f0 to drop
the specific duplicate-label that is causing problems
right now, so patch 2 here will need the obvious trivial
update. But I do think this is a better approach than
forever avoiding defining labels in .rst.inc files...

thanks
-- PMM

On Tue, 29 Apr 2025 at 17:32, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Sphinx requires that labels within documents are unique across the
> whole manual.  This is because the "create a hyperlink" directive
> specifies only the name of the label, not a filename+label.  Some
> Sphinx versions will warn about duplicate labels, but even if there
> is no warning there is still an ambiguity and no guarantee that the
> hyperlink will be created to the intended target.
>
> For QEMU this is awkward, because we have various .rst.inc fragments
> which we include into multiple .rst files.  If you define a label in
> the .rst.inc file then it will be a duplicate label.  We have mostly
> worked around this by not putting labels into those .rst.inc files,
> or by adding "insert a label" functionality into the hxtool extension
> (see commit 1eeb432a953b0 "doc/sphinx/hxtool.py: add optional label
> argument to SRST directive"). However, we let one into the codebase
> without initially noticing, in commit 7f6314427e ("docs/devel: add a
> codebase section"), because not all versions of Sphinx warn about
> the duplicate labels.
>
> This patchset resolves the problem by implementing a small Sphinx
> extension. The extension lets you write in a .rst.inc:
>
>   .. uniquelabel:: mylabel
>
> and it will be as if you had written:
>
>   .. _foo/bar-mylabel
>
> where foo/bar.rst is the top level document that includes the
> .rst.inc file.
>
> Patch 1 is the extension; patch 2 is the use of it to fix the
> problem in qemu-block-drivers.rst.inc. (Concretely, the result is
> that instead of an ambiguous "nbd" label, we now have separate
> "system/images-nbd" and "system/qemu-block-drivers-nbd" labels.
> We want to link to the former, because the latter is in the
> manpage, not the proper HTML manual.)
>
> This patchset is a bit RFC quality -- I have not tested it
> super thoroughly, and the extension itself is written based on
> our existing ones, because I'm neither a Python nor a Sphinx
> expert. I figured I'd send it out to see if people agreed that
> it was the right way to solve this problem.
>
> (In theory we could remove the SRST(label) functionality from
> the hxtool extension and have the .hx files use uniquelabel.
> Not sure that's worthwhile at this point.)
>
> PS: I find that our extensions are confused about whether they
> should set "required_arguments = 1" or "required_argument = 1";
> probably the latter are all bugs that happen to have no bad
> side effects...
>
> thanks
> -- PMM
>
> Peter Maydell (2):
>   docs: Create a uniquelabel Sphinx extension
>   docs: Use uniquelabel in qemu-block-drivers.rst.inc
>
>  docs/conf.py                           |  1 +
>  docs/devel/codebase.rst                |  2 +-
>  docs/sphinx/uniquelabel.py             | 74 ++++++++++++++++++++++++++
>  docs/system/qemu-block-drivers.rst.inc |  2 +-
>  4 files changed, 77 insertions(+), 2 deletions(-)
>  create mode 100644 docs/sphinx/uniquelabel.py
>
> --

Ping^2 on this one?

-- PMM

On Mon, 19 May 2025 at 15:32, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Ping? Any opinions on this?
>
> In the interim we've applied commit 82707dd4f0 to drop
> the specific duplicate-label that is causing problems
> right now, so patch 2 here will need the obvious trivial
> update. But I do think this is a better approach than
> forever avoiding defining labels in .rst.inc files...
>
> thanks
> -- PMM
>
> On Tue, 29 Apr 2025 at 17:32, Peter Maydell <peter.maydell@linaro.org> wrote:
> >
> > Sphinx requires that labels within documents are unique across the
> > whole manual.  This is because the "create a hyperlink" directive
> > specifies only the name of the label, not a filename+label.  Some
> > Sphinx versions will warn about duplicate labels, but even if there
> > is no warning there is still an ambiguity and no guarantee that the
> > hyperlink will be created to the intended target.
> >
> > For QEMU this is awkward, because we have various .rst.inc fragments
> > which we include into multiple .rst files.  If you define a label in
> > the .rst.inc file then it will be a duplicate label.  We have mostly
> > worked around this by not putting labels into those .rst.inc files,
> > or by adding "insert a label" functionality into the hxtool extension
> > (see commit 1eeb432a953b0 "doc/sphinx/hxtool.py: add optional label
> > argument to SRST directive"). However, we let one into the codebase
> > without initially noticing, in commit 7f6314427e ("docs/devel: add a
> > codebase section"), because not all versions of Sphinx warn about
> > the duplicate labels.
> >
> > This patchset resolves the problem by implementing a small Sphinx
> > extension. The extension lets you write in a .rst.inc:
> >
> >   .. uniquelabel:: mylabel
> >
> > and it will be as if you had written:
> >
> >   .. _foo/bar-mylabel
> >
> > where foo/bar.rst is the top level document that includes the
> > .rst.inc file.
> >
> > Patch 1 is the extension; patch 2 is the use of it to fix the
> > problem in qemu-block-drivers.rst.inc. (Concretely, the result is
> > that instead of an ambiguous "nbd" label, we now have separate
> > "system/images-nbd" and "system/qemu-block-drivers-nbd" labels.
> > We want to link to the former, because the latter is in the
> > manpage, not the proper HTML manual.)
> >
> > This patchset is a bit RFC quality -- I have not tested it
> > super thoroughly, and the extension itself is written based on
> > our existing ones, because I'm neither a Python nor a Sphinx
> > expert. I figured I'd send it out to see if people agreed that
> > it was the right way to solve this problem.
> >
> > (In theory we could remove the SRST(label) functionality from
> > the hxtool extension and have the .hx files use uniquelabel.
> > Not sure that's worthwhile at this point.)
> >
> > PS: I find that our extensions are confused about whether they
> > should set "required_arguments = 1" or "required_argument = 1";
> > probably the latter are all bugs that happen to have no bad
> > side effects...
> >
> > thanks
> > -- PMM
> >
> > Peter Maydell (2):
> >   docs: Create a uniquelabel Sphinx extension
> >   docs: Use uniquelabel in qemu-block-drivers.rst.inc
> >
> >  docs/conf.py                           |  1 +
> >  docs/devel/codebase.rst                |  2 +-
> >  docs/sphinx/uniquelabel.py             | 74 ++++++++++++++++++++++++++
> >  docs/system/qemu-block-drivers.rst.inc |  2 +-
> >  4 files changed, 77 insertions(+), 2 deletions(-)
> >  create mode 100644 docs/sphinx/uniquelabel.py
> >
> > --

On 6/9/25 18:18, Peter Maydell wrote:
> Ping^2 on this one?

No objections if it's the easiest way to solve the issue.

Alternatively, is there a Sphinx way to write something in the spirit of

#ifdef DEFINE_THE_LABEL
.. _label
#endif

This way, you could have the nbd label enabled when including into 
system/images, but not when including into system/qemu-block-drivers.

Paolo

On Mon, 9 Jun 2025 at 17:38, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 6/9/25 18:18, Peter Maydell wrote:
> > Ping^2 on this one?
>
> No objections if it's the easiest way to solve the issue.
>
> Alternatively, is there a Sphinx way to write something in the spirit of
>
> #ifdef DEFINE_THE_LABEL
> .. _label
> #endif

Unfortunately not. Sphinx's conditional-content stuff is a bit
odd, and only works for actual docs, not for sections, labels, etc:

https://www.sphinx-doc.org/en/master/usage/extensions/ifconfig.html
https://www.sphinx-doc.org/en/master/usage/restructuredtext/directives.html#including-content-based-on-tags

IIRC this is because under the hood the conditionalisation
only takes effect quite late, when it's about to emit
something: they really act just as filters on the output.
So you can't conditionalise a directive or anything that
needs to take effect at the parsing stage.

https://pypi.org/project/sphinx-selective-exclude/
is an extension that tries to work around this, but it
hasn't been updated in years and it probably doesn't work
with newer sphinx versions.

-- PMM