[sub PR for merge] Add dockerfile and build script#339
Conversation
![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 Docker image build script (docker/build_image.sh) and several multi-stage Dockerfiles for Ubuntu and openEuler environments to support different Python versions. Additionally, it updates a SWE-bench configuration file. Feedback on these changes highlights a critical syntax error due to unresolved git conflict markers in the SWE-bench configuration. In the Docker build script, improvements are recommended to prevent word splitting by double-quoting variables, handle potential directory navigation failures, and correct an invalid usage example. Finally, the Dockerfiles should be refactored to address redundant package installations and fragile file-copying practices across build stages, either by simplifying them to single-stage builds or properly utilizing Python virtual environments.
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.
| <<<<<<< HEAD | ||
| path="", # necessary for mini datasets, get dataset from https://modelers.cn/datasets/AISBench/SWE-Bench_Multilingual_mini | ||
| ======= | ||
| path="", # Set local path to the mini dataset. Download from https://modelers.cn/datasets/AISBench/SWE-Bench_Multilingual_mini | ||
| >>>>>>> master_center |
There was a problem hiding this comment.
Unresolved git conflict markers (<<<<<<< HEAD, =======, >>>>>>> master_center) are present in this file. This will cause a SyntaxError when the Python interpreter attempts to load or run this configuration file. Please resolve the conflict and remove the markers.
path="", # Set local path to the mini dataset. Download from https://modelers.cn/datasets/AISBench/SWE-Bench_Multilingual_mini| echo "" | ||
| echo "示例:" | ||
| echo " $0 --tag v3.1-20260522-master" | ||
| echo " $0 --tag v3.1-20260522-master --os ubuntu22.04 --py-version py311" |
There was a problem hiding this comment.
The usage example suggests running with --os ubuntu22.04 --py-version py311. However, there is no corresponding Dockerfile.py311.ubuntu22.04 in the repository (only py310.ubuntu22.04 and py312.ubuntu24.04 are provided for Ubuntu). Running this example command will result in a 'Dockerfile does not exist' error. Please update the example to use a valid combination, such as --os ubuntu24.04 --py-version py312 or --os openeuler24.03 --py-version py311.
| echo " $0 --tag v3.1-20260522-master --os ubuntu22.04 --py-version py311" | |
| echo " $0 --tag v3.1-20260522-master --os ubuntu24.04 --py-version py312" |
| if docker images -q ${image_name} > /dev/null 2>&1; then | ||
| docker rmi -f ${image_name} |
There was a problem hiding this comment.
Double-quote ${image_name} to prevent word splitting if the image name contains spaces or special characters.
| if docker images -q ${image_name} > /dev/null 2>&1; then | |
| docker rmi -f ${image_name} | |
| if docker images -q "${image_name}" > /dev/null 2>&1; then | |
| docker rmi -f "${image_name}" |
| docker build \ | ||
| --network host \ | ||
| --build-arg GIT_TAG=${TAG} \ | ||
| -f ${dockerfile_path} \ | ||
| -t ${image_name} \ | ||
| $(dirname "$0") |
There was a problem hiding this comment.
Variables and command substitutions passed to docker build should be double-quoted to prevent word splitting if paths or tags contain spaces.
| docker build \ | |
| --network host \ | |
| --build-arg GIT_TAG=${TAG} \ | |
| -f ${dockerfile_path} \ | |
| -t ${image_name} \ | |
| $(dirname "$0") | |
| docker build \ | |
| --network host \ | |
| --build-arg GIT_TAG="${TAG}" \ | |
| -f "${dockerfile_path}" \ | |
| -t "${image_name}" \ | |
| "$(dirname "$0")" |
| docker build \ | ||
| --no-cache \ | ||
| --network host \ | ||
| --build-arg GIT_TAG=${TAG} \ | ||
| -f ${dockerfile_path} \ | ||
| -t ${image_name} \ | ||
| $(dirname "$0") |
There was a problem hiding this comment.
Variables and command substitutions passed to docker build should be double-quoted to prevent word splitting if paths or tags contain spaces.
| docker build \ | |
| --no-cache \ | |
| --network host \ | |
| --build-arg GIT_TAG=${TAG} \ | |
| -f ${dockerfile_path} \ | |
| -t ${image_name} \ | |
| $(dirname "$0") | |
| docker build \ | |
| --no-cache \ | |
| --network host \ | |
| --build-arg GIT_TAG="${TAG}" \ | |
| -f "${dockerfile_path}" \ | |
| -t "${image_name}" \ | |
| "$(dirname "$0")" |
| cd ${obsutils_path} | ||
| ./obsutil cp ${offline_pkg_full_path} obs://aisbench/images/benchmark/github/${offline_pkg_name} -f |
There was a problem hiding this comment.
If the cd command fails, the script will continue executing ./obsutil in the current working directory, which can lead to unexpected behavior or failures. Additionally, variables like ${obsutils_path}, ${offline_pkg_full_path}, and ${offline_pkg_name} should be double-quoted to prevent word splitting and globbing issues if they contain spaces or special characters.
| cd ${obsutils_path} | |
| ./obsutil cp ${offline_pkg_full_path} obs://aisbench/images/benchmark/github/${offline_pkg_name} -f | |
| cd "${obsutils_path}" || exit 1 | |
| ./obsutil cp "${offline_pkg_full_path}" "obs://aisbench/images/benchmark/github/${offline_pkg_name}" -f |
| RUN ln -s /usr/local/bin/python3.10 /usr/local/bin/python3 && \ | ||
| ln -s /usr/local/bin/pip3.10 /usr/local/bin/pip3 | ||
|
|
||
| COPY --from=builder /usr/local/lib/python3.10 /usr/local/lib/python3.10 | ||
| COPY --from=builder /usr/local/bin /usr/local/bin |
There was a problem hiding this comment.
Creating symlinks in /usr/local/bin before copying /usr/local/bin from the builder stage is redundant and risky. The COPY instruction will overwrite the directory contents, and the Python make install in the builder stage already creates the necessary python3 and pip3 symlinks. These lines should be removed, or if symlinks are indeed needed, they should be created after the COPY step.
COPY --from=builder /usr/local/lib/python3.10 /usr/local/lib/python3.10
COPY --from=builder /usr/local/bin /usr/local/bin
| COPY --from=builder /usr/local/lib/python3.10 /usr/local/lib/python3.10 | ||
| COPY --from=builder /usr/local/bin /usr/local/bin | ||
| COPY --from=builder /usr/lib/python3/dist-packages /usr/lib/python3/dist-packages | ||
| COPY --from=builder /benchmark /benchmark | ||
| COPY --from=builder /usr/share/nltk_data /usr/share/nltk_data |
There was a problem hiding this comment.
The current multi-stage build pattern is highly inefficient and defeats the purpose of multi-stage builds.
- Redundancy: You are installing the exact same heavy development dependencies (like
python3.10-dev,python3-pip,git,curl,vim) in both thebuilderstage and the final stage. - Fragility: Copying system directories like
/usr/local/lib/python3.10,/usr/local/bin, and/usr/lib/python3/dist-packagesfrom the builder stage is extremely fragile. It can easily overwrite or conflict with files installed byaptin the final stage, leading to broken dependencies or runtime issues.
Recommended Solutions:
- Option A (Single-stage build): Since the final stage installs almost the same packages as the builder stage, you can simplify this into a single-stage Dockerfile. This reduces complexity and avoids fragile directory copying.
- Option B (Proper Multi-stage with Virtual Environment): Use a Python virtual environment (e.g., in
/opt/venv) in the builder stage to install all pip dependencies. In the final stage, install only the runtime dependencies (e.g.,python3.10,python3.10-distutilswithout-devorpip), copy/opt/venvfrom the builder, and prepend/opt/venv/binto thePATH. This keeps the final image clean and significantly smaller.
| COPY --from=builder /usr/local/lib/python3.12 /usr/local/lib/python3.12 | ||
| COPY --from=builder /usr/local/bin /usr/local/bin | ||
| COPY --from=builder /usr/lib/python3/dist-packages /usr/lib/python3/dist-packages | ||
| COPY --from=builder /benchmark /benchmark | ||
| COPY --from=builder /usr/share/nltk_data /usr/share/nltk_data |
There was a problem hiding this comment.
Similar to the Python 3.10 Dockerfile, this multi-stage build installs the same development dependencies (like python3.12-dev, python3-pip) in both stages and manually copies system python directories (/usr/local/lib/python3.12, /usr/lib/python3/dist-packages). This is fragile and redundant. Please consider either simplifying this to a single-stage build or using a Python virtual environment (venv) to cleanly copy only the installed packages to the final stage without copying system directories.
| COPY --from=builder /usr/share/nltk_data /usr/share/nltk_data | ||
| COPY --from=builder /usr/local/bin /usr/local/bin | ||
| COPY --from=builder /usr/local/lib64/python3.11/site-packages /usr/local/lib64/python3.11/site-packages | ||
| COPY --from=builder /usr/local/lib/python3.11/site-packages /usr/local/lib/python3.11/site-packages | ||
| COPY --from=builder /usr/lib64/python3.11/site-packages /usr/lib64/python3.11/site-packages | ||
| COPY --from=builder /usr/lib/python3.11/site-packages /usr/lib/python3.11/site-packages |
There was a problem hiding this comment.
This multi-stage build manually copies multiple site-packages directories from the builder stage. This is extremely fragile and prone to breaking Python's package resolution or overwriting system packages. It is highly recommended to use a Python virtual environment (venv) in the builder stage and copy only the virtual environment directory to the final stage, or simplify this to a single-stage build.
2 participants
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献，我们非常重视。以下说明将使您的拉取请求更健康，更易于获得反馈。如果您不理解某些项目，请不要担心，只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #(issue ID / issue 编号) / Relates to #(issue ID / issue 编号)
🔍 Motivation / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
#332
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接，例如 CI 管道、测试报告等。
Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改？如果是，请描述它如何破坏兼容性，以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。
If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降，请描述性能下降的影响和预期的性能改进。
🌟 Use cases (Optional) / 使用案例（可选）
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能，最好在此处列出一些用例并更新文档。
✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build