From 4dd24cad977352419e80164b113affea3ec1647b Mon Sep 17 00:00:00 2001 From: Arnon Keereena Date: Mon, 1 May 2017 17:19:05 +0200 Subject: [PATCH] CommitIterator now not hold branch as reference. Make private var instead to prevent unexpected state change from external influence. Uses enum instead of tuple of stop and error to indicate state of Git command result. --- SwiftGit2/CommitIterator.swift | 123 +++++++++++++++------------- SwiftGit2Tests/RepositorySpec.swift | 42 ++++++---- 2 files changed, 92 insertions(+), 73 deletions(-) diff --git a/SwiftGit2/CommitIterator.swift b/SwiftGit2/CommitIterator.swift index 05a95bc..40faf64 100644 --- a/SwiftGit2/CommitIterator.swift +++ b/SwiftGit2/CommitIterator.swift @@ -7,61 +7,68 @@ import Result import libgit2 public class CommitIterator: IteratorProtocol { - public typealias Element = Result - var repo: Repository - var branch: Branch - var revisionWalker: OpaquePointer? = nil - var oid: git_oid - private var unsafeCommit: OpaquePointer? = nil - - public init(repo: Repository, branch: Branch) { - self.repo = repo - self.branch = branch - self.oid = branch.oid.oid - setupRevisionWalker() - } - - deinit { - git_revwalk_free(self.revisionWalker) - } - - private func setupRevisionWalker() { - git_revwalk_new(&revisionWalker, repo.pointer) - git_revwalk_sorting(revisionWalker, GIT_SORT_TOPOLOGICAL.rawValue) - git_revwalk_sorting(revisionWalker, GIT_SORT_TIME.rawValue) - git_revwalk_push(revisionWalker, &oid) - } - - private func result(withName name: String, from result: Int32) -> (stop: Bool, error: NSError?) { - guard result == GIT_OK.rawValue else { - if result == GIT_ITEROVER.rawValue { - return (stop: true, error: nil) - } else { - return (stop: false, error: NSError(gitError: result, pointOfFailure: name)) - } - } - return (stop: false, error: nil) - } - - public func next() -> Element? { - let revwalkGitResult = git_revwalk_next(&self.oid, self.revisionWalker) - let revwalkResult = result(withName: "git_revwalk_next", from: revwalkGitResult) - if revwalkResult.stop { - return nil - } else if let error = revwalkResult.error { - return Result.failure(error) - } - let lookupGitResult = git_commit_lookup(&self.unsafeCommit, self.repo.pointer, &self.oid) - let lookupResult = result(withName: "git_commit_lookup", from: lookupGitResult) - if lookupResult.stop { - return nil - } else if let error = lookupResult.error { - return Result.failure(error) - } - guard let commit = unsafeCommit else { - return nil - } - git_commit_free(unsafeCommit) - return Result.success(Commit(commit)) - } -} \ No newline at end of file + public typealias Element = Result + let repo: Repository + private var oid: git_oid + private var revisionWalker: OpaquePointer? = nil + + private enum Next { + case over + case ok + case error(NSError) + + init(_ result: Int32, name: String) { + switch result { + case GIT_ITEROVER.rawValue: + self = .over + case GIT_OK.rawValue: + self = .ok + default: + self = .error(NSError(gitError: result, pointOfFailure: name)) + } + } + } + + init(repo: Repository, branch: Branch) { + self.repo = repo + self.oid = branch.oid.oid + setupRevisionWalker() + } + + deinit { + git_revwalk_free(self.revisionWalker) + } + + private func setupRevisionWalker() { + git_revwalk_new(&revisionWalker, repo.pointer) + git_revwalk_sorting(revisionWalker, GIT_SORT_TOPOLOGICAL.rawValue) + git_revwalk_sorting(revisionWalker, GIT_SORT_TIME.rawValue) + git_revwalk_push(revisionWalker, &oid) + } + + private func next(withName name: String, from result: Int32) -> Next { + if result == GIT_OK.rawValue || result == GIT_ITEROVER.rawValue { + return Next(result, name: name) + } else { + return Next.error(NSError(gitError: result, pointOfFailure: name)) + } + } + + public func next() -> Element? { + var unsafeCommit: OpaquePointer? = nil + let revwalkGitResult = git_revwalk_next(&oid, revisionWalker) + let nextResult = next(withName: "git_revwalk_next", from: revwalkGitResult) + if case let .error(error) = nextResult { + return Result.failure(error) + } else if case .over = nextResult { + return nil + } + guard git_commit_lookup(&unsafeCommit, repo.pointer, &oid) == GIT_OK.rawValue, + let unwrapCommit = unsafeCommit else { + return nil + } + let result: Element = Result.success(Commit(unwrapCommit)) + git_commit_free(unsafeCommit) + return result + } +} diff --git a/SwiftGit2Tests/RepositorySpec.swift b/SwiftGit2Tests/RepositorySpec.swift index 4853ba5..9019e7d 100644 --- a/SwiftGit2Tests/RepositorySpec.swift +++ b/SwiftGit2Tests/RepositorySpec.swift @@ -92,18 +92,18 @@ class RepositorySpec: QuickSpec { let env = ProcessInfo.processInfo.environment if let privateRepo = env["SG2TestPrivateRepo"], - let gitUsername = env["SG2TestUsername"], - let publicKey = env["SG2TestPublicKey"], - let privateKey = env["SG2TestPrivateKey"], - let passphrase = env["SG2TestPassphrase"] { + let gitUsername = env["SG2TestUsername"], + let publicKey = env["SG2TestPublicKey"], + let privateKey = env["SG2TestPrivateKey"], + let passphrase = env["SG2TestPassphrase"] { it("should be able to clone a remote repository requiring credentials") { let remoteRepoURL = URL(string: privateRepo) let localURL = self.temporaryURL(forPurpose: "private-remote-clone") let credentials = Credentials.sshMemory(username: gitUsername, - publicKey: publicKey, - privateKey: privateKey, - passphrase: passphrase) + publicKey: publicKey, + privateKey: privateKey, + passphrase: passphrase) let cloneResult = Repository.clone(from: remoteRepoURL!, to: localURL, credentials: credentials) @@ -600,19 +600,31 @@ class RepositorySpec: QuickSpec { expect(repo.HEAD().value?.longName).to(equal("HEAD")) } } - - fdescribe("Repository.allCommits(in:)") { + + describe("Repository.allCommits(in:)") { it("should return all (9) commits") { let repo = Fixtures.simpleRepository let branches = repo.localBranches().value! - var count = 0 - let expected = 9 + let expectedCount = 9 + let expectedMessages: [String] = [ + "List branches in README\n", + "Create a README\n", + "Merge branch 'alphabetize'\n", + "Alphabetize branches\n", + "List new branches\n", + "List branches in README\n", + "Create a README\n", + "List branches in README\n", + "Create a README\n" + ] + var commitMessages: [String] = [] for branch in branches { - while repo.commits(in: branch).next() != nil { - count += 1 - } + while let commit = repo.commits(in: branch).next() { + commitMessages.append(commit.value!.message) + } } - expect(count).to(equal(expected)) + expect(commitMessages.count).to(equal(expectedCount)) + expect(commitMessages).to(equal(expectedMessages)) } } }