Skip to content

Add a customized GitRepositoryError and funnel all Git operations thr ough a method that throws one of those if the operation fails #2987

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 1 commit into from
Oct 20, 2020
Merged

Add a customized GitRepositoryError and funnel all Git operations thr ough a method that throws one of those if the operation fails #2987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Sources/PackageGraph/RepositoryPackageContainerProvider.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ public class RepositoryPackageContainer: BasePackageContainer, CustomStringConve
}.1
} catch {
// Examine the error to see if we can come up with a more informative and actionable error message. We know that the revision is expected to be a branch name or a hash (tags are handled through a different code path).
if let gitInvocationError = error as? ProcessResult.Error, gitInvocationError.description.contains("Needed a single revision") {
if let error = error as? GitRepositoryError, error.description.contains("Needed a single revision") {
// It was a Git process invocation error. Take a look at the repository to see if we can come up with a reasonable diagnostic.
if let rev = try? repository.resolveRevision(identifier: revision), repository.exists(revision: rev) {
// Revision does exist, so something else must be wrong.
Expand Down
216 changes: 138 additions & 78 deletions Sources/SourceControl/GitRepository.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/*
This source file is part of the Swift.org open source project

Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
Licensed under Apache License v2.0 with Runtime Library Exception

See http://swift.org/LICENSE.txt for license information
Expand All @@ -12,21 +12,6 @@ import TSCBasic
import Dispatch
import TSCUtility

public struct GitCloneError: Swift.Error, CustomStringConvertible {

/// The repository that was being cloned.
public let repository: String

/// The process result.
public let result: ProcessResult

public var description: String {
let stdout = (try? result.utf8Output()) ?? ""
let stderr = (try? result.utf8stderrOutput()) ?? ""
let output = (stdout + stderr).spm_chomp().spm_multilineIndent(count: 4)
return "Failed to clone \(repository):\n\(output)"
}
}

/// A `git` repository provider.
public class GitRepositoryProvider: RepositoryProvider {
Expand All @@ -38,6 +23,29 @@ public class GitRepositoryProvider: RepositoryProvider {
self.processSet = processSet
}

/// Private function to invoke the Git tool with its default environment and given set of arguments. The specified
/// failure message is used only in case of error. This function waits for the invocation to finish and returns the
/// output as a string.
@discardableResult
private func callGit(_ args: String..., environment: [String: String] = Git.environment, failureMessage: String = "", repository: RepositorySpecifier) throws -> String {
let process = Process(arguments: [Git.tool] + args, environment: environment, outputRedirection: .collect)
let result: ProcessResult
do {
try processSet?.add(process)
try process.launch()
result = try process.waitUntilExit()
}
catch {
// Handle a failure to even launch the Git tool by synthesizing a result that we can wrap an error around.
result = ProcessResult(arguments: process.arguments, environment: process.environment,
exitStatus: .terminated(code: -1), output: .failure(error), stderrOutput: .failure(error))
}
guard result.exitStatus == .terminated(code: 0) else {
throw GitCloneError(repository: repository, message: failureMessage, result: result)
}
return try result.utf8Output()
}

public func fetch(repository: RepositorySpecifier, to path: AbsolutePath) throws {
// Perform a bare clone.
//
Expand All @@ -47,24 +55,9 @@ public class GitRepositoryProvider: RepositoryProvider {

precondition(!localFileSystem.exists(path))

// FIXME: We need infrastructure in this subsystem for reporting
// status information.

let process = Process(
args: Git.tool, "clone", "--mirror", repository.url, path.pathString, environment: Git.environment)
// Add to process set.
try processSet?.add(process)

try process.launch()
let result = try process.waitUntilExit()

// Throw if cloning failed.
guard result.exitStatus == .terminated(code: 0) else {
throw GitCloneError(
repository: repository.url,
result: result
)
}
// FIXME: Ideally we should pass `--progress` here and report status regularly. We currently don't have callbacks for that.
try callGit("clone", "--mirror", repository.url, path.pathString,
failureMessage: "Failed to clone repository \(repository.url)", repository: repository)
}

public func open(repository: RepositorySpecifier, at path: AbsolutePath) -> Repository {
Expand All @@ -82,8 +75,8 @@ public class GitRepositoryProvider: RepositoryProvider {
// For editable clones, i.e. the user is expected to directly work on them, first we create
// a clone from our cache of repositories and then we replace the remote to the one originally
// present in the bare repository.
try Process.checkNonZeroExit(args:
Git.tool, "clone", "--no-checkout", sourcePath.pathString, destinationPath.pathString)
try callGit("clone", "--no-checkout", sourcePath.pathString, destinationPath.pathString,
failureMessage: "Failed to clone repository \(repository.url)", repository: repository)
// The default name of the remote.
let origin = "origin"
// In destination repo remove the remote which will be pointing to the source repo.
Expand All @@ -101,8 +94,8 @@ public class GitRepositoryProvider: RepositoryProvider {
// re-resolve such that the objects in this repository changed, we would
// only ever expect to get back a revision that remains present in the
// object storage.
try Process.checkNonZeroExit(args:
Git.tool, "clone", "--shared", "--no-checkout", sourcePath.pathString, destinationPath.pathString)
try callGit("clone", "--shared", "--no-checkout", sourcePath.pathString, destinationPath.pathString,
failureMessage: "Failed to clone repository \(repository.url)", repository: repository)
}
}

Expand All @@ -118,6 +111,33 @@ public class GitRepositoryProvider: RepositoryProvider {
}
}


public struct GitCloneError: Error, CustomStringConvertible, DiagnosticLocationProviding {
public let repository: RepositorySpecifier
public let message: String
public let result: ProcessResult

public struct Location: DiagnosticLocation {
public let repository: RepositorySpecifier
public var description: String {
return repository.url
}
}

public var diagnosticLocation: DiagnosticLocation? {
return Location(repository: repository)
}

public var description: String {
let stdout = (try? result.utf8Output()) ?? ""
let stderr = (try? result.utf8stderrOutput()) ?? ""
let output = (stdout + stderr).spm_chomp().spm_multilineIndent(count: 4)
return "\(message):\n\(output)"
}
}



enum GitInterfaceError: Swift.Error {
/// This indicates a problem communicating with the `git` tool.
case malformedResponse(String)
Expand All @@ -135,7 +155,7 @@ enum GitInterfaceError: Swift.Error {
// abstract, and change the provider to just return an adaptor around this
// class.
//
/// A basic `git` repository. This class is thread safe.
/// A basic Git repository in the local file system (almost always a clone of a remote). This class is thread safe.
public class GitRepository: Repository, WorkingCheckout {
/// A hash object.
public struct Hash: Hashable {
Expand Down Expand Up @@ -225,7 +245,7 @@ public class GitRepository: Repository, WorkingCheckout {
public let contents: [Entry]
}

/// The path of the repository on disk.
/// The path of the repository in the local file system.
public let path: AbsolutePath

/// The (serial) queue to execute git cli on.
Expand All @@ -238,13 +258,34 @@ public class GitRepository: Repository, WorkingCheckout {
self.path = path
self.isWorkingRepo = isWorkingRepo
do {
let isBareRepo = try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "rev-parse", "--is-bare-repository").spm_chomp() == "true"
let isBareRepo = try callGit("rev-parse", "--is-bare-repository").spm_chomp() == "true"
assert(isBareRepo != isWorkingRepo)
} catch {
// Ignore if we couldn't run popen for some reason.
}
}

/// Private function to invoke the Git tool with its default environment and given set of arguments, specifying the
/// path of the repository as the one to operate on. The specified failure message is used only in case of error.
/// This function waits for the invocation to finish and returns the output as a string.
@discardableResult
private func callGit(_ args: String..., environment: [String: String] = Git.environment, failureMessage: String = "") throws -> String {
let process = Process(arguments: [Git.tool, "-C", path.pathString] + args, environment: environment, outputRedirection: .collect)
let result: ProcessResult
do {
try process.launch()
result = try process.waitUntilExit()
}
catch {
// Handle a failure to even launch the Git tool by synthesizing a result that we can wrap an error around.
result = ProcessResult(arguments: process.arguments, environment: process.environment,
exitStatus: .terminated(code: -1), output: .failure(error), stderrOutput: .failure(error))
}
guard result.exitStatus == .terminated(code: 0) else {
throw GitRepositoryError(path: self.path, message: failureMessage, result: result)
}
return try result.utf8Output()
}

/// Changes URL for the remote.
///
Expand All @@ -253,8 +294,8 @@ public class GitRepository: Repository, WorkingCheckout {
/// - url: The new url of the remote.
public func setURL(remote: String, url: String) throws {
try queue.sync {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "remote", "set-url", remote, url)
try callGit("remote", "set-url", remote, url,
failureMessage: "Couldn’t set the URL of the remote ‘\(remote)’ to ‘\(url)’")
return
}
}
Expand All @@ -265,13 +306,13 @@ public class GitRepository: Repository, WorkingCheckout {
public func remotes() throws -> [(name: String, url: String)] {
return try queue.sync {
// Get the remote names.
let remoteNamesOutput = try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "remote").spm_chomp()
let remoteNamesOutput = try callGit("remote",
failureMessage: "Couldn’t get the list of remotes").spm_chomp()
let remoteNames = remoteNamesOutput.split(separator: "\n").map(String.init)
return try remoteNames.map({ name in
// For each remote get the url.
let url = try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "config", "--get", "remote.\(name).url").spm_chomp()
let url = try callGit("config", "--get", "remote.\(name).url",
failureMessage: "Couldn’t get the URL of the remote ‘\(name)’").spm_chomp()
return (name, url)
})
}
Expand All @@ -297,8 +338,8 @@ public class GitRepository: Repository, WorkingCheckout {
/// Returns the tags present in repository.
private func getTags() -> [String] {
// FIXME: Error handling.
let tagList = try! Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "tag", "-l").spm_chomp()
let tagList = try! callGit("tag", "-l",
failureMessage: "Couldn’t get the list of tags").spm_chomp()
return tagList.split(separator: "\n").map(String.init)
}

Expand All @@ -312,20 +353,17 @@ public class GitRepository: Repository, WorkingCheckout {

public func fetch() throws {
try queue.sync {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "remote", "update", "-p", environment: Git.environment)
try callGit("remote", "update", "-p",
failureMessage: "Couldn’t fetch updates from remote repositories")
self.tagsCache = nil
}
}

public func hasUncommittedChanges() -> Bool {
// Only a work tree can have changes.
// Only a working repository can have changes.
guard isWorkingRepo else { return false }
return queue.sync {
// Check nothing has been changed
guard let result = try? Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "status", "-s") else
{
guard let result = try? callGit("status", "-s") else {
return false
}
return !result.spm_chomp().isEmpty
Expand All @@ -340,26 +378,25 @@ public class GitRepository: Repository, WorkingCheckout {

public func hasUnpushedCommits() throws -> Bool {
return try queue.sync {
let hasOutput = try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "log", "--branches", "--not", "--remotes").spm_chomp().isEmpty
let hasOutput = try callGit("log", "--branches", "--not", "--remotes",
failureMessage: "Couldn’t check for unpushed commits").spm_chomp().isEmpty
return !hasOutput
}
}

public func getCurrentRevision() throws -> Revision {
return try queue.sync {
return try Revision(
identifier: Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "rev-parse", "--verify", "HEAD").spm_chomp())
return try Revision(identifier: callGit("rev-parse", "--verify", "HEAD",
failureMessage: "Couldn’t get current revision").spm_chomp())
}
}

public func checkout(tag: String) throws {
// FIXME: Audit behavior with off-branch tags in remote repositories, we
// may need to take a little more care here.
try queue.sync {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "reset", "--hard", tag)
try callGit("reset", "--hard", tag,
failureMessage: "Couldn’t check out tag ‘\(tag)’")
try self.updateSubmoduleAndClean()
}
}
Expand All @@ -368,34 +405,32 @@ public class GitRepository: Repository, WorkingCheckout {
// FIXME: Audit behavior with off-branch tags in remote repositories, we
// may need to take a little more care here.
try queue.sync {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "checkout", "-f", revision.identifier)
try callGit("checkout", "-f", revision.identifier,
failureMessage: "Couldn’t check out revision ‘\(revision.identifier)’")
try self.updateSubmoduleAndClean()
}
}

/// Initializes and updates the submodules, if any, and cleans left over the files and directories using git-clean.
private func updateSubmoduleAndClean() throws {
try Process.checkNonZeroExit(args: Git.tool,
"-C", path.pathString, "submodule", "update", "--init", "--recursive", environment: Git.environment)
try Process.checkNonZeroExit(args: Git.tool,
"-C", path.pathString, "clean", "-ffdx")
try callGit("submodule", "update", "--init", "--recursive",
failureMessage: "Couldn’t update repository submodules")
try callGit("clean", "-ffdx",
failureMessage: "Couldn’t clean repository submodules")
}

/// Returns true if a revision exists.
public func exists(revision: Revision) -> Bool {
return queue.sync {
let result = try? Process.popen(
args: Git.tool, "-C", path.pathString, "rev-parse", "--verify", revision.identifier)
return result?.exitStatus == .terminated(code: 0)
return (try? callGit("rev-parse", "--verify", revision.identifier)) != nil
}
}

public func checkout(newBranch: String) throws {
precondition(isWorkingRepo, "This operation should run in a working repo.")
precondition(isWorkingRepo, "This operation is only valid in a working repository")
try queue.sync {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "checkout", "-b", newBranch)
try callGit("checkout", "-b", newBranch,
failureMessage: "Couldn’t check out new branch ‘\(newBranch)’")
return
}
}
Expand Down Expand Up @@ -459,8 +494,8 @@ public class GitRepository: Repository, WorkingCheckout {
}
let response = try queue.sync {
try cachedHashes.memo(key: specifier) {
try Process.checkNonZeroExit(
args: Git.tool, "-C", path.pathString, "rev-parse", "--verify", specifier).spm_chomp()
try callGit("rev-parse", "--verify", specifier,
failureMessage: "Couldn’t get revision ‘\(specifier)’").spm_chomp()
}
}
if let hash = Hash(response) {
Expand Down Expand Up @@ -754,3 +789,28 @@ private class GitFileSystemView: FileSystem {
fatalError("will never be supported")
}
}


public struct GitRepositoryError: Error, CustomStringConvertible, DiagnosticLocationProviding {
public let path: AbsolutePath
public let message: String
public let result: ProcessResult

public struct Location: DiagnosticLocation {
public let path: AbsolutePath
public var description: String {
return path.pathString
}
}

public var diagnosticLocation: DiagnosticLocation? {
return Location(path: path)
}

public var description: String {
let stdout = (try? result.utf8Output()) ?? ""
let stderr = (try? result.utf8stderrOutput()) ?? ""
let output = (stdout + stderr).spm_chomp().spm_multilineIndent(count: 4)
return "\(message):\n\(output)"
}
}
Loading