Skip to content

Add isGitIgnored method for checking if a file is ignored by git #1613

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.

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 11 commits into from
Jun 28, 2018
Merged
38 changes: 38 additions & 0 deletions Sources/SourceControl/GitRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import Basic
import Dispatch
import Utility
import struct Foundation.Data

public enum GitRepositoryProviderError: Swift.Error {
case gitCloneFailure(errorOutput: String)
Expand Down Expand Up @@ -92,6 +93,13 @@ public class GitRepositoryProvider: RepositoryProvider {
}
}

public func checkoutExists(at path: AbsolutePath) throws -> Bool {
precondition(exists(path))
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we return false if the path doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could return false, but I think that the purpose of this method is to determine if the checkout exists, not the path itself, so in my opinion, it's more like a precondition.


let result = try Process.popen(args: Git.tool, "-C", path.asString, "rev-parse", "--is-bare-repository")
return try result.exitStatus == .terminated(code: 0) && result.utf8Output().chomp() == "false"
}

public func openCheckout(at path: AbsolutePath) throws -> WorkingCheckout {
return GitRepository(path: path)
}
Expand All @@ -100,6 +108,9 @@ public class GitRepositoryProvider: RepositoryProvider {
enum GitInterfaceError: Swift.Error {
/// This indicates a problem communicating with the `git` tool.
case malformedResponse(String)

/// This indicates that a fatal error was encountered
case fatalError
}

/// A basic `git` repository. This class is thread safe.
Expand Down Expand Up @@ -387,6 +398,33 @@ public class GitRepository: Repository, WorkingCheckout {
return localFileSystem.isDirectory(AbsolutePath(firstLine))
}

/// Returns true if the file at `path` is ignored by `git`
public func isGitIgnored(_ paths: [AbsolutePath]) throws -> [Bool] {
Copy link
Contributor

@aciidgh aciidgh Jun 28, 2018

Choose a reason for hiding this comment

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

How about arePathsIgnored? or just areIgnored is also fine with me.

return try queue.sync {
let stringPaths = paths.map({ $0.asString })
let pathsFileContent = stringPaths.joined(separator: "\0")
Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be better for performance if you append to the stream directly:

try localFileSystem.writeFileContents(pathsFile.path) { 
    for path in paths {
        $0 <<< path.asString <<< "\0"
    }
}


let pathsFile = try TemporaryFile()
let pathsData = Data(pathsFileContent.utf8)
pathsFile.fileHandle.write(pathsData)
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use BufferedOutputByteStream and localFileSystem to APIs to write to the temp file.


let args = [Git.tool, "-C", self.path.asString, "check-ignore", "-z", "--stdin", "<", "\(pathsFile.path.asString)"]
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to shell quotes the arguments here as we'll be using sh -c.

Copy link
Contributor

Choose a reason for hiding this comment

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

We should have a test for this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to shell escape the args here, we should also have a test for it.

let argsWithSh = ["sh", "-c", args.joined(separator: " ")]
let result = try Process.popen(arguments: argsWithSh)
let output = try result.output.dematerialize()

let outputs: [String] = output.split(separator: 0).map(Array.init).map({ (bytes: [Int8]) -> String in
return String(cString: bytes + [0])
})

guard result.exitStatus == .terminated(code: 0) || result.exitStatus == .terminated(code: 1) else {
Copy link
Contributor

Choose a reason for hiding this comment

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

The documentation says check-ignore will return 1 when "None of the provided paths are ignored". We shouldn't throw fatal in that case, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This doesn't throw on exit code 1. Only on exit codes different than 0 or 1.
This case is also included in the unit test.

Copy link
Contributor

Choose a reason for hiding this comment

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

oh ya, right!

throw GitInterfaceError.fatalError
}

return stringPaths.map(outputs.contains)
}
}

// MARK: Git Operations

/// Resolve a "treeish" to a concrete hash.
Expand Down
8 changes: 8 additions & 0 deletions Sources/SourceControl/InMemoryGitRepository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,10 @@ extension InMemoryGitRepository: WorkingCheckout {
public func isAlternateObjectStoreValid() -> Bool {
return true
}

public func isGitIgnored(_ paths: [AbsolutePath]) throws -> [Bool] {
return [false]
}
}

/// This class implement provider for in memeory git repository.
Expand Down Expand Up @@ -323,6 +327,10 @@ public final class InMemoryGitRepositoryProvider: RepositoryProvider {
try checkout.installHead()
}

public func checkoutExists(at path: AbsolutePath) throws -> Bool {
return checkoutsMap.keys.contains(path)
}

public func openCheckout(at path: AbsolutePath) throws -> WorkingCheckout {
return checkoutsMap[path]!
}
Expand Down
6 changes: 6 additions & 0 deletions Sources/SourceControl/Repository.swift
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ public protocol RepositoryProvider {
to destinationPath: AbsolutePath,
editable: Bool) throws

/// Returns true if a working repository exists at `path`
func checkoutExists(at path: AbsolutePath) throws -> Bool

/// Open a working repository copy.
///
/// - Parameters:
Expand Down Expand Up @@ -193,6 +196,9 @@ public protocol WorkingCheckout {

/// Returns true if there is an alternative store in the checkout and it is valid.
func isAlternateObjectStoreValid() -> Bool

/// Returns true if the file at `path` is ignored by `git`
func isGitIgnored(_ paths: [AbsolutePath]) throws -> [Bool]
}

/// A single repository revision.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,10 @@ private class MockRepositories: RepositoryProvider {
// No-op.
assert(repositories.index(forKey: repository.url) != nil)
}

func checkoutExists(at path: AbsolutePath) throws -> Bool {
return false
}

func open(repository: RepositorySpecifier, at path: AbsolutePath) throws -> Repository {
return repositories[repository.url]!
Expand Down
23 changes: 23 additions & 0 deletions Tests/SourceControlTests/GitRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ class GitRepositoryTests: XCTestCase {
// Test the provider.
let testCheckoutPath = path.appending(component: "checkout")
let provider = GitRepositoryProvider()
XCTAssertTrue(try provider.checkoutExists(at: testRepoPath))
let repoSpec = RepositorySpecifier(url: testRepoPath.asString)
try! provider.fetch(repository: repoSpec, to: testCheckoutPath)

Expand Down Expand Up @@ -604,6 +605,27 @@ class GitRepositoryTests: XCTestCase {
}
}

func testGitIgnored() throws {
mktmpdir { path in
// Create a repo.
let testRepoPath = path.appending(component: "test_repo")
try makeDirectories(testRepoPath)
initGitRepo(testRepoPath)
let repo = GitRepository(path: testRepoPath)

// Add a .gitignore
try localFileSystem.writeFileContents(testRepoPath.appending(component: ".gitignore"), bytes: "ignored_file1\nignored file2")

let ignored = try repo.isGitIgnored([testRepoPath.appending(component: "ignored_file1"), testRepoPath.appending(component: "ignored file2"), testRepoPath.appending(component: "not ignored")])
XCTAssertTrue(ignored[0])
XCTAssertTrue(ignored[1])
XCTAssertFalse(ignored[2])

let notIgnored = try repo.isGitIgnored([testRepoPath.appending(component: "not_ignored")])
XCTAssertFalse(notIgnored[0])
}
}

static var allTests = [
("testBranchOperations", testBranchOperations),
("testCheckoutRevision", testCheckoutRevision),
Expand All @@ -620,5 +642,6 @@ class GitRepositoryTests: XCTestCase {
("testSubmodules", testSubmodules),
("testUncommitedChanges", testUncommitedChanges),
("testAlternativeObjectStoreValidation", testAlternativeObjectStoreValidation),
("testGitIgnored", testGitIgnored),
]
}
2 changes: 2 additions & 0 deletions Tests/SourceControlTests/InMemoryGitRepositoryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,9 @@ class InMemoryGitRepositoryTests: XCTestCase {
XCTAssert(fooRepo.exists(revision: try fooRepo.resolveRevision(tag: v1)))

let fooCheckoutPath = AbsolutePath("/fooCheckout")
XCTAssertFalse(try provider.checkoutExists(at: fooCheckoutPath))
try provider.cloneCheckout(repository: specifier, at: fooRepoPath, to: fooCheckoutPath, editable: false)
XCTAssertTrue(try provider.checkoutExists(at: fooCheckoutPath))
let fooCheckout = try provider.openCheckout(at: fooCheckoutPath)

XCTAssertEqual(fooCheckout.tags.sorted(), [v1, v2])
Expand Down
4 changes: 4 additions & 0 deletions Tests/SourceControlTests/RepositoryManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ private class DummyRepositoryProvider: RepositoryProvider {
try localFileSystem.writeFileContents(destinationPath.appending(component: "README.txt"), bytes: "Hi")
}

func checkoutExists(at path: AbsolutePath) throws -> Bool {
return false
}

func openCheckout(at path: AbsolutePath) throws -> WorkingCheckout {
fatalError("unsupported")
}
Expand Down