Skip to content

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

Open
wants to merge 2 commits into
base: v3-alpha
Choose a base branch
from

Conversation

leaanthony
Copy link
Member

@leaanthony leaanthony commented Jun 11, 2025

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-in overrides 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:

  • APT (Debian/Ubuntu): WebKit 4.1 → 4.0 fallback detection
  • DNF (Fedora/RHEL): Enhanced webkit2gtk4.1-devel, webkit2gtk3-devel, webkit2gtk4.0-devel support
  • Zypper (SUSE): Added webkit2gtk4_1-devel for modern SUSE distributions
  • Emerge (Gentoo): Support for both webkit-gtk:6 and webkit-gtk:4 slots
  • Pacman (Arch): webkit2gtk-4.1 → webkit2gtk fallback support

🎯 How It Works

For Packaging (nfpm automatically selects based on format):

  • wails3 tool package -format deb → WebKit 4.1 dependencies
  • wails3 tool package -format rpm → WebKit 4.0 dependencies
  • wails3 tool package -format archlinux → WebKit 4.1 with Arch packages

For Development (doctor command detects what's available):

  • wails3 doctor checks for available WebKit packages in order of preference
  • Provides appropriate installation commands for the user's specific distribution
  • Falls back gracefully when newer WebKit versions aren't available

📋 Benefits

  • Comprehensive solution - Handles both packaging and development workflows
  • Distribution-aware - Each package format/distribution gets appropriate dependencies
  • WebKit compatibility - Handles WebKit 4.0 vs 4.1 differences automatically
  • Fallback support - Graceful degradation when newer packages aren't available
  • Developer-friendly - Better guidance through enhanced doctor command
  • Maintainable - Uses standard nfpm features and existing package manager interfaces

🔄 Backward Compatibility

This change is fully backward compatible. Existing build processes will continue to work unchanged, but will now:

  • Automatically get the correct dependencies for their target distribution when packaging
  • Receive better dependency detection guidance when running wails3 doctor

📝 Example Usage

Packaging:

# Create a DEB package for Debian/Ubuntu (WebKit 4.1)
wails3 tool package -format deb -config ./build/linux/nfpm/nfpm.yaml -name myapp

# Create an RPM package for RHEL/CentOS (WebKit 4.0)  
wails3 tool package -format rpm -config ./build/linux/nfpm/nfpm.yaml -name myapp

# Create an Arch package for Arch Linux (WebKit 4.1)
wails3 tool package -format archlinux -config ./build/linux/nfpm/nfpm.yaml -name myapp

Development Setup:

# Check what dependencies are needed and available
wails3 doctor

# Follow the provided installation commands for your distribution

Files Changed

  • internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl - Enhanced with distribution-specific overrides
  • internal/doctor/packagemanager/apt.go - Added WebKit 4.1/4.0 fallback support
  • internal/doctor/packagemanager/dnf.go - Enhanced WebKit package detection for Fedora/RHEL
  • internal/doctor/packagemanager/emerge.go - Added WebKit slot support for Gentoo
  • internal/doctor/packagemanager/pacman.go - Added WebKit fallback support for Arch
  • internal/doctor/packagemanager/zypper.go - Added WebKit 4.1 support for SUSE

Closes #4339

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • Improved package dependency management for Linux distributions, with distribution-specific dependency overrides for Debian/Ubuntu, RPM-based distros, and Arch Linux.
  • Bug Fixes
    • Enhanced recognition of required development packages for WebKit and GTK across multiple Linux package managers, supporting additional versions and variants.

Copy link
Contributor

coderabbitai bot commented Jun 11, 2025

Walkthrough

The nfpm.yaml template was updated to specify more precise build dependencies for Debian, Ubuntu, RPM-based, and Arch Linux distributions. A new overrides section introduces distribution-specific dependency lists, ensuring correct package names and versions are used for each target platform and WebKit ABI version. Additionally, various package manager files were updated to recognize multiple WebKitGTK versions across distributions.

Changes

File(s) Change Summary
v3/internal/commands/updatable_build_assets/linux/nfpm/nfpm.yaml.tmpl Updated default dependencies for Debian/Ubuntu; added overrides for RPM and Arch Linux with distribution-specific dependency lists.
v3/internal/doctor/packagemanager/apt.go, dnf.go, emerge.go, pacman.go, zypper.go Added or updated WebKitGTK package entries to support multiple versions (4.0 and 4.1) across different package managers.

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
Loading

Assessment against linked issues

Objective Addressed Explanation
Update nfpm config to use correct build dependencies for each Linux distribution (#4339)
Add conditional overrides for RPM and Arch Linux to reflect distribution-specific packages (#4339)
Ensure dependency lists match the ABI and WebKit version requirements per distro (#4339)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes detected.

Suggested reviewers

  • leaanthony

Poem

In the warren where builds are spun,
Each distro’s needs are finely done.
With overrides set and packages right,
No more dependency woes at night!
Rabbits hop, the YAML’s neat—
Every bunny’s build is sweet! 🐇✨

✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 unversioned webkit2gtk 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 with webkit2gtk-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 declaration

The 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 inlining

Purely 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() executes pkg-config each time NewCapabilities() is called.
If NewCapabilities() is created per window or event, this can spawn dozens of short-lived processes.

A simple sync.Once cache inside operatingsystem (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 assumptions

Returning []*Package{4.1, 4.0} under the same "webkit2gtk" key makes sense for graceful degradation, yet other code that previously assumed Packages()["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: Swallowing pkg-config execution errors hides diagnostic information

getWebkitVersionFromPkgConfig drops the err from cmd.Output() and simply returns an empty struct.
While that keeps the control-flow simple, users running wails doctor won’t know why detection failed (missing pkg-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 yield 0, 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 templating

You 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

📥 Commits

Reviewing files that changed from the base of the PR and between fe29d14 and ed182c2.

📒 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 added

Adding 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 coverage

Gentoo’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.1

On Fedora 40+ the devel package is usually webkit2gtk4.1-devel, but older naming schemes sometimes drop the minor version. Please confirm with dnf info webkit2gtk4\*devel to avoid broken deps at install time.

Comment on lines 25 to 30
# Dependencies for RHEL/CentOS with WebKit 4.0
depends:
- gtk3-devel
- webkit2gtk3-devel
- gcc-c++
- pkg-config
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Copy link

cloudflare-workers-and-pages bot commented Jun 11, 2025

Deploying wails with  Cloudflare Pages  Cloudflare Pages

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

View logs

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>
@leaanthony leaanthony force-pushed the v3/feature-4339-linux-build-dependencies branch from ed182c2 to a1ecb1a Compare June 11, 2025 14:15
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>
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 to webkit2gtk3-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: Reconsider base-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 to makedepends).

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ed182c2 and a1ecb1a.

📒 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, and pkg-config are meant as runtime deps

Placing 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 suggestion

Add 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.

Copy link

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:L This PR changes 100-499 lines, ignoring generated files. labels Jun 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 versions

Now 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}, // legacy

This tiny hint avoids accidental regressions without altering behaviour.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a1ecb1a and 5f38b3c.

📒 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

@dosubot dosubot bot added the Enhancement New feature or request label Jun 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli Enhancement New feature or request Linux size:M This PR changes 30-99 lines, ignoring generated files. v3-alpha
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant