SG-45110 Make QtOpenGL import optional in PySide6Patcher - #1133
Closed
julien-lang wants to merge 3 commits into
Closed
julien-lang wants to merge 3 commits into
julien-lang wants to merge 3 commits into
Conversation
QtOpenGL requires OpenGL/EGL shared libraries (e.g. libGL, libEGL) that may be missing on headless servers such as Rundeck's doc-build environment. Today, if that import fails, QtImporter silently swallows the ImportError and returns None for QtCore/QtGui entirely, breaking all Qt functionality instead of just the handful of Qt4-compat OpenGL classes it provides. Follow the same pattern already used for QtWebEngineWidgets/QtWebEngineCore (SG-38470): fetch QtOpenGL via _import_module_by_name() so a failure only degrades gracefully (with a warning) instead of taking down the whole PySide6 binding.
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Add an importer-boundary regression test covering failed QtOpenGL loading.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Makes QtOpenGL optional so PySide6 remains usable in headless environments.
Changes:
- Loads
QtOpenGLseparately with graceful failure handling. - Skips OpenGL compatibility classes and emits a warning when unavailable.
- Passes optional
QtOpenGLsupport to the patcher.
File summaries
| File | Description |
|---|---|
python/tank/util/qt_importer.py |
Handles optional QtOpenGL loading. |
python/tank/util/pyside6_patcher.py |
Gracefully handles missing OpenGL support. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # QtOpenGL requires OpenGL/EGL shared libraries that may be missing on headless | ||
| # servers, so import it separately rather than letting it take down the whole | ||
| # PySide6 import. | ||
| QtOpenGL = self._import_module_by_name("PySide6", "QtOpenGL") |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1133 +/- ##
==========================================
+ Coverage 80.09% 80.11% +0.01%
==========================================
Files 203 203
Lines 19537 19538 +1
==========================================
+ Hits 15649 15652 +3
+ Misses 3888 3886 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PySide6Patcher.patch()does an unconditionalfrom PySide6 import (QtCore, QtGui, QtOpenGL, QtWidgets).QtOpenGLrequires OpenGL/EGL shared libraries (e.g.libGL.so.1,libEGL) that may not be present on headless servers, such as the machine that runs Rundeck's Sphinx doc-generation job for tk-framework-qtwidgets (SG-45110).When that import fails, the
ImportErrorpropagates out ofPySide6Patcher.patch(), andqt_importer.py's_import_modules()silently swallows it (except ImportError: pass), soQtImporter().QtCoreends upNoneentirely, breaking all Qt functionality in that environment, not just the handful of Qt4-compat OpenGL classesQtOpenGLprovides.Fix
Follow the same pattern already established for
QtWebEngineWidgets/QtWebEngineCore(SG-38470, #1012): fetchQtOpenGLvia the existing_import_module_by_name()helper, which catches any exception and returnsNoneon failure, and pass it intoPySide6Patcher.patch()as an optional parameter. WhenQtOpenGLis unavailable,patch()now degrades gracefully (skips restoring the ~15 Qt4-compat OpenGL classes intoQtGui, emits aRuntimeWarning) instead of losing the entire PySide6 binding.Behavior impact
QtOpenGLimports successfully (the normal case for DCCs bundling a full Qt with OpenGL support).QtOpenGLfails to import: previouslyQtCore/QtGuiwere bothNone(all Qt functionality broken). Now they're valid, minusQOpenGLBuffer,QOpenGLShader, and similar rarely used classes. A workspace-wide search across alltk-*repos found no app/engine/framework code referencing any of these classes through the Qt shim.Testing
tests/util_tests/test_pyside6_patcher.py::PySide6PatcherTests::test_patchstill passes (now emits the newRuntimeWarningsince it callspatch(None, None)withoutQtOpenGL).patch()returns valid, workingQtCore/QtGuishims both with and withoutQtOpenGLpassed in.Related to SG-45110 (Rundeck doc-build failure) and SG-44795 (PySide as a tk-toolchain dependency).