build: add libcurl-dev to Dockerfiles#172
Conversation
📝 WalkthroughWalkthroughThe pull request adds Changes
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
dockerfiles/pyopenms/manylinux/Dockerfile (1)
11-11: Consolidate yum install commands and add cache cleanup to reduce image size.Lines 10–12 contain three separate
RUN yum installcommands without cache cleanup, leaving yum metadata in the image (Trivy DS-0015). Combine these into a singleRUNcommand with cleanup:♻️ Proposed consolidation
-RUN yum install -y wget -RUN yum install -y xz qt6-qtbase-devel qt6-qtsvg-devel libcurl-devel -RUN yum install -y libtool cmake3 +RUN yum install -y \ + wget \ + xz qt6-qtbase-devel qt6-qtsvg-devel libcurl-devel \ + libtool cmake3 \ + && yum clean all \ + && rm -rf /var/cache/yum🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dockerfiles/pyopenms/manylinux/Dockerfile` at line 11, Consolidate the multiple RUN yum install invocations into a single RUN that installs all packages together and add yum/dnf cache cleanup to reduce image size and remove leftover metadata; locate the RUN yum install lines (e.g., the existing "RUN yum install -y xz qt6-qtbase-devel qt6-qtsvg-devel libcurl-devel" and the other two RUN yum install lines) and merge their package lists into one RUN, then append cache cleanup commands such as "yum clean all" and "rm -rf /var/cache/yum /var/log/yum.* /var/cache/dnf" (or the appropriate cache paths for the base image) so the final layer does not retain package metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dockerfiles/pyopenms/manylinux/Dockerfile`:
- Line 11: Consolidate the multiple RUN yum install invocations into a single
RUN that installs all packages together and add yum/dnf cache cleanup to reduce
image size and remove leftover metadata; locate the RUN yum install lines (e.g.,
the existing "RUN yum install -y xz qt6-qtbase-devel qt6-qtsvg-devel
libcurl-devel" and the other two RUN yum install lines) and merge their package
lists into one RUN, then append cache cleanup commands such as "yum clean all"
and "rm -rf /var/cache/yum /var/log/yum.* /var/cache/dnf" (or the appropriate
cache paths for the base image) so the final layer does not retain package
metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e651f668-abe8-493f-be28-10e731a6a50d
📒 Files selected for processing (2)
dockerfiles/Dockerfiledockerfiles/pyopenms/manylinux/Dockerfile
OpenMS core library now uses libcurl instead of Qt Network for HTTP operations. Add the development packages so the build can find curl headers and libraries. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
timosachsenberg
force-pushed
the
feature/add-libcurl-dependency
branch
from
5899519 to
8f242de
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dockerfiles/Dockerfile`:
- Line 22: Replace the duplicate package name libcurl4-openssl-dev with
libssl-dev in the apt packages list so the system provides OpenSSL development
headers required by the curl build; update the packages list entry that
currently reads libcurl4-openssl-dev to libssl-dev (the other occurrence of
libcurl4-openssl-dev on the file can remain) to ensure
libraries.cmake/curl.cmake can build with -DCURL_USE_OPENSSL=ON.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9a9f030e-5c9b-4a46-a948-75717f11533c
📒 Files selected for processing (1)
dockerfiles/Dockerfile
| zlib1g-dev \ | ||
| libxerces-c-dev \ | ||
| libbz2-dev \ | ||
| libcurl4-openssl-dev \ |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== curl build configuration =="
rg -n -C2 'OPENMS_CONTRIB_BUILD_CURL|CURL_USE_OPENSSL|CURL_USE_SCHANNEL|CURL_USE_SECTRANSP' libraries.cmake/curl.cmake
echo
echo "== relevant Dockerfile packages =="
rg -n 'libcurl4-openssl-dev|libssl-dev' dockerfiles/DockerfileRepository: OpenMS/contrib
Length of output: 622
🏁 Script executed:
sed -n '15,40p' dockerfiles/Dockerfile | cat -nRepository: OpenMS/contrib
Length of output: 826
🏁 Script executed:
rg -n 'libssl' dockerfiles/DockerfileRepository: OpenMS/contrib
Length of output: 40
Replace libcurl4-openssl-dev with libssl-dev on this line.
libcurl4-openssl-dev is already present on line 35. Since libraries.cmake/curl.cmake builds curl from source with -DCURL_USE_OPENSSL=ON, the required system dependency is OpenSSL development headers (libssl-dev), not a duplicate libcurl package. Without libssl-dev, the curl build will fail.
Proposed fix
- libcurl4-openssl-dev \
+ libssl-dev \📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| libcurl4-openssl-dev \ | |
| libssl-dev \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@dockerfiles/Dockerfile` at line 22, Replace the duplicate package name
libcurl4-openssl-dev with libssl-dev in the apt packages list so the system
provides OpenSSL development headers required by the curl build; update the
packages list entry that currently reads libcurl4-openssl-dev to libssl-dev (the
other occurrence of libcurl4-openssl-dev on the file can remain) to ensure
libraries.cmake/curl.cmake can build with -DCURL_USE_OPENSSL=ON.
1 participant
OpenMS core library now uses libcurl instead of Qt Network for HTTP operations. Add the development packages so the build can find curl headers and libraries.
Summary by CodeRabbit