From c7a996f379aa255bcb7f47e2b559ec57f38e4091 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pedro=20Pi=C3=B1era=20Buend=C3=ADa?= Date: Wed, 19 Sep 2018 18:43:43 +0200 Subject: [PATCH] Check Swift version before installing (#134) * Check Swift version before installing * Test changes * Include the type of error to catch * Add CHANGELOG entry --- CHANGELOG.md | 4 +++ Sources/TuistCore/Utils/System.swift | 11 ++++++++ .../TuistCoreTesting/Utils/MockSystem.swift | 5 ++++ Sources/TuistEnvKit/GitHub/GitHubClient.swift | 24 +++++++++++++++-- .../GitHub/GitHubRequestsFactory.swift | 17 +++++++++--- Sources/TuistEnvKit/Installer/Installer.swift | 26 ++++++++++++++++++- .../TuistKit/Generator/LinkGenerator.swift | 4 +-- .../GitHub/GitHubClientTests .swift | 5 ++-- .../GitHub/GitHubRequestsPoviderTests.swift | 14 +++++++--- .../GitHub/Mocks/MockGitHubClient.swift | 5 ++++ .../Installer/InstallerTests.swift | 20 ++++++++++++++ 11 files changed, 122 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 47d1b6e22..d8b5ce23c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ Please, check out guidelines: https://keepachangelog.com/en/1.0.0/ ## Next version +### Added + +- Check that the local Swift version is compatible with the version that will be installed https://github.com/tuist/tuist/pull/134 by @pepibumur. + ### Changed - Bump xcodeproj to 6.0.0 https://github.com/tuist/tuist/pull/133 by @pepibumur. diff --git a/Sources/TuistCore/Utils/System.swift b/Sources/TuistCore/Utils/System.swift index f9b49dfe5..9964fd3d4 100644 --- a/Sources/TuistCore/Utils/System.swift +++ b/Sources/TuistCore/Utils/System.swift @@ -8,6 +8,8 @@ public protocol Systeming { func capture(_ args: String..., verbose: Bool) throws -> SystemResult func popen(_ args: String..., verbose: Bool) throws func popen(_ args: [String], verbose: Bool) throws + + func swiftVersion() throws -> String? } public struct SystemError: FatalError, Equatable { @@ -51,6 +53,8 @@ public struct SystemResult { } public final class System: Systeming { + private static var swiftVersionRegex = try! NSRegularExpression(pattern: "Apple Swift version\\s(.+)\\s\\(.+\\)", options: []) + // MARK: - Attributes let printer: Printing @@ -89,6 +93,13 @@ public final class System: Systeming { _ = task(arguments, print: true).wait() } + public func swiftVersion() throws -> String? { + let output = try capture("swift", "--version").throwIfError().stdout + let range = NSRange(location: 0, length: output.count) + guard let match = System.swiftVersionRegex.firstMatch(in: output, options: [], range: range) else { return nil } + return NSString(string: output).substring(with: match.range(at: 1)).chomp() + } + // MARK: - Fileprivate fileprivate func task(_ args: [String], print: Bool = false) -> SignalProducer { diff --git a/Sources/TuistCoreTesting/Utils/MockSystem.swift b/Sources/TuistCoreTesting/Utils/MockSystem.swift index 164f58cb6..191d35beb 100644 --- a/Sources/TuistCoreTesting/Utils/MockSystem.swift +++ b/Sources/TuistCoreTesting/Utils/MockSystem.swift @@ -4,6 +4,7 @@ import TuistCore public final class MockSystem: Systeming { private var stubs: [String: (stderror: String?, stdout: String?, exitstatus: Int32?)] = [:] private var calls: [String] = [] + var swiftVersionStub: (() throws -> String?)? public init() {} @@ -41,6 +42,10 @@ public final class MockSystem: Systeming { } } + public func swiftVersion() throws -> String? { + return try swiftVersionStub?() + } + public func called(_ args: String...) -> Bool { let command = args.joined(separator: " ") return calls.contains(command) diff --git a/Sources/TuistEnvKit/GitHub/GitHubClient.swift b/Sources/TuistEnvKit/GitHub/GitHubClient.swift index ba18d62fb..a131fe11c 100644 --- a/Sources/TuistEnvKit/GitHub/GitHubClient.swift +++ b/Sources/TuistEnvKit/GitHub/GitHubClient.swift @@ -5,29 +5,34 @@ import Utility protocol GitHubClienting: AnyObject { func releases() throws -> [Release] func release(tag: String) throws -> Release + func getContent(ref: String, path: String) throws -> String } enum GitHubClientError: FatalError { case sessionError(Error) case missingData case decodingError(Error) + case invalidResponse var type: ErrorType { switch self { case .sessionError: return .abort case .missingData: return .abort case .decodingError: return .bug + case .invalidResponse: return .bug } } var description: String { switch self { case let .sessionError(error): - return "Session error: \(error.localizedDescription)." + return "Session error: \(error.localizedDescription)" case .missingData: - return "No data received from the GitHub API." + return "No data received from the GitHub API" case let .decodingError(error): return "Error decoding JSON from API: \(error.localizedDescription)" + case .invalidResponse: + return "Received an invalid response from the GitHub API" } } } @@ -79,4 +84,19 @@ class GitHubClient: GitHubClienting { throw GitHubClientError.decodingError(error) } } + + func getContent(ref: String, path: String) throws -> String { + let data = try execute(request: requestFactory.getContent(ref: ref, path: path)) + do { + guard let json = try JSONSerialization.jsonObject(with: data, options: []) as? [String: Any], + let content = json["content"] as? String, + let base64Data = Data(base64Encoded: content.chomp()), + let decodedContent = String(data: base64Data, encoding: .utf8) else { + throw GitHubClientError.invalidResponse + } + return decodedContent + } catch { + throw GitHubClientError.decodingError(error) + } + } } diff --git a/Sources/TuistEnvKit/GitHub/GitHubRequestsFactory.swift b/Sources/TuistEnvKit/GitHub/GitHubRequestsFactory.swift index b0c038431..4bd5c0c78 100644 --- a/Sources/TuistEnvKit/GitHub/GitHubRequestsFactory.swift +++ b/Sources/TuistEnvKit/GitHub/GitHubRequestsFactory.swift @@ -3,7 +3,7 @@ import Foundation class GitHubRequestsFactory { /// MARK: - Constants - static let releasesRepository: String = "tuist/tuist" + static let repository: String = "tuist/tuist" // MARK: - Attributes @@ -16,7 +16,7 @@ class GitHubRequestsFactory { } func releases() -> URLRequest { - let path = "/repos/\(GitHubRequestsFactory.releasesRepository)/releases" + let path = "/repos/\(GitHubRequestsFactory.repository)/releases" let url = baseURL.appendingPathComponent(path) var request = URLRequest(url: url) request.httpMethod = "GET" @@ -24,10 +24,21 @@ class GitHubRequestsFactory { } func release(tag: String) -> URLRequest { - let path = "/repos/\(GitHubRequestsFactory.releasesRepository)/releases/tags/\(tag)" + let path = "/repos/\(GitHubRequestsFactory.repository)/releases/tags/\(tag)" let url = baseURL.appendingPathComponent(path) var request = URLRequest(url: url) request.httpMethod = "GET" return request } + + func getContent(ref: String, path: String) -> URLRequest { + let path = "/repos/\(GitHubRequestsFactory.repository)/contents/\(path)" + let url = baseURL.appendingPathComponent(path) + var components = URLComponents(url: url, resolvingAgainstBaseURL: true)! + components.queryItems = [] + components.queryItems?.append(URLQueryItem(name: "ref", value: ref)) + var request = URLRequest(url: components.url!) + request.httpMethod = "GET" + return request + } } diff --git a/Sources/TuistEnvKit/Installer/Installer.swift b/Sources/TuistEnvKit/Installer/Installer.swift index 546021f64..420f18ed0 100644 --- a/Sources/TuistEnvKit/Installer/Installer.swift +++ b/Sources/TuistEnvKit/Installer/Installer.swift @@ -8,17 +8,21 @@ protocol Installing: AnyObject { enum InstallerError: FatalError, Equatable { case versionNotFound(String) + case incompatibleSwiftVersion(local: String, expected: String) var type: ErrorType { switch self { case .versionNotFound: return .abort + case .incompatibleSwiftVersion: return .abort } } var description: String { switch self { case let .versionNotFound(version): - return "Version \(version) not found." + return "Version \(version) not found" + case let .incompatibleSwiftVersion(local, expected): + return "Found \(local) Swift version but expected \(expected)" } } @@ -26,6 +30,10 @@ enum InstallerError: FatalError, Equatable { switch (lhs, rhs) { case let (.versionNotFound(lhsVersion), .versionNotFound(rhsVersion)): return lhsVersion == rhsVersion + case let (.incompatibleSwiftVersion(lhsLocal, lhsExpected), .incompatibleSwiftVersion(rhsLocal, rhsExpected)): + return lhsLocal == rhsLocal && lhsExpected == rhsExpected + default: + return false } } } @@ -64,6 +72,8 @@ final class Installer: Installing { } func install(version: String, temporaryDirectory: TemporaryDirectory) throws { + try verifySwiftVersion(version: version) + var bundleURL: URL? do { bundleURL = try self.bundleURL(version: version) @@ -79,6 +89,20 @@ final class Installer: Installing { } } + func verifySwiftVersion(version: String) throws { + guard let localVersion = try system.swiftVersion() else { return } + printer.print("Verifying the Swift version is compatible with your version \(localVersion)") + var remoteVersion: String! + do { + remoteVersion = try githubClient.getContent(ref: version, path: ".swift-version").chomp() + } catch is GitHubClientError { + printer.print(warning: "Couldn't get the Swift version needed for \(version). Continuing...") + } + if remoteVersion != nil && localVersion != remoteVersion { + throw InstallerError.incompatibleSwiftVersion(local: localVersion, expected: remoteVersion) + } + } + func bundleURL(version: String) throws -> URL? { guard let release = try? githubClient.release(tag: version) else { printer.print(warning: "The release \(version) couldn't be obtained from GitHub") diff --git a/Sources/TuistKit/Generator/LinkGenerator.swift b/Sources/TuistKit/Generator/LinkGenerator.swift index 8143ccb9f..8659ed902 100644 --- a/Sources/TuistKit/Generator/LinkGenerator.swift +++ b/Sources/TuistKit/Generator/LinkGenerator.swift @@ -141,14 +141,14 @@ final class LinkGenerator: LinkGenerating { } .map({ $0.removingLastComponent() }) .map({ $0.relative(to: sourceRootPath).asString }) - .sorted() .map({ "$(SRCROOT)/\($0)" }) + if paths.isEmpty { return } let configurationList = pbxTarget.buildConfigurationList let buildConfigurations = configurationList?.buildConfigurations - let pathsValue = Set(paths).joined(separator: " ") + let pathsValue = Set(paths).sorted().joined(separator: " ") buildConfigurations?.forEach { buildConfiguration in var frameworkSearchPaths = (buildConfiguration.buildSettings["FRAMEWORK_SEARCH_PATHS"] as? String) ?? "" if frameworkSearchPaths.isEmpty { diff --git a/Tests/TuistEnvKitTests/GitHub/GitHubClientTests .swift b/Tests/TuistEnvKitTests/GitHub/GitHubClientTests .swift index fa0243a85..352a9c9a6 100644 --- a/Tests/TuistEnvKitTests/GitHub/GitHubClientTests .swift +++ b/Tests/TuistEnvKitTests/GitHub/GitHubClientTests .swift @@ -5,9 +5,10 @@ import XCTest final class GitHubClientErrorTests: XCTestCase { func test_errorDescription() { let error = NSError(domain: "test", code: 1, userInfo: nil) - XCTAssertEqual(GitHubClientError.sessionError(error).description, "Session error: \(error.localizedDescription).") - XCTAssertEqual(GitHubClientError.missingData.description, "No data received from the GitHub API.") + XCTAssertEqual(GitHubClientError.sessionError(error).description, "Session error: \(error.localizedDescription)") + XCTAssertEqual(GitHubClientError.missingData.description, "No data received from the GitHub API") XCTAssertEqual(GitHubClientError.decodingError(error).description, "Error decoding JSON from API: \(error.localizedDescription)") + XCTAssertEqual(GitHubClientError.invalidResponse.description, "Received an invalid response from the GitHub API") } } diff --git a/Tests/TuistEnvKitTests/GitHub/GitHubRequestsPoviderTests.swift b/Tests/TuistEnvKitTests/GitHub/GitHubRequestsPoviderTests.swift index 2d9f3cae2..db6e42daa 100644 --- a/Tests/TuistEnvKitTests/GitHub/GitHubRequestsPoviderTests.swift +++ b/Tests/TuistEnvKitTests/GitHub/GitHubRequestsPoviderTests.swift @@ -13,20 +13,28 @@ final class GitHubRequestsFactoryTests: XCTestCase { } func test_releasesRepository() { - XCTAssertEqual(GitHubRequestsFactory.releasesRepository, "tuist/tuist") + XCTAssertEqual(GitHubRequestsFactory.repository, "tuist/tuist") } func test_releases() { let got = subject.releases() XCTAssertEqual(got.httpMethod, "GET") - XCTAssertEqual(got.url, baseURL.appendingPathComponent("/repos/\(GitHubRequestsFactory.releasesRepository)/releases")) + XCTAssertEqual(got.url, baseURL.appendingPathComponent("/repos/\(GitHubRequestsFactory.repository)/releases")) XCTAssertNil(got.httpBody) } func test_release() { let got = subject.release(tag: "1.2.3") XCTAssertEqual(got.httpMethod, "GET") - XCTAssertEqual(got.url, baseURL.appendingPathComponent("/repos/\(GitHubRequestsFactory.releasesRepository)/releases/tags/1.2.3")) + XCTAssertEqual(got.url, baseURL.appendingPathComponent("/repos/\(GitHubRequestsFactory.repository)/releases/tags/1.2.3")) XCTAssertNil(got.httpBody) } + + func test_getContent() { + let got = subject.getContent(ref: "master", path: "path/to/file") + XCTAssertEqual(got.httpMethod, "GET") + let components = URLComponents(url: got.url!, resolvingAgainstBaseURL: true)! + XCTAssertEqual(components.query, "ref=master") + XCTAssertEqual(components.path, "/repos/\(GitHubRequestsFactory.repository)/contents/path/to/file") + } } diff --git a/Tests/TuistEnvKitTests/GitHub/Mocks/MockGitHubClient.swift b/Tests/TuistEnvKitTests/GitHub/Mocks/MockGitHubClient.swift index 5c6ebfa32..8aa1a896d 100644 --- a/Tests/TuistEnvKitTests/GitHub/Mocks/MockGitHubClient.swift +++ b/Tests/TuistEnvKitTests/GitHub/Mocks/MockGitHubClient.swift @@ -4,6 +4,7 @@ import Foundation final class MockGitHubClient: GitHubClienting { var releasesStub: (() throws -> [Release])? var releaseWithTagStub: ((String) throws -> Release)? + var getContentStub: ((String, String) throws -> String)? func releases() throws -> [Release] { return try releasesStub?() ?? [] @@ -12,4 +13,8 @@ final class MockGitHubClient: GitHubClienting { func release(tag: String) throws -> Release { return try releaseWithTagStub?(tag) ?? Release.test() } + + func getContent(ref: String, path: String) throws -> String { + return try getContentStub?(ref, path) ?? "" + } } diff --git a/Tests/TuistEnvKitTests/Installer/InstallerTests.swift b/Tests/TuistEnvKitTests/Installer/InstallerTests.swift index 9ad249c74..69a41448f 100644 --- a/Tests/TuistEnvKitTests/Installer/InstallerTests.swift +++ b/Tests/TuistEnvKitTests/Installer/InstallerTests.swift @@ -33,6 +33,26 @@ final class InstallerTests: XCTestCase { githubClient: githubClient) } + func test_install_when_invalid_swift_version() throws { + let version = "3.2.1" + let temporaryDirectory = try TemporaryDirectory(removeTreeOnDeinit: true) + system.swiftVersionStub = { "8.8.8" } + githubClient.getContentStub = { ref, path in + if ref == version && path == ".swift-version" { + return "7.7.7" + } else { + throw NSError.test() + } + } + + let expectedError = InstallerError.incompatibleSwiftVersion(local: "8.8.8", expected: "7.7.7") + XCTAssertThrowsError(try subject.install(version: version, + temporaryDirectory: temporaryDirectory)) { error in + XCTAssertEqual(error as? InstallerError, expectedError) + } + XCTAssertTrue(printer.printArgs.contains("Verifying the Swift version is compatible with your version 8.8.8")) + } + func test_install_when_bundled_release() throws { let version = "3.2.1" let temporaryDirectory = try TemporaryDirectory(removeTreeOnDeinit: true)