fix(ci): run macdeployqt to bundle QML module plugins on macOS - #292
Conversation
The macOS CI previously did manual framework and QML module copying, but missed the module plugins QQuickWidget needs at runtime (Controls, Layouts, Dialogs, Effects, Templates, NativeStyle). Installed bundles rendered the inspector panel blank and the material editor list would not open. macdeployqt scans the qml/ source tree for imports and pulls in the missing module plugins plus their backing frameworks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 37 minutes and 56 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA macOS packaging step was added to the deploy workflow that conditionally runs Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/deploy.yml (1)
1519-1519: Optional: extract themacdeployqtinvocation into its own step.The new block is appended inside the
Copy Qt libs to app folderstep, which makes the step name a little misleading (it now also bundles QML modules and adjusts rpaths viamacdeployqt). A dedicated step (e.g.,Run macdeployqt to bundle QML modules) would read better in the Actions UI and make failures easier to localize — no functional change needed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/deploy.yml at line 1519, The step currently named "Copy Qt libs to app folder" also runs macdeployqt and adjusts rpaths; split the macdeployqt invocation into a separate GitHub Actions step (e.g., "Run macdeployqt to bundle QML modules") so the original step only copies Qt libs and the new step runs macdeployqt and any rpath/QML bundling commands; update the workflow to move the macdeployqt commands out of the "Copy Qt libs to app folder" step and place them in the new step while keeping the same commands and dependencies, referencing the existing macdeployqt invocation and the "Copy Qt libs to app folder" step to preserve ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/deploy.yml:
- Around line 1575-1588: The macdeployqt step can place QML imports under
Contents/Resources/qml (mismatching the later qt.conf Qml2Imports = PlugIns/qml)
and its exit status is masked by the pipe to tail; update the job to (1) inspect
the created bundle after running MACDEPLOYQT (referencing MACDEPLOYQT and the
app path used) to list Contents/PlugIns/qml vs Contents/Resources/qml so you can
confirm where newly-added modules landed, (2) fix runtime lookup by either
setting Qml2Imports = Resources/qml in the qt.conf writer or adding
Resources/qml alongside PlugIns/qml or moving/symlinking the modules from
Resources/qml into PlugIns/qml after macdeployqt, and (3) preserve macdeployqt's
exit code by enabling pipefail or capturing $PIPESTATUS (i.e., record
macdeployqt's exit status before piping to tail) so failures are not swallowed.
---
Nitpick comments:
In @.github/workflows/deploy.yml:
- Line 1519: The step currently named "Copy Qt libs to app folder" also runs
macdeployqt and adjusts rpaths; split the macdeployqt invocation into a separate
GitHub Actions step (e.g., "Run macdeployqt to bundle QML modules") so the
original step only copies Qt libs and the new step runs macdeployqt and any
rpath/QML bundling commands; update the workflow to move the macdeployqt
commands out of the "Copy Qt libs to app folder" step and place them in the new
step while keeping the same commands and dependencies, referencing the existing
macdeployqt invocation and the "Copy Qt libs to app folder" step to preserve
ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 452ab132-b2c9-4ce7-ade2-606cc7f6c55f
📒 Files selected for processing (1)
.github/workflows/deploy.yml
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07b2c0b2b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sudo "$MACDEPLOYQT" "${{github.workspace}}/bin/QtMeshEditor.app" \ | ||
| -qmldir="${{github.workspace}}/qml" \ | ||
| -verbose=2 \ | ||
| -no-strip 2>&1 | tail -60 || echo "macdeployqt reported warnings, continuing..." |
There was a problem hiding this comment.
In the macOS Copy Qt libs to app folder step, the macdeployqt command is wrapped with || echo "...continuing", which converts any non-zero exit into a successful step. When macdeployqt fails (for example due to unresolved QML imports or plugin copy/link errors), CI will still publish a bundle, and the app can ship with missing Qt modules despite this fix. This should fail the job (or only ignore explicitly-known benign conditions) so broken bundles are caught during CI.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 56326ba — the step now captures macdeployqt's exit code into $rc, prints the tail of the log, and exit $rc on non-zero. A missing macdeployqt binary is also now a hard error.
Previously the macdeployqt exit code was masked by the `| tail -60` pipe and a trailing `|| echo`. A hard failure would still succeed the step and ship a broken bundle. Capture the exit code explicitly, print the tail, and exit non-zero on failure. Also treat a missing macdeployqt binary as a hard error instead of a warning, since we now depend on it for QML module discovery. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
macdeployqt already adds @executable_path/../Frameworks to the main binary, so our subsequent install_name_tool -add_rpath fails with "would duplicate path" and aborts the step (shell runs with -e). Silence the duplicate errors so the step remains idempotent whether or not macdeployqt touched the binary first. The dylib loop below already uses the same pattern.
|
1 participant
Summary
macdeployqton the macOS app bundle after the manual framework/QML module copy step so Qt can discover QML imports and bundle the missing module plugins (Controls, Layouts, Dialogs, Effects, Templates, NativeStyle, ...)Root cause
Our manual
cp -R .../qml/QtQml .../PlugIns/qml/only copies the top-levelQtQmlandQtQuicktrees. QQuickWidget loadsQtQuick.Controls,QtQuick.Layouts,QtQuick.Dialogs,QtQuick.Effects,QtQuick.Templates,QtQuick.NativeStyle, etc. at runtime — those module plugins and their backingQtQuickControls2*.frameworkdylibs were missing from installed bundles.macdeployqt -qmldir=<workspace>/qmlscans our QML source for imports and pulls in every missing module plugin plus its framework, and also fixes up rpaths/install_names.Test plan
QtMeshEditor.app— onlyQtQmlandQtQuickappeared underContents/PlugIns/qml/macdeployqtinvocation against the same bundle —Contents/PlugIns/qml/now includesQtQuick/{Controls,Dialogs,Effects,Layouts,Templates,NativeStyle,Window,Shapes,...}andContents/Frameworks/includes the matchingQtQuickControls2*.frameworkdylibs🤖 Generated with Claude Code
Summary by CodeRabbit