Merge pull request #29 from SwiftPackageIndex/fix-relative-path-zipping

Fix relative path zipping

Author: finestructure

.github/workflows/ci.yml

@@ -28,6 +28,6 @@ jobs:
       image: swift:5.10.0-amazonlinux2
     steps:
       - name: Checkout
-        uses: actions/checkout@v3
+        uses: actions/checkout@v4
       - name: Run tests
         run: swift test

.github/workflows/deploy.yml

@@ -27,7 +27,7 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           # Fetch all history to ensure the version number (tag) is resolved to create the
           # version info during the build step.

Sources/DocUploadBundle/DocUploadBundle.swift

@@ -110,14 +110,7 @@ public struct DocUploadBundle {
         let metadataURL = URL(fileURLWithPath: "\(workDir)/metadata.json")
         try JSONEncoder().encode(metadata).write(to: metadataURL)
 
-        switch method {
-            case .library, .zipTool(workingDirectory: .some(_)):
-                try Zipper.zip(paths: [metadataURL, URL(fileURLWithPath: sourcePath)], to: archiveURL, method: method)
-
-            case .zipTool(.none):
-                // By default, run the zip tool in the working directory
-                try Zipper.zip(paths: [metadataURL, URL(fileURLWithPath: sourcePath)], to: archiveURL, method: .zipTool(workingDirectory: workDir))
-        }
+        try Zipper.zip(paths: [metadataURL, URL(fileURLWithPath: sourcePath)], to: archiveURL, method: method)
 
         return archiveURL.path
     }

Tests/DocUploadBundleTests/TempDir.swift renamed to Sources/DocUploadBundle/TempDir.swift

@@ -32,3 +32,8 @@ func withTempDir<T>(body: (String) async throws -> T) async throws -> T {
     let tmp = try TempDir()
     return try await body(tmp.path)
 }
+
+func withTempDir<T>(body: (String) throws -> T) throws -> T {
+    let tmp = try TempDir()
+    return try body(tmp.path)
+}

Sources/DocUploadBundle/Zipper.swift

