#459 Invalid circular inclusion warning when including multiple documents from a directive

[Ian Wienand]

The recent sphinx release has found what I think is a problem in the circular-reference detection in docutils.

We have a situation where we build a page with a directive that automatically includes a file (readme from a sub-component). In some cases, these readme's might then also include another file (e.g. similar components share a common file about their arguments, etc.). So we have a include hierarchy like

 .. include:: ./component/README.rst
   .. include:: common.rst
.. include:: ./component2/README.rst
  .. include:: common.rst

Our directive works by reading the README.rst file and then using self.state_machine.insert_input()

Our build started failing due a circular inclusion in "include" directive which comes from the second inclusion of the common.rst above. In this situation, I don't believe there is any circular inclusion -- we want the same content included twice; but it is not referencing itself.

I think it has something to do with self.state.document.include_log thinking that it has seen common.rst before.

I have isolated this down to the smallest example I can think of and attached that; it is also at https://paste.opendev.org/show/brTxHYllHebBIs1wHbfM/

When I run this I get

$ ./docutils-venv/bin/python3 ./recursive.py 
Write combined.rst
Write file1.rst
Write file2.rst
Write common.rst
file1.rst:1: (WARNING/2) circular inclusion in "include" directive:
file1.rst
> file1.rst

[Günter Milde]

Thank you for reporting a problem.
It seems related to the changes required to fix [bugs:#366] in [r8851].

With the stock include directive, Docutils correctly identifies "common.rst" as included from different files and does not throw the "circular inclusion" error.
The custom directive does not add the logging information required for the detection of this case. Compare the last lines of directives.misc.Include.run (lines 198 ff. in docutils/parsers/rst/directives/misc.py).

[Ian Wienand]

Thanks -- I guess the problem here is that this directive uses insert_input to insert input which then just happens to have a .. include:: directive inside it.

It seems like the suggestion is to directly modify the document.include_log which is probably close to the change I've suggested in our actual code at

https://review.opendev.org/c/zuul/zuul-sphinx/+/862215/2/zuul_sphinx/zuul.py

It feels like a bit of a breaking change to insert_input if the include log needs to be manually managed around that call?

[Günter Milde]

insert_input() is an undocumented (internal) auxiliary method.
Maybe the custom directive can subclass or call directives.misc.Include instead.

Just emptying the document's include_log breaks the detection of circular inclusions :(
(You will only recognise this in CI, if there are test cases that have both, the custom directive and a circular inclusion.)

Checking the test example file, I realized that it reports:

file1.rst:1: (WARNING/2) circular inclusion in "include" directive:
file1.rst
> file1.rst

i.e. the inclusion detection assumes that "file1.rst" includes itself recursively!
This is due to a wrong usage of state_machine.insert_input():
the line written by DirectiveThatIncludes.run() will (when parsed) include the source "name", but it is not from source "name". With

    def run(self):
        name = self.content[0]
        source = self.state_machine.input_lines.source(
            self.lineno - self.state_machine.input_offset - 1)
        self.state_machine.insert_input(
            ['.. include:: %s' % name], source)
        return []

there is no warning (because, the inclusion of "file1" and "file2" uses the stock misc.Directiveclass that handles circular inclusion detection fine).
Even when using

    def run(self):
        name = self.content[0]
        with open(name) as f:
            lines = f.read()
        self.state_machine.insert_input(lines, name)
        return []

the circular-inclusion warning does not appear here.

[Ian Wienand]

Thank you for your time looking at this. I'm still a bit confused as to what is going on, as we do try in the plugin to set the name/source field correctly when using insert_input.

In the above, I'm not sure the lines = f.read() quite works because I don't think that puts it into a nice array of lines for parsing; so I think it might hide the issue.

I have been back to our build environment and found some more details -- including bisecting this down to a change between sphinx 5.2.3 and 5.3; see https://github.com/sphinx-doc/sphinx/issues/10951

Additionally, this only seems to occur with docutils 0.17 -- if I downgrade to 0.16 or upgrade to 0.18 I don't the same build error! However we remain on 0.17 due to a dependency from rtd_sphinx_theme

[Günter Milde]

I'm not sure the lines = f.read() quite works because I don't think that puts it into a nice array of lines for parsing ...

The lines = f.read() in my small example is a stub.
The point is, that if you insist on statemachine.insert_input() to inject generated lines to the document while parsing it, either use the currently active source or a "fancy description" as second argument, e.g.

def run(self):
    name = self.content[0]
    self.state_machine.insert_input(
        ['.. include:: %s' % name], 'mytrickydirective')
    return []

This works in 0.18 but I can't tell for 0.17.

[Günter Milde]

Additionally, this only seems to occur with docutils 0.17 -- if I downgrade to 0.16 or upgrade to 0.18 I don't the same build error!

This is very important information, because [bugs:#366] was reported for 0.17 and fixed in 0.18.

The bug report described cases of "false positives" in the circular inclusion detection, reproducible in stock Docutils for specific input cases. Maybe your problem is triggered by the same fault to the 0.17 version of the detection algorithm.
The fix includes incompatible changes to the "manual management" of the include_log required around an insert_input call. This means any fix on your side should be tested across Docutils versions :(

[Günter Milde]

Thanks for reporting.

if I downgrade to 0.16 or upgrade to 0.18 I don't the same build error!

Closing as "out-of-date", as we are working on 0.20 by now.