update to OpenMS 3.4 #268
update to OpenMS 3.4 #268
Conversation
WalkthroughThe Dockerfile was updated to use OpenMS branch Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-openms
- GitHub Check: build-simple-app
🔇 Additional comments (1)
Dockerfile (1)
12-12: Confirm build compatibility with OpenMS 3.4 branch
BumpingOPENMS_BRANCHtorelease/3.4.0is straightforward, but the build flags (Line 84) and the THIRDPARTY layout occasionally change between minor releases. Please make sure:
-DPY_MEMLEAK_DISABLE=Onis still accepted.- Required third-party binaries for 3.4 are still present in
THIRDPARTY/Linux/64bit.If any flag or folder moved, the compile stage will fail late in the pipeline.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
Dockerfile (1)
61-61: Un-pinned NumPy / Pandas may break wheels in the future
Whileautowrapandcythonare constrained,numpyandpandasare not. A future major release (NumPy 2.0 is imminent) is likely to break the pyOpenMS wheel build.Consider pinning to the version used by the OpenMS conda recipe (currently 1.26.* and pandas < 3) or at least adding an upper bound.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Dockerfile(4 hunks)
🧰 Additional context used
🪛 Hadolint (2.12.0)
Dockerfile
[error] 61-61: Ensure the shebang uses an absolute path to the interpreter.
(SC2239)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: build-openms
- GitHub Check: build-full-app
- GitHub Check: build-simple-app
- GitHub Check: test (ubuntu-latest, 3.11)
🔇 Additional comments (2)
Dockerfile (2)
12-12: Confirm 3.4.1 branch really compiles with our CMake flags
Jumping two minor releases can introduce new/renamed options (e.g.OPENMS_BUILD_GUI, contributor hashes). Please make sure thecmakeinvocation later (Line 84) still matches what 3.4.1 expects; otherwise the build will silently drop features or fail late.
71-71: Verify new third-party path exists in every tagged release
The layout change fromLinux/64bittoLinux/x86_64works for current nightlies, but older tags occasionally miss thex86_64directory (see 3.3.0-RC). A guarded copy or a fallback would make the build more robust.
| libboost-math1.74-dev \ | ||
| libboost-random1.74-dev | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests qtbase5-dev libqt5svg5-dev libqt5opengl5-dev | ||
| RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev |
There was a problem hiding this comment.
Header package for Qt6OpenGLWidgets is missing – build will likely fail
libqt6openglwidgets6 is the runtime library; the headers live in libqt6openglwidgets6-dev.
OpenMS’ GUI parts (and several topp tools) include <QtOpenGLWidgets/...> headers, so the -dev variant is required at compile time.
-RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev
+RUN apt-get install -y --no-install-recommends --no-install-suggests \
+ qt6-base-dev \
+ libqt6svg6-dev \
+ libqt6opengl6-dev \
+ libqt6openglwidgets6-dev \
+ libgl-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.
| RUN apt-get install -y --no-install-recommends --no-install-suggests qt6-base-dev libqt6svg6-dev libqt6opengl6-dev libqt6openglwidgets6 libgl-dev | |
| RUN apt-get install -y --no-install-recommends --no-install-suggests \ | |
| qt6-base-dev \ | |
| libqt6svg6-dev \ | |
| libqt6opengl6-dev \ | |
| libqt6openglwidgets6-dev \ | |
| libgl-dev |
🤖 Prompt for AI Agents
In Dockerfile at line 34, the package list includes the runtime library
libqt6openglwidgets6 but is missing the corresponding development headers
package libqt6openglwidgets6-dev. Replace libqt6openglwidgets6 with
libqt6openglwidgets6-dev to ensure the Qt6OpenGLWidgets header files are
available for compilation, preventing build failures related to missing headers.
Summary by CodeRabbit