Updated the dockerfile with new OpenMS version issue #128 #165
Conversation
WalkthroughThe Dockerfile has been revised to update the OpenMS branch from Changes
Sequence Diagram(s)sequenceDiagram
participant D as Docker Build
participant F as Dockerfile
participant P as Package Installer
participant E as Environment Setup
participant V as Verification
participant C as CMake Configuration
D->>F: Start Docker build
F->>P: Remove Qt5 packages and install Qt6 packages
F->>E: Set environment variables (Qt6_DIR & CMAKE_PREFIX_PATH)
E->>F: Create symbolic link for qmake
F->>V: Verify Qt6 installation (qmake version & cmake directory)
F->>C: Pass Qt6 paths to CMake configuration
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Dockerfile (2)
40-41: Setting Qt6 Environment Variables
DefiningQt6_DIRandCMAKE_PREFIX_PATHfor Qt6 is a good practice to ensure the build system can locate the correct Qt6 libraries and tools.Suggestion: Consider using these environment variables directly in the CMake configuration (see comments in lines 98-99) to avoid duplicating or hardcoding similar paths.
98-99: Align CMake Prefix Path with Environment Variables
The CMake configuration command currently hardcodes the prefix path as/OpenMS/contrib-build/;/usr/;/usr/local;/usr/lib/qt6. Since the environment variableCMAKE_PREFIX_PATHwas set earlier to/usr/lib/x86_64-linux-gnu/cmake/Qt6:/usr/lib/qt6, it would help to integrate this variable to ensure consistency and simplify future updates. For example, you might consider updating the command as follows:-RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH='/OpenMS/contrib-build/;/usr/;/usr/local;/usr/lib/qt6' -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On" +RUN /bin/bash -c "cmake -DCMAKE_BUILD_TYPE='Release' -DCMAKE_PREFIX_PATH=\"$CMAKE_PREFIX_PATH;/OpenMS/contrib-build/;/usr/;/usr/local\" -DHAS_XSERVER=OFF -DBOOST_USE_STATIC=OFF -DPYOPENMS=ON ../OpenMS -DPY_MEMLEAK_DISABLE=On"This change leverages the previously set environment variable, minimizes duplication, and reduces potential errors if the Qt6 paths change later.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-openms
- GitHub Check: build-simple-app
- GitHub Check: build-full-app
🔇 Additional comments (5)
Dockerfile (5)
12-12: Update OpenMS Branch Argument
The branch argument has been successfully updated torelease/3.3.0. This ensures that the build now pulls from the updated OpenMS release.
34-36: Remove Legacy Qt5 Packages
The removal of Qt5 packages (usingapt-get remove -y ... || true) is implemented correctly to avoid conflicts with the new Qt6 installations. Ensure that none of the removed packages are needed in any other part of the build process.
37-38: Installation of Qt6 Dependencies
The new Qt6 package groups (qt6-base-dev,qt6-base-dev-tools,libqt6svg6-dev,libqt6opengl6-dev,qt6-l10n-tools, andqt6-tools-dev) are installed appropriately. This aligns with the goal of moving from Qt5 to Qt6.
44-44: Ensure qmake Points to Qt6
Creating a symbolic link forqmaketo point to the Qt6 executable is correctly implemented. This ensures that any tool or script invokingqmakewill use the Qt6 version.
47-48: Verification of Qt6 Installation
Runningqmake --versionand listing the/usr/lib/x86_64-linux-gnu/cmake/Qt6directory are effective sanity checks to confirm a correct Qt6 installation.
Ensured Qt6 is used instead of Qt5 by:
Optimized the Docker build process by:
Summary by CodeRabbit