Implement auto-update system with Sparkle framework #1
Conversation
- Added UpdateNotificationCard component for displaying update notifications. - Integrated UpdaterManager to handle update checks and installations. - Updated ContentView to show update notifications based on UpdaterManager state. - Configured app constants for auto-update settings including appcast URLs and update intervals. - Modified build script to include Sparkle configuration in Info.plist for automatic update checks. - Created Product Requirements Document detailing the auto-update system's objectives, features, and implementation phases. - Developed Sparkle setup guide for generating EdDSA keys and configuring GitHub secrets. - Implemented scripts for generating EdDSA keys and signatures for release assets.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @jsonify, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a robust auto-update system for the application, leveraging the Sparkle framework to provide seamless update notifications, management, and installation. The changes encompass client-side update logic, server-side appcast generation from GitHub releases, user interface components for update interaction, and comprehensive documentation for setup, security, and future development.
Highlights
- Core Auto-Update Logic: I've introduced
UpdaterManager.swift, which serves as the central orchestrator for the Sparkle auto-update system. This manager handles update checks, manages user preferences like automatic updates and beta channel participation, and exposes update status and information to the UI. It also implements theSPUUpdaterDelegateprotocol to interact directly with the Sparkle framework. - Appcast Generation: A new
AppcastGenerator.swiftfile has been added to programmatically create Sparkle-compatible appcast XML feeds. This utility fetches release information directly from GitHub's API, filters releases based on criteria (e.g., beta vs. stable), and formats the data into the required XML structure, which is essential for Sparkle to discover available updates. - User Interface Integration: I've added two new SwiftUI components:
DeveloperUpdateButton.swiftandUpdateNotificationCard.swift. TheDeveloperUpdateButtonprovides a simple manual update check and install button, primarily for development and testing. TheUpdateNotificationCardis a user-facing component that displays update availability, version details, and offers actions like viewing release notes, installing the update, or skipping a version. These components are integrated intoContentView.swift. - Build System and Configuration Updates: The
Package.swiftfile now includes Sparkle as a dependency.AppConstants.swifthas been updated with new configurations for the update system, including appcast URLs, check intervals, andUserDefaultskeys for user preferences. Thebuild_app.shscript has also been modified to embed Sparkle-specific keys directly into the application'sInfo.plistduring the build process. - Security and Documentation: To support secure updates, I've added Python scripts (
generate_eddsa_keys.pyandgenerate_signatures.py) for generating EdDSA key pairs and signing release assets. Comprehensive documentation files (auto-update-mvp-roadmap.md,auto-update-system-prd.md,sparkle-setup-guide.md) have been included to detail the system's design, implementation roadmap, and setup instructions for developers and maintainers.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive auto-update system using the Sparkle framework. My review highlights a critical bug in the handling of release notes, and some areas for improvement in terms of script robustness, code clarity, and performance.
| /// Get release notes as attributed string if available | ||
| func getReleaseNotes() -> AttributedString? { | ||
| guard let releaseNotesURL = updateReleaseNotes, | ||
| let releaseNotesData = releaseNotesURL.data(using: .utf8) else { | ||
| return nil | ||
| } | ||
|
|
||
| // Simple plain text conversion - could be enhanced to support HTML/Markdown | ||
| let plainText = String(data: releaseNotesData, encoding: .utf8) ?? "Release notes unavailable" | ||
| return AttributedString(plainText) | ||
| } |
There was a problem hiding this comment.
updateReleaseNotes stores the URL of the release notes as a string, but this code converts the URL string itself into data, not the content at that URL. The UI will display the URL instead of the actual release notes. Perform a network request to fetch the content from the URL, making the function async and updating the call site in UpdateDetailsView to handle an asynchronous operation.
/// Get release notes as attributed string if available
func getReleaseNotes() async -> AttributedString? {
guard let releaseNotesURLString = updateReleaseNotes,
let url = URL(string: releaseNotesURLString) else {
return nil
}
do {
let (data, _) = try await URLSession.shared.data(from: url)
// The appcast generator creates HTML, so we should handle that.
// For now, simple text conversion is a good first step.
let plainText = String(data: data, encoding: .utf8) ?? "Release notes unavailable"
return AttributedString(plainText)
} catch {
print("Failed to download release notes: \(error.localizedDescription)")
return AttributedString("Failed to load release notes.")
}
}| /// Trigger the update installation process | ||
| func installUpdate() { | ||
| updaterController.updater.checkForUpdates() | ||
| } |
There was a problem hiding this comment.
The method installUpdate() initiates a new update check rather than installing an already downloaded update. Consider renaming the method to something like showUpdatePrompt() to more accurately reflect its behavior, and add a comment explaining why checkForUpdates() is called here.
| /// Trigger the update installation process | |
| func installUpdate() { | |
| updaterController.updater.checkForUpdates() | |
| } | |
| /// Triggers the Sparkle UI to show the update prompt by re-initiating a check. | |
| /// Sparkle will show its UI if a valid update is found. | |
| func showUpdatePrompt() { | |
| updaterController.updater.checkForUpdates() | |
| } |
| def update_appcast_with_signatures(appcast_path, asset_signatures): | ||
| """Update appcast XML with signature information.""" | ||
| try: | ||
| with open(appcast_path, 'r', encoding='utf-8') as f: | ||
| content = f.read() | ||
|
|
||
| # Add signatures to enclosure tags | ||
| for asset_name, signature in asset_signatures.items(): | ||
| if signature: | ||
| # Find the enclosure tag for this asset | ||
| lines = content.split('\n') | ||
| for i, line in enumerate(lines): | ||
| if f'url=' in line and asset_name in line and 'enclosure' in line: | ||
| # Add signature attribute to the enclosure tag | ||
| if 'sparkle:edSignature=' not in line: | ||
| line = line.rstrip(' />') | ||
| line += f' sparkle:edSignature="{signature}" />' | ||
| lines[i] = line | ||
| break | ||
|
|
||
| content = '\n'.join(lines) | ||
|
|
||
| # Write updated content back | ||
| with open(appcast_path, 'w', encoding='utf-8') as f: | ||
| f.write(content) | ||
|
|
||
| print(f"✅ Updated {appcast_path} with signatures") | ||
|
|
||
| except Exception as e: | ||
| print(f"Error updating appcast: {e}") |
There was a problem hiding this comment.
Modifying XML by splitting the content into lines and performing string replacements is fragile. Use a proper XML parsing library, like Python's built-in xml.etree.ElementTree, to robustly parse and modify the appcast file.
import xml.etree.ElementTree as ET
def update_appcast_with_signatures(appcast_path, asset_signatures):
"""Update appcast XML with signature information."""
try:
# Register namespaces to preserve them in the output
ET.register_namespace('sparkle', "http://www.andymatuschak.org/xml-namespaces/sparkle")
ET.register_namespace('dc', "http://purl.org/dc/elements/1.1/")
tree = ET.parse(appcast_path)
root = tree.getroot()
# Find all enclosure tags and add signatures
for enclosure in root.findall('.//enclosure'):
url = enclosure.get('url')
if not url:
continue
asset_name = Path(url).name
if asset_name in asset_signatures and asset_signatures[asset_name]:
signature = asset_signatures[asset_name]
enclosure.set('{http://www.andymatuschak.org/xml-namespaces/sparkle}edSignature', signature)
# Write updated content back
tree.write(appcast_path, encoding='utf-8', xml_declaration=True)
print(f"✅ Updated {appcast_path} with signatures")
except Exception as e:
print(f"Error updating appcast: {e}")| private static func parseDate(_ dateString: String?) -> Date? { | ||
| guard let dateString = dateString else { return nil } | ||
|
|
||
| let formatter = ISO8601DateFormatter() | ||
| formatter.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | ||
|
|
||
| return formatter.date(from: dateString) ?? ISO8601DateFormatter().date(from: dateString) | ||
| } |
There was a problem hiding this comment.
Creating ISO8601DateFormatter instances is computationally expensive. Consider defining the date formatters as static, lazily-initialized constants, similar to how RFC822DateFormatter is defined, to ensure they are created only once.
| #if DEBUG | ||
| // In development builds, only manual checking is enabled | ||
| if !AppConstants.DeveloperUpdateConfig.manualCheckOnly, | ||
| updaterManager.autoUpdateEnabled, | ||
| let timeSinceLastCheck = updaterManager.timeSinceLastCheck, | ||
| timeSinceLastCheck > AppConstants.updateCheckInterval { | ||
| updaterManager.checkForUpdates() | ||
| } | ||
| #else | ||
| // In production builds, use normal automatic checking | ||
| if updaterManager.autoUpdateEnabled, | ||
| let timeSinceLastCheck = updaterManager.timeSinceLastCheck, | ||
| timeSinceLastCheck > AppConstants.updateCheckInterval { | ||
| updaterManager.checkForUpdates() | ||
| } | ||
| #endif |
There was a problem hiding this comment.
The logic for checking for updates on launch is duplicated. Simplify to avoid repetition and improve readability by defining a single condition for checking and using the preprocessor directive only for the part that differs.
#if DEBUG
let canCheckAutomatically = !AppConstants.DeveloperUpdateConfig.manualCheckOnly
#else
let canCheckAutomatically = true
#endif
if canCheckAutomatically,
updaterManager.autoUpdateEnabled,
let timeSinceLastCheck = updaterManager.timeSinceLastCheck,
timeSinceLastCheck > AppConstants.updateCheckInterval {
updaterManager.checkForUpdates()
}| // Private initializer to prevent instantiation | ||
| private init() {} |
There was a problem hiding this comment.
This private init() is a duplicate. Remove this redundant one to avoid confusion.
Introduce an auto-update system using the Sparkle framework, enhancing user experience with update notifications and management. The implementation includes configuration for automatic checks, update intervals, and a user interface for manual update checks. Additionally, provide documentation for setup and key generation.