Skip to content

Commit efd4f6d

Browse files
Irfan Ahmadclaude
authored andcommitted
fix: correct uv sync/export flags that silently dropped project deps
Found via real CI runs after opening the PR (unit tests, Quality Others, and the ReadTheDocs build all failed): 1. Makefile's test-requirements used `uv sync --only-group testing`, which EXCLUDES [project.dependencies] entirely (confirmed: `--only-group` replaces the dependency set rather than adding to it, unlike `--group`). This meant Django, XBlock, and the rest of the actual application were never installed for test runs -- ModuleNotFoundError: No module named 'xblock' on every unit test shard. Fixed to `--no-default-groups --group testing`, matching what tox-uv itself generates for the equivalent tox environment. 2. .readthedocs.yaml had the same bug in its doc-requirements.txt export (`--only-group doc`). Since [project.dependencies] were excluded from that export, the subsequent `pip install -e .` resolved the whole dependency tree completely unconstrained by uv.lock's [tool.uv].constraint-dependencies -- picking setuptools==82.0.1 (violates setuptools<82) and Django==6.0.6 (violates Django<6.0). The setuptools violation broke fs/pyfilesystem2's pkg_resources import, crashing the Sphinx build via Django app loading. Fixed to `--group doc` plus `--no-deps` on the `pip install -e .` step, so dependencies only ever come from the properly-constrained export. 3. scripts/xsslint_config.py's SKIP_DIRS didn't exclude .venv. Under the old pip-compile system, dependencies installed into the system Python outside the repo checkout, so this never mattered. Now that `uv sync` creates .venv/ inside the checkout, xsslint's directory walk (which defaults to scanning the whole cwd) swept up thousands of vendored third-party files, inflating violations from 64 (the accepted baseline) to 316. Verified all three: real pytest run (59 passed) plus full Django `manage.py check` for both LMS and CMS against the corrected test-requirements; a local simulation of the RTD build job sequence confirms Django==5.2.15/setuptools==81.0.0 (both constraint-compliant) and a successful Sphinx build. Audited every other `--only-group` usage introduced in this migration (assets.txt compat export, semgrep.yml) -- both are intentionally project-dependency-free, matching the original files' documented behavior, and their CI checks already passed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent d07c449 commit efd4f6d

3 files changed

Lines changed: 15 additions & 10 deletions

File tree

.readthedocs.yaml

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,20 @@ build:
77
jobs:
88
post_create_environment:
99
- pip install uv
10-
# Export the `doc` dependency-group to a plain requirements.txt so it can be
11-
# installed into RTD's own managed virtualenv below, rather than a separate
12-
# uv-managed one -- this keeps RTD's normal sphinx auto-build detection intact.
13-
# constraint-dependencies (e.g. setuptools<82, needed by fs/pyfilesystem2) are
14-
# applied automatically as part of this export, so no separate pre-pinning step
15-
# is needed the way requirements/pip-tools.txt used to provide.
16-
- uv export --frozen --no-hashes --only-group doc -o doc-requirements.txt
10+
# Export [project.dependencies] *and* the `doc` group together (--group, not
11+
# --only-group -- the latter excludes [project.dependencies] entirely, which
12+
# would leave Django/XBlock/etc. for `pip install -e .` below to resolve on
13+
# its own, unconstrained by uv.lock's [tool.uv].constraint-dependencies, e.g.
14+
# picking setuptools>=82 and breaking fs/pyfilesystem2's pkg_resources import)
15+
# to a plain requirements.txt, installed into RTD's own managed virtualenv
16+
# below rather than a separate uv-managed one -- this keeps RTD's normal
17+
# sphinx auto-build detection intact.
18+
- uv export --frozen --no-hashes --group doc --no-emit-project -o doc-requirements.txt
1719
post_install:
1820
- python -m pip install -r doc-requirements.txt
19-
- python -m pip install -e .
21+
# --no-deps: dependencies are already installed, pinned, from doc-requirements.txt
22+
# above; this step only registers the local package itself for import.
23+
- python -m pip install -e . --no-deps
2024

2125
sphinx:
2226
configuration: docs/conf.py

Makefile

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ dev-requirements: ## install development environment requirements
7676
base-requirements: ## install only production/runtime dependencies
7777
uv sync --no-default-groups --frozen
7878

79-
test-requirements: ## install only testing dependencies (used by CI and tox)
80-
uv sync --only-group testing --frozen
79+
test-requirements: ## install production dependencies plus the testing group (used by CI and tox)
80+
uv sync --no-default-groups --group testing --frozen
8181

8282
requirements: dev-requirements ## install development environment requirements
8383

scripts/xsslint_config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
# Define the directories that should be ignored by the script.
2222
SKIP_DIRS = (
2323
'.git',
24+
'.venv',
2425
'.pycharm_helpers',
2526
'common/static/xmodule/modules',
2627
'common/static/bundles',

0 commit comments

Comments
 (0)