Skip to content

Comments

Support recursive requirements and constraints inclusion#15657

Merged
konstin merged 3 commits intomainfrom
konsti/support-recursive-requirements-inclusion
Sep 5, 2025
Merged

Support recursive requirements and constraints inclusion#15657
konstin merged 3 commits intomainfrom
konsti/support-recursive-requirements-inclusion

Conversation

@konstin
Copy link
Member

@konstin konstin commented Sep 3, 2025

uv currently panics with a stack overflow when requirements or constraints are recursively included. Instead, we ignore files we have already seen. The one complexity here is that we have to track whether we're in a requirements inclusion or in a constraints inclusion, to allow including a file separately for requirements and for constraints, and to handle -r inside or -c (which we treat as constraints too).

Fixes #15650

uv currently panics with a stack overflow when requirements or constraints are recursively included. Instead, we ignore files we have already seen. The one complexity here is that we have to track whether we're in a requirements inclusion or in a constraints inclusion, to allow including a file separately for requirements and for constraints, and to handle `-r` inside or `-c` (which we treat as constraints too).
@konstin konstin added the bug Something isn't working label Sep 3, 2025
requirements_txt: impl AsRef<Path>,
working_dir: impl AsRef<Path>,
client_builder: &BaseClientBuilder<'_>,
visited: &mut VisitedFiles<'_>,
Copy link
Member Author

Choose a reason for hiding this comment

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

We have to use nested &mut to convince rustc that the inner &mut FxHashSet remain unique.

@konstin konstin temporarily deployed to uv-test-registries September 3, 2025 12:11 — with GitHub Actions Inactive
if requirements.contains(&sub_file) {
continue;
}
requirements.insert(sub_file.clone())
Copy link
Member

Choose a reason for hiding this comment

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

Consider using the return value of requirements.insert instead of the contains-insert?

/// The requirements are included as constraints, all recursive inclusions are considered
/// constraints.
Constraints {
constraints: &'a mut FxHashSet<PathBuf>,
Copy link
Member

Choose a reason for hiding this comment

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

Why do these need to be unowned? Why not requirements: FxHashSet?

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree that they are ugly; What I'm trying to do is keep track of the visited files for requirements and constraints separately, but upon recursing into -c, keep track of only constraints, since -r nested in a -c'd file is also considered a constraint, like in the logic below.

                    // Switch to constraints mode, if we aren't in it already.
                    let mut visited = match visited {
                        VisitedFiles::Requirements { constraints, .. } => {
                            if !constraints.insert(sub_file.clone()) {
                                continue;
                            }
                            VisitedFiles::Constraints { constraints }
                        }
                        VisitedFiles::Constraints { constraints } => {
                            if !constraints.insert(sub_file.clone()) {
                                continue;
                            }
                            VisitedFiles::Constraints { constraints }
                        }
                    };

Copy link
Member Author

Choose a reason for hiding this comment

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

Going to merge this, I'll refactor it if someone knows how to write this in more elegant rust.

let mut visited = VisitedFiles::Requirements {
requirements: &mut FxHashSet::default(),
constraints: &mut FxHashSet::default(),
};
Copy link
Member

Choose a reason for hiding this comment

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

Consider a default impl?

Copy link
Member Author

Choose a reason for hiding this comment

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

It doesn't allow one for &mut unfortunately.

@konstin konstin temporarily deployed to uv-test-registries September 3, 2025 15:18 — with GitHub Actions Inactive
@konstin konstin temporarily deployed to uv-test-registries September 3, 2025 16:04 — with GitHub Actions Inactive
@konstin konstin merged commit 549fb12 into main Sep 5, 2025
240 of 242 checks passed
@konstin konstin deleted the konsti/support-recursive-requirements-inclusion branch September 5, 2025 09:20
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Sep 12, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.8.15` -> `0.8.17` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.8.17`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0817)

[Compare Source](astral-sh/uv@0.8.16...0.8.17)

Released on 2025-09-10.

##### Enhancements

- Improve error message for HTTP validation in auth services ([#&#8203;15768](astral-sh/uv#15768))
- Respect `PYX_API_URL` when suggesting `uv auth login` on 401 ([#&#8203;15774](astral-sh/uv#15774))
- Add pyx as a supported PyTorch index URL ([#&#8203;15769](astral-sh/uv#15769))

##### Bug fixes

- Avoid initiating login flow for invalid API keys ([#&#8203;15773](astral-sh/uv#15773))
- Do not search for a password for requests with a token attached already ([#&#8203;15772](astral-sh/uv#15772))
- Filter pre-release Python versions in `uv init --script` ([#&#8203;15747](astral-sh/uv#15747))

### [`v0.8.16`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0816)

[Compare Source](astral-sh/uv@0.8.15...0.8.16)

##### Enhancements

- Allow `--editable` to override `editable = false` annotations ([#&#8203;15712](astral-sh/uv#15712))
- Allow `editable = false` for workspace sources ([#&#8203;15708](astral-sh/uv#15708))
- Show a dedicated error for virtual environments in source trees on build ([#&#8203;15748](astral-sh/uv#15748))
- Support Android platform tags ([#&#8203;15646](astral-sh/uv#15646))
- Support iOS platform tags ([#&#8203;15640](astral-sh/uv#15640))
- Support scripts with inline metadata in `--with-requirements` and `--requirements` ([#&#8203;12763](astral-sh/uv#12763))

##### Preview features

- Support `--no-project` in `uv format` ([#&#8203;15572](astral-sh/uv#15572))
- Allow `uv format` in unmanaged projects ([#&#8203;15553](astral-sh/uv#15553))

##### Bug fixes

- Avoid erroring when `match-runtime` target is optional ([#&#8203;15671](astral-sh/uv#15671))
- Ban empty usernames and passwords in `uv auth` ([#&#8203;15743](astral-sh/uv#15743))
- Error early for parent path in build backend ([#&#8203;15733](astral-sh/uv#15733))
- Retry on IO errors during HTTP/2 streaming ([#&#8203;15675](astral-sh/uv#15675))
- Support recursive requirements and constraints inclusion ([#&#8203;15657](astral-sh/uv#15657))
- Use token store credentials for `uv publish` ([#&#8203;15759](astral-sh/uv#15759))
- Fix virtual environment activation script compatibility with latest nushell ([#&#8203;15272](astral-sh/uv#15272))
- Skip Python interpreters that cannot be queried with permission errors ([#&#8203;15685](astral-sh/uv#15685))

##### Documentation

- Clarify that `uv auth` commands take a URL ([#&#8203;15664](astral-sh/uv#15664))
- Improve the CLI help for options that accept requirements files ([#&#8203;15706](astral-sh/uv#15706))
- Adds example for caching for managed Python downloads in Docker builds ([#&#8203;15689](astral-sh/uv#15689))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MS45OC4xIiwidXBkYXRlZEluVmVyIjoiNDEuOTkuNiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Infinite recursion when parsing circular -r references in multiple requirements.txt

2 participants