-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add distribution-specific build dependencies for Linux (#4339) #4345
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
base: v3-alpha
Are you sure you want to change the base?
Conversation
WalkthroughThe nfpm.yaml template was updated to specify more precise build dependencies for Debian, Ubuntu, RPM-based, and Arch Linux distributions. A new Changes
Sequence Diagram(s)sequenceDiagram
participant BuildSystem
participant nfpm.yaml.tmpl
participant DistroSelector
BuildSystem->>nfpm.yaml.tmpl: Load build configuration
nfpm.yaml.tmpl->>DistroSelector: Determine target distribution/format
DistroSelector-->>nfpm.yaml.tmpl: Return distro/format info
nfpm.yaml.tmpl->>nfpm.yaml.tmpl: Apply default depends
nfpm.yaml.tmpl->>nfpm.yaml.tmpl: Apply overrides for rpm/archlinux if applicable
nfpm.yaml.tmpl-->>BuildSystem: Final dependency list for packaging
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Suggested reviewers
Poem
✨ Finishing Touches
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
Documentation and Community
|
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
🧹 Nitpick comments (7)
v3/internal/doctor/packagemanager/pacman.go (1)
32-35
: Consider reversing the fallback order for Arch Linux
pacman
’s official repos expose the development headers under the unversionedwebkit2gtk
package (currently 2.42.x).
webkit2gtk-4.1
exists only in AUR / third-party repos. If the consuming code iterates the slice in order and stops at the first “available” match, starting withwebkit2gtk-4.1
will cause a false-negative on a stock Arch install.- "webkit2gtk": []*Package{ - {Name: "webkit2gtk-4.1", SystemPackage: true, Library: true}, - {Name: "webkit2gtk", SystemPackage: true, Library: true}, + "webkit2gtk": []*Package{ + // Arch repos (pacman) provide the headers here; try first. + {Name: "webkit2gtk", SystemPackage: true, Library: true}, + // AUR / bleeding-edge fallback + {Name: "webkit2gtk-4.1", SystemPackage: true, Library: true}, },Please verify the lookup logic in
doctor
prefers the first satisfiable entry, or switch the order as shown.v3/internal/doctor/packagemanager/dnf.go (1)
27-32
: Minor style nit – slice literal could be in declarationThe temporary
webkitPackages
variable is only referenced once. Inline it into the map literal to reduce noise.- webkitPackages := []*Package{ + // Fedora 40+ ships 4.1; keep explicit fallbacks for older releases. + webkitPackages := []*Package{ // <- consider inliningPurely cosmetic – feel free to ignore.
v3/internal/capabilities/capabilities_linux.go (1)
5-23
: Cache the pkg-config probe to avoid repeated subprocess overhead
operatingsystem.GetWebkitVersion()
executespkg-config
each timeNewCapabilities()
is called.
IfNewCapabilities()
is created per window or event, this can spawn dozens of short-lived processes.A simple
sync.Once
cache insideoperatingsystem
(or here) avoids the overhead:var cached Version var once sync.Once func GetWebkitVersion() Version { - // current implementation … + once.Do(func() { + cached = probeWithPkgConfig() + }) + return cached }Not critical, but worth tightening.
v3/internal/doctor/packagemanager/apt.go (1)
27-32
: Explicit ordering for fall-back is good, but double-check downstream assumptionsReturning
[]*Package{4.1, 4.0}
under the same"webkit2gtk"
key makes sense for graceful degradation, yet other code that previously assumedPackages()["webkit2gtk"][0]
is “the” package might now silently pick 4.1 only.
If later logic walks the slice until it finds an installed entry this is fine; if it just dereferences the first element it will never consider 4.0. Please verify the consumer logic across other managers (dnf/pacman/…) and in the doctor UI.Also applies to: 37-37
v3/internal/operatingsystem/webkit_linux.go (2)
36-45
: Swallowingpkg-config
execution errors hides diagnostic information
getWebkitVersionFromPkgConfig
drops theerr
fromcmd.Output()
and simply returns an empty struct.
While that keeps the control-flow simple, users runningwails doctor
won’t know why detection failed (missingpkg-config
, wrong package name, PATH issues, etc.).-func getWebkitVersionFromPkgConfig(packageName string) WebkitVersion { - cmd := exec.Command("pkg-config", "--modversion", packageName) - output, err := cmd.Output() - if err != nil { - return WebkitVersion{} // Package not found - } +func getWebkitVersionFromPkgConfig(packageName string) WebkitVersion { + cmd := exec.Command("pkg-config", "--modversion", packageName) + output, err := cmd.Output() + if err != nil { + // Keep quiet in library mode but surface the reason in doctor/verbose paths + // log.Debugf("pkg-config %s failed: %v", packageName, err) + return WebkitVersion{} + }Even a debug-level log would help troubleshooting without changing behaviour.
47-63
: Parsing routine ignores conversion errors
strconv.ParseUint
return values are discarded. Although unlikely, a malformed version string would silently yield0
, potentially making capability checks mis-behave.-major, _ := strconv.ParseUint(matches[1], 10, 32) -minor, _ := strconv.ParseUint(matches[2], 10, 32) +major, err1 := strconv.ParseUint(matches[1], 10, 32) +minor, err2 := strconv.ParseUint(matches[2], 10, 32) +if err1 != nil || err2 != nil { + return WebkitVersion{} +}Same applies to
micro
. Handling the error (or at least logging) would make the function more robust.v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_41-deb_suse_arch.yaml.tmpl (1)
25-35
: Arch & SUSE alternative names are helpful – consider templatingYou already document the alternatives in comments. To avoid maintaining three nearly identical templates, you could expose the package names as Go template variables (e.g.
{{.WebkitDevPackage}}
) and fill them per distro in the build pipeline, reducing duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
v3/internal/capabilities/capabilities_linux.go
(1 hunks)v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_40-rhel.yaml.tmpl
(1 hunks)v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_41-deb_suse_arch.yaml.tmpl
(1 hunks)v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_41-fedora.yaml.tmpl
(1 hunks)v3/internal/doctor/packagemanager/apt.go
(1 hunks)v3/internal/doctor/packagemanager/dnf.go
(1 hunks)v3/internal/doctor/packagemanager/emerge.go
(1 hunks)v3/internal/doctor/packagemanager/pacman.go
(1 hunks)v3/internal/doctor/packagemanager/zypper.go
(1 hunks)v3/internal/operatingsystem/webkit_linux.go
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
v3/internal/doctor/packagemanager/apt.go (1)
v3/internal/doctor/packagemanager/pm.go (2)
Package
(6-14)Packagemap
(16-16)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (macos-latest, 1.24)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
- GitHub Check: semgrep/ci
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
v3/internal/doctor/packagemanager/zypper.go (1)
34-35
: LGTM – new 4.1 devel package addedAdding
webkit2gtk4_1-devel
provides a clean upgrade path for newer openSUSE releases without breaking the existing fallbacks.v3/internal/doctor/packagemanager/emerge.go (1)
33-35
: Slot 4 added – double-check GTK API coverageGentoo’s
net-libs/webkit-gtk:4
is the GTK 4 build, whereas:6
is still GTK 3 with libsoup-3. Ensure Wails’ Gtk3 code paths don’t accidentally pick the GTK 4 variant first, or enforce the slot selection elsewhere.v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_41-fedora.yaml.tmpl (1)
25-31
: Verify Fedora package name for WebKit 4.1On Fedora 40+ the devel package is usually
webkit2gtk4.1-devel
, but older naming schemes sometimes drop the minor version. Please confirm withdnf info webkit2gtk4\*devel
to avoid broken deps at install time.
# Dependencies for RHEL/CentOS with WebKit 4.0 | ||
depends: | ||
- gtk3-devel | ||
- webkit2gtk3-devel | ||
- gcc-c++ | ||
- pkg-config |
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.
Possible mismatch: RHEL’s 4.0 devel package is often webkit2gtk4.0-devel
The spec lists webkit2gtk3-devel
. That is the GTK 3 version (2.x series), not 4.0.
If the intent is WebKit GTK 4.0, the correct RPM name on RHEL 9 is usually webkit2gtk4.0-devel
. Double-check to prevent pulling the wrong ABI.
🤖 Prompt for AI Agents
In v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm_40-rhel.yaml.tmpl
around lines 25 to 30, the dependency listed as webkit2gtk3-devel is likely
incorrect for WebKit GTK 4.0 on RHEL 9. Replace webkit2gtk3-devel with
webkit2gtk4.0-devel to ensure the correct development package for WebKit GTK 4.0
is used, preventing ABI mismatches.
Deploying wails with
|
Latest commit: |
5f38b3c
|
Status: | ✅ Deploy successful! |
Preview URL: | https://a509d241.wails.pages.dev |
Branch Preview URL: | https://v3-feature-4339-linux-build.wails.pages.dev |
Implements feature #4339 by enhancing the nfpm.yaml.tmpl to use nfpm's built-in overrides feature for different Linux distributions and package formats. ## Changes Made: ### Enhanced nfpm configuration with overrides: - **Default**: Debian 12/Ubuntu 22.04+ with WebKit 4.1 dependencies - Uses libgtk-3-dev, libwebkit2gtk-4.1-dev, build-essential, pkg-config - **RPM override**: RHEL/CentOS/AlmaLinux/Rocky Linux with WebKit 4.0 dependencies - Uses gtk3-devel, webkit2gtk3-devel, gcc-c++, pkg-config - **Arch override**: Arch Linux with WebKit 4.1 dependencies - Uses gtk3, webkit2gtk-4.1, base-devel, pkgconf This approach uses nfpm's native override system to automatically select the correct dependencies based on the target package format, ensuring that each distribution gets the appropriate WebKit version and package names. ### How it works: - DEB packages get WebKit 4.1 dependencies (default) - RPM packages get WebKit 4.0 dependencies (RHEL/CentOS compatibility) - Arch packages get WebKit 4.1 dependencies with Arch-specific package names Fixes #4339 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
ed182c2
to
a1ecb1a
Compare
Improves the doctor command to better detect WebKit dependencies across different Linux distributions by adding fallback package support. ## Changes Made: ### Enhanced Package Manager Detection: - **APT (Debian/Ubuntu)**: Added WebKit 4.1 → 4.0 fallback support - **DNF (Fedora/RHEL)**: Added webkit2gtk4.0-devel fallback for older systems - **Zypper (SUSE)**: Added webkit2gtk4_1-devel for modern SUSE distributions - **Emerge (Gentoo)**: Added support for both webkit-gtk:6 and webkit-gtk:4 slots - **Pacman (Arch)**: Added fallback from webkit2gtk-4.1 to webkit2gtk ### Improved Developer Experience: - Doctor command now tries newer WebKit versions first, falls back gracefully - Provides more accurate dependency detection across distributions - Better guidance for developers on different Linux systems ### How It Works: - Each package manager lists multiple WebKit package options in order of preference - The dependency system tries packages in order until it finds one that's available - Developers get appropriate installation commands for their specific distribution This complements the nfpm overrides by ensuring developers can properly set up their development environment regardless of distribution. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
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 (2)
v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl (2)
35-43
: RPM override shadows the root list—4.1 support for Fedora 40+?
overrides.rpm.depends
replaces, not merges with, the default list. Fedora 40+ ships WebKit 4.1, yet the override pins towebkit2gtk3-devel
(4.0).
If Fedora 4.1 is handled in a separate template (nfpm_41-fedora.yaml.tmpl
), add a short in-file comment to make that explicit; otherwise update the override or use an OR-style dependency string.
45-50
: Reconsiderbase-devel
as a runtime dependency on Arch
base-devel
is a meta-group that installs the full compiler suite; pacman users typically only expect it for build time. Unless the Wails CLI genuinely requires a compiler at runtime, you may want to drop it (or, once nfpm supports it, move it tomakedepends
).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: auto-label
- GitHub Check: Run JS Tests (20.x)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (2)
v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl (2)
27-33
: Confirm that “*-dev”,build-essential
, andpkg-config
are meant as runtime depsPlacing these in
depends
means every machine that installs the resulting Wails package will also pull in the full build tool-chain. If the template targets end-users rather than developers, consider instead:• listing only the shared-library packages (
libgtk-3-0
,libwebkit2gtk-4.*-0
);
• or providing a separate “builder” template that keeps the-dev
/compiler deps.Please verify the intended audience.
27-33
: 🛠️ Refactor suggestionAdd a fallback to WebKit 4.0 for Debian/Ubuntu
libwebkit2gtk-4.1-dev
is not present in the default repos of Debian 12 or Ubuntu 22.04, so the generated.deb
will fail to install there. nfpm accepts full dpkg syntax, so you can express an OR-dependency:- - libwebkit2gtk-4.1-dev + - "libwebkit2gtk-4.1-dev | libwebkit2gtk-4.0-dev"Doing the same for the runtime library (if needed) will let one template service both ABIs without manual tweaking.
Likely an incorrect or invalid review 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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
v3/internal/doctor/packagemanager/dnf.go (1)
31-35
: Clarify fallback precedence for multiple WebKitGTK versionsNow that the slice contains three mutually-exclusive development packages (
4.1
,3
,4.0
), downstream logic must rely on the slice order to decide which version to test / install first.
If that assumption is intentional, add a short comment (or sort newest → oldest) so future maintainers don’t inadvertently change the order and break the fallback strategy."webkit2gtk": []*Package{ - {Name: "webkit2gtk4.1-devel", SystemPackage: true, Library: true}, - {Name: "webkit2gtk3-devel", SystemPackage: true, Library: true}, - {Name: "webkit2gtk4.0-devel", SystemPackage: true, Library: true}, + // Try 4.1 first (Fedora ≥ 40), then 3 (generic), finally 4.0 (RHEL-compatible) + {Name: "webkit2gtk4.1-devel", SystemPackage: true, Library: true}, // preferred + {Name: "webkit2gtk3-devel", SystemPackage: true, Library: true}, // fallback + {Name: "webkit2gtk4.0-devel", SystemPackage: true, Library: true}, // legacyThis tiny hint avoids accidental regressions without altering behaviour.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
v3/internal/doctor/packagemanager/apt.go
(1 hunks)v3/internal/doctor/packagemanager/dnf.go
(1 hunks)v3/internal/doctor/packagemanager/emerge.go
(1 hunks)v3/internal/doctor/packagemanager/pacman.go
(1 hunks)v3/internal/doctor/packagemanager/zypper.go
(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- v3/internal/doctor/packagemanager/zypper.go
- v3/internal/doctor/packagemanager/apt.go
- v3/internal/doctor/packagemanager/emerge.go
- v3/internal/doctor/packagemanager/pacman.go
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Run Go Tests v3 (windows-latest, 1.24)
- GitHub Check: Run Go Tests v3 (ubuntu-latest, 1.24)
- GitHub Check: Cloudflare Pages
Summary
Implements feature #4339 with a comprehensive approach that includes both nfpm overrides for packaging and enhanced doctor command support for development dependency detection across different Linux distributions.
Changes Made
🔧 Enhanced nfpm Configuration with Overrides
Updated
nfpm.yaml.tmpl
to use nfpm's built-inoverrides
feature:Default (DEB packages): Debian 12/Ubuntu 22.04+ with WebKit 4.1 dependencies
libgtk-3-dev
,libwebkit2gtk-4.1-dev
,build-essential
,pkg-config
RPM Override: RHEL/CentOS/AlmaLinux/Rocky Linux with WebKit 4.0 dependencies
gtk3-devel
,webkit2gtk3-devel
,gcc-c++
,pkg-config
Arch Override: Arch Linux with WebKit 4.1 dependencies
gtk3
,webkit2gtk-4.1
,base-devel
,pkgconf
🩺 Enhanced wails doctor Command
Improved package manager detection with WebKit fallback support:
🎯 How It Works
For Packaging (nfpm automatically selects based on format):
wails3 tool package -format deb
→ WebKit 4.1 dependencieswails3 tool package -format rpm
→ WebKit 4.0 dependencieswails3 tool package -format archlinux
→ WebKit 4.1 with Arch packagesFor Development (doctor command detects what's available):
wails3 doctor
checks for available WebKit packages in order of preference📋 Benefits
🔄 Backward Compatibility
This change is fully backward compatible. Existing build processes will continue to work unchanged, but will now:
wails3 doctor
📝 Example Usage
Packaging:
Development Setup:
Files Changed
internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl
- Enhanced with distribution-specific overridesinternal/doctor/packagemanager/apt.go
- Added WebKit 4.1/4.0 fallback supportinternal/doctor/packagemanager/dnf.go
- Enhanced WebKit package detection for Fedora/RHELinternal/doctor/packagemanager/emerge.go
- Added WebKit slot support for Gentoointernal/doctor/packagemanager/pacman.go
- Added WebKit fallback support for Archinternal/doctor/packagemanager/zypper.go
- Added WebKit 4.1 support for SUSECloses #4339
🤖 Generated with Claude Code
Summary by CodeRabbit