- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 10.6k
Add appimage support for linux #20334
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: main
Are you sure you want to change the base?
Conversation
c91f2f7    to
    caba75a      
    Compare
  
    | 
 Need CI before this PR is merged. Where are the app images installed and is that an expected/sensible location? Maybe we should reuse  | 
| 
 I considered it, but deemed it too confusing. Especially with it not working on macOS. 
 In  | 
| @SMillerDev sounds good 👍🏻 | 
| I'm also considering adding a desktop file for these so the various Linux systems can pick it up easily. Ref: https://specifications.freedesktop.org/desktop-entry-spec/latest/ | 
| This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. | 
| This is waiting for the following pieces: 
 | 
| This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. | 
| Not stale, but it is waiting for Linux CI for casks, and that's not going as fast as I'd like. So help would be much appreciated. | 
| Okay, now that Linux CI for casks is merged I'm planning to: 
 | 
caba75a    to
    85b07ae      
    Compare
  
    85b07ae    to
    3f62e6a      
    Compare
  
    3f62e6a    to
    740bac5      
    Compare
  
    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.
Pull Request Overview
This PR adds AppImage support as a Linux-only cask stanza, extending Homebrew's cross-platform capabilities. It implements a new artifact type app_image that allows Linux users to install AppImage applications through Homebrew casks.
- Implements AppImageartifact class with symlink functionality
- Updates OS-specific installer logic to validate platform compatibility
- Enhances CI matrix generation to support Linux cask testing
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description | 
|---|---|
| Library/Homebrew/cask/artifact/appimage.rb | New AppImage artifact class that extends symlinked artifacts | 
| Library/Homebrew/cask/artifact.rb | Registers AppImage in artifact system and defines Linux-only artifacts | 
| Library/Homebrew/cask/config.rb | Adds appimagedir configuration option with default path | 
| Library/Homebrew/cli/parser.rb | Adds --appimagedir CLI flag for configuring AppImage installation directory | 
| Library/Homebrew/cask/dsl.rb | Registers AppImage as an ordinary artifact class in DSL | 
| Library/Homebrew/cask/cask.rb | Updates platform support detection logic for macOS and Linux | 
| Library/Homebrew/extend/os/mac/cask/installer.rb | New macOS-specific installer validation | 
| Library/Homebrew/extend/os/linux/cask/installer.rb | Updates Linux installer validation logic | 
| Library/Homebrew/extend/os/cask/installer.rb | Includes macOS-specific installer extensions | 
| Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb | Enhances CI matrix generation for Linux cask support | 
Comments suppressed due to low confidence (1)
Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb:1
- Empty comment should either be removed or expanded to explain why the exception is being silently ignored, unlike the macOS case which logs the error message.
# typed: strict
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| I guess this is an implementation of #15808 | 
9dcac65    to
    0c8c28c      
    Compare
  
    0c8c28c    to
    4b7f3af      
    Compare
  
    4b7f3af    to
    06e8a20      
    Compare
  
    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.
Looks good so far.
| def supports_macos? | ||
| return true if font? | ||
|  | ||
| if @dsl.on_system_blocks_exist? | 
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.
Given this block is nearly identical in the macOS and Linux cases: it'd be nicer to have a shared helper method in this class that both cases call.
| # we don't need to run simulated archs on Linux | ||
| next if runner.fetch(:symbol) == :linux && !native_runner_arch | ||
| # we don't need to run simulated archs on macOS | ||
| # we don't need to run simulated archs on macOS Sequoia | 
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.
Why not?
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.
Because an intel runner exists for it, in contrary to all the other macOS versions
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.
| # we don't need to run simulated archs on macOS Sequoia | |
| # we don't need to run simulated archs on macOS Sequoia | |
| # because it has a GitHub hosted x86_64 runner | 
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.
Should I add the same to the Linux block above?
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.
if that's the reason: yup!
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?This adds support for the first linux-only cask stanza:
app_image.We should probably add CI for Linux casks, but haven't been able to get that working yet.