Fix small prefix, menu and install bugs from #248

app/Sources/NotProtonApp/App.swift

@@ -19,7 +19,8 @@ struct NotProtonApp: App {
                 .environment(status)
                 .environment(prefixes)
                 .frame(minWidth: 720, minHeight: 460)
-                .onChange(of: pane) { prefixes.forgetOutcome() }
+                // Status never shows the prefix banner, so a result that lands there is kept
+                .onChange(of: pane) { old, _ in if old != .status { prefixes.forgetOutcome() } }
                 .onChange(of: status.snapshot?.installedRunners.map(\.id)) {
                     if prefixes.hasLoaded { Task { await prefixes.load() } }
                 }
@@ -61,21 +62,24 @@ struct NotProtonApp: App {
         }
 
         CommandMenu("Setup") {
-            Button("Install") { Task { await status.requestInstall() } }
+            Button("Install") { pane = .status; Task { await status.requestInstall() } }
                 .keyboardShortcut("i", modifiers: .command)
                 .disabled(!status.canInstall)
 
-            Button("Set Up Compatibility Tool") { Task { await status.requestCompatibilityTool() } }
-                .disabled(!status.canInstall || status.setupSource == nil)
+            Button("Set Up Compatibility Tool") {
+                pane = .status
+                Task { await status.requestCompatibilityTool() }
+            }
+            .disabled(!status.canInstall || status.setupSource == nil)
 
-            Button("Fetch Valve Binaries") { Task { await status.fetchValveBinaries() } }
+            Button("Fetch Valve Binaries") { pane = .status; Task { await status.fetchValveBinaries() } }
                 .disabled(!status.canInstall)
 
             Divider()
 
             updateBlockCommand
 
-            Button("Repair Steam") { status.pendingConfirmation = .replaceSteam }
+            Button("Repair Steam") { pane = .status; status.pendingConfirmation = .replaceSteam }
                 .disabled(!status.isIdle)
 
             Divider()
@@ -117,6 +121,7 @@ struct NotProtonApp: App {
 
             Button(PrefixPrompt.backUpButton(selectionTargets)) {
                 if !selectionTargets.isEmpty {
+                    pane = .prefixes
                     prefixes.pendingConfirmation = .backUp(selectionTargets)
                 }
             }
@@ -126,6 +131,7 @@ struct NotProtonApp: App {
                 Menu(PrefixPrompt.rebuildButton(selectionTargets)) {
                     ForEach(prefixes.tools) { tool in
                         Button(tool.display) {
+                            pane = .prefixes
                             prefixes.pendingConfirmation = .rebuild(selectionTargets, tool)
                         }
                     }
@@ -134,6 +140,7 @@ struct NotProtonApp: App {
             } else {
                 Button(PrefixPrompt.rebuildButton(selectionTargets)) {
                     if let tool = prefixes.tools.first, !selectionTargets.isEmpty {
+                        pane = .prefixes
                         prefixes.pendingConfirmation = .rebuild(selectionTargets, tool)
                     }
                 }
@@ -142,6 +149,7 @@ struct NotProtonApp: App {
 
             Button(PrefixPrompt.deleteButton(selectionTargets)) {
                 if !selectionTargets.isEmpty {
+                    pane = .prefixes
                     prefixes.pendingConfirmation = .delete(selectionTargets)
                 }
             }
@@ -160,10 +168,10 @@ struct NotProtonApp: App {
     @ViewBuilder
     private var updateBlockCommand: some View {
         if status.snapshot?.updateBlocked == true {
-            Button("Allow Client Updates") { Task { await status.setUpdateBlock(false) } }
+            Button("Allow Client Updates") { pane = .status; Task { await status.setUpdateBlock(false) } }
                 .disabled(!status.isIdle)
         } else {
-            Button("Block Client Updates") { status.pendingConfirmation = .blockUpdates }
+            Button("Block Client Updates") { pane = .status; status.pendingConfirmation = .blockUpdates }
                 .disabled(!status.isIdle || status.snapshot == nil)
         }
     }

app/Sources/NotProtonApp/Model/PrefixStore.swift

@@ -69,9 +69,8 @@ enum PrefixStore {
 
         var roots: [URL] = []
         for line in text.split(separator: "\n") {
-            let fields = line.split(separator: "\"").map(String.init)
-                .filter { !$0.trimmingCharacters(in: .whitespaces).isEmpty }
-            guard fields.count >= 2, fields[0] == "path" else { continue }
+            let fields = vdfFields(line)
+            guard fields.count >= 2, fields[0] == "path", !fields[1].isEmpty else { continue }
             roots.append(URL(filePath: fields[1]))
         }
 
@@ -225,23 +224,27 @@ enum PrefixStore {
         let manifest = library.steamapps.appending(path: "appmanifest_\(appID).acf")
         guard let text = try? String(contentsOf: manifest, encoding: .utf8) else { return nil }
         for line in text.split(separator: "\n") {
-            // Steam escapes \" \\ \n and \t inside quoted values.
-            let fields = line.matches(of: #/"((?:[^"\\]|\\.)*)"/#)
-                .map {
-                    String($0.1).replacing(#/\\(.)/#) { escape -> String in
-                        switch escape.1 {
-                        case "n": "\n"
-                        case "t": "\t"
-                        case "\\", "\"": String(escape.1)
-                        default: String(escape.0)
-                        }
-                    }
-                }
+            let fields = vdfFields(line)
             if fields.count >= 2, fields[0] == key { return fields[1] }
         }
         return nil
     }
 
+    // Steam escapes \" \\ \n and \t inside quoted values.
+    private static func vdfFields(_ line: Substring) -> [String] {
+        line.matches(of: #/"((?:[^"\\]|\\.)*)"/#)
+            .map {
+                String($0.1).replacing(#/\\(.)/#) { escape -> String in
+                    switch escape.1 {
+                    case "n": "\n"
+                    case "t": "\t"
+                    case "\\", "\"": String(escape.1)
+                    default: String(escape.0)
+                    }
+                }
+            }
+    }
+
     static let backupPrefix = "pfx.previous"
 
     static func backupClock() -> DateFormatter {
@@ -394,14 +397,15 @@ enum PrefixStore {
         _ prefix: WinePrefix, root: URL = serverRoot, lsof: String = "/usr/sbin/lsof",
         drainTimeout: DispatchTimeInterval = .seconds(2)
     ) -> Bool {
+        isClaimed(prefix) || serverIsBusy(prefix, root: root, lsof: lsof, drainTimeout: drainTimeout)
+    }
+
+    // The lock only, without the wineserver check. A running game does not hold it.
+    static func isClaimed(_ prefix: WinePrefix) -> Bool {
         let lock = open(lockPath(of: prefix), O_RDWR | O_NOFOLLOW | O_CLOEXEC)
-        if lock >= 0 {
-            defer { FileLock.release(lock) }
-            guard locked(lock) else { return true }
-        } else if errno != ENOENT {
-            return true
-        }
-        return serverIsBusy(prefix, root: root, lsof: lsof, drainTimeout: drainTimeout)
+        guard lock >= 0 else { return errno != ENOENT }
+        defer { FileLock.release(lock) }
+        return !locked(lock)
     }
 
     // A launch takes this lock while it sets the prefix up. Holding it stops a launch from

app/Sources/NotProtonApp/Model/PrefixTools.swift

@@ -200,12 +200,20 @@ enum PrefixTools {
         )
     }
 
+    // A launch setting the prefix up or a rebuild started from Steam holds the lock in another
+    // process. Wine started now would write into a prefix that is half made or about to be replaced.
+    private static func requireUnclaimed(step: String, prefix: WinePrefix) throws {
+        guard !PrefixStore.isClaimed(prefix) else {
+            throw StepFailure(step: step, detail: String(localized: "\(prefix.title) is running. Quit the game first."))
+        }
+    }
+
     static func launch(
         _ tool: WineTool, in prefix: WinePrefix, runner: URL, flavor: CompatTool.Flavor = .fex
     ) throws {
-        let loader = try readyLoader(
-            step: String(localized: "Open \(tool.label)"), prefix: prefix, runner: runner, flavor: flavor
-        )
+        let step = String(localized: "Open \(tool.label)")
+        try requireUnclaimed(step: step, prefix: prefix)
+        let loader = try readyLoader(step: step, prefix: prefix, runner: runner, flavor: flavor)
         try Shell.detach(
             loader.path(percentEncoded: false),
             [tool.rawValue],
@@ -228,6 +236,7 @@ enum PrefixTools {
         flavor: CompatTool.Flavor = .fex
     ) throws {
         let step = String(localized: "Run \(executable.lastPathComponent)")
+        try requireUnclaimed(step: step, prefix: prefix)
         let loader = try readyLoader(step: step, prefix: prefix, runner: runner, flavor: flavor)
         guard FileManager.default.fileExists(atPath: executable.path(percentEncoded: false)) else {
             throw StepFailure(step: step, detail: String(localized: "No file at \(executable.path(percentEncoded: false))."))

app/Sources/NotProtonApp/Model/RunnerInstaller.swift

@@ -32,10 +32,23 @@ enum RunnerInstaller {
             defer { try? fm.removeItem(at: staging) }
             try copyPayload(from: install.crossOverRoot, to: staging)
             try verifyClone(build: build, root: staging.appending(path: "CrossOver"))
-            if occupied {
-                try fm.removeItem(at: target)
+            guard occupied else {
+                try fm.moveItem(at: staging, to: target)
+                return build
             }
-            try fm.moveItem(at: staging, to: target)
+            // Deleting in place can fail part way and leave half a runner. A fresh name each time
+            // keeps a leftover that would not delete from blocking the next repair.
+            let trash = target.deletingLastPathComponent()
+                .appending(path: ".\(target.lastPathComponent).\(UUID().uuidString).removing")
+            removeLeftoverRemovals(runners: runners)
+            try fm.moveItem(at: target, to: trash)
+            do {
+                try fm.moveItem(at: staging, to: target)
+            } catch {
+                try? fm.moveItem(at: trash, to: target)
+                throw error
+            }
+            try? fm.removeItem(at: trash)
             return build
         }
 

app/Sources/NotProtonApp/Model/SteamBundle.swift

@@ -44,7 +44,11 @@ enum SteamBundle {
         let deployedDylib = SupportPaths.Steam.deployedDylib(inBundle: app)
             .path(percentEncoded: false)
         let components = insert.split(separator: ":").map(String.init)
-        guard components.contains(deployedDylib) else { return .foreign(insert: insert) }
+        guard components.contains(deployedDylib) else {
+            // Our dylib at another path means Steam.app moved after install. Installing again rewrites it.
+            let others = components.filter { URL(filePath: $0).lastPathComponent != SupportPaths.dylibName }
+            return others.isEmpty ? .notInstalled : .foreign(insert: insert)
+        }
 
         guard files.fileExists(atPath: deployedDylib) else { return .notInstalled }
 

app/Sources/NotProtonApp/Model/SteamInstaller.swift

@@ -263,7 +263,9 @@ enum SteamInstaller {
         guard let insert = SteamBundle.currentInsert(at: plist), !insert.isEmpty else { return }
 
         let deployed = dylib.path(percentEncoded: false)
-        let foreign = insert.split(separator: ":").map(String.init).filter { $0 != deployed }
+        let foreign = insert.split(separator: ":").map(String.init).filter {
+            $0 != deployed && URL(filePath: $0).lastPathComponent != SupportPaths.dylibName
+        }
         guard foreign.isEmpty else {
             throw StepFailure(
                 step: step,

app/Sources/NotProtonApp/PrefixesModel.swift

@@ -10,7 +10,10 @@ final class PrefixesModel {
 
     private(set) var prefixes: [WinePrefix] = []
     private(set) var usage: [String: PrefixUsage] = [:]
-    private(set) var isLoading = false
+    // A load that finishes in the middle of a rebuild or backup must not end the loading state
+    var isLoading: Bool { listing || working }
+    private var listing = false
+    private var working = false
     private(set) var records: [String: PrefixBuildRecord] = [:]
     private(set) var tools: [InstalledTool] = []
 
@@ -87,8 +90,8 @@ final class PrefixesModel {
     func load() async {
         loadGeneration += 1
         let run = loadGeneration
-        isLoading = true
-        defer { if run == loadGeneration { isLoading = false } }
+        listing = true
+        defer { if run == loadGeneration { listing = false } }
 
         let roots = libraries()
         let found = await Task.detached { PrefixStore.all(libraries: roots) }.value
@@ -124,14 +127,14 @@ final class PrefixesModel {
         }
         kept.sort { ($0.taken ?? .distantPast) > ($1.taken ?? .distantPast) }
         backups = kept
+        // Sizes fill in as they are measured, so a big library does not hold up the list
+        hasLoaded = true
 
         for prefix in found {
             let measured = await Task.detached { PrefixStore.usage(of: prefix) }.value
             guard run == loadGeneration else { return }
             usage[prefix.id] = measured
         }
-
-        hasLoaded = true
     }
 
     func open(_ tool: WineTool, for prefix: WinePrefix) {
@@ -205,7 +208,7 @@ final class PrefixesModel {
     func deleteBackups(_ targets: [PrefixBackup]) async {
         guard !targets.isEmpty else { return }
         forgetOutcome()
-        isLoading = true
+        working = true
 
         var cleared: [PrefixBackup] = []
         var refused: [Error] = []
@@ -222,7 +225,7 @@ final class PrefixesModel {
         report = FailureReport(refused)
         let gone = Set(cleared.map(\.id))
         backups.removeAll { gone.contains($0.id) }
-        isLoading = false
+        working = false
         await load()
     }
 
@@ -248,7 +251,7 @@ final class PrefixesModel {
     ) async -> [Step<Made>] {
         guard !targets.isEmpty else { return [] }
         forgetOutcome()
-        isLoading = true
+        working = true
         busy = Set(targets.map(\.id))
 
         var done: [Step<Made>] = []
@@ -267,7 +270,7 @@ final class PrefixesModel {
         report = FailureReport(refused)
 
         busy = []
-        isLoading = false
+        working = false
         await load()
         return done
     }

app/Sources/NotProtonApp/SystemStatus.swift

@@ -415,6 +415,8 @@ final class SystemStatus {
         }
 
         await perform(from: RunnerSetup.Phase.cloning.label) { progress in
+            // Cached sizes outlive a recopy of the same build
+            if case .supported(let build) = install.support { runnerSizes[build.id] = nil }
             let lock = try DeploymentContent.acquireInstallationLock()
             defer { lock.release() }
             try await requireInstallableContent()

app/Sources/NotProtonApp/Views/PrefixesView.swift

@@ -383,6 +383,7 @@ struct PrefixesView: View {
             if let prefix { model.chooseExecutable(for: prefix) }
         }
         Button("Winetricks…") {
+            winetricksVerbs = ""
             winetricksPrefix = prefix
         }
         Divider()

app/Sources/NotProtonApp/Views/StatusView.swift

@@ -564,7 +564,7 @@ struct StatusView: View {
                     label: String(localized: "Repair"),
                     isProminent: true,
                     help: String(localized: "Set up the compatibility tool again."),
-                    isEnabled: status.isIdle && status.setupSource != nil
+                    isEnabled: status.canInstall && status.setupSource != nil
                 ) { Task { await status.requestCompatibilityTool() } }
             )
         }

app/Tests/NotProtonAppTests/PrefixStoreTests.swift

@@ -50,6 +50,53 @@ struct PrefixStoreTests {
         #expect(libraries.last?.compatdata.lastPathComponent == "compatdata")
     }
 
+    @Test("A library path with an escaped quote or backslash is read whole")
+    func readsEscapedLibraryPaths() throws {
+        let dir = try scratchDirectory("pfx")
+        defer { try? FileManager.default.removeItem(at: dir) }
+
+        let vdf = dir.appending(path: "libraryfolders.vdf")
+        try write(
+            #"""
+            "libraryfolders"
+            {
+            \#t"0"
+            \#t{
+            \#t\#t"path"\#t\#t"/Volumes/Bob\"s Drive/Back\\Slash/SteamLibrary"
+            \#t}
+            }
+            """#, to: vdf)
+
+        let libraries = PrefixStore.libraries(vdf: vdf)
+        #expect(libraries.map { $0.root.path(percentEncoded: false) } == [#"/Volumes/Bob"s Drive/Back\Slash/SteamLibrary"#])
+    }
+
+    // The old splitter dropped empty fields, so an empty path was never read as a library
+    @Test("An empty library path is skipped")
+    func skipsEmptyLibraryPath() throws {
+        let dir = try scratchDirectory("pfx")
+        defer { try? FileManager.default.removeItem(at: dir) }
+
+        let vdf = dir.appending(path: "libraryfolders.vdf")
+        try write(
+            #"""
+            "libraryfolders"
+            {
+            \#t"0"
+            \#t{
+            \#t\#t"path"\#t\#t""
+            \#t}
+            \#t"1"
+            \#t{
+            \#t\#t"path"\#t\#t"/Volumes/Games/SteamLibrary"
+            \#t}
+            }
+            """#, to: vdf)
+
+        let libraries = PrefixStore.libraries(vdf: vdf)
+        #expect(libraries.map { $0.root.path(percentEncoded: false) } == ["/Volumes/Games/SteamLibrary"])
+    }
+
     @Test("A missing or empty file still yields the default library")
     func fallsBackToTheDefaultLibrary() throws {
         let dir = try scratchDirectory("pfx")