chore: add source distribution collection to container builds for license compliance - #1266
Conversation
bd6bff2 to
4ffc6c1
Compare
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesSource collection and image integration
License metadata corrections
Documentation notice
Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant DockerBake
participant DockerImageBuild
participant SourceCollectors
participant SourceDistributionDirectory
ReleaseWorkflow->>DockerBake: set NMP_COLLECT_SOURCES=1
DockerBake->>DockerImageBuild: pass collection argument
DockerImageBuild->>SourceCollectors: collect APT, CPython, workspace, and Python sources
SourceCollectors->>SourceDistributionDirectory: write archives and manifests
DockerImageBuild->>SourceDistributionDirectory: copy artifacts into runtime image
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/base/Dockerfile.nmp-python-base`:
- Around line 37-49: Run every listed collector through the appropriate uv
runner, using `uv run --no-project --python <runner>
/usr/local/bin/<collector>.py ...` while preserving each collector’s `--python`
argument. In docker/base/Dockerfile.nmp-python-base lines 37-49, move the uv
copy before the first collector and update both collectors; apply the same
collector updates in docker/Dockerfile.auditor-tasks lines 93-112 and 135-138,
docker/Dockerfile.nmp-api lines 58-66, docker/Dockerfile.nmp-core lines 21-29,
docker/Dockerfile.nmp-cpu-tasks lines 14-22, and
docker/rl/Dockerfile.nmp-rl-base lines 434-454. In
docker/rl/Dockerfile.nmp-rl-training lines 65-79, use uv-glue as the runner and
preserve UV_BIN=uv-glue.
In `@docker/Dockerfile.safe-synthesizer-tasks`:
- Around line 203-217: Add `/build/packages/nmp_common` as an additional
`--project` argument to the `collect-workspace-sdists.py` invocation in the
safe-synthesizer task image, ensuring the local nmp-common source distribution
is collected alongside the existing workspace projects.
In `@docker/scripts/collect-apt-sources.sh`:
- Around line 82-122: Update the installed-package collection and
source-download flow around dpkg-query, the source_packages_file loop, and
apt-get source so installed packages retain both their source name and installed
source version. Resolve each installed binary package’s source metadata, store
entries as source_package=source_version when --installed is enabled, and pass
that exact version to apt-get source; preserve the existing unversioned behavior
for explicitly requested packages.
In `@docker/scripts/collect-workspace-sdists.py`:
- Around line 69-72: Update the explicit-project handling loop around
args.project so paths that do not exist or lack pyproject.toml are recorded in
missing-workspace-sdists.txt, while valid projects continue to be added to
projects. Reuse the existing manifest-writing mechanism and preserve its
expected path format.
In `@README.md`:
- Around line 200-201: Update the NOTICE AND DISCLAIMER text to limit the “not
distributed with this software” claim to materials fetched only at runtime.
Explicitly state that materials embedded in distributed artifacts, including
Docker images, remain subject to their original licenses and notices, while
preserving the existing responsibility and warranty language.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7c901e32-fb27-4aab-a10c-3ff514d17150
📒 Files selected for processing (17)
README.mddocker/Dockerfile.auditor-tasksdocker/Dockerfile.nmp-apidocker/Dockerfile.nmp-coredocker/Dockerfile.nmp-cpu-tasksdocker/Dockerfile.nmp-customizer-tasksdocker/Dockerfile.nmp-unsloth-trainingdocker/Dockerfile.safe-synthesizer-tasksdocker/automodel/Dockerfile.nmp-automodel-basedocker/automodel/Dockerfile.nmp-automodel-trainingdocker/base/Dockerfile.nmp-python-basedocker/rl/Dockerfile.nmp-rl-basedocker/rl/Dockerfile.nmp-rl-trainingdocker/scripts/collect-apt-sources.shdocker/scripts/collect-cpython-source.pydocker/scripts/collect-python-sdists.pydocker/scripts/collect-workspace-sdists.py
|
4ffc6c1 to
6d5ce1d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/automodel/Dockerfile.nmp-automodel-base`:
- Around line 76-89: Declare NMP_COLLECT_SOURCES before the grouped_gemm and
bitsandbytes build commands, and condition each corresponding git archive
operation on the same enabled values used by the collectors. Ensure archives are
not created or shipped when collection is disabled, while preserving the
existing clone, checkout, and installation steps.
In `@docker/scripts/collect-cpython-source.py`:
- Around line 40-53: Update download to verify the fetched CPython archive
against the official release signature or a trusted checksum before accepting it
as valid. Perform verification before writing or recording the destination
artifact, retry or fail when verification fails, and preserve the existing
successful-download and error behavior for valid archives.
In `@docker/scripts/collect-workspace-sdists.py`:
- Around line 81-88: Update the exception handling around purelib_for() and
direct_url_projects() to catch OSError in addition to
subprocess.CalledProcessError, so missing or non-executable args.python values
are recorded in workspace-sdist-errors.txt instead of terminating the script.
Preserve the existing error message and file-writing behavior.
- Around line 99-108: Update the workspace sdist collection flow so the function
returns a non-zero status when the missing list is non-empty, while retaining
status 0 when all builds succeed. Apply this to the return path after writing
the built and missing manifests.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 020d3fb5-6a4f-4a86-8840-0eddaec3c8fd
📒 Files selected for processing (21)
.github/workflows/release.yamlREADME.mddocker-bake.hcldocker/Dockerfile.auditor-tasksdocker/Dockerfile.nmp-apidocker/Dockerfile.nmp-coredocker/Dockerfile.nmp-cpu-tasksdocker/Dockerfile.nmp-customizer-tasksdocker/Dockerfile.nmp-unsloth-trainingdocker/Dockerfile.safe-synthesizer-tasksdocker/automodel/Dockerfile.nmp-automodel-basedocker/automodel/Dockerfile.nmp-automodel-trainingdocker/base/Dockerfile.nmp-python-basedocker/rl/Dockerfile.nmp-rl-basedocker/rl/Dockerfile.nmp-rl-trainingdocker/scripts/collect-apt-sources.shdocker/scripts/collect-cpython-source.pydocker/scripts/collect-python-sdists.pydocker/scripts/collect-workspace-sdists.pytests/unit/test_collect_apt_sources.pytests/unit/test_collect_cpython_source.py
🚧 Files skipped from review as they are similar to previous changes (7)
- docker/automodel/Dockerfile.nmp-automodel-training
- README.md
- docker/Dockerfile.nmp-unsloth-training
- docker/rl/Dockerfile.nmp-rl-base
- docker/Dockerfile.nmp-customizer-tasks
- docker/Dockerfile.auditor-tasks
- docker/rl/Dockerfile.nmp-rl-training
bc668a3 to
d6cf20e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tools/nemo-platform-sdk-tools/tests/license/test_license_utils.py`:
- Around line 236-260: Extend
test_reviewed_license_overrides_cover_corrected_inventory_values to load
third_party/licenses.jsonl, build a package-to-license mapping from its
generated rows, and assert the corrected_packages values against it as well as
overrides. Normalize package names and use the generator’s uppercase
license-expression convention for the inventory assertion.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 36009cc1-2842-4c2d-9fc1-65b31aa6ffb9
📒 Files selected for processing (16)
docker/Dockerfile.auditor-tasksdocker/Dockerfile.nmp-apidocker/Dockerfile.nmp-coredocker/Dockerfile.nmp-customizer-tasksdocker/Dockerfile.nmp-unsloth-trainingdocker/Dockerfile.safe-synthesizer-tasksdocker/automodel/Dockerfile.nmp-automodel-basedocker/automodel/Dockerfile.nmp-automodel-trainingdocker/rl/Dockerfile.nmp-rl-basedocker/rl/Dockerfile.nmp-rl-trainingdocker/scripts/collect-cpython-source.pytests/unit/test_collect_apt_sources.pytests/unit/test_collect_cpython_source.pythird_party/licenses.jsonltools/nemo-platform-sdk-tools/src/nemo_platform_sdk_tools/license/overrides.yamltools/nemo-platform-sdk-tools/tests/license/test_license_utils.py
🚧 Files skipped from review as they are similar to previous changes (5)
- docker/Dockerfile.nmp-core
- docker/Dockerfile.auditor-tasks
- docker/Dockerfile.nmp-unsloth-training
- docker/automodel/Dockerfile.nmp-automodel-training
- docker/automodel/Dockerfile.nmp-automodel-base
d6cf20e to
79fa62c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docker/scripts/collect-workspace-sdists.py`:
- Around line 101-105: Update the exception handling around the uv build
invocation to also catch OSError alongside subprocess.CalledProcessError,
appending the affected project and str(error) to missing so uv startup failures
are recorded and processing continues.
In `@NOTICE`:
- Around line 6-7: Update the NOTICE disclaimer to distinguish materials
retrieved at runtime from source archives bundled in release Docker images under
the source-distributions directory, and state that bundled materials remain
subject to their own licenses and notices. Preserve the existing responsibility,
warranty, and liability language.
In `@tests/unit/test_collect_workspace_sdists.py`:
- Around line 39-40: Update the test setup around main() in
test_collect_workspace_sdists to remove the UV_BIN environment variable before
invocation, ensuring the expected command uses the default executable regardless
of the external test environment. Preserve the existing NMP_COLLECT_SOURCES
setup and assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: dcb5829f-65cf-4738-b7c4-935d6bf6a6b9
📒 Files selected for processing (4)
NOTICEdocker/scripts/collect-workspace-sdists.pytests/unit/test_collect_workspace_sdists.pytools/nemo-platform-sdk-tools/tests/license/test_license_utils.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/nemo-platform-sdk-tools/tests/license/test_license_utils.py
2b1e183 to
47f7fe1
Compare
47f7fe1 to
5c84055
Compare
Signed-off-by: Ryan S <267728323+ironcommit@users.noreply.github.com>
c26efcb to
bd8b214
Compare
Summary
Adds automated source distribution collection to all container images so that shipped containers include the source code required by open-source licenses (GPL, LGPL, etc.). Also adds a third-party materials disclaimer to the README.
Changes
New collection scripts (
docker/scripts/)collect-apt-sources.sh— downloads Debian/Ubuntu source packages for installed or explicitly listed apt packages, withdeb-srcauto-enablement for official reposcollect-cpython-source.py— fetches the CPython source tarball matching the interpreter version from python.orgcollect-python-sdists.py— collects PyPI source distributions for all non-editable packages in a virtualenv, usinguv.lockURLs when available and falling back to the PyPI JSON API; supports baseline freeze diffing, hash verification, and labeled multi-environment manifestscollect-workspace-sdists.py— builds sdists for local workspace packages (editable installs) usinguv build --sdistDockerfile integration
Each container image now runs the appropriate collection scripts during build and copies results to
/usr/share/nemo-platform/source-distributions/<image>/:Dockerfile.nmp-python-base— apt sources, CPython source, seed venv sdistsDockerfile.nmp-api— workspace + PyPI sdistsDockerfile.nmp-core— workspace + PyPI sdistsDockerfile.nmp-cpu-tasks— workspace + PyPI sdistsDockerfile.nmp-customizer-tasks— apt sources, workspace + PyPI sdistsDockerfile.nmp-unsloth-training— apt sources, git archives (bitsandbytes), workspace + PyPI sdistsDockerfile.safe-synthesizer-tasks— apt sources, CPython source, workspace + PyPI sdistsDockerfile.auditor-tasks— apt sources, CPython source, workspace + PyPI sdists (app + garak venvs)Dockerfile.nmp-automodel-base— apt sources, git archives (grouped_gemm, bitsandbytes), workspace + PyPI sdistsDockerfile.nmp-automodel-training— workspace + delta PyPI sdists (baseline freeze diff)Dockerfile.nmp-rl-base— apt sources, workspace + PyPI sdists across nemo-rl, ray, and gym venvsDockerfile.nmp-rl-training— workspace + delta PyPI sdists (baseline freeze diff)Other changes
COPY --chownfrom1000:1000tonvs:nvsinDockerfile.nmp-apiandDockerfile.nmp-coreREADME.mdType of Change
Quality Gates
Verification
Signed-off-by:traileruv run pre-commit run -apasses, or any blocked checks are identified belowSummary by CodeRabbit
New Features
Documentation
Chores