From fbfcec34534d4ece73ce779d0d39fdf951835794 Mon Sep 17 00:00:00 2001 From: Zorg Date: Fri, 19 Aug 2022 21:36:50 -0700 Subject: [PATCH] Fix recently introduced bugs in generate_appcast with multiple feeds (#2232) We fix the validation check for using the output option when multiple feeds are present, fix not generating new feeds when no prior feed is available, and fix incorrect logic with cleaning up delta files when multiple feeds are present. --- generate_appcast/Appcast.swift | 113 ++++++++++++++++++--------------- generate_appcast/main.swift | 21 +++--- 2 files changed, 74 insertions(+), 60 deletions(-) diff --git a/generate_appcast/Appcast.swift b/generate_appcast/Appcast.swift index 41a59074..b60548d8 100644 --- a/generate_appcast/Appcast.swift +++ b/generate_appcast/Appcast.swift @@ -41,12 +41,6 @@ func makeAppcasts(archivesSourceDir: URL, outputPathURL: URL?, cacheDirectory ca update.downloadUrlPrefix = downloadURLPrefix update.releaseNotesURLPrefix = releaseNotesURLPrefix } - - // If a (single) output filename was specified on the command-line, but more than one - // appcast file was found in the archives, then it's an error. - if let outputPathURL = outputPathURL, allUpdates.count > 1 { - throw makeError(code: .appcastError, "Cannot write to \(outputPathURL.path): multiple appcasts found") - } // Group updates by appcast feed var updatesByAppcast: [FeedName: [ArchiveItem]] = [:] @@ -55,6 +49,12 @@ func makeAppcasts(archivesSourceDir: URL, outputPathURL: URL?, cacheDirectory ca updatesByAppcast[appcastFile, default: []].append(update) } + // If a (single) output filename was specified on the command-line, but more than one + // appcast file was found in the archives, then it's an error. + if let outputPathURL = outputPathURL, updatesByAppcast.count > 1 { + throw makeError(code: .appcastError, "Cannot write to \(outputPathURL.path): multiple appcasts found") + } + let group = DispatchGroup() var appcastByFeed: [FeedName: Appcast] = [:] @@ -66,13 +66,14 @@ func makeAppcasts(archivesSourceDir: URL, outputPathURL: URL?, cacheDirectory ca let feedURL = outputPathURL ?? archivesSourceDir.appendingPathComponent(feed) - guard let reachable = try? feedURL.checkResourceIsReachable(), reachable else { - continue + // Find all the update versions & branches from our existing feed (if available) + let feedUpdateBranches: [UpdateVersion: UpdateBranch] + if let reachable = try? feedURL.checkResourceIsReachable(), reachable { + feedUpdateBranches = try readAppcast(archives: archivesTable, appcastURL: feedURL) + } else { + feedUpdateBranches = [:] } - // Find all the update versions & branches from our existing feed - let feedUpdateBranches: [UpdateVersion: UpdateBranch] = try readAppcast(archives: archivesTable, appcastURL: feedURL) - // Find which versions are new and old but aren't in the feed and that we should ignore/skip var ignoredVersionsToInsert: Set = Set() var ignoredOldVersions: Set = Set() @@ -365,7 +366,7 @@ func makeAppcasts(archivesSourceDir: URL, outputPathURL: URL?, cacheDirectory ca return appcastByFeed } -func moveOldUpdatesFromAppcast(archivesSourceDir: URL, oldFilesDirectory: URL, cacheDirectory: URL, appcast: Appcast, autoPruneUpdates: Bool) -> (movedCount: Int, prunedCount: Int) { +func moveOldUpdatesFromAppcasts(archivesSourceDir: URL, oldFilesDirectory: URL, cacheDirectory: URL, appcasts: [Appcast], autoPruneUpdates: Bool) -> (movedCount: Int, prunedCount: Int) { let fileManager = FileManager.default let suFileManager = SUFileManager() @@ -396,50 +397,52 @@ func moveOldUpdatesFromAppcast(archivesSourceDir: URL, oldFilesDirectory: URL, c var movedItemsCount = 0 // Move aside all old unused update items - let versionsInFeedSet = Set(appcast.versionsInFeed) - for (version, update) in appcast.archives { - guard !versionsInFeedSet.contains(version) && !appcast.deltaFromVersionsUsed.contains(version) && !appcast.ignoredVersionsToInsert.contains(version) else { - continue - } - - let archivePath = update.archivePath - - guard makeOldFilesDirectory() else { - return (movedItemsCount, 0) - } - - do { - try suFileManager.updateModificationAndAccessTimeOfItem(at: archivePath) - } catch { - print("Warning: failed to update modification time for \(archivePath.path): \(error)") - } - - do { - try fileManager.moveItem(at: archivePath, to: oldFilesDirectory.appendingPathComponent(archivePath.lastPathComponent)) + for appcast in appcasts { + let versionsInFeedSet = Set(appcast.versionsInFeed) + for (version, update) in appcast.archives { + guard !versionsInFeedSet.contains(version) && !appcast.deltaFromVersionsUsed.contains(version) && !appcast.ignoredVersionsToInsert.contains(version) else { + continue + } - movedItemsCount += 1 + let archivePath = update.archivePath - // Remove cache for the update - let appCachePath = update.appPath.deletingLastPathComponent() - let _ = try? fileManager.removeItem(at: appCachePath) - } catch { - print("Warning: failed to move \(archivePath.lastPathComponent) to \(oldFilesDirectory.lastPathComponent): \(error)") - } - - let releaseNotesFile = archivePath.deletingPathExtension().appendingPathExtension("html") - if fileManager.fileExists(atPath: releaseNotesFile.path) { - do { - try suFileManager.updateModificationAndAccessTimeOfItem(at: releaseNotesFile) - } catch { - print("Warning: failed to update modification time for \(releaseNotesFile.path): \(error)") + guard makeOldFilesDirectory() else { + return (movedItemsCount, 0) } do { - try fileManager.moveItem(at: releaseNotesFile, to: oldFilesDirectory.appendingPathComponent(releaseNotesFile.lastPathComponent)) + try suFileManager.updateModificationAndAccessTimeOfItem(at: archivePath) + } catch { + print("Warning: failed to update modification time for \(archivePath.path): \(error)") + } + + do { + try fileManager.moveItem(at: archivePath, to: oldFilesDirectory.appendingPathComponent(archivePath.lastPathComponent)) movedItemsCount += 1 + + // Remove cache for the update + let appCachePath = update.appPath.deletingLastPathComponent() + let _ = try? fileManager.removeItem(at: appCachePath) } catch { - print("Warning: failed to move \(releaseNotesFile.lastPathComponent) to \(oldFilesDirectory.lastPathComponent): \(error)") + print("Warning: failed to move \(archivePath.lastPathComponent) to \(oldFilesDirectory.lastPathComponent): \(error)") + } + + let releaseNotesFile = archivePath.deletingPathExtension().appendingPathExtension("html") + if fileManager.fileExists(atPath: releaseNotesFile.path) { + do { + try suFileManager.updateModificationAndAccessTimeOfItem(at: releaseNotesFile) + } catch { + print("Warning: failed to update modification time for \(releaseNotesFile.path): \(error)") + } + + do { + try fileManager.moveItem(at: releaseNotesFile, to: oldFilesDirectory.appendingPathComponent(releaseNotesFile.lastPathComponent)) + + movedItemsCount += 1 + } catch { + print("Warning: failed to move \(releaseNotesFile.lastPathComponent) to \(oldFilesDirectory.lastPathComponent): \(error)") + } } } } @@ -455,8 +458,18 @@ func moveOldUpdatesFromAppcast(archivesSourceDir: URL, oldFilesDirectory: URL, c } let deltaURL = archivesSourceDir.appendingPathComponent(filename) - guard !appcast.deltaPathsUsed.contains(deltaURL.path) else { - continue + do { + var foundDeltaItemUsage = false + for appcast in appcasts { + if appcast.deltaPathsUsed.contains(deltaURL.path) { + foundDeltaItemUsage = true + break + } + } + + guard !foundDeltaItemUsage else { + continue + } } guard makeOldFilesDirectory() else { diff --git a/generate_appcast/main.swift b/generate_appcast/main.swift index 9ff18ac8..6a8f5686 100644 --- a/generate_appcast/main.swift +++ b/generate_appcast/main.swift @@ -273,6 +273,8 @@ struct GenerateAppcast: ParsableCommand { let oldFilesDirectory = archivesSourceDir.appendingPathComponent(GenerateAppcast.oldFilesDirectoryName) + let pluralizeWord = { $0 == 1 ? $1 : "\($1)s" } + for (appcastFile, appcast) in appcastsByFeed { // If an output filename was specified, use it. // Otherwise, use the name of the appcast file found in the archive. @@ -283,21 +285,20 @@ struct GenerateAppcast: ParsableCommand { let (numNewUpdates, numExistingUpdates, numUpdatesRemoved) = try writeAppcast(appcastDestPath: appcastDestPath, appcast: appcast, fullReleaseNotesLink: fullReleaseNotesURL, maxCDATAThreshold: maxCDATAThreshold, link: link, newChannel: channel, majorVersion: majorVersion, ignoreSkippedUpgradesBelowVersion: ignoreSkippedUpgradesBelowVersion, phasedRolloutInterval: phasedRolloutInterval, criticalUpdateVersion: criticalUpdateVersion, informationalUpdateVersions: informationalUpdateVersions) // Inform the user, pluralizing "update" if necessary - let pluralizeWord = { $0 == 1 ? $1 : "\($1)s" } let pluralizeUpdates = { pluralizeWord($0, "update") } let newUpdatesString = pluralizeUpdates(numNewUpdates) let existingUpdatesString = pluralizeUpdates(numExistingUpdates) let removedUpdatesString = pluralizeUpdates(numUpdatesRemoved) - print("Wrote \(numNewUpdates) new \(newUpdatesString), updated \(numExistingUpdates) existing \(existingUpdatesString), and removed \(numUpdatesRemoved) old \(removedUpdatesString)") - - let (moveCount, prunedCount) = moveOldUpdatesFromAppcast(archivesSourceDir: archivesSourceDir, oldFilesDirectory: oldFilesDirectory, cacheDirectory: GenerateAppcast.cacheDirectory, appcast: appcast, autoPruneUpdates: autoPruneUpdates) - if moveCount > 0 { - print("Moved \(moveCount) old update \(pluralizeWord(moveCount, "file")) to \(oldFilesDirectory.lastPathComponent)") - } - if prunedCount > 0 { - print("Pruned \(prunedCount) old update \(pluralizeWord(prunedCount, "file"))") - } + print("Wrote \(numNewUpdates) new \(newUpdatesString), updated \(numExistingUpdates) existing \(existingUpdatesString), and removed \(numUpdatesRemoved) old \(removedUpdatesString) in \(appcastFile)") + } + + let (moveCount, prunedCount) = moveOldUpdatesFromAppcasts(archivesSourceDir: archivesSourceDir, oldFilesDirectory: oldFilesDirectory, cacheDirectory: GenerateAppcast.cacheDirectory, appcasts: Array(appcastsByFeed.values), autoPruneUpdates: autoPruneUpdates) + if moveCount > 0 { + print("Moved \(moveCount) old update \(pluralizeWord(moveCount, "file")) to \(oldFilesDirectory.lastPathComponent)") + } + if prunedCount > 0 { + print("Pruned \(prunedCount) old update \(pluralizeWord(prunedCount, "file"))") } } catch { print("Error generating appcast from directory", archivesSourceDir.path, "\n", error)