From 87239922d2cdbe6934defaa6e45ec069c10a5cf2 Mon Sep 17 00:00:00 2001 From: Jatinjit Singh Date: Thu, 29 Oct 2026 12:56:19 +0900 Subject: [PATCH 1/4] Fix P0 issues: vanishing bottles, runtime install, log growth, CPU spin Process output (Whisky-App/Whisky#1010, #1031) - Clear each pipe's readability handler at end of file. Foundation keeps calling it on a closed pipe, so every finished process left a reader spinning a full core for the life of the app. - Finish the stream once the process exited and both pipes closed instead of blocking the termination handler in readToEnd() while children such as a game started by `wine start` still hold the pipes. Logs (#1005, #1115, #1261) - Cap each process log at 100 MB with a single truncation notice. - Prune logs older than 7 days, then the oldest until the folder fits in 1 GB. Log names are unique so runs in the same second no longer share and truncate one file. WhiskyWine download and install (#801, #995, #1020, #1271, #1275) - Reject non-200 responses and move the download out of URLSession's temporary location before the completion handler returns. - Extract into a staging folder and replace Libraries only once the new runtime has a version file; tar failures are no longer ignored. - Show download and install failures with Retry instead of stalling or reporting success. - Updating no longer uninstalls the runtime before the new one downloads. Bottle creation (#359, #760, #798, #819, #1029, #1079, #1110, #1191, #1232, #1237, #1278, #1306, #1311, #1364) - Report why a bottle could not be created instead of silently removing it: WhiskyWine missing, a folder name ending in a period, or Wine not creating the prefix (with Wine's output). - Remove the half-created bottle folder on failure. - Keep bottles that are still being created across list reloads. - Run creation on the main actor rather than mutating published state from a detached task. - Set aside an unreadable bottle list instead of overwriting it with an empty one. Tests - Add a WhiskyKit test target covering log capping and pruning, download validation, staged installs, and process stream completion and idling. Run it in CI. --- .github/workflows/CI.yml | 3 + Whisky.xcodeproj/project.pbxproj | 4 + Whisky/Localizable.xcstrings | 70 ++++++++ Whisky/View Models/BottleVM.swift | 105 ++++++++---- Whisky/Views/ContentView.swift | 64 ++++--- Whisky/Views/Setup/SetupFailureView.swift | 42 +++++ Whisky/Views/Setup/SetupView.swift | 12 +- .../Views/Setup/WhiskyWineDownloadView.swift | 133 +++++++++------ .../Views/Setup/WhiskyWineInstallView.swift | 37 ++-- Whisky/Views/WhiskyApp.swift | 41 +---- WhiskyKit/Package.swift | 4 + .../Extensions/FileHandle+Extensions.swift | 60 ------- .../Extensions/Process+Extensions.swift | 91 +++++----- .../Sources/WhiskyKit/Logging/LogFile.swift | 137 +++++++++++++++ .../Sources/WhiskyKit/Logging/WineLogs.swift | 71 ++++++++ WhiskyKit/Sources/WhiskyKit/Tar.swift | 52 +++--- .../Sources/WhiskyKit/Utils/Rosetta2.swift | 30 +--- .../Sources/WhiskyKit/Whisky/BottleData.swift | 50 +++--- .../WhiskyWine/WhiskyWineInstaller.swift | 158 ++++++++++++------ WhiskyKit/Sources/WhiskyKit/Wine/Wine.swift | 53 ++---- .../Tests/WhiskyKitTests/LogFileTests.swift | 61 +++++++ .../WhiskyKitTests/ProcessStreamTests.swift | 87 ++++++++++ .../WhiskyKitTests/TemporaryFolder.swift | 41 +++++ .../WhiskyWineInstallerTests.swift | 133 +++++++++++++++ .../Tests/WhiskyKitTests/WineLogsTests.swift | 70 ++++++++ 25 files changed, 1201 insertions(+), 408 deletions(-) create mode 100644 Whisky/Views/Setup/SetupFailureView.swift create mode 100644 WhiskyKit/Sources/WhiskyKit/Logging/LogFile.swift create mode 100644 WhiskyKit/Sources/WhiskyKit/Logging/WineLogs.swift create mode 100644 WhiskyKit/Tests/WhiskyKitTests/LogFileTests.swift create mode 100644 WhiskyKit/Tests/WhiskyKitTests/ProcessStreamTests.swift create mode 100644 WhiskyKit/Tests/WhiskyKitTests/TemporaryFolder.swift create mode 100644 WhiskyKit/Tests/WhiskyKitTests/WhiskyWineInstallerTests.swift create mode 100644 WhiskyKit/Tests/WhiskyKitTests/WineLogsTests.swift diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 3e899dce9..3ffc5a6b3 100644 --- a/.github/workflows/CI.yml +++ b/.github/workflows/CI.yml @@ -43,6 +43,9 @@ jobs: path: .build/SourcePackages key: spm-${{ hashFiles('Whisky.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved', 'WhiskyKit/Package.swift') }} + - name: Test WhiskyKit + run: swift test --package-path WhiskyKit + - name: Build Whisky run: xcodebuild build -project Whisky.xcodeproj -scheme Whisky -configuration Release -clonedSourcePackagesDirPath .build/SourcePackages -disableAutomaticPackageResolution diff --git a/Whisky.xcodeproj/project.pbxproj b/Whisky.xcodeproj/project.pbxproj index 9a782462a..bac6cee2a 100644 --- a/Whisky.xcodeproj/project.pbxproj +++ b/Whisky.xcodeproj/project.pbxproj @@ -36,6 +36,7 @@ 6E5B61B62AC34E6D007B1913 /* SemanticVersion in Frameworks */ = {isa = PBXBuildFile; productRef = 6E5B61B52AC34E6D007B1913 /* SemanticVersion */; }; 6E621CEF2A5F631300C9AAB3 /* Winetricks.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E621CEE2A5F631200C9AAB3 /* Winetricks.swift */; }; 6E6C0CF22A419A6800356232 /* WelcomeView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E6C0CF12A419A6800356232 /* WelcomeView.swift */; }; + 6E7A1F012F00000000000001 /* SetupFailureView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E7A1F012F00000000000002 /* SetupFailureView.swift */; }; 6E6C0CF42A419A7600356232 /* RosettaView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E6C0CF32A419A7600356232 /* RosettaView.swift */; }; 6E6C0CF62A419A8300356232 /* WhiskyWineDownloadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E6C0CF52A419A8300356232 /* WhiskyWineDownloadView.swift */; }; 6E6C0CF82A419A8C00356232 /* WhiskyWineInstallView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6E6C0CF72A419A8C00356232 /* WhiskyWineInstallView.swift */; }; @@ -138,6 +139,7 @@ 6E50D98429CDF25B008C39F6 /* BottleCreationView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BottleCreationView.swift; sourceTree = ""; }; 6E621CEE2A5F631200C9AAB3 /* Winetricks.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Winetricks.swift; sourceTree = ""; }; 6E6C0CF12A419A6800356232 /* WelcomeView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WelcomeView.swift; sourceTree = ""; }; + 6E7A1F012F00000000000002 /* SetupFailureView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SetupFailureView.swift; sourceTree = ""; }; 6E6C0CF32A419A7600356232 /* RosettaView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RosettaView.swift; sourceTree = ""; }; 6E6C0CF52A419A8300356232 /* WhiskyWineDownloadView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhiskyWineDownloadView.swift; sourceTree = ""; }; 6E6C0CF72A419A8C00356232 /* WhiskyWineInstallView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WhiskyWineInstallView.swift; sourceTree = ""; }; @@ -368,6 +370,7 @@ children = ( 6EF557972A410599001A4F09 /* SetupView.swift */, 6E6C0CF12A419A6800356232 /* WelcomeView.swift */, + 6E7A1F012F00000000000002 /* SetupFailureView.swift */, 6E6C0CF32A419A7600356232 /* RosettaView.swift */, 6E6C0CF52A419A8300356232 /* WhiskyWineDownloadView.swift */, 6E6C0CF72A419A8C00356232 /* WhiskyWineInstallView.swift */, @@ -618,6 +621,7 @@ 6E49E0212AECB7DB00009CAC /* SettingsView.swift in Sources */, 6E7C07C02AAF570100F6E66B /* FileOpenView.swift in Sources */, 6E6C0CF22A419A6800356232 /* WelcomeView.swift in Sources */, + 6E7A1F012F00000000000001 /* SetupFailureView.swift in Sources */, 6E40498129CCA8B0006E3F1B /* BottleView.swift in Sources */, 6E355E5E29D7D85D002D83BE /* ProgramView.swift in Sources */, ); diff --git a/Whisky/Localizable.xcstrings b/Whisky/Localizable.xcstrings index e49e73b17..fe8583a20 100644 --- a/Whisky/Localizable.xcstrings +++ b/Whisky/Localizable.xcstrings @@ -8164,6 +8164,56 @@ } } }, + "create.error.message.%@.%@" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "“%1$@” could not be created. %2$@" + } + } + } + }, + "create.error.prefixNotCreated.%@" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Wine did not set up the bottle. Wine output: %@" + } + } + } + }, + "create.error.title" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Couldn't Create Bottle" + } + } + } + }, + "create.error.unsupportedLocation.%@" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Wine can't use folders whose names end with a period. Choose a different location than %@." + } + } + } + }, + "create.error.whiskyWineNotInstalled" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "WhiskyWine is not installed. Install it from Whisky > Setup, then try again." + } + } + } + }, "create.name" : { "localizations" : { "ar" : { @@ -17298,6 +17348,16 @@ } } }, + "setup.whiskywine.download.failed" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Download Failed" + } + } + } + }, "setup.whiskywine.download.subtitle" : { "localizations" : { "ar" : { @@ -17706,6 +17766,16 @@ } } }, + "setup.whiskywine.install.failed" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Installation Failed" + } + } + } + }, "setup.whiskywine.install.subtitle" : { "localizations" : { "ar" : { diff --git a/Whisky/View Models/BottleVM.swift b/Whisky/View Models/BottleVM.swift index 7cb864d9b..c15ed5777 100644 --- a/Whisky/View Models/BottleVM.swift +++ b/Whisky/View Models/BottleVM.swift @@ -17,9 +17,33 @@ // import Foundation +import os import SemanticVersion import WhiskyKit +enum BottleCreationError: LocalizedError { + case whiskyWineNotInstalled + case unsupportedLocation(URL) + case prefixNotCreated(wineOutput: String) + + var errorDescription: String? { + switch self { + case .whiskyWineNotInstalled: + return String(localized: "create.error.whiskyWineNotInstalled") + case .unsupportedLocation(let url): + return String(localized: "create.error.unsupportedLocation.\(url.prettyPath())") + case .prefixNotCreated(let wineOutput): + return String(localized: "create.error.prefixNotCreated.\(wineOutput.suffix(1000).description)") + } + } +} + +struct BottleCreationFailure: Identifiable { + let id = UUID() + let bottleName: String + let error: Error +} + // swiftlint:disable:next todo // TODO: Don't use unchecked! final class BottleVM: ObservableObject, @unchecked Sendable { @@ -27,52 +51,73 @@ final class BottleVM: ObservableObject, @unchecked Sendable { var bottlesList = BottleData() @Published var bottles: [Bottle] = [] + @Published var bottleCreationFailure: BottleCreationFailure? + /// Bottles still being created are not in the bottle list yet, so they are kept across reloads @MainActor func loadBottles() { - bottles = bottlesList.loadBottles() + let bottlesInCreation = bottles.filter { $0.inFlight && !bottlesList.paths.contains($0.url) } + bottles = bottlesList.loadBottles() + bottlesInCreation } func countActive() -> Int { return bottles.filter { $0.isAvailable == true }.count } + @MainActor func createNewBottle(bottleName: String, winVersion: WinVersion, bottleURL: URL) -> URL { let newBottleDir = bottleURL.appending(path: UUID().uuidString) - Task.detached { - var bottleId: Bottle? + Task { do { - try FileManager.default.createDirectory(atPath: newBottleDir.path(percentEncoded: false), - withIntermediateDirectories: true) - let bottle = Bottle(bottleUrl: newBottleDir, inFlight: true) - bottleId = bottle - - await MainActor.run { - self.bottles.append(bottle) - } - - bottle.settings.windowsVersion = winVersion - bottle.settings.name = bottleName - try await Wine.changeWinVersion(bottle: bottle, win: winVersion) - let wineVer = try await Wine.wineVersion() - bottle.settings.wineVersion = SemanticVersion(wineVer) ?? SemanticVersion(0, 0, 0) - // Add record - await MainActor.run { - self.bottlesList.paths.append(newBottleDir) - self.loadBottles() - } + try await setUpBottle(at: newBottleDir, name: bottleName, winVersion: winVersion) } catch { - print("Failed to create new bottle: \(error)") - if let bottle = bottleId { - await MainActor.run { - if let index = self.bottles.firstIndex(of: bottle) { - self.bottles.remove(at: index) - } - } - } + Logger.wineKit.error("Failed to create bottle `\(bottleName)`: \(error)") + discardBottle(at: newBottleDir) + bottleCreationFailure = BottleCreationFailure(bottleName: bottleName, error: error) } } return newBottleDir } + + @MainActor + private func setUpBottle(at bottleDir: URL, name: String, winVersion: WinVersion) async throws { + guard WhiskyWineInstaller.isWhiskyWineInstalled() else { + throw BottleCreationError.whiskyWineNotInstalled + } + // Wine drops trailing dots when mapping Unix paths to Windows ones, so the prefix cannot load + guard !bottleDir.pathComponents.contains(where: { $0.hasSuffix(".") }) else { + throw BottleCreationError.unsupportedLocation(bottleDir) + } + + try FileManager.default.createDirectory(at: bottleDir, withIntermediateDirectories: true) + let bottle = Bottle(bottleUrl: bottleDir, inFlight: true) + bottles.append(bottle) + + bottle.settings.windowsVersion = winVersion + bottle.settings.name = name + let wineOutput = try await Wine.changeWinVersion(bottle: bottle, win: winVersion) + guard FileManager.default.fileExists(atPath: bottleDir.appending(path: "drive_c").path(percentEncoded: false)) + else { + throw BottleCreationError.prefixNotCreated(wineOutput: wineOutput) + } + + let wineVer = try await Wine.wineVersion() + bottle.settings.wineVersion = SemanticVersion(wineVer) ?? SemanticVersion(0, 0, 0) + bottlesList.paths.append(bottleDir) + loadBottles() + } + + @MainActor + private func discardBottle(at bottleDir: URL) { + bottles.removeAll { $0.url == bottleDir } + guard FileManager.default.fileExists(atPath: bottleDir.path(percentEncoded: false)) else { return } + do { + try FileManager.default.removeItem(at: bottleDir) + } catch { + Logger.wineKit.error( + "Failed to remove incomplete bottle `\(bottleDir.path(percentEncoded: false))`: \(error)" + ) + } + } } diff --git a/Whisky/Views/ContentView.swift b/Whisky/Views/ContentView.swift index c15e5c733..cc3960a0a 100644 --- a/Whisky/Views/ContentView.swift +++ b/Whisky/Views/ContentView.swift @@ -20,6 +20,7 @@ import SwiftUI import UniformTypeIdentifiers import WhiskyKit import SemanticVersion +import os struct ContentView: View { @AppStorage("selectedBottleURL") private var selectedBottleURL: URL? @@ -28,6 +29,7 @@ struct ContentView: View { @State private var selected: URL? @State private var showBottleCreation: Bool = false + @State private var showWhiskyWineUpdate: Bool = false @State private var bottlesLoaded: Bool = false @State private var showBottleSelection: Bool = false @State private var newlyCreatedBottleURL: URL? @@ -77,6 +79,17 @@ struct ContentView: View { .sheet(isPresented: $showSetup) { SetupView(showSetup: $showSetup, firstTime: false) } + .sheet(isPresented: $showWhiskyWineUpdate) { + SetupView(showSetup: $showWhiskyWineUpdate, firstTime: false, initialPath: [.whiskyWineDownload]) + } + .alert( + "create.error.title", + isPresented: isShowingBottleCreationFailure, + presenting: bottleVM.bottleCreationFailure + ) { _ in + } message: { failure in + Text("create.error.message.\(failure.bottleName).\(failure.error.localizedDescription)") + } .sheet(item: $openedFileURL) { url in FileOpenView(fileURL: url, currentBottle: selected, @@ -104,28 +117,39 @@ struct ContentView: View { if !WhiskyWineInstaller.isWhiskyWineInstalled() { showSetup = true } - let task = Task.detached { - return await WhiskyWineInstaller.shouldUpdateWhiskyWine() - } - let updateInfo = await task.value - if updateInfo.0 { - let alert = NSAlert() - alert.messageText = String(localized: "update.whiskywine.title") - alert.informativeText = String(format: String(localized: "update.whiskywine.description"), - String(WhiskyWineInstaller.whiskyWineVersion() - ?? SemanticVersion(0, 0, 0)), - String(updateInfo.1)) - alert.alertStyle = .warning - alert.addButton(withTitle: String(localized: "update.whiskywine.update")) - alert.addButton(withTitle: String(localized: "button.removeAlert.cancel")) + await offerWhiskyWineUpdate() + } + } - let response = alert.runModal() + private var isShowingBottleCreationFailure: Binding { + Binding( + get: { bottleVM.bottleCreationFailure != nil }, + set: { if !$0 { bottleVM.bottleCreationFailure = nil } } + ) + } - if response == .alertFirstButtonReturn { - WhiskyWineInstaller.uninstall() - showSetup = true - } - } + private func offerWhiskyWineUpdate() async { + let remoteVersion: SemanticVersion? + do { + remoteVersion = try await WhiskyWineInstaller.availableUpdate() + } catch { + Logger.wineKit.error("Failed to check for a WhiskyWine update: \(error)") + return + } + guard let remoteVersion else { return } + + let alert = NSAlert() + alert.messageText = String(localized: "update.whiskywine.title") + alert.informativeText = String(format: String(localized: "update.whiskywine.description"), + String(WhiskyWineInstaller.whiskyWineVersion() + ?? SemanticVersion(0, 0, 0)), + String(remoteVersion)) + alert.alertStyle = .warning + alert.addButton(withTitle: String(localized: "update.whiskywine.update")) + alert.addButton(withTitle: String(localized: "button.removeAlert.cancel")) + + if alert.runModal() == .alertFirstButtonReturn { + showWhiskyWineUpdate = true } } diff --git a/Whisky/Views/Setup/SetupFailureView.swift b/Whisky/Views/Setup/SetupFailureView.swift new file mode 100644 index 000000000..642cc72de --- /dev/null +++ b/Whisky/Views/Setup/SetupFailureView.swift @@ -0,0 +1,42 @@ +// +// SetupFailureView.swift +// Whisky +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import SwiftUI + +struct SetupFailureView: View { + let title: LocalizedStringKey + let message: String + let retry: () -> Void + + var body: some View { + VStack(spacing: 8) { + Label(title, systemImage: "xmark.octagon.fill") + .foregroundStyle(.red) + .font(.headline) + Text(message) + .font(.subheadline) + .foregroundStyle(.secondary) + .multilineTextAlignment(.center) + .lineLimit(3) + .textSelection(.enabled) + Button("setup.retry", action: retry) + .keyboardShortcut(.defaultAction) + } + .padding(.horizontal) + } +} diff --git a/Whisky/Views/Setup/SetupView.swift b/Whisky/Views/Setup/SetupView.swift index 213854fc5..60bc87188 100644 --- a/Whisky/Views/Setup/SetupView.swift +++ b/Whisky/Views/Setup/SetupView.swift @@ -25,10 +25,16 @@ enum SetupStage { } struct SetupView: View { - @State private var path: [SetupStage] = [] - @State var tarLocation: URL = URL(fileURLWithPath: "") + @State private var path: [SetupStage] + @State private var tarLocation: URL = URL(fileURLWithPath: "") @Binding var showSetup: Bool - var firstTime: Bool = true + var firstTime: Bool + + init(showSetup: Binding, firstTime: Bool = true, initialPath: [SetupStage] = []) { + _showSetup = showSetup + self.firstTime = firstTime + _path = State(initialValue: initialPath) + } var body: some View { VStack { diff --git a/Whisky/Views/Setup/WhiskyWineDownloadView.swift b/Whisky/Views/Setup/WhiskyWineDownloadView.swift index c3a60ab51..be380e375 100644 --- a/Whisky/Views/Setup/WhiskyWineDownloadView.swift +++ b/Whisky/Views/Setup/WhiskyWineDownloadView.swift @@ -27,6 +27,7 @@ struct WhiskyWineDownloadView: View { @State private var downloadTask: URLSessionDownloadTask? @State private var observation: NSKeyValueObservation? @State private var startTime: Date? + @State private var failureMessage: String? @Binding var tarLocation: URL @Binding var path: [SetupStage] var body: some View { @@ -39,61 +40,97 @@ struct WhiskyWineDownloadView: View { .font(.subheadline) .foregroundStyle(.secondary) Spacer() - VStack { - ProgressView(value: fractionProgress, total: 1) - HStack { - HStack { - Text(String(format: String(localized: "setup.whiskywine.progress"), - formatBytes(bytes: completedBytes), - formatBytes(bytes: totalBytes))) - + Text(String(" ")) - + (shouldShowEstimate() ? - Text(String(format: String(localized: "setup.whiskywine.eta"), - formatRemainingTime(remainingBytes: totalBytes - completedBytes))) - : Text(String())) - Spacer() - } - .font(.subheadline) - .monospacedDigit() + if let failureMessage { + SetupFailureView(title: "setup.whiskywine.download.failed", message: failureMessage) { + startDownload() } + } else { + progress } - .padding(.horizontal) Spacer() } Spacer() } .frame(width: 400, height: 200) .onAppear { - Task { - if let url: URL = URL(string: "https://data.getwhisky.app/Wine/Libraries.tar.gz") { - downloadTask = URLSession(configuration: .ephemeral).downloadTask(with: url) { url, _, _ in - Task.detached { - await MainActor.run { - if let url = url { - tarLocation = url - proceed() - } - } - } - } - observation = downloadTask?.observe(\.countOfBytesReceived) { task, _ in - Task { - await MainActor.run { - let currentTime = Date() - let elapsedTime = currentTime.timeIntervalSince(startTime ?? currentTime) - if completedBytes > 0 { - downloadSpeed = Double(completedBytes) / elapsedTime - } - totalBytes = task.countOfBytesExpectedToReceive - completedBytes = task.countOfBytesReceived - fractionProgress = Double(completedBytes) / Double(totalBytes) - } - } - } - startTime = Date() - downloadTask?.resume() + startDownload() + } + .onDisappear { + observation?.invalidate() + downloadTask?.cancel() + } + } + + private var progress: some View { + VStack { + ProgressView(value: fractionProgress, total: 1) + HStack { + HStack { + Text(String(format: String(localized: "setup.whiskywine.progress"), + formatBytes(bytes: completedBytes), + formatBytes(bytes: totalBytes))) + + Text(String(" ")) + + (shouldShowEstimate() ? + Text(String(format: String(localized: "setup.whiskywine.eta"), + formatRemainingTime(remainingBytes: totalBytes - completedBytes))) + : Text(String())) + Spacer() + } + .font(.subheadline) + .monospacedDigit() + } + } + .padding(.horizontal) + } + + private func startDownload() { + failureMessage = nil + fractionProgress = 0 + completedBytes = 0 + totalBytes = 0 + downloadSpeed = 0 + + let task = URLSession(configuration: .ephemeral) + .downloadTask(with: WhiskyWineInstaller.librariesArchiveURL) { location, response, error in + let result = Result { + try WhiskyWineInstaller.acceptDownload(at: location, response: response, error: error) + } + Task { @MainActor in + finishDownload(result) } } + observation = task.observe(\.countOfBytesReceived) { task, _ in + let received = task.countOfBytesReceived + let expected = task.countOfBytesExpectedToReceive + Task { @MainActor in + updateProgress(received: received, expected: expected) + } + } + downloadTask = task + startTime = Date() + task.resume() + } + + private func updateProgress(received: Int64, expected: Int64) { + let elapsedTime = Date().timeIntervalSince(startTime ?? Date()) + if received > 0 && elapsedTime > 0 { + downloadSpeed = Double(received) / elapsedTime + } + completedBytes = received + totalBytes = max(expected, 0) + fractionProgress = totalBytes > 0 ? Double(completedBytes) / Double(totalBytes) : 0 + } + + private func finishDownload(_ result: Result) { + observation?.invalidate() + switch result { + case .success(let archive): + tarLocation = archive + path.append(.whiskyWineInstall) + case .failure(let error as URLError) where error.code == .cancelled: + break + case .failure(let error): + failureMessage = error.localizedDescription } } @@ -106,7 +143,7 @@ struct WhiskyWineDownloadView: View { func shouldShowEstimate() -> Bool { let elapsedTime = Date().timeIntervalSince(startTime ?? Date()) - return Int(elapsedTime.rounded()) > 5 && completedBytes != 0 + return Int(elapsedTime.rounded()) > 5 && completedBytes != 0 && totalBytes > 0 && downloadSpeed > 0 } func formatRemainingTime(remainingBytes: Int64) -> String { @@ -121,8 +158,4 @@ struct WhiskyWineDownloadView: View { return "" } } - - func proceed() { - path.append(.whiskyWineInstall) - } } diff --git a/Whisky/Views/Setup/WhiskyWineInstallView.swift b/Whisky/Views/Setup/WhiskyWineInstallView.swift index fde12e9a3..2b7d05593 100644 --- a/Whisky/Views/Setup/WhiskyWineInstallView.swift +++ b/Whisky/Views/Setup/WhiskyWineInstallView.swift @@ -20,7 +20,8 @@ import SwiftUI import WhiskyKit struct WhiskyWineInstallView: View { - @State var installing: Bool = true + @State private var installing: Bool = true + @State private var failureMessage: String? @Binding var tarLocation: URL @Binding var path: [SetupStage] @Binding var showSetup: Bool @@ -35,7 +36,11 @@ struct WhiskyWineInstallView: View { .font(.subheadline) .foregroundStyle(.secondary) Spacer() - if installing { + if let failureMessage { + SetupFailureView(title: "setup.whiskywine.install.failed", message: failureMessage) { + path.removeLast() + } + } else if installing { ProgressView() .progressViewStyle(.circular) .frame(width: 80) @@ -50,20 +55,24 @@ struct WhiskyWineInstallView: View { Spacer() } .frame(width: 400, height: 200) - .onAppear { - Task.detached { - await WhiskyWineInstaller.install(from: tarLocation) - await MainActor.run { - installing = false - } - sleep(2) - await proceed() - } + .task { + await install() } } - @MainActor - func proceed() { - showSetup = false + private func install() async { + let archive = tarLocation + do { + try await Task.detached { + try WhiskyWineInstaller.install(from: archive) + }.value + installing = false + try await Task.sleep(for: .seconds(2)) + showSetup = false + } catch is CancellationError { + return + } catch { + failureMessage = error.localizedDescription + } } } diff --git a/Whisky/Views/WhiskyApp.swift b/Whisky/Views/WhiskyApp.swift index ec5979ceb..ef9dc302d 100644 --- a/Whisky/Views/WhiskyApp.swift +++ b/Whisky/Views/WhiskyApp.swift @@ -19,6 +19,7 @@ import SwiftUI import Sparkle import WhiskyKit +import os @main struct WhiskyApp: App { @@ -56,7 +57,11 @@ struct WhiskyApp: App { NSWindow.allowsAutomaticWindowTabbing = false Task.detached { - await WhiskyApp.deleteOldLogs() + do { + try WineLogs.prune() + } catch { + Logger.wineKit.error("Failed to prune logs: \(error)") + } } } } @@ -144,39 +149,7 @@ struct WhiskyApp: App { } static func openLogsFolder() { - NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: Wine.logsFolder.path) - } - - static func deleteOldLogs() { - let pastDate = Date().addingTimeInterval(-7 * 24 * 60 * 60) - - guard let urls = try? FileManager.default.contentsOfDirectory( - at: Wine.logsFolder, - includingPropertiesForKeys: [.creationDateKey]) else { - return - } - - let logs = urls.filter { url in - url.pathExtension == "log" - } - - let oldLogs = logs.filter { url in - do { - let resourceValues = try url.resourceValues(forKeys: [.creationDateKey]) - - return resourceValues.creationDate ?? Date() < pastDate - } catch { - return false - } - } - - for log in oldLogs { - do { - try FileManager.default.removeItem(at: log) - } catch { - print("Failed to delete log: \(error)") - } - } + NSWorkspace.shared.selectFile(nil, inFileViewerRootedAtPath: WineLogs.folder.path) } static func wipeShaderCaches() { diff --git a/WhiskyKit/Package.swift b/WhiskyKit/Package.swift index b6f8d36cd..2bb362e59 100644 --- a/WhiskyKit/Package.swift +++ b/WhiskyKit/Package.swift @@ -37,6 +37,10 @@ let package = Package( .target( name: "WhiskyKit", dependencies: ["SemanticVersion"] + ), + .testTarget( + name: "WhiskyKitTests", + dependencies: ["WhiskyKit"] ) ], swiftLanguageModes: [.v6] diff --git a/WhiskyKit/Sources/WhiskyKit/Extensions/FileHandle+Extensions.swift b/WhiskyKit/Sources/WhiskyKit/Extensions/FileHandle+Extensions.swift index 589b6b62f..e14a932f0 100644 --- a/WhiskyKit/Sources/WhiskyKit/Extensions/FileHandle+Extensions.swift +++ b/WhiskyKit/Sources/WhiskyKit/Extensions/FileHandle+Extensions.swift @@ -17,8 +17,6 @@ // import Foundation -import os.log -import SemanticVersion extension FileHandle { func extract(_ type: T.Type, offset: UInt64 = 0) -> T? { @@ -33,62 +31,4 @@ extension FileHandle { return nil } } - - func write(line: String) { - do { - guard let data = line.data(using: .utf8) else { return } - try write(contentsOf: data) - } catch { - Logger.wineKit.info("Failed to write line: \(error)") - } - } - - // swiftlint:disable line_length - func writeApplicaitonInfo() { - var header = String() - let macOSVersion = ProcessInfo.processInfo.operatingSystemVersion - - header += "Whisky Version: \(Bundle.main.infoDictionary?["CFBundleShortVersionString"] ?? "")\n" - header += "Date: \(ISO8601DateFormatter().string(from: Date.now))\n" - header += "macOS Version: \(macOSVersion.majorVersion).\(macOSVersion.minorVersion).\(macOSVersion.patchVersion)\n\n" - write(line: header) - } - // swiftlint:enable line_length - - func writeInfo(for process: Process) { - var header = String() - - if let arguments = process.arguments { - header += "Arguments: \(arguments.joined(separator: " "))\n\n" - } - - if let environment = process.environment, !environment.isEmpty { - header += "Environment:\n\(environment as AnyObject)\n\n" - } - - write(line: header) - } - - func writeInfo(for bottle: Bottle) { - var header = String() - header += "Bottle Name: \(bottle.settings.name)\n" - header += "Bottle URL: \(bottle.url.path)\n\n" - - if let version = WhiskyWineInstaller.whiskyWineVersion() { - header += "WhiskyWine Version: \(version.major).\(version.minor).\(version.patch)\n" - } - header += "Windows Version: \(bottle.settings.windowsVersion)\n" - header += "Enhanced Sync: \(bottle.settings.enhancedSync)\n\n" - - header += "Metal HUD: \(bottle.settings.metalHud)\n" - header += "Metal Trace: \(bottle.settings.metalTrace)\n\n" - - if bottle.settings.dxvk { - header += "DXVK: \(bottle.settings.dxvk)\n" - header += "DXVK Async: \(bottle.settings.dxvkAsync)\n" - header += "DXVK HUD: \(bottle.settings.dxvkHud)\n\n" - } - - write(line: header) - } } diff --git a/WhiskyKit/Sources/WhiskyKit/Extensions/Process+Extensions.swift b/WhiskyKit/Sources/WhiskyKit/Extensions/Process+Extensions.swift index 31a0947b5..cb9d0d732 100644 --- a/WhiskyKit/Sources/WhiskyKit/Extensions/Process+Extensions.swift +++ b/WhiskyKit/Sources/WhiskyKit/Extensions/Process+Extensions.swift @@ -17,7 +17,7 @@ // import Foundation -import os.log +import os public enum ProcessOutput: Hashable { case started(Process) @@ -28,21 +28,34 @@ public enum ProcessOutput: Hashable { public extension Process { /// Run the process returning a stream output - func runStream(name: String, fileHandle: FileHandle?) throws -> AsyncStream { - let stream = makeStream(name: name, fileHandle: fileHandle) + func runStream(name: String, logFile: LogFile?) throws -> AsyncStream { + let stream = makeStream(name: name, logFile: logFile) self.logProcessInfo(name: name) - fileHandle?.writeInfo(for: self) + logFile?.writeInfo(for: self) try run() return stream } - private func makeStream(name: String, fileHandle: FileHandle?) -> AsyncStream { - let pipe = Pipe() + /// The stream finishes once the process has exited and both pipes reached end of file, + /// because children (e.g. a game started by `wine start`) keep writing after the parent exits + private func makeStream(name: String, logFile: LogFile?) -> AsyncStream { + let outputPipe = Pipe() let errorPipe = Pipe() - standardOutput = pipe + standardOutput = outputPipe standardError = errorPipe return AsyncStream { continuation in + let openSources = OSAllocatedUnfairLock(initialState: 3) + let closeSource: @Sendable () -> Void = { + let remaining = openSources.withLock { count in + count -= 1 + return count + } + guard remaining == 0 else { return } + logFile?.close() + continuation.finish() + } + continuation.onTermination = { termination in switch termination { case .finished: @@ -57,38 +70,42 @@ public extension Process { continuation.yield(.started(self)) - pipe.fileHandleForReading.readabilityHandler = { pipe in - guard let line = pipe.nextLine() else { return } - continuation.yield(.message(line)) - guard !line.isEmpty else { return } - Logger.wineKit.info("\(line, privacy: .public)") - fileHandle?.write(line: line) - } - - errorPipe.fileHandleForReading.readabilityHandler = { pipe in - guard let line = pipe.nextLine() else { return } - continuation.yield(.error(line)) - guard !line.isEmpty else { return } - Logger.wineKit.warning("\(line, privacy: .public)") - fileHandle?.write(line: line) - } + Self.forward(outputPipe, isError: false, logFile: logFile, continuation: continuation, + onEndOfFile: closeSource) + Self.forward(errorPipe, isError: true, logFile: logFile, continuation: continuation, + onEndOfFile: closeSource) terminationHandler = { (process: Process) in - do { - _ = try pipe.fileHandleForReading.readToEnd() - _ = try errorPipe.fileHandleForReading.readToEnd() - try fileHandle?.close() - } catch { - Logger.wineKit.error("Error while clearing data: \(error)") - } - process.logTermination(name: name) continuation.yield(.terminated(process)) - continuation.finish() + closeSource() } } } + private static func forward( + _ pipe: Pipe, isError: Bool, logFile: LogFile?, + continuation: AsyncStream.Continuation, onEndOfFile: @escaping @Sendable () -> Void + ) { + pipe.fileHandleForReading.readabilityHandler = { handle in + guard let text = handle.readAvailableText() else { + // The handler keeps firing at end of file until it is removed, spinning a core + handle.readabilityHandler = nil + onEndOfFile() + return + } + + if isError { + continuation.yield(.error(text)) + Logger.wineKit.warning("\(text, privacy: .public)") + } else { + continuation.yield(.message(text)) + Logger.wineKit.info("\(text, privacy: .public)") + } + logFile?.write(text) + } + } + private func logTermination(name: String) { if terminationStatus == 0 { Logger.wineKit.info( @@ -120,12 +137,10 @@ public extension Process { } extension FileHandle { - func nextLine() -> String? { - guard let line = String(data: availableData, encoding: .utf8) else { return nil } - if !line.isEmpty { - return line - } else { - return nil - } + /// The text currently available, or `nil` at end of file + func readAvailableText() -> String? { + let data = availableData + guard !data.isEmpty else { return nil } + return String(bytes: data, encoding: .utf8) ?? "[\(data.count) bytes of non-UTF-8 output]\n" } } diff --git a/WhiskyKit/Sources/WhiskyKit/Logging/LogFile.swift b/WhiskyKit/Sources/WhiskyKit/Logging/LogFile.swift new file mode 100644 index 000000000..1dd8bf7cc --- /dev/null +++ b/WhiskyKit/Sources/WhiskyKit/Logging/LogFile.swift @@ -0,0 +1,137 @@ +// +// LogFile.swift +// WhiskyKit +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation +import os + +/// A log file for one process run that stops growing once it reaches `maxSize` +public final class LogFile: Sendable { + public static let defaultMaxSize = 100 * 1024 * 1024 + + private struct State { + var bytesWritten = 0 + var isAcceptingWrites = true + var isClosed = false + } + + public let url: URL + private let maxSize: Int + private let handle: FileHandle + private let state = OSAllocatedUnfairLock(initialState: State()) + + init(url: URL, maxSize: Int = defaultMaxSize) throws { + guard FileManager.default.createFile(atPath: url.path(percentEncoded: false), contents: nil) else { + throw CocoaError(.fileWriteUnknown, userInfo: [NSURLErrorKey: url]) + } + self.url = url + self.maxSize = maxSize + self.handle = try FileHandle(forWritingTo: url) + } + + public func write(_ text: String) { + state.withLock { state in + guard state.isAcceptingWrites else { return } + let data = Data(text.utf8) + let remaining = maxSize - state.bytesWritten + + do { + if data.count <= remaining { + try handle.write(contentsOf: data) + state.bytesWritten += data.count + } else { + try handle.write(contentsOf: data.prefix(remaining)) + try handle.write(contentsOf: Data(truncationNotice.utf8)) + state.bytesWritten = maxSize + state.isAcceptingWrites = false + } + } catch { + state.isAcceptingWrites = false + Logger.wineKit.error("Stopped writing log `\(self.url.lastPathComponent)`: \(error)") + } + } + } + + public func close() { + state.withLock { state in + guard !state.isClosed else { return } + state.isClosed = true + state.isAcceptingWrites = false + do { + try handle.close() + } catch { + Logger.wineKit.error("Failed to close log `\(self.url.lastPathComponent)`: \(error)") + } + } + } + + private var truncationNotice: String { + let limit = ByteCountFormatter.string(fromByteCount: Int64(maxSize), countStyle: .file) + return "\n\n[Whisky] Log truncated after reaching \(limit); further output is not recorded.\n" + } +} + +public extension LogFile { + // swiftlint:disable line_length + func writeApplicationInfo() { + var header = String() + let macOSVersion = ProcessInfo.processInfo.operatingSystemVersion + + header += "Whisky Version: \(Bundle.main.infoDictionary?["CFBundleShortVersionString"] ?? "")\n" + header += "Date: \(ISO8601DateFormatter().string(from: Date.now))\n" + header += "macOS Version: \(macOSVersion.majorVersion).\(macOSVersion.minorVersion).\(macOSVersion.patchVersion)\n\n" + write(header) + } + // swiftlint:enable line_length + + func writeInfo(for process: Process) { + var header = String() + + if let arguments = process.arguments { + header += "Arguments: \(arguments.joined(separator: " "))\n\n" + } + + if let environment = process.environment, !environment.isEmpty { + header += "Environment:\n\(environment as AnyObject)\n\n" + } + + write(header) + } + + func writeInfo(for bottle: Bottle) { + var header = String() + header += "Bottle Name: \(bottle.settings.name)\n" + header += "Bottle URL: \(bottle.url.path)\n\n" + + if let version = WhiskyWineInstaller.whiskyWineVersion() { + header += "WhiskyWine Version: \(version.major).\(version.minor).\(version.patch)\n" + } + header += "Windows Version: \(bottle.settings.windowsVersion)\n" + header += "Enhanced Sync: \(bottle.settings.enhancedSync)\n\n" + + header += "Metal HUD: \(bottle.settings.metalHud)\n" + header += "Metal Trace: \(bottle.settings.metalTrace)\n\n" + + if bottle.settings.dxvk { + header += "DXVK: \(bottle.settings.dxvk)\n" + header += "DXVK Async: \(bottle.settings.dxvkAsync)\n" + header += "DXVK HUD: \(bottle.settings.dxvkHud)\n\n" + } + + write(header) + } +} diff --git a/WhiskyKit/Sources/WhiskyKit/Logging/WineLogs.swift b/WhiskyKit/Sources/WhiskyKit/Logging/WineLogs.swift new file mode 100644 index 000000000..0c061a0df --- /dev/null +++ b/WhiskyKit/Sources/WhiskyKit/Logging/WineLogs.swift @@ -0,0 +1,71 @@ +// +// WineLogs.swift +// WhiskyKit +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation + +/// The folder holding one log file per process run, kept within an age and total size budget +public enum WineLogs { + public static let folder = FileManager.default.urls( + for: .libraryDirectory, in: .userDomainMask + )[0].appending(path: "Logs").appending(path: Bundle.whiskyBundleIdentifier) + + public static let maxAge: TimeInterval = 7 * 24 * 60 * 60 + public static let maxTotalSize = 1024 * 1024 * 1024 + + private static let fileExtension = "log" + + public static func makeLogFile() throws -> LogFile { + try makeLogFile(in: folder) + } + + static func makeLogFile(in folder: URL, maxSize: Int = LogFile.defaultMaxSize) throws -> LogFile { + try FileManager.default.createDirectory(at: folder, withIntermediateDirectories: true) + + // Processes started within the same second must not share (and truncate) a file + let name = "\(Date.now.ISO8601Format())-\(UUID().uuidString.prefix(8))" + return try LogFile(url: folder.appending(path: name).appendingPathExtension(fileExtension), maxSize: maxSize) + } + + /// Deletes logs older than `maxAge`, then the oldest remaining logs until the folder fits in `maxTotalSize` + public static func prune() throws { + try prune(in: folder, maxAge: maxAge, maxTotalSize: maxTotalSize, now: .now) + } + + static func prune(in folder: URL, maxAge: TimeInterval, maxTotalSize: Int, now: Date) throws { + guard FileManager.default.fileExists(atPath: folder.path(percentEncoded: false)) else { return } + + let keys: Set = [.creationDateKey, .fileSizeKey] + let logs = try FileManager.default.contentsOfDirectory(at: folder, includingPropertiesForKeys: Array(keys)) + .filter { $0.pathExtension == fileExtension } + .map { url in + let values = try url.resourceValues(forKeys: keys) + return (url: url, created: values.creationDate ?? now, size: values.fileSize ?? 0) + } + .sorted { $0.created > $1.created } + + let oldestAllowed = now.addingTimeInterval(-maxAge) + var keptSize = 0 + for log in logs { + if log.created >= oldestAllowed && keptSize + log.size <= maxTotalSize { + keptSize += log.size + } else { + try FileManager.default.removeItem(at: log.url) + } + } + } +} diff --git a/WhiskyKit/Sources/WhiskyKit/Tar.swift b/WhiskyKit/Sources/WhiskyKit/Tar.swift index 8afc82a6f..105595103 100644 --- a/WhiskyKit/Sources/WhiskyKit/Tar.swift +++ b/WhiskyKit/Sources/WhiskyKit/Tar.swift @@ -18,50 +18,44 @@ import Foundation -public class Tar { - static let tarBinary: URL = URL(fileURLWithPath: "/usr/bin/tar") - - public static func tar(folder: URL, toURL: URL) throws { - let process = Process() - let pipe = Pipe() +public enum TarError: LocalizedError { + case failed(status: Int32, output: String) - process.executableURL = tarBinary - process.arguments = ["-zcf", "\(toURL.path)", "\(folder.path)"] - process.standardOutput = pipe - process.standardError = pipe + public var errorDescription: String? { + switch self { + case .failed(let status, let output): + return "tar exited with status \(status): \(output)" + } + } +} - try process.run() +public enum Tar { + static let tarBinary: URL = URL(fileURLWithPath: "/usr/bin/tar") - if let output = try pipe.fileHandleForReading.readToEnd() { - let outputString = String(data: output, encoding: .utf8) ?? String() - process.waitUntilExit() - let status = process.terminationStatus - if status != 0 { - throw outputString - } - } + public static func tar(folder: URL, toURL: URL) throws { + try run(arguments: ["-zcf", toURL.path, folder.path]) } public static func untar(tarBall: URL, toURL: URL) throws { + try run(arguments: ["-xzf", tarBall.path, "-C", toURL.path]) + } + + private static func run(arguments: [String]) throws { let process = Process() let pipe = Pipe() process.executableURL = tarBinary - process.arguments = ["-xzf", "\(tarBall.path)", "-C", "\(toURL.path)"] + process.arguments = arguments process.standardOutput = pipe process.standardError = pipe try process.run() + let output = try pipe.fileHandleForReading.readToEnd() ?? Data() + process.waitUntilExit() - if let output = try pipe.fileHandleForReading.readToEnd() { - let outputString = String(data: output, encoding: .utf8) ?? String() - process.waitUntilExit() - let status = process.terminationStatus - if status != 0 { - throw outputString - } + guard process.terminationStatus == 0 else { + let message = String(bytes: output, encoding: .utf8) ?? "\(output.count) bytes of non-UTF-8 output" + throw TarError.failed(status: process.terminationStatus, output: message) } } } - -extension String: @retroactive Error {} diff --git a/WhiskyKit/Sources/WhiskyKit/Utils/Rosetta2.swift b/WhiskyKit/Sources/WhiskyKit/Utils/Rosetta2.swift index feff48723..b74e8e37b 100644 --- a/WhiskyKit/Sources/WhiskyKit/Utils/Rosetta2.swift +++ b/WhiskyKit/Sources/WhiskyKit/Utils/Rosetta2.swift @@ -17,7 +17,6 @@ // import Foundation -import os.log public class Rosetta2 { private static let rosetta2RuntimeBin = "/Library/Apple/usr/libexec/oah/libRosettaRuntime" @@ -28,31 +27,18 @@ public class Rosetta2 { public static func installRosetta() async throws -> Bool { let process = Process() - let fileHandle = try Wine.makeFileHandle() - - process.launchPath = "/usr/sbin/softwareupdate" + process.executableURL = URL(filePath: "/usr/sbin/softwareupdate") process.arguments = ["--install-rosetta", "--agree-to-license"] - process.standardOutput = fileHandle - process.standardError = fileHandle - fileHandle.writeApplicaitonInfo() - fileHandle.writeInfo(for: process) - return try await withCheckedThrowingContinuation { continuation in - process.terminationHandler = { (process: Process) in - do { - try fileHandle.close() - continuation.resume(returning: process.terminationStatus == 0) - } catch { - Logger.wineKit.error("Error while closing file handle: \(error)") - continuation.resume(throwing: error) - } - } + let logFile = try WineLogs.makeLogFile() + logFile.writeApplicationInfo() - do { - try process.run() - } catch { - continuation.resume(throwing: error) + var succeeded = false + for await output in try process.runStream(name: "softwareupdate", logFile: logFile) { + if case .terminated(let process) = output { + succeeded = process.terminationStatus == 0 } } + return succeeded } } diff --git a/WhiskyKit/Sources/WhiskyKit/Whisky/BottleData.swift b/WhiskyKit/Sources/WhiskyKit/Whisky/BottleData.swift index 411d63e0f..ce47d62c0 100644 --- a/WhiskyKit/Sources/WhiskyKit/Whisky/BottleData.swift +++ b/WhiskyKit/Sources/WhiskyKit/Whisky/BottleData.swift @@ -17,6 +17,7 @@ // import Foundation +import os import SemanticVersion enum BottleDataError: Error { @@ -60,11 +61,38 @@ public struct BottleData: Codable { public init() { fileVersion = Self.currentVersion - if !decode() { + guard FileManager.default.fileExists(atPath: Self.bottleEntriesDir.path(percentEncoded: false)) else { encode() + return + } + + do { + let stored = try Self.read(from: Self.bottleEntriesDir) + guard stored.fileVersion == Self.currentVersion else { + throw BottleDataError.unsupportedFileVersion(stored.fileVersion) + } + self = stored + } catch { + // Starting from an empty list would overwrite the user's bottle list on the next change + Logger.wineKit.error("Unreadable bottle list: \(error)") + do { + try Self.setAsideUnreadableEntries() + encode() + } catch { + Logger.wineKit.error("Failed to set aside the unreadable bottle list: \(error)") + } } } + private static func setAsideUnreadableEntries() throws { + let backup = bottleEntriesDir + .deletingPathExtension() + .appendingPathExtension("unreadable-\(Date.now.ISO8601Format())") + .appendingPathExtension("plist") + try FileManager.default.moveItem(at: bottleEntriesDir, to: backup) + Logger.wineKit.error("Moved the unreadable bottle list to `\(backup.path(percentEncoded: false))`") + } + private init(paths: [URL]) { fileVersion = Self.currentVersion self.paths = paths @@ -109,20 +137,6 @@ public struct BottleData: Codable { try PropertyListDecoder().decode(BottleData.self, from: Data(contentsOf: entriesURL)) } - @discardableResult - private mutating func decode() -> Bool { - do { - self = try Self.read(from: Self.bottleEntriesDir) - if self.fileVersion != Self.currentVersion { - print("Invalid file version \(self.fileVersion)") - return false - } - return true - } catch { - return false - } - } - private func write() throws { let encoder = PropertyListEncoder() encoder.outputFormat = .xml @@ -131,13 +145,11 @@ public struct BottleData: Codable { try encoder.encode(self).write(to: Self.bottleEntriesDir) } - @discardableResult - private func encode() -> Bool { + private func encode() { do { try write() - return true } catch { - return false + Logger.wineKit.error("Failed to save the bottle list: \(error)") } } } diff --git a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift index 450c1aad0..3365e256c 100644 --- a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift +++ b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift @@ -17,8 +17,26 @@ // import Foundation +import os import SemanticVersion +public enum WhiskyWineInstallerError: LocalizedError { + case missingResponse + case unexpectedStatus(Int) + case missingVersionInfo + + public var errorDescription: String? { + switch self { + case .missingResponse: + return "The server did not respond." + case .unexpectedStatus(let status): + return "The server responded with HTTP \(status)." + case .missingVersionInfo: + return "The downloaded archive does not contain a WhiskyWine version file." + } + } +} + public class WhiskyWineInstaller { /// The Whisky application folder public static let applicationFolder = applicationFolder(for: Bundle.whiskyBundleIdentifier) @@ -29,6 +47,18 @@ public class WhiskyWineInstaller { /// URL to the installed `wine` `bin` directory public static let binFolder: URL = libraryFolder.appending(path: "Wine").appending(path: "bin") + private static let distributionURL: URL = { + guard let url = URL(string: "https://data.getwhisky.app/Wine/") else { + preconditionFailure("Invalid WhiskyWine distribution URL") + } + return url + }() + + /// The archive holding the `Libraries` folder + public static let librariesArchiveURL = distributionURL.appending(path: "Libraries.tar.gz") + + private static let remoteVersionURL = distributionURL.appending(path: "WhiskyWineVersion.plist") + static func applicationFolder(for bundleIdentifier: String) -> URL { FileManager.default.urls(for: .applicationSupportDirectory, in: .userDomainMask)[0] .appending(path: bundleIdentifier) @@ -42,20 +72,60 @@ public class WhiskyWineInstaller { return whiskyWineVersion() != nil } - public static func install(from: URL) { + /// Validates a finished `URLSessionDownloadTask` and moves its file out of the session's temporary + /// location, which is deleted as soon as the completion handler returns + public static func acceptDownload(at location: URL?, response: URLResponse?, error: Error?) throws -> URL { + if let error { + throw error + } + guard let location, let response = response as? HTTPURLResponse else { + throw WhiskyWineInstallerError.missingResponse + } + guard response.statusCode == 200 else { + throw WhiskyWineInstallerError.unexpectedStatus(response.statusCode) + } + + let destination = FileManager.default.temporaryDirectory + .appending(path: UUID().uuidString) + .appendingPathExtension("tar.gz") + try FileManager.default.moveItem(at: location, to: destination) + return destination + } + + /// Installs the archive, replacing the current `Libraries` folder only once the new one is extracted and valid + public static func install(from archive: URL) throws { + try install(from: archive, into: applicationFolder) + } + + static func install(from archive: URL, into applicationFolder: URL) throws { + let fileManager = FileManager.default + let stagingFolder = applicationFolder.appending(path: ".staging-\(UUID().uuidString)") + try fileManager.createDirectory(at: stagingFolder, withIntermediateDirectories: true) + defer { + removeLeftover(stagingFolder) + removeLeftover(archive) + } + + try Tar.untar(tarBall: archive, toURL: stagingFolder) + let stagedLibraries = libraryFolder(in: stagingFolder) + guard whiskyWineVersion(in: stagedLibraries) != nil else { + throw WhiskyWineInstallerError.missingVersionInfo + } + + let installedLibraries = libraryFolder(in: applicationFolder) + if fileManager.fileExists(atPath: installedLibraries.path(percentEncoded: false)) { + _ = try fileManager.replaceItemAt(installedLibraries, withItemAt: stagedLibraries) + } else { + try fileManager.moveItem(at: stagedLibraries, to: installedLibraries) + } + } + + private static func removeLeftover(_ url: URL) { + guard FileManager.default.fileExists(atPath: url.path(percentEncoded: false)) else { return } do { - if !FileManager.default.fileExists(atPath: applicationFolder.path) { - try FileManager.default.createDirectory(at: applicationFolder, withIntermediateDirectories: true) - } else { - // Recreate it - try FileManager.default.removeItem(at: applicationFolder) - try FileManager.default.createDirectory(at: applicationFolder, withIntermediateDirectories: true) - } - - try Tar.untar(tarBall: from, toURL: applicationFolder) - try FileManager.default.removeItem(at: from) + try FileManager.default.removeItem(at: url) } catch { - print("Failed to install WhiskyWine: \(error)") + Logger.wineKit.error("Failed to remove `\(url.path(percentEncoded: false))`: \(error)") } } @@ -67,57 +137,37 @@ public class WhiskyWineInstaller { } } - public static func shouldUpdateWhiskyWine() async -> (Bool, SemanticVersion) { - let versionPlistURL = "https://data.getwhisky.app/Wine/WhiskyWineVersion.plist" - let localVersion = whiskyWineVersion() - - var remoteVersion: SemanticVersion? - - if let remoteUrl = URL(string: versionPlistURL) { - remoteVersion = await withCheckedContinuation { continuation in - URLSession(configuration: .ephemeral).dataTask(with: URLRequest(url: remoteUrl)) { data, _, error in - do { - if error == nil, let data = data { - let decoder = PropertyListDecoder() - let remoteInfo = try decoder.decode(WhiskyWineVersion.self, from: data) - let remoteVersion = remoteInfo.version - - continuation.resume(returning: remoteVersion) - return - } - if let error = error { - print(error) - } - } catch { - print(error) - } - - continuation.resume(returning: nil) - }.resume() - } - } + /// The newer WhiskyWine version available for download, or `nil` when the installed one is current + public static func availableUpdate() async throws -> SemanticVersion? { + guard let localVersion = whiskyWineVersion() else { return nil } - if let localVersion = localVersion, let remoteVersion = remoteVersion { - if localVersion < remoteVersion { - return (true, remoteVersion) - } + let (data, response) = try await URLSession(configuration: .ephemeral).data(from: remoteVersionURL) + guard let response = response as? HTTPURLResponse else { + throw WhiskyWineInstallerError.missingResponse + } + guard response.statusCode == 200 else { + throw WhiskyWineInstallerError.unexpectedStatus(response.statusCode) } - return (false, SemanticVersion(0, 0, 0)) + let remoteVersion = try PropertyListDecoder().decode(WhiskyWineVersion.self, from: data).version + return localVersion < remoteVersion ? remoteVersion : nil } public static func whiskyWineVersion() -> SemanticVersion? { - do { - let versionPlist = libraryFolder - .appending(path: "WhiskyWineVersion") - .appendingPathExtension("plist") + whiskyWineVersion(in: libraryFolder) + } + + private static func whiskyWineVersion(in libraryFolder: URL) -> SemanticVersion? { + let versionPlist = libraryFolder + .appending(path: "WhiskyWineVersion") + .appendingPathExtension("plist") + guard FileManager.default.fileExists(atPath: versionPlist.path(percentEncoded: false)) else { return nil } - let decoder = PropertyListDecoder() + do { let data = try Data(contentsOf: versionPlist) - let info = try decoder.decode(WhiskyWineVersion.self, from: data) - return info.version + return try PropertyListDecoder().decode(WhiskyWineVersion.self, from: data).version } catch { - print(error) + Logger.wineKit.error("Unreadable WhiskyWine version file: \(error)") return nil } } diff --git a/WhiskyKit/Sources/WhiskyKit/Wine/Wine.swift b/WhiskyKit/Sources/WhiskyKit/Wine/Wine.swift index c485d0712..5bace8086 100644 --- a/WhiskyKit/Sources/WhiskyKit/Wine/Wine.swift +++ b/WhiskyKit/Sources/WhiskyKit/Wine/Wine.swift @@ -30,7 +30,7 @@ public class Wine { /// Run a process on a executable file given by the `executableURL` private static func runProcess( name: String? = nil, args: [String], environment: [String: String], executableURL: URL, directory: URL? = nil, - fileHandle: FileHandle? + logFile: LogFile? ) throws -> AsyncStream { let process = Process() process.executableURL = executableURL @@ -40,29 +40,29 @@ public class Wine { process.qualityOfService = .userInitiated return try process.runStream( - name: name ?? args.joined(separator: " "), fileHandle: fileHandle + name: name ?? args.joined(separator: " "), logFile: logFile ) } /// Run a `wine` process with the given arguments and environment variables returning a stream of output private static func runWineProcess( name: String? = nil, args: [String], environment: [String: String] = [:], - fileHandle: FileHandle? + logFile: LogFile? ) throws -> AsyncStream { return try runProcess( name: name, args: args, environment: environment, executableURL: wineBinary, - fileHandle: fileHandle + logFile: logFile ) } /// Run a `wineserver` process with the given arguments and environment variables returning a stream of output private static func runWineserverProcess( name: String? = nil, args: [String], environment: [String: String] = [:], - fileHandle: FileHandle? + logFile: LogFile? ) throws -> AsyncStream { return try runProcess( name: name, args: args, environment: environment, executableURL: wineserverBinary, - fileHandle: fileHandle + logFile: logFile ) } @@ -70,14 +70,14 @@ public class Wine { public static func runWineProcess( name: String? = nil, args: [String], bottle: Bottle, environment: [String: String] = [:] ) throws -> AsyncStream { - let fileHandle = try makeFileHandle() - fileHandle.writeApplicaitonInfo() - fileHandle.writeInfo(for: bottle) + let logFile = try WineLogs.makeLogFile() + logFile.writeApplicationInfo() + logFile.writeInfo(for: bottle) return try runWineProcess( name: name, args: args, environment: constructWineEnvironment(for: bottle, environment: environment), - fileHandle: fileHandle + logFile: logFile ) } @@ -85,14 +85,14 @@ public class Wine { public static func runWineserverProcess( name: String? = nil, args: [String], bottle: Bottle, environment: [String: String] = [:] ) throws -> AsyncStream { - let fileHandle = try makeFileHandle() - fileHandle.writeApplicaitonInfo() - fileHandle.writeInfo(for: bottle) + let logFile = try WineLogs.makeLogFile() + logFile.writeApplicationInfo() + logFile.writeInfo(for: bottle) return try runWineserverProcess( name: name, args: args, environment: constructWineServerEnvironment(for: bottle, environment: environment), - fileHandle: fileHandle + logFile: logFile ) } @@ -171,16 +171,16 @@ public class Wine { _ args: [String], bottle: Bottle?, environment: [String: String] = [:] ) async throws -> String { var result: [String] = [] - let fileHandle = try makeFileHandle() - fileHandle.writeApplicaitonInfo() + let logFile = try WineLogs.makeLogFile() + logFile.writeApplicationInfo() var environment = environment if let bottle = bottle { - fileHandle.writeInfo(for: bottle) + logFile.writeInfo(for: bottle) environment = constructWineEnvironment(for: bottle, environment: environment) } - for await output in try runWineProcess(args: args, environment: environment, fileHandle: fileHandle) { + for await output in try runWineProcess(args: args, environment: environment, logFile: logFile) { switch output { case .started, .terminated: break @@ -266,23 +266,6 @@ enum RegistryType: String { case string = "REG_SZ" } -extension Wine { - public static let logsFolder = FileManager.default.urls( - for: .libraryDirectory, in: .userDomainMask - )[0].appending(path: "Logs").appending(path: Bundle.whiskyBundleIdentifier) - - public static func makeFileHandle() throws -> FileHandle { - if !FileManager.default.fileExists(atPath: Self.logsFolder.path) { - try FileManager.default.createDirectory(at: Self.logsFolder, withIntermediateDirectories: true) - } - - let dateString = Date.now.ISO8601Format() - let fileURL = Self.logsFolder.appending(path: dateString).appendingPathExtension("log") - try "".write(to: fileURL, atomically: true, encoding: .utf8) - return try FileHandle(forWritingTo: fileURL) - } -} - extension Wine { private enum RegistryKey: String { case currentVersion = #"HKLM\Software\Microsoft\Windows NT\CurrentVersion"# diff --git a/WhiskyKit/Tests/WhiskyKitTests/LogFileTests.swift b/WhiskyKit/Tests/WhiskyKitTests/LogFileTests.swift new file mode 100644 index 000000000..2b148f33f --- /dev/null +++ b/WhiskyKit/Tests/WhiskyKitTests/LogFileTests.swift @@ -0,0 +1,61 @@ +// +// LogFileTests.swift +// WhiskyKitTests +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation +import Testing +@testable import WhiskyKit + +struct LogFileTests { + @Test func writesEverythingBelowTheLimit() throws { + let folder = try TemporaryFolder() + let logFile = try LogFile(url: folder.url.appending(path: "run.log"), maxSize: 64) + + logFile.write("first\n") + logFile.write("second\n") + logFile.close() + + #expect(try String(contentsOf: logFile.url, encoding: .utf8) == "first\nsecond\n") + } + + @Test func stopsAtTheLimitWithASingleNotice() throws { + let folder = try TemporaryFolder() + let logFile = try LogFile(url: folder.url.appending(path: "run.log"), maxSize: 10) + + logFile.write("0123456789ABCDEF") + logFile.write("never written") + logFile.close() + + let contents = try String(contentsOf: logFile.url, encoding: .utf8) + #expect(contents.hasPrefix("0123456789\n\n[Whisky] Log truncated")) + #expect(!contents.contains("ABCDEF")) + #expect(!contents.contains("never written")) + #expect(contents.components(separatedBy: "Log truncated").count == 2) + } + + @Test func ignoresWritesAfterClosing() throws { + let folder = try TemporaryFolder() + let logFile = try LogFile(url: folder.url.appending(path: "run.log"), maxSize: 64) + + logFile.write("kept") + logFile.close() + logFile.write("dropped") + logFile.close() + + #expect(try String(contentsOf: logFile.url, encoding: .utf8) == "kept") + } +} diff --git a/WhiskyKit/Tests/WhiskyKitTests/ProcessStreamTests.swift b/WhiskyKit/Tests/WhiskyKitTests/ProcessStreamTests.swift new file mode 100644 index 000000000..840bf3986 --- /dev/null +++ b/WhiskyKit/Tests/WhiskyKitTests/ProcessStreamTests.swift @@ -0,0 +1,87 @@ +// +// ProcessStreamTests.swift +// WhiskyKitTests +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation +import Testing +@testable import WhiskyKit + +struct ProcessStreamTests { + private func collect(_ script: String, logFile: LogFile? = nil) async throws -> [ProcessOutput] { + let process = Process() + process.executableURL = URL(filePath: "/bin/sh") + process.arguments = ["-c", script] + + var outputs: [ProcessOutput] = [] + for await output in try process.runStream(name: "sh", logFile: logFile) { + outputs.append(output) + } + return outputs + } + + private func text(in outputs: [ProcessOutput]) -> (messages: String, errors: String) { + var messages = "" + var errors = "" + for output in outputs { + switch output { + case .message(let text): messages += text + case .error(let text): errors += text + case .started, .terminated: break + } + } + return (messages, errors) + } + + @Test func deliversBothPipesAndFinishes() async throws { + let folder = try TemporaryFolder() + let logFile = try WineLogs.makeLogFile(in: folder.url) + + let outputs = try await collect("echo out; echo err 1>&2", logFile: logFile) + + let (messages, errors) = text(in: outputs) + #expect(messages == "out\n") + #expect(errors == "err\n") + #expect(outputs.contains { if case .terminated = $0 { true } else { false } }) + + let logged = try String(contentsOf: logFile.url, encoding: .utf8) + #expect(logged.contains("out\n") && logged.contains("err\n")) + } + + private func cpuTime() -> TimeInterval { + var usage = rusage() + getrusage(RUSAGE_SELF, &usage) + let user = TimeInterval(usage.ru_utime.tv_sec) + TimeInterval(usage.ru_utime.tv_usec) / 1_000_000 + let system = TimeInterval(usage.ru_stime.tv_sec) + TimeInterval(usage.ru_stime.tv_usec) / 1_000_000 + return user + system + } + + @Test func goesIdleAfterEndOfFile() async throws { + _ = try await collect("echo done") + + let before = cpuTime() + try await Task.sleep(for: .milliseconds(500)) + let spent = cpuTime() - before + + #expect(spent < 0.1, "Readers kept polling the closed pipes for \(spent)s of CPU") + } + + @Test func waitsForChildrenThatKeepThePipesOpen() async throws { + let outputs = try await collect("(sleep 0.3; echo late) & echo early") + + #expect(text(in: outputs).messages == "early\nlate\n") + } +} diff --git a/WhiskyKit/Tests/WhiskyKitTests/TemporaryFolder.swift b/WhiskyKit/Tests/WhiskyKitTests/TemporaryFolder.swift new file mode 100644 index 000000000..b8761fbd9 --- /dev/null +++ b/WhiskyKit/Tests/WhiskyKitTests/TemporaryFolder.swift @@ -0,0 +1,41 @@ +// +// TemporaryFolder.swift +// WhiskyKitTests +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation + +/// A folder under the system temporary directory that is removed when the value is deinitialized +final class TemporaryFolder { + let url: URL + + init() throws { + url = FileManager.default.temporaryDirectory.appending(path: "WhiskyKitTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: url, withIntermediateDirectories: true) + } + + deinit { + do { + try FileManager.default.removeItem(at: url) + } catch { + print("Failed to remove test folder `\(url.path(percentEncoded: false))`: \(error)") + } + } + + func contents(of folder: URL? = nil) throws -> [String] { + try FileManager.default.contentsOfDirectory(atPath: (folder ?? url).path(percentEncoded: false)).sorted() + } +} diff --git a/WhiskyKit/Tests/WhiskyKitTests/WhiskyWineInstallerTests.swift b/WhiskyKit/Tests/WhiskyKitTests/WhiskyWineInstallerTests.swift new file mode 100644 index 000000000..4fb55f417 --- /dev/null +++ b/WhiskyKit/Tests/WhiskyKitTests/WhiskyWineInstallerTests.swift @@ -0,0 +1,133 @@ +// +// WhiskyWineInstallerTests.swift +// WhiskyKitTests +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation +import SemanticVersion +import Testing +@testable import WhiskyKit + +struct WhiskyWineInstallerTests { + private let archiveURL = URL(filePath: "/Libraries.tar.gz") + + private func response(status: Int) throws -> HTTPURLResponse { + try #require(HTTPURLResponse(url: archiveURL, statusCode: status, httpVersion: nil, headerFields: nil)) + } + + /// Builds an archive laid out like the published one: a top-level `Libraries` folder + private func makeArchive(in folder: URL, version: SemanticVersion?, marker: String) throws -> URL { + let source = folder.appending(path: "source-\(marker)") + let libraries = source.appending(path: "Libraries") + try FileManager.default.createDirectory(at: libraries, withIntermediateDirectories: true) + try Data(marker.utf8).write(to: libraries.appending(path: "marker")) + if let version { + let plist = try PropertyListEncoder().encode(WhiskyWineVersion(version: version)) + try plist.write(to: libraries.appending(path: "WhiskyWineVersion.plist")) + } + + let archive = folder.appending(path: "\(marker).tar.gz") + let process = Process() + process.executableURL = Tar.tarBinary + process.arguments = ["-zcf", archive.path, "-C", source.path, "Libraries"] + try process.run() + process.waitUntilExit() + try #require(process.terminationStatus == 0) + return archive + } + + private func installedMarker(in applicationFolder: URL) throws -> String { + let marker = WhiskyWineInstaller.libraryFolder(in: applicationFolder).appending(path: "marker") + return try String(contentsOf: marker, encoding: .utf8) + } + + @Test func rejectsANonSuccessResponse() throws { + let folder = try TemporaryFolder() + let download = folder.url.appending(path: "download") + try Data("Not Found".utf8).write(to: download) + + #expect { + try WhiskyWineInstaller.acceptDownload(at: download, response: response(status: 404), error: nil) + } throws: { error in + guard case WhiskyWineInstallerError.unexpectedStatus(404) = error else { return false } + return true + } + } + + @Test func passesThroughTheTransportError() throws { + #expect(throws: URLError(.timedOut)) { + try WhiskyWineInstaller.acceptDownload(at: nil, response: nil, error: URLError(.timedOut)) + } + } + + @Test func movesASuccessfulDownloadOutOfTheSessionLocation() throws { + let folder = try TemporaryFolder() + let download = folder.url.appending(path: "download") + try Data("archive".utf8).write(to: download) + + let accepted = try WhiskyWineInstaller.acceptDownload(at: download, response: response(status: 200), error: nil) + defer { + do { + try FileManager.default.removeItem(at: accepted) + } catch { + Issue.record(error) + } + } + + #expect(!FileManager.default.fileExists(atPath: download.path(percentEncoded: false))) + #expect(try String(contentsOf: accepted, encoding: .utf8) == "archive") + } + + @Test func installsAndThenReplacesTheLibraries() throws { + let folder = try TemporaryFolder() + let applicationFolder = folder.url.appending(path: "Application") + + let first = try makeArchive(in: folder.url, version: SemanticVersion(2, 4, 0), marker: "first") + try WhiskyWineInstaller.install(from: first, into: applicationFolder) + #expect(try installedMarker(in: applicationFolder) == "first") + + let second = try makeArchive(in: folder.url, version: SemanticVersion(2, 5, 0), marker: "second") + try WhiskyWineInstaller.install(from: second, into: applicationFolder) + #expect(try installedMarker(in: applicationFolder) == "second") + + #expect(try folder.contents(of: applicationFolder) == ["Libraries"]) + #expect(!FileManager.default.fileExists(atPath: second.path(percentEncoded: false))) + } + + @Test func keepsTheInstalledLibrariesWhenTheArchiveIsInvalid() throws { + let folder = try TemporaryFolder() + let applicationFolder = folder.url.appending(path: "Application") + let valid = try makeArchive(in: folder.url, version: SemanticVersion(2, 4, 0), marker: "valid") + try WhiskyWineInstaller.install(from: valid, into: applicationFolder) + + let unversioned = try makeArchive(in: folder.url, version: nil, marker: "unversioned") + #expect { + try WhiskyWineInstaller.install(from: unversioned, into: applicationFolder) + } throws: { error in + guard case WhiskyWineInstallerError.missingVersionInfo = error else { return false } + return true + } + + let notAnArchive = folder.url.appending(path: "page.tar.gz") + try Data("Not Found".utf8).write(to: notAnArchive) + #expect(throws: TarError.self) { + try WhiskyWineInstaller.install(from: notAnArchive, into: applicationFolder) + } + + #expect(try installedMarker(in: applicationFolder) == "valid") + #expect(try folder.contents(of: applicationFolder) == ["Libraries"]) + } +} diff --git a/WhiskyKit/Tests/WhiskyKitTests/WineLogsTests.swift b/WhiskyKit/Tests/WhiskyKitTests/WineLogsTests.swift new file mode 100644 index 000000000..5dcc30984 --- /dev/null +++ b/WhiskyKit/Tests/WhiskyKitTests/WineLogsTests.swift @@ -0,0 +1,70 @@ +// +// WineLogsTests.swift +// WhiskyKitTests +// +// This file is part of Whisky. +// +// Whisky is free software: you can redistribute it and/or modify it under the terms +// of the GNU General Public License as published by the Free Software Foundation, +// either version 3 of the License, or (at your option) any later version. +// +// Whisky is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; +// without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. +// See the GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License along with Whisky. +// If not, see https://www.gnu.org/licenses/. +// + +import Foundation +import Testing +@testable import WhiskyKit + +struct WineLogsTests { + private let now = Date(timeIntervalSince1970: 1_000_000) + + private func makeLog(_ name: String, in folder: URL, size: Int, age: TimeInterval) throws { + let url = folder.appending(path: name) + try Data(repeating: 0, count: size).write(to: url) + try FileManager.default.setAttributes([.creationDate: now.addingTimeInterval(-age)], + ofItemAtPath: url.path(percentEncoded: false)) + } + + @Test func createsDistinctFilesForRunsInTheSameSecond() throws { + let folder = try TemporaryFolder() + + let first = try WineLogs.makeLogFile(in: folder.url) + let second = try WineLogs.makeLogFile(in: folder.url) + + #expect(first.url != second.url) + #expect(try folder.contents().count == 2) + } + + @Test func removesLogsOlderThanTheMaximumAge() throws { + let folder = try TemporaryFolder() + try makeLog("recent.log", in: folder.url, size: 1, age: 60) + try makeLog("stale.log", in: folder.url, size: 1, age: 3600) + try makeLog("notes.txt", in: folder.url, size: 1, age: 3600) + + try WineLogs.prune(in: folder.url, maxAge: 600, maxTotalSize: 1024, now: now) + + #expect(try folder.contents() == ["notes.txt", "recent.log"]) + } + + @Test func removesTheOldestLogsUntilTheFolderFitsTheBudget() throws { + let folder = try TemporaryFolder() + try makeLog("newest.log", in: folder.url, size: 400, age: 10) + try makeLog("middle.log", in: folder.url, size: 400, age: 20) + try makeLog("oldest.log", in: folder.url, size: 400, age: 30) + + try WineLogs.prune(in: folder.url, maxAge: 600, maxTotalSize: 1000, now: now) + + #expect(try folder.contents() == ["middle.log", "newest.log"]) + } + + @Test func ignoresAMissingFolder() throws { + let folder = try TemporaryFolder() + + try WineLogs.prune(in: folder.url.appending(path: "missing"), maxAge: 600, maxTotalSize: 1000, now: now) + } +} From 175dc9c9f67c5265e0dd7435a6eba471d55b0ea0 Mon Sep 17 00:00:00 2001 From: Jatinjit Singh Date: Thu, 29 Oct 2026 12:59:08 +0900 Subject: [PATCH 2/4] Host WhiskyWine on this repository's whiskywine release data.getwhisky.app no longer serves the runtime (both the archive and the version file return 404), so setup could never install WhiskyWine. - Download Libraries.tar.gz and WhiskyWineVersion.plist from the assets of the whiskywine release. - Add scripts/publish-whiskywine.sh, which packages a built Libraries folder and uploads both assets, replacing the previous runtime. - Skip the Discord and Homebrew release job for the whiskywine release, which is not an app release. --- .github/workflows/Release.yml | 2 + .../WhiskyWine/WhiskyWineInstaller.swift | 3 +- scripts/publish-whiskywine.sh | 55 +++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100755 scripts/publish-whiskywine.sh diff --git a/.github/workflows/Release.yml b/.github/workflows/Release.yml index 65c253c97..7248184e0 100644 --- a/.github/workflows/Release.yml +++ b/.github/workflows/Release.yml @@ -7,6 +7,8 @@ on: jobs: release: name: Release + # The whiskywine release only hosts the runtime the app downloads; it is not an app release + if: github.event.release.tag_name != 'whiskywine' runs-on: ubuntu-latest steps: - name: Create Discord Embed diff --git a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift index 3365e256c..ceaf6bb99 100644 --- a/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift +++ b/WhiskyKit/Sources/WhiskyKit/WhiskyWine/WhiskyWineInstaller.swift @@ -47,8 +47,9 @@ public class WhiskyWineInstaller { /// URL to the installed `wine` `bin` directory public static let binFolder: URL = libraryFolder.appending(path: "Wine").appending(path: "bin") + /// Assets of the `whiskywine` release, which `scripts/publish-whiskywine.sh` replaces on every runtime update private static let distributionURL: URL = { - guard let url = URL(string: "https://data.getwhisky.app/Wine/") else { + guard let url = URL(string: "https://github.com/iStorry/Whisky/releases/download/whiskywine/") else { preconditionFailure("Invalid WhiskyWine distribution URL") } return url diff --git a/scripts/publish-whiskywine.sh b/scripts/publish-whiskywine.sh new file mode 100755 index 000000000..649ce3997 --- /dev/null +++ b/scripts/publish-whiskywine.sh @@ -0,0 +1,55 @@ +#!/bin/bash +# +# publish-whiskywine.sh +# Whisky +# +# Publishes a built WhiskyWine `Libraries` folder as the assets Whisky downloads: +# Libraries.tar.gz and WhiskyWineVersion.plist on the `whiskywine` release. +# +# Usage: scripts/publish-whiskywine.sh +# + +set -euo pipefail + +readonly RELEASE_TAG="whiskywine" +readonly ARCHIVE_NAME="Libraries.tar.gz" +readonly VERSION_FILE="WhiskyWineVersion.plist" + +if [[ $# -ne 1 ]]; then + echo "Usage: $0 " >&2 + exit 64 +fi + +libraries="$(cd "$1" && pwd)" +if [[ "$(basename "$libraries")" != "Libraries" ]]; then + echo "The folder must be named Libraries; the app extracts the archive expecting that top-level folder." >&2 + exit 65 +fi +for required in "$VERSION_FILE" "Wine/bin/wine64" "Wine/bin/wineserver"; do + if [[ ! -e "$libraries/$required" ]]; then + echo "Missing $required in $libraries" >&2 + exit 65 + fi +done + +version_component() { + plutil -extract "version.$1" raw -o - "$libraries/$VERSION_FILE" +} +version="$(version_component major).$(version_component minor).$(version_component patch)" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +echo "Packaging WhiskyWine $version" +COPYFILE_DISABLE=1 tar -zcf "$work/$ARCHIVE_NAME" -C "$(dirname "$libraries")" Libraries +cp "$libraries/$VERSION_FILE" "$work/$VERSION_FILE" + +if ! gh release view "$RELEASE_TAG" > /dev/null 2>&1; then + gh release create "$RELEASE_TAG" --title "WhiskyWine runtime" --latest=false \ + --notes "Wine runtime downloaded by Whisky during setup. Not an app release." +fi +gh release upload "$RELEASE_TAG" "$work/$ARCHIVE_NAME" "$work/$VERSION_FILE" --clobber +gh release edit "$RELEASE_TAG" --notes "Wine runtime downloaded by Whisky during setup. Not an app release. + +Current version: $version" + +echo "Published WhiskyWine $version to the $RELEASE_TAG release" From fecdc27833fd8921bb5f68ba85b1a33d9951f2bf Mon Sep 17 00:00:00 2001 From: Jatinjit Singh Date: Thu, 29 Oct 2026 13:11:59 +0900 Subject: [PATCH 3/4] Publish WhiskyWine to iStorry/Whisky explicitly This checkout also has the upstream remote and no gh default repository, so release commands without --repo could target the wrong repository. --- scripts/publish-whiskywine.sh | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/scripts/publish-whiskywine.sh b/scripts/publish-whiskywine.sh index 649ce3997..80b40a40f 100755 --- a/scripts/publish-whiskywine.sh +++ b/scripts/publish-whiskywine.sh @@ -11,6 +11,8 @@ set -euo pipefail +# Must match WhiskyWineInstaller.distributionURL; this checkout also has the upstream remote +readonly REPOSITORY="iStorry/Whisky" readonly RELEASE_TAG="whiskywine" readonly ARCHIVE_NAME="Libraries.tar.gz" readonly VERSION_FILE="WhiskyWineVersion.plist" @@ -43,13 +45,13 @@ echo "Packaging WhiskyWine $version" COPYFILE_DISABLE=1 tar -zcf "$work/$ARCHIVE_NAME" -C "$(dirname "$libraries")" Libraries cp "$libraries/$VERSION_FILE" "$work/$VERSION_FILE" -if ! gh release view "$RELEASE_TAG" > /dev/null 2>&1; then - gh release create "$RELEASE_TAG" --title "WhiskyWine runtime" --latest=false \ +if ! gh release view "$RELEASE_TAG" --repo "$REPOSITORY" > /dev/null 2>&1; then + gh release create "$RELEASE_TAG" --repo "$REPOSITORY" --title "WhiskyWine runtime" --latest=false \ --notes "Wine runtime downloaded by Whisky during setup. Not an app release." fi -gh release upload "$RELEASE_TAG" "$work/$ARCHIVE_NAME" "$work/$VERSION_FILE" --clobber -gh release edit "$RELEASE_TAG" --notes "Wine runtime downloaded by Whisky during setup. Not an app release. +gh release upload "$RELEASE_TAG" --repo "$REPOSITORY" "$work/$ARCHIVE_NAME" "$work/$VERSION_FILE" --clobber +gh release edit "$RELEASE_TAG" --repo "$REPOSITORY" --notes "Wine runtime downloaded by Whisky during setup. Not an app release. Current version: $version" -echo "Published WhiskyWine $version to the $RELEASE_TAG release" +echo "Published WhiskyWine $version to the $RELEASE_TAG release of $REPOSITORY" From 91f748999c3cbe05dc42e404bd4605b7ae0812e8 Mon Sep 17 00:00:00 2001 From: Jatinjit Singh Date: Thu, 29 Oct 2026 13:12:26 +0900 Subject: [PATCH 4/4] Remove the Discord and Homebrew release workflow It announced releases on upstream's Discord and bumped the Whisky-App Homebrew cask. Neither applies to this fork while it is being built, and it ran on every published release, including the whiskywine runtime. --- .github/workflows/Release.yml | 28 ---------------------------- 1 file changed, 28 deletions(-) delete mode 100644 .github/workflows/Release.yml diff --git a/.github/workflows/Release.yml b/.github/workflows/Release.yml deleted file mode 100644 index 7248184e0..000000000 --- a/.github/workflows/Release.yml +++ /dev/null @@ -1,28 +0,0 @@ -name: Release - -on: - release: - types: [published] - -jobs: - release: - name: Release - # The whiskywine release only hosts the runtime the app downloads; it is not an app release - if: github.event.release.tag_name != 'whiskywine' - runs-on: ubuntu-latest - steps: - - name: Create Discord Embed - uses: tsickert/discord-webhook@v5.3.0 - with: - webhook-url: ${{ secrets.WEBHOOK }} - embed-title: ${{ github.event.release.name }} - embed-url: https://github.com/Whisky-App/Whisky/releases/download/${{ github.event.release.tag_name }}/Whisky.zip - embed-description: ${{ github.event.release.body }} - embed-color: 9442302 - - - name: Update Homebrew Cask - uses: macauley/action-homebrew-bump-cask@v1 - with: - token: ${{secrets.BREW_TOKEN}} - org: Whisky-App - cask: whisky