v0.4.0 dev#6
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 updates the Dockerfile base image to rocker/geospatial:4.5.2, replaces apt-get with apt, and adds version verification for several bioinformatics tools. It also installs fastp, removes salmon and QualiMap, adds R package verification, and updates the application image version in the job submission template. Feedback recommends reverting to apt-get for stability, using HTTPS and the correct version for the fastp download, and purging build-time dependencies like cmake to optimize image size.
| RUN wget http://opengene.org/fastp/fastp.1.3.2 && \ | ||
| chmod a+x fastp.1.3.2 && \ | ||
| mv fastp.1.3.2 /usr/local/bin/fastp && \ | ||
| fastp --version |
There was a problem hiding this comment.
There are two issues here:
- Security: The download uses
httpinstead ofhttps, making it vulnerable to man-in-the-middle attacks. - Correctness: The version
1.3.2appears to be a typo. Thefastptool typically uses0.x.yversioning (e.g.,0.23.4).
It is safer and more reliable to download the binary from the official GitHub releases using HTTPS.
RUN wget https://github.com/OpenGene/fastp/releases/download/v0.23.4/fastp && \
chmod a+x fastp && \
mv fastp /usr/local/bin/fastp && \
fastp --version
| libcurl4-openssl-dev \ | ||
| libssl-dev \ | ||
| default-jre \ | ||
| RUN apt update && DEBIAN_FRONTEND=noninteractive apt install -y \ |
There was a problem hiding this comment.
It is recommended to use apt-get instead of apt in Dockerfiles. apt is intended for interactive use and its interface is not considered stable for scripting, which can lead to warnings or unexpected behavior in automated builds. Additionally, apt-get is the standard practice recommended by Docker documentation.
RUN apt-get update && DEBIAN_FRONTEND=noninteractive apt-get install -y \
| RUN apt update && \ | ||
| apt install -y --no-install-recommends \ |
There was a problem hiding this comment.
Consistent with the previous block, apt-get should be used instead of apt for stability in non-interactive environments.
RUN apt-get update && \
apt-get install -y --no-install-recommends \
| apt-get clean && rm -rf /var/lib/apt/lists/* && \ | ||
| bamtools --version |
There was a problem hiding this comment.
Since cmake is only required to build bamtools and is not used elsewhere in the image, it should be purged after the installation to reduce the final image size. Also, ensure consistency by using apt-get if the rest of the file is reverted.
apt-get purge -y --auto-remove cmake && \
apt-get clean && rm -rf /var/lib/apt/lists/* && \
bamtools --version
|
looks good, fastp looks correct |
2 participants
Updating and fixing the installs to avoid the failing library install issue that popped up during the March 2026 workshop.
Major changes: