Skip to content

Adds license check for docker containers#6210

Draft
kellyguo11 wants to merge 10 commits into
isaac-sim:release/3.0.0-beta2from
kellyguo11:kellyg/docker-license-check
Draft

Adds license check for docker containers#6210
kellyguo11 wants to merge 10 commits into
isaac-sim:release/3.0.0-beta2from
kellyguo11:kellyg/docker-license-check

Conversation

@kellyguo11

Copy link
Copy Markdown
Contributor

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (existing functionality will not work without user modification)
  • Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@kellyguo11 kellyguo11 requested a review from pascal-roth as a code owner June 17, 2026 14:00
@kellyguo11 kellyguo11 marked this pull request as draft June 17, 2026 14:00
@github-actions github-actions Bot added isaac-lab Related to Isaac Lab team infrastructure labels Jun 17, 2026
@greptile-apps

greptile-apps Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR replaces the old bash/jq-based license workflow with a new Python tool (tools/check_dependency_licenses.py) that checks both pip and apt package licenses inside Docker containers, and wires it into the base-image build job so license compliance is verified on every Docker build.

  • New license checker: check_dependency_licenses.py supports both pip (via importlib or pip-licenses) and apt (via Debian copyright files), reads exceptions from the existing JSON file, and can emit JSON/Markdown reports for artifact upload.
  • Docker Dockerfile changes: Both Dockerfile.base and Dockerfile.curobo snapshot the Isaac Sim base image's apt package list before Isaac Lab installs its own packages (used as the apt baseline in CI); swig is refactored into a temporary install/purge block inside the nlopt build step so it does not appear as a manually-installed package in the license report.
  • CI workflow changes: build.yaml gains verify, capture, check (pip + apt), and upload steps after the base-image build; license-check.yaml is simplified to delegate to the same Python tool.

Confidence Score: 3/5

The CI and Python tool changes are solid, but libgmp-dev was dropped from both Dockerfiles' arm64 build deps without a temporary install/purge replacement, which would silently break pytetwild builds on arm64.

The swig package was carefully moved to a scoped install/purge block to avoid license noise, but libgmp-dev — which the original code explicitly flagged as required by pytetwild's FTetWild source build on aarch64 — was simply deleted. pytetwild is still referenced in the exceptions file, so if no aarch64 wheel exists at the pinned version, arm64 Docker builds will fail at isaaclab.sh --install. This needs either confirmation that an aarch64 wheel is now available or the same temporary install/purge treatment given to swig.

docker/Dockerfile.base and docker/Dockerfile.curobo — specifically the removal of libgmp-dev from the arm64 section.

Important Files Changed

Filename Overview
tools/check_dependency_licenses.py New 542-line license checker supporting both pip (importlib + pip-licenses collectors) and apt; logic is solid; minor unguarded unpack in _collect_apt_versions on blank dpkg-query lines.
docker/Dockerfile.base Adds apt-baseline snapshot and moves swig to a temporary install/purge block; libgmp-dev (needed for pytetwild arm64 source build) was removed without a replacement temporary step.
docker/Dockerfile.curobo Same structural changes as Dockerfile.base; same libgmp-dev removal risk on arm64.
.github/workflows/build.yaml Adds license-capture and check steps to the base-image build job; uses a create/start/cp/rm pattern to collect artifacts even on container failure; logic is correct.
.github/actions/ecr-build-push-pull/action.yml In the deps-cache-hit path, adds docker pull + docker tag so the locally-reused image is available for subsequent steps that rely on it by tag.
.github/workflows/license-check.yaml Replaces the old bash-scripted jq-based license check with calls to the new check_dependency_licenses.py tool; simpler and consistent with the docker path.
.github/workflows/license-exceptions.json Adds exceptions for cumotion (pip) and several apt packages (build-essential, cmake, git, libglib2.0-0, libglib2.0-0t64, ncurses-term, wget) required by the Docker image.
source/isaaclab/isaaclab/cli/commands/install.py Removes swig auto-install on ARM; now prints an informational message instead, since nlopt is pre-installed in the Dockerfile before optional deps are resolved.
source/isaaclab/test/install_ci/Dockerfile.installci Comment-only updates to reflect that swig is no longer auto-installed by the CI path.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant CI as GitHub Actions Runner
    participant ECR as ECR (Docker Registry)
    participant Container as Docker Container
    participant Artifact as Artifact Store

    CI->>ECR: ecr-build-push-pull (build or cache hit)
    ECR-->>CI: docker pull + tag (cache-hit path)
    CI->>CI: Verify image available locally
    CI->>Container: docker create (entrypoint bash)
    CI->>Container: docker start -a
    Note over Container: capture base-apt-manual.txt<br/>collect pip license JSON<br/>run dpkg-query
    Container-->>CI: exit (0 or non-zero)
    CI->>CI: docker cp /tmp/license-reports to license-reports/
    CI->>CI: docker rm container
    CI->>Container: docker run check_dependency_licenses.py pip
    Container-->>CI: pass / fail
    CI->>Container: docker run check_dependency_licenses.py apt
    Container-->>CI: pass / fail
    CI->>Artifact: upload-artifact (license-reports/, if: always())
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant CI as GitHub Actions Runner
    participant ECR as ECR (Docker Registry)
    participant Container as Docker Container
    participant Artifact as Artifact Store

    CI->>ECR: ecr-build-push-pull (build or cache hit)
    ECR-->>CI: docker pull + tag (cache-hit path)
    CI->>CI: Verify image available locally
    CI->>Container: docker create (entrypoint bash)
    CI->>Container: docker start -a
    Note over Container: capture base-apt-manual.txt<br/>collect pip license JSON<br/>run dpkg-query
    Container-->>CI: exit (0 or non-zero)
    CI->>CI: docker cp /tmp/license-reports to license-reports/
    CI->>CI: docker rm container
    CI->>Container: docker run check_dependency_licenses.py pip
    Container-->>CI: pass / fail
    CI->>Container: docker run check_dependency_licenses.py apt
    Container-->>CI: pass / fail
    CI->>Artifact: upload-artifact (license-reports/, if: always())
Loading

Comments Outside Diff (1)

  1. tools/check_dependency_licenses.py, line 831-836 (link)

    P2 Unguarded tuple unpack on dpkg-query output

    line.split("\t", 1) returns a one-element list for any blank or malformed line (e.g. a stray empty line), causing package, version = ... to raise ValueError and crash the checker. Adding a if not line.strip(): continue guard before the unpack makes the function resilient to unexpected output from dpkg-query.

Reviews (1): Last reviewed commit: "Fix Docker license report capture" | Re-trigger Greptile

Comment thread docker/Dockerfile.base
Comment on lines 61 to 66
RUN if [ "$(dpkg --print-architecture)" = "arm64" ]; then \
apt-get update && \
apt-get install -y --no-install-recommends \
libgl1-mesa-dev libopengl-dev libglx-dev \
libx11-dev libxcursor-dev libxi-dev libxinerama-dev libxrandr-dev \
libgmp-dev \
swig; \
libx11-dev libxcursor-dev libxi-dev libxinerama-dev libxrandr-dev; \
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 libgmp-dev silently dropped from arm64 build deps

The original file carried an explicit comment: libgmp-dev is required by pytetwild's fTetWild source build (no aarch64 wheel for 0.2.3). This PR removes libgmp-dev from the always-installed arm64 block but adds no temporary install/purge step for it (the pattern used for swig in the nlopt block). pytetwild is still present in license-exceptions.json, so it is still expected to be installed; if an aarch64 wheel is still unavailable at the pinned version, isaaclab.sh --install will fail on arm64 when pytetwild's CMake build can't find GMP headers. The same gap exists in Dockerfile.curobo.

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

Labels

infrastructure isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant