Resolve docker hiccups around regexp build#1101
Conversation
* add abnf-to-regexp as requirement * split namespace_patterns.py out from patterns.py as file that can be generated fully
* add Makefile rule * add autogeneration warning to namespace_patterns.py resolve #899
… namespace_patterns.py - Move abnf-to-regexp from runtime to dev dependency group in pyproject.toml - Replace dynamic ABNF parsing in patterns.py with static import from namespace_patterns.py (no runtime dependency on abnf-to-regexp) - Add namespace_patterns.py with pre-generated static regex patterns - Add Makefile rule to regenerate namespace_patterns.py from ABNF source - Add --no-dev to 'uv sync --frozen' in Dockerfile dependencies stage so abnf-to-regexp (and its sdist-only regex==2021.4.4 dep) are not installed in production images - Add gcc+libc-dev to test stage for compiling regex when running dev sync Fixes docker-compose --project-directory docker build --no-cache Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…x-1067-with-1065 # Conflicts: # Makefile
ahouseholder
requested review from
cgyarbrough and
sei-vsarvepalli
as code owners
ahouseholder
changed the title
There was a problem hiding this comment.
Pull request overview
This PR refactors SSVC namespace regex handling by switching from runtime ABNF parsing to build-time generation of a committed Python module, while adjusting dependency groups and Docker/Make targets to keep builds working without production installing abnf-to-regexp.
Changes:
- Replace runtime ABNF-to-regexp execution with import of pre-generated
namespace_patterns.py. - Move
abnf-to-regexpto dev dependencies only (and update lockfile accordingly). - Update Makefile and Dockerfile to align generation/install steps with the new approach.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Moves abnf-to-regexp from main deps to dev deps in the lock metadata. |
pyproject.toml |
Moves abnf-to-regexp into [dependency-groups].dev. |
src/ssvc/utils/patterns.py |
Stops runtime ABNF parsing and instead imports pre-generated namespace regex components. |
src/ssvc/utils/namespace_patterns.py |
Adds the generated namespace regex components module. |
Makefile |
Adds/threads a generation rule for namespace_patterns.py into common targets. |
docker/Dockerfile |
Installs only prod deps in the main image and dev deps in the test image, adding build tooling for tests. |
| src/ssvc/utils/namespace_patterns.py: src/ssvc/utils/ssvc_namespace_pattern.abnf | ||
| echo "# AUTOGENERATED, DO NOT CHANGE manually, see Makefile" >$@ | ||
| uv run abnf-to-regexp --format python-nested -i $< >>$@ | ||
|
|
||
| dev: src/ssvc/utils/namespace_patterns.py | ||
| @echo "Set up dev environment..." | ||
| uv sync --dev |
There was a problem hiding this comment.
make dev now depends on src/ssvc/utils/namespace_patterns.py, but generating that file runs uv run abnf-to-regexp before uv sync --dev executes. Since abnf-to-regexp is a dev-only dependency, a fresh checkout is likely to fail when running make dev. Consider generating the file after uv sync --dev (e.g., call $(MAKE) from the recipe) or otherwise ensuring the dev dependency is available before invoking abnf-to-regexp (and using $(UV_RUN) for consistency).
| # AUTOGENERATED, DO NOT CHANGE manually, see Makefile | ||
| alnum = '[a-zA-Z0-9]' | ||
| lower = '[a-z]' | ||
| alnumlow = f'({lower}|[0-9])' | ||
| dash = '-' |
There was a problem hiding this comment.
This new generated Python module does not include the standard CMU copyright/license header that appears to be present in essentially all other src/ssvc/**/*.py files (including other generated helpers). If the project requires that header in distributed source files, update the generation rule (Makefile) to emit the full header before the autogenerated content so the generated file stays compliant.
There was a problem hiding this comment.
@ahouseholder can we configure the makefile to add the legalese?
- Create separate 'codegen' dependency group for abnf-to-regexp so that 'uv sync --frozen --dev' in the test Docker stage does NOT pull in abnf-to-regexp (and its sdist-only regex==2021.4.4 dependency that requires gcc to build) - Update uv.lock to reflect codegen group membership - Remove gcc/libc6-dev from test Docker stage (no longer needed) - Add --no-sync to all 'uv run' CMD instructions in Dockerfile so containers don't re-sync on startup (which would pull dev/codegen deps) - Update Makefile to invoke abnf-to-regexp via --group codegen Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
sei-renae
left a comment
sei-renae
left a comment
There was a problem hiding this comment.
it builds, but I've got a couple of concerns in the comments.
| # AUTOGENERATED, DO NOT CHANGE manually, see Makefile | ||
| alnum = '[a-zA-Z0-9]' | ||
| lower = '[a-z]' | ||
| alnumlow = f'({lower}|[0-9])' | ||
| dash = '-' |
There was a problem hiding this comment.
@ahouseholder can we configure the makefile to add the legalese?
| src/ssvc/utils/namespace_patterns.py: src/ssvc/utils/ssvc_namespace_pattern.abnf | ||
| echo "# AUTOGENERATED, DO NOT CHANGE manually, see Makefile" >$@ | ||
| uv run abnf-to-regexp --format python-nested -i $< >>$@ | ||
|
|
||
| dev: src/ssvc/utils/namespace_patterns.py | ||
| @echo "Set up dev environment..." | ||
| uv sync --dev |
This PR
This pull request refactors how namespace pattern regular expressions are generated and used within the project. Instead of generating these patterns dynamically at runtime using the
abnf-to-regexplibrary, the patterns are now generated ahead of time and stored in a dedicated, auto-generated Python file. This change simplifies the runtime dependencies and setup, and moves theabnf-to-regexpdependency to development only. The Makefile and Dockerfile are updated to ensure the generated patterns file is always up to date before running development, tests, or documentation.Key changes include:
Namespace pattern generation and usage:
src/ssvc/utils/namespace_patterns.pyat build time using a Makefile rule, rather than generated dynamically at runtime. The runtime code now imports these pre-generated patterns. [1] [2] [3] [4]Dependency management:
abnf-to-regexpdependency is moved from the main dependencies list to the development dependencies group inpyproject.toml, so it is no longer installed in production environments. [1] [2]abnf-to-regexp) are installed only in the test image.Build and test process:
src/ssvc/utils/namespace_patterns.pyfile, ensuring it is always up to date before these tasks run.Minor code cleanup:
src/ssvc/utils/patterns.py.These changes improve reliability and reproducibility by ensuring regular expressions are always up to date and by reducing unnecessary runtime dependencies.