Build a docker image to setup the QNN SDK environment - #3736
Conversation
dosubot
Bot
added
the
size:L
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Dockerfile ( ChangesQNN SDK Docker Build and Validation
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant BuildWorkflow as qnn-sdk-docker-build.yaml
participant GHCR
participant TestWorkflow as test-qnn-sdk-docker.yaml
BuildWorkflow->>GHCR: Build and push qnn-sdk:2.40 image
TestWorkflow->>GHCR: Pull qnn-sdk:2.40 container
TestWorkflow->>TestWorkflow: Validate QNN CLI tools via --help
Possibly related PRs
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Dockerfile (Dockerfile-2.40) to set up an environment with Ubuntu 22.04, Android NDK r29, QNN SDK 2.40, and various Python dependencies. The review feedback identifies critical issues in the Dockerfile's shell execution steps: first, operator precedence pitfalls in Bash chaining (&& and ||) can lead to silent installation failures of both Linux and Python dependencies; second, running check-linux-dependency.sh will fail because package lists were previously deleted, requiring an apt-get update beforehand; and third, there are duplicate dependencies (pyyaml and numpy) in the pip installation layers.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| RUN cd ${QNN_SDK_ROOT}/bin && . envsetup.sh \ | ||
| && yes | ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh || true \ | ||
| && rm -rf /var/lib/apt/lists/* |
There was a problem hiding this comment.
Bash Operator Precedence & Missing Package Lists\n\nThere are two issues in this step:\n1. Operator Precedence Pitfall: In Bash, && and || have equal precedence and are left-associative. The expression A && B && C || true && D is evaluated as (((A && B && C) || true) && D). If cd or source envsetup.sh fails, the chain A && B && C fails, but the || true evaluates to true, and the shell proceeds to run D (rm -rf ...). This silently ignores failures in sourcing envsetup.sh.\n2. Missing Package Lists: The check-linux-dependency.sh script runs apt-get install under the hood. However, since /var/lib/apt/lists/* was deleted in the previous layer, apt-get will fail to locate packages. Running apt-get update before the script ensures dependencies can be successfully installed.\n\nTo fix both, wrap the dependency check in { ... || true; } and run apt-get update beforehand.
RUN apt-get update && cd ${QNN_SDK_ROOT}/bin && . envsetup.sh \\n && { yes | ${QNN_SDK_ROOT}/bin/check-linux-dependency.sh || true; } \\n && rm -rf /var/lib/apt/lists/*
| RUN . /opt/py310/bin/activate && \ | ||
| pip install --no-cache-dir --upgrade pip && \ | ||
| pip install --no-cache-dir \ | ||
| mock \ | ||
| numpy \ | ||
| opencv-python \ | ||
| optuna \ | ||
| packaging \ | ||
| pandas \ | ||
| paramiko \ | ||
| pathlib2 \ | ||
| pillow \ | ||
| plotly \ | ||
| protobuf \ | ||
| psutil \ | ||
| pydantic \ | ||
| pytest \ | ||
| pyyaml \ | ||
| rich \ | ||
| scikit-optimize \ | ||
| scipy \ | ||
| six \ | ||
| tabulate \ | ||
| typing-extensions \ | ||
| xlsxwriter \ | ||
| && python3 "${QNN_SDK_ROOT}/bin/check-python-dependency" || true \ | ||
| && pip install --no-cache-dir \ | ||
| torch==2.0.0+cpu -f https://download.pytorch.org/whl/torch \ | ||
| kaldi_native_fbank \ | ||
| "numpy<2" \ | ||
| onnx==1.17.0 \ | ||
| onnxruntime \ | ||
| soundfile \ | ||
| librosa \ | ||
| onnxsim \ | ||
| sentencepiece \ | ||
| pyyaml \ | ||
| && pip cache purge 2>/dev/null || true |
There was a problem hiding this comment.
Silent Installation Failures & Duplicate Dependencies\n\nThere are two main issues in this block:\n1. Silent Failures due to Operator Precedence: In Bash, && and || have equal precedence and are left-associative. The expression A && B || true && C && D || true is evaluated as ((((A && B) || true) && C) && D) || true. If the first pip install fails, the || true evaluates to true, and the shell proceeds to the second pip install. More critically, if the second pip install (which installs major packages like PyTorch and ONNX) fails, the trailing || true causes the entire RUN instruction to succeed, silently producing a broken Docker image.\n2. Duplicate Dependencies: pyyaml is listed twice (lines 82 and 101). numpy is also listed twice (once as numpy on line 69 and once as \"numpy<2\" on line 94), which causes pip to install the latest numpy first and then downgrade it later.\n\nGrouping the commands with { ... || true; } and cleaning up the duplicates resolves these issues.
RUN . /opt/py310/bin/activate && \\n pip install --no-cache-dir --upgrade pip && \\n pip install --no-cache-dir \\n mock \\n \"numpy<2\" \\n opencv-python \\n optuna \\n packaging \\n pandas \\n paramiko \\n pathlib2 \\n pillow \\n plotly \\n protobuf \\n psutil \\n pydantic \\n pytest \\n pyyaml \\n rich \\n scikit-optimize \\n scipy \\n six \\n tabulate \\n typing-extensions \\n xlsxwriter && \\n { python3 \"${QNN_SDK_ROOT}/bin/check-python-dependency\" || true; } && \\n pip install --no-cache-dir \\n torch==2.0.0+cpu -f https://download.pytorch.org/whl/torch \\n kaldi_native_fbank \\n onnx==1.17.0 \\n onnxruntime \\n soundfile \\n librosa \\n onnxsim \\n sentencepiece && \\n { pip cache purge 2>/dev/null || true; }
1 participant
See https://github.com/k2-fsa/sherpa-onnx/pkgs/container/qnn-sdk
Summary by CodeRabbit