Skip to content

Fix swift-metrics unzipping #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 1, 2024
Merged

Fix swift-metrics unzipping #27

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1544443
Add swift-dependencies regression test
finestructure Jun 27, 2024
dc4ba4b
Extend test to show round-tripping fails
finestructure Jun 27, 2024
886b06f
Update ZipTests to show that ZipFoundation isn't recursing into direc…
finestructure Jun 27, 2024
377bfe4
Dropping superfluous test
finestructure Jun 27, 2024
a614c8b
Revert "Merge pull request #23 from SwiftPackageIndex/switch-zip-libr…
finestructure Jun 27, 2024
07db6e4
Fall back to os level unzip in case an unzip fails
finestructure Jun 27, 2024
65f65be
Fix Linux compile issues
finestructure Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,12 @@
}
},
{
"identity" : "zipfoundation",
"identity" : "zip",
"kind" : "remoteSourceControl",
"location" : "https://github.com/weichsel/ZIPFoundation.git",
"location" : "https://github.com/marmelroy/Zip.git",
"state" : {
"revision" : "02b6abe5f6eef7e3cbd5f247c5cc24e246efcfe0",
"version" : "0.9.19"
"revision" : "67fa55813b9e7b3b9acee9c0ae501def28746d76",
"version" : "2.1.2"
}
}
],
Expand Down
4 changes: 2 additions & 2 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ let package = Package(
.package(url: "https://github.com/swift-server/swift-aws-lambda-events.git", from: "0.1.0"),
.package(url: "https://github.com/swift-server/swift-aws-lambda-runtime.git", from: "1.0.0-alpha.1"),
.package(url: "https://github.com/soto-project/soto-s3-file-transfer.git", from: "1.2.0"),
.package(url: "https://github.com/weichsel/ZIPFoundation.git", from: "0.9.19"),
.package(url: "https://github.com/marmelroy/Zip.git", from: "2.1.2"),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't use ZIPFoundation, because it doesn't actually zip directories recursively. Best I can tell there's no way to do it other than by listing all the files ourselves. marmelroy/Zip supports that, so it's easier to stick with it for now.

.package(url: "https://github.com/pointfreeco/swift-dependencies.git", from: "1.0.0")
],
targets: [
Expand All @@ -56,7 +56,7 @@ let package = Package(
linkerSettings: linkerSettings
),
.target(name: "DocUploadBundle", dependencies: [
.product(name: "ZIPFoundation", package: "zipfoundation"),
.product(name: "Zip", package: "Zip"),
.product(name: "Dependencies", package: "swift-dependencies")
]),
.testTarget(name: "DocUploadBundleTests", dependencies: ["DocUploadBundle"], exclude: ["Fixtures"]),
Expand Down
18 changes: 11 additions & 7 deletions Sources/DocUploadBundle/Zipper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,24 @@

import Foundation

import ZIPFoundation
import Zip


enum Zipper {
static func zip(paths inputPaths: [URL], to outputPath: URL) throws {
let archive = try Archive(url: outputPath, accessMode: .create)
for url in inputPaths {
try archive.addEntry(with: url.lastPathComponent, fileURL: url)
}
try Zip.zipFiles(paths: inputPaths, zipFilePath: outputPath, password: nil, progress: nil)
}

static func unzip(from inputPath: URL, to outputPath: URL, fileOutputHandler: ((_ unzippedFile: URL) -> Void)? = nil) throws {
try FileManager.default.createDirectory(at: outputPath, withIntermediateDirectories: true)
try FileManager.default.unzipItem(at: inputPath, to: outputPath)
do {
try Zip.unzipFile(inputPath, destination: outputPath, overwrite: true, password: nil, fileOutputHandler: fileOutputHandler)
} catch ZipError.unzipFail {
// Try OS level unzip as a fallback
// See https://github.com/SwiftPackageIndex/SwiftPackageIndex-Server/issues/3069
let unzip = URL(fileURLWithPath: "/usr/bin/unzip")
let process = try Process.run(unzip, arguments: ["-q", inputPath.path, "-d", outputPath.path])
process.waitUntilExit()
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've verified that /usr/bin/unzip is available at that path on the amazon linux image.

}
}

Expand Down
25 changes: 21 additions & 4 deletions Tests/DocUploadBundleTests/DocUploadBundleTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,11 +105,28 @@ final class DocUploadBundleTests: XCTestCase {
try await withTempDir { tempDir in
let url = fixtureUrl(for: "prod-apple-swift-metrics-main-e6a00d36.zip")
XCTAssertNoThrow(
try DocUploadBundle.unzip(bundle: url.path, outputPath: tempDir)
try DocUploadBundle.unzip(bundle: url.path(), outputPath: tempDir)
)
XCTAssert(FileManager.default.fileExists(atPath: tempDir + "/metadata.json"))
XCTAssert(FileManager.default.fileExists(atPath: tempDir + "/main/index.html"))
XCTAssert(FileManager.default.fileExists(atPath: tempDir + "/main/index/index.json"))
for pathComponent in ["metadata.json",
"main/index.html",
"main/index/index.json"] {
let path = tempDir + "/" + pathComponent
XCTAssertTrue(FileManager.default.fileExists(atPath: path), "does not exist: \(path)")
}
// test roundtrip, to ensure the zip library can zip/unzip its own product
// zip
let urls = [tempDir + "/metadata.json",
tempDir + "/main"].map(URL.init(fileURLWithPath:))
let zipped = URL(fileURLWithPath: tempDir + "/out.zip")
try Zipper.zip(paths: urls, to: zipped)
XCTAssertTrue(FileManager.default.fileExists(atPath: zipped.path()))
// unzip
let out = URL(fileURLWithPath: tempDir + "/out")
try Zipper.unzip(from: zipped, to: out)
XCTAssertTrue(FileManager.default.fileExists(atPath: out.path()))
XCTAssertTrue(FileManager.default.fileExists(atPath: out.appendingPathComponent("metadata.json").path()))
XCTAssertTrue(FileManager.default.fileExists(atPath: out.appendingPathComponent("main/index.html").path()))
XCTAssertTrue(FileManager.default.fileExists(atPath: out.appendingPathComponent("main/index/index.json").path()))
}
}

Expand Down
56 changes: 35 additions & 21 deletions Tests/DocUploadBundleTests/ZipTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,29 @@ import XCTest


final class ZipTests: XCTestCase {

func test_unzip() async throws {
// Test basic unzip behaviour we expect from the library we use
try await withTempDir { tempDir in
let tempURL = URL(fileURLWithPath: tempDir)
let zipFile = fixtureUrl(for: "out.zip")
let outDir = tempURL.appendingPathComponent("out")
try Zipper.unzip(from: zipFile, to: outDir)
XCTAssert(FileManager.default.fileExists(atPath: outDir.path))

// out/a.txt
// out/subdir/b.txt
let fileA = outDir.appendingPathComponent("a.txt")
let fileB = outDir.appendingPathComponent("subdir").appendingPathComponent("b.txt")
XCTAssert(FileManager.default.fileExists(atPath: fileA.path))
XCTAssert(FileManager.default.fileExists(atPath: fileB.path))
XCTAssertEqual(try String(contentsOf: fileA), "a")
XCTAssertEqual(try String(contentsOf: fileB), "b")
}
}

func test_zip() async throws {
// Test basic zip behaviour we expect from the library we use
func test_zip_roundtrip() async throws {
// Test basic zip roundtrip
try await withTempDir { tempDir in
// temp
let tempURL = URL(fileURLWithPath: tempDir)
Expand All @@ -40,26 +60,20 @@ final class ZipTests: XCTestCase {
let zipFile = tempURL.appendingPathComponent("out.zip")
try Zipper.zip(paths: [fileA, subdir], to: zipFile)
XCTAssert(FileManager.default.fileExists(atPath: zipFile.path))
}
}

func test_unzip() async throws {
// Test basic unzip behaviour we expect from the library we use
try await withTempDir { tempDir in
let tempURL = URL(fileURLWithPath: tempDir)
let zipFile = fixtureUrl(for: "out.zip")
let outDir = tempURL.appendingPathComponent("out")
try Zipper.unzip(from: zipFile, to: outDir)
XCTAssert(FileManager.default.fileExists(atPath: outDir.path))

// out/a.txt
// out/subdir/b.txt
let fileA = outDir.appendingPathComponent("a.txt")
let fileB = outDir.appendingPathComponent("subdir").appendingPathComponent("b.txt")
XCTAssert(FileManager.default.fileExists(atPath: fileA.path))
XCTAssert(FileManager.default.fileExists(atPath: fileB.path))
XCTAssertEqual(try String(contentsOf: fileA), "a")
XCTAssertEqual(try String(contentsOf: fileB), "b")
do { // unzip what we zipped and check results
let roundtrip = tempURL.appendingPathComponent("roundtrip")
try Zipper.unzip(from: zipFile, to: roundtrip)
XCTAssert(FileManager.default.fileExists(atPath: roundtrip.path))
// roundtrip/a.txt
// roundtrip/subdir/b.txt
let fileA = roundtrip.appendingPathComponent("a.txt")
let fileB = roundtrip.appendingPathComponent("subdir").appendingPathComponent("b.txt")
XCTAssert(FileManager.default.fileExists(atPath: fileA.path))
XCTAssert(FileManager.default.fileExists(atPath: fileB.path))
XCTAssertEqual(try String(contentsOf: fileA), "a")
XCTAssertEqual(try String(contentsOf: fileB), "b")
}
}
}

Expand Down
Loading