-
Notifications
You must be signed in to change notification settings - Fork 3
Fix/sp 3645/install errors on linux #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
EditMenu is a macOS-only feature that enables standard shortcuts (Cmd+C, Cmd+V, Cmd+Z). Appending it on Linux causes the app to crash. Added platform check to conditionally append EditMenu only on darwin, as recommended in the Wails documentation. See: https://wails.io/docs/reference/menus/
Added build_webkit41 target to Makefile for Ubuntu 24.04+ and Debian 13+ support, which require the -tags webkit2_41 flag due to webkit2gtk-4.1. See: https://wails.io/docs/gettingstarted/building
WalkthroughAdds WebKit 4.1 support and a Linux CI matrix (webkit40/webkit41), updates release artifact naming and packaging for both variants, restricts the Edit menu to macOS, and updates docs/installation instructions to offer OS-version–specific binaries and build targets. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Trigger as release.yml
participant Matrix as linux matrix
participant Job40 as webkit40 job
participant Job41 as webkit41 job
participant Artifacts as artifact store
participant Release as create_release job
Trigger->>Matrix: start with variants (webkit40, webkit41)
par parallel builds
Matrix->>Job40: run on ubuntu-22.04\nmake build (build_target), output suffix linux-amd64
Job40->>Job40: install libwebkit2gtk-4.0-dev\nbuild, zip as *-linux-amd64.zip
Job40->>Artifacts: upload artifact_l_webkit40
and
Matrix->>Job41: run on ubuntu-24.04\nmake build_webkit41, output suffix linux-amd64-webkit41
Job41->>Job41: install libwebkit2gtk-4.1-dev\nbuild, zip as *-linux-amd64-webkit41.zip
Job41->>Artifacts: upload artifact_l_webkit41
end
Release->>Artifacts: download artifact_l_webkit40 and artifact_l_webkit41
Release->>Release: aggregate zips, generate SHA256SUMS
Release->>GitHub: create release with all zips + SHA256SUMS
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ 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 |
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/release.yml(4 hunks)CHANGELOG.md(2 hunks)INSTALLATION.md(2 hunks)Makefile(2 hunks)README.md(3 hunks)app.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app.go (1)
frontend/wailsjs/runtime/runtime.js (1)
Environment(180-182)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (13)
app.go (1)
169-171: LGTM! Platform-specific menu handling looks correct.The EditMenu is now correctly restricted to macOS, which should resolve the Linux startup crash mentioned in the PR objectives.
CHANGELOG.md (1)
10-19: LGTM! Changelog entry is well-documented.The changelog accurately captures the changes in this PR, including the new build target, Linux CI matrix, and the EditMenu fix.
Makefile (2)
21-21: LGTM! Help regex correctly updated to support numeric targets.The addition of
0-9to the help pattern ensures the newbuild_webkit41target will appear in the help output.
76-78: LGTM! WebKit 4.1 build target implemented correctly.The new target properly uses the
webkit2_41tags and follows the same clean pattern as the other build targets.INSTALLATION.md (1)
220-226: LGTM! Binary selection guidance is clear.The section clearly distinguishes which binary to download based on the Ubuntu/Debian version and WebKit support.
README.md (3)
82-95: LGTM! Linux installation instructions are clear and accurate.The version-specific binary selection and installation commands correctly guide users to the appropriate WebKit variant for their system.
153-178: LGTM! Development dependencies correctly specify WebKit versions.The system dependencies section properly distinguishes between WebKit 4.0 (for Ubuntu 22.04 and older) and WebKit 4.1 (for Ubuntu 24.04+/Debian 13+), which aligns with the CI matrix and build targets.
218-223: LGTM! Build instructions for different Linux versions are accurate.The section correctly maps Ubuntu versions to their respective make targets and WebKit versions.
.github/workflows/release.yml (5)
188-202: LGTM! Linux build matrix is well-structured.The matrix correctly defines two variants for WebKit 4.0 and 4.1 support with appropriate runner versions, build targets, output suffixes, and webkit packages. This aligns perfectly with the Makefile changes and documentation.
231-231: LGTM! Dependency installation properly uses matrix variable.Using
matrix.webkit_packageensures each build variant installs the correct WebKit library version.
242-243: LGTM! Build step correctly uses matrix variables.The build command uses
matrix.build_targetto invoke the appropriate make target, and the binary is correctly renamed withmatrix.output_suffixto distinguish the variants.
259-260: LGTM! Artifact handling correctly incorporates matrix variant.The artifact naming scheme includes
matrix.variantand usesmatrix.output_suffixin the path, ensuring proper identification of webkit40 vs webkit41 builds.
374-382: LGTM! Release artifact downloads handle both WebKit variants.The separate download steps for
artifact_l_webkit40andartifact_l_webkit41ensure both Linux binaries are included in the release.
…bian/Ubuntu 22.04 and older
SCANOSS SCAN Completed 🚀
View more details on SCANOSS Action Summary |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
INSTALLATION.md (1)
377-390: Uninstall section should be version-aware for consistency.The uninstall commands for Ubuntu/Debian (line 380) only reference
libwebkit2gtk-4.0-37, which is incorrect for Ubuntu 24.04+ / Debian 13+ users who would havelibwebkit2gtk-4.1-0installed instead. While the section correctly notes that dependencies may be used by other applications, consistency with the version-specific approach used throughout the installation and troubleshooting sections would improve clarity.Consider splitting the uninstall command by version to match the pattern used elsewhere in the documentation:
**Note:** System dependencies (GTK3, WebKit2GTK) are not removed as they may be used by other applications. If you want to remove them: ```bash # Ubuntu/Debian -sudo apt-get remove libgtk-3-0 libwebkit2gtk-4.0-37 +# Ubuntu 22.04 and older / Debian 12 and older +sudo apt-get remove libgtk-3-0 libwebkit2gtk-4.0-37 + +# Ubuntu 24.04+ / Debian 13+ +sudo apt-get remove libgtk-3-0 libwebkit2gtk-4.1-0 # Fedora/RHEL sudo dnf remove gtk3 webkit2gtk4.0
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
INSTALLATION.md(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). (1)
- GitHub Check: build
🔇 Additional comments (4)
INSTALLATION.md (4)
220-226: Binary selection documentation is clear and well-structured.The new "Choosing the Right Binary" subsection effectively communicates the WebKit version differences and provides an unambiguous decision point for users.
233-243: Installation commands are version-specific and correct.The split between webkit40 and webkit41 binaries, along with appropriate move commands, aligns with the binary distribution strategy.
247-255: Critical issue resolved: WebKit library versions are now correct.The previous incorrect specification of
libwebkit2gtk-4.1-0for Ubuntu 22.04 and older has been fixed. The documentation now correctly specifies:
- Ubuntu 22.04 and older:
libwebkit2gtk-4.0-37✓- Ubuntu 24.04+ / Debian 13+:
libwebkit2gtk-4.1-0✓This aligns with the version-specific binary approach and matches the troubleshooting section.
444-451: Troubleshooting section is version-aware and consistent.The separation of WebKit dependency installation by OS version in the troubleshooting section mirrors the installation guidance and ensures users find correct information regardless of which section they consult.
matiasdaloia
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
See individual commits
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chore