@@ -21,39 +21,63 @@ public enum Zipper {
     public static func zip(paths inputPaths: [URL], to outputPath: URL, method: Method = .library) throws {
         switch method {
             case .library:
-                do { try Zip.zipFiles(paths: inputPaths, zipFilePath: outputPath, password: nil, progress: nil) } 
-                catch ZipError.fileNotFound { throw Error.fileNotFound }
-                catch ZipError.unzipFail { throw Error.unzipFail }
-                catch ZipError.zipFail { throw Error.zipFail }
-                catch { throw Error.generic(reason: "\(error)") }
+                do {
+                    try Zip.zipFiles(paths: inputPaths, zipFilePath: outputPath, password: nil, progress: nil)
+                } catch let error as ZipError {
+                    switch error {
+                        case .fileNotFound: throw Error.fileNotFound
+                        case .unzipFail: throw Error.unzipFail
+                        case .zipFail: throw Error.zipFail
+                    }
+                }
+                catch {
+                    throw Error.generic(reason: "\(error)")
+                }
 
-            case let .zipTool(cwd):
+            case .zipTool:
                 do {
-                    let process = Process()
-                    process.executableURL = zip
-                    process.arguments = ["-q", "-r", outputPath.path] + inputPaths.map(\.lastPathComponent)
-                    process.currentDirectoryURL = cwd.map(URL.init(fileURLWithPath:))
-                    try process.run()
-                    process.waitUntilExit()
+                    try withTempDir { tempDir in
+                        let tempURL = URL(fileURLWithPath: tempDir)
+                        // Copy inputs to tempDir
+                        for source in inputPaths {
+                            let target = tempURL.appendingPathComponent(source.lastPathComponent)
+                            try FileManager.default.copyItem(at: source, to: target)
+                        }
+
+                        // Run zip
+                        let process = Process()
+                        process.executableURL = zip
+                        process.arguments = ["-q", "-r", outputPath.path] + inputPaths.map(\.lastPathComponent)
+                        process.currentDirectoryURL = tempURL
+                        try process.run()
+                        process.waitUntilExit()
+                    }
                 } catch {
                     throw Error.generic(reason: "\(error)")
                 }
         }
     }
 
     public static func unzip(from inputPath: URL, to outputPath: URL, fileOutputHandler: ((_ unzippedFile: URL) -> Void)? = nil) throws {
-        do { try Zip.unzipFile(inputPath, destination: outputPath, overwrite: true, password: nil, fileOutputHandler: fileOutputHandler) }
-        catch ZipError.fileNotFound { throw Error.fileNotFound }
-        catch ZipError.unzipFail { throw Error.unzipFail }
-        catch ZipError.zipFail { throw Error.zipFail }
-        catch { throw Error.generic(reason: "\(error)") }
+        do {
+            try Zip.unzipFile(inputPath, destination: outputPath, overwrite: true, password: nil, fileOutputHandler: fileOutputHandler)
+        } catch let error as ZipError {
+            switch error {
+                case .fileNotFound: throw Error.fileNotFound
+                case .unzipFail: throw Error.unzipFail
+                case .zipFail: throw Error.zipFail
+            }
+        }
+        catch {
+            throw Error.generic(reason: "\(error)")
+        }
     }
 
     static let zip = URL(fileURLWithPath: "/usr/bin/zip")
 
     public enum Method {
         case library
-        case zipTool(workingDirectory: String? = nil)
+        case zipTool
     }
 
     public enum Error: Swift.Error {

Tests/DocUploadBundleTests/ZipTests.swift

@@ -21,7 +21,7 @@ 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
+        try withTempDir { tempDir in
             let tempURL = URL(fileURLWithPath: tempDir)
             let zipFile = fixtureUrl(for: "out.zip")
             let outDir = tempURL.appendingPathComponent("out")
@@ -41,7 +41,7 @@ final class ZipTests: XCTestCase {
 
     func test_zip_roundtrip() async throws {
         // Test basic zip roundtrip
-        try await withTempDir { tempDir in
+        try withTempDir { tempDir in
             //  temp
             let tempURL = URL(fileURLWithPath: tempDir)
 
@@ -92,7 +92,7 @@ final class ZipTests: XCTestCase {
         try XCTSkipIf(!FileManager.default.fileExists(atPath: Zipper.zip.path))
 
         // Test basic zip roundtrip with the shellTool method
-        try await withTempDir { tempDir in
+        try withTempDir { tempDir in
             //  temp
             let tempURL = URL(fileURLWithPath: tempDir)
 
@@ -117,7 +117,7 @@ final class ZipTests: XCTestCase {
             try "c".write(to: fileC, atomically: true, encoding: .utf8)
 
             let zipFile = tempURL.appendingPathComponent("out.zip")
-            try Zipper.zip(paths: [fileA, subdir], to: zipFile, method: .zipTool(workingDirectory: tempDir))
+            try Zipper.zip(paths: [fileA, subdir], to: zipFile, method: .zipTool)
             XCTAssert(FileManager.default.fileExists(atPath: zipFile.path))
 
             do { // unzip what we zipped and check results
@@ -139,4 +139,41 @@ final class ZipTests: XCTestCase {
         }
     }
 
+    func test_zip_roundtrip_shellTool_relative_paths() async throws {
+        try XCTSkipIf(!FileManager.default.fileExists(atPath: Zipper.zip.path))
+
+        // Test basic zip roundtrip with the shellTool method and relative paths
+        try withTempDir { tempDir in
+            // DocBundle components
+            // metadataURL: tempDir/metadata.json
+            // sourceURL:   tempDir/.docs/owner/repo/ref
+            // should be zipped as
+            //   - metadata.json
+            //   - ref
+            // at the top level as relative paths.
+            let tempURL = URL(fileURLWithPath: tempDir)
+            let metadataURL = tempURL.appendingPathComponent("metadata.json")
+            try "metadata".write(to: metadataURL, atomically: true, encoding: .utf8)
+            let sourceURL = tempURL.appendingPathComponent("docs/owner/repo/ref")
+            try FileManager.default.createDirectory(at: sourceURL, withIntermediateDirectories: true)
+            let indexHTML = sourceURL.appendingPathComponent("index.html")
+            try "index".write(to: indexHTML, atomically: true, encoding: .utf8)
+
+            // MUT
+            let zipFile = tempURL.appendingPathComponent("out.zip")
+            try Zipper.zip(paths: [metadataURL, sourceURL], to: zipFile, method: .zipTool)
+
+            do {  // validate
+                let unzipDir = tempURL.appendingPathComponent("unzip")
+                try Zipper.unzip(from: zipFile, to: unzipDir)
+                let metadataURL = unzipDir.appendingPathComponent("metadata.json")
+                let indexHTML = unzipDir.appendingPathComponent("ref/index.html")
+                XCTAssert(FileManager.default.fileExists(atPath: metadataURL.path))
+                XCTAssert(FileManager.default.fileExists(atPath: indexHTML.path))
+                XCTAssertEqual(try String(contentsOf: metadataURL), "metadata")
+                XCTAssertEqual(try String(contentsOf: indexHTML), "index")
+            }
+        }
+    }
+
 }