Skip to content

Conversation

@RR5555
Copy link
Contributor

@RR5555 RR5555 commented Jul 28, 2025

Issue

Circular transclusions lead to:

E           RecursionError: maximum recursion depth exceeded while calling a Python object

/home/gitpod/.local/share/uv/python/cpython-3.9.23-linux-x86_64-gnu/lib/python3.9/pathlib.py:646: RecursionError

It might be nice to catch it as soon as it happens, and provide a more comprehensive error message including the names of the files that collide.


This PR

This PR introduces tests for circular transclusions that shows that these are not really dealt with in a user-friendly manner in addition to taking some unnecessary time.
The tests are currently skipped to not interfere with normal condition testing, and commented with a TODO line similar to the last one in the previous section Issue.

NOTE: The test with demo_transclude_circular_bis will fail because of the circular recursion.
The test with demo_transclude_circular will:

  • currently fail with a yaml.composer.ComposerError: expected a single document in the stream
  • if https://github.com/copier-org/copier/pull/2251 is accepted, also fail because of the circular recursion.
    But it might be a better test as it needs to go to the deepest transclusion to get the _excludes.

Circular transclusions lead to:
```bash
E           RecursionError: maximum recursion depth exceeded while calling a Python object

/home/gitpod/.local/share/uv/python/cpython-3.9.23-linux-x86_64-gnu/lib/python3.9/pathlib.py:646: RecursionError
```
It might be nice to catch as soon as it happens, and provides a more comprehensive error message including the names of the files that collide.
The tests are skipped to not intefer with normal condition testing, and commented with a TODO with a line to the one right before this one.

> NOTE: The test with `demo_transclude_circular_bis` will fail because of the circular recursion.
> The test with `demo_transclude_circular` will:
> - currently fail with a `yaml.composer.ComposerError: expected a single document in the stream`
> - if `https://github.com/copier-org/copier/pull/2251` is accepted, also fail because of the circular recursion.
> But it might be a better test as it needs to go to the deepest transclusion to get the `_excludes`.
Copy link
Member

@sisp sisp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this bad error management and providing test cases, @RR5555! 🙇

We might even be able to add cycle detection with just some minor change. The YAML loader with support for the !include constructor is built inside the load_template_config() function. As the !include handler is a closure in this function, we could record the included YAML files in a set created in this function,, and when including another YAML file which has been included before, we could raise a proper error like CircularConfigFileIncludeError (declared in copier/errors.py). WDYT?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants