From b2efd36e79be3e8855ea58d2502213911a19fc62 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sat, 13 Jan 2018 02:48:46 +0100 Subject: [PATCH 1/3] Convert git-repository-provider to JS --- src/git-repository-provider.coffee | 84 ---------------------- src/git-repository-provider.js | 107 +++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 84 deletions(-) delete mode 100644 src/git-repository-provider.coffee create mode 100644 src/git-repository-provider.js diff --git a/src/git-repository-provider.coffee b/src/git-repository-provider.coffee deleted file mode 100644 index 593324d0c..000000000 --- a/src/git-repository-provider.coffee +++ /dev/null @@ -1,84 +0,0 @@ -fs = require 'fs' -{Directory} = require 'pathwatcher' -GitRepository = require './git-repository' - -# Returns the .gitdir path in the agnostic Git symlink .git file given, or -# null if the path is not a valid gitfile. -# -# * `gitFile` {String} path of gitfile to parse -gitFileRegex = RegExp "^gitdir: (.+)" -pathFromGitFile = (gitFile) -> - try - gitFileBuff = fs.readFileSync(gitFile, 'utf8') - return gitFileBuff?.match(gitFileRegex)[1] - -# Checks whether a valid `.git` directory is contained within the given -# directory or one of its ancestors. If so, a Directory that corresponds to the -# `.git` folder will be returned. Otherwise, returns `null`. -# -# * `directory` {Directory} to explore whether it is part of a Git repository. -findGitDirectorySync = (directory) -> - # TODO: Fix node-pathwatcher/src/directory.coffee so the following methods - # can return cached values rather than always returning new objects: - # getParent(), getFile(), getSubdirectory(). - gitDir = directory.getSubdirectory('.git') - gitDirPath = pathFromGitFile(gitDir.getPath?()) - if gitDirPath - gitDir = new Directory(directory.resolve(gitDirPath)) - if gitDir.existsSync?() and isValidGitDirectorySync gitDir - gitDir - else if directory.isRoot() - return null - else - findGitDirectorySync directory.getParent() - -# Returns a boolean indicating whether the specified directory represents a Git -# repository. -# -# * `directory` {Directory} whose base name is `.git`. -isValidGitDirectorySync = (directory) -> - # To decide whether a directory has a valid .git folder, we use - # the heuristic adopted by the valid_repository_path() function defined in - # node_modules/git-utils/deps/libgit2/src/repository.c. - return directory.getSubdirectory('objects').existsSync() and - directory.getFile('HEAD').existsSync() and - directory.getSubdirectory('refs').existsSync() - -# Provider that conforms to the atom.repository-provider@0.1.0 service. -module.exports = -class GitRepositoryProvider - - constructor: (@project, @config) -> - # Keys are real paths that end in `.git`. - # Values are the corresponding GitRepository objects. - @pathToRepository = {} - - # Returns a {Promise} that resolves with either: - # * {GitRepository} if the given directory has a Git repository. - # * `null` if the given directory does not have a Git repository. - repositoryForDirectory: (directory) -> - # TODO: Currently, this method is designed to be async, but it relies on a - # synchronous API. It should be rewritten to be truly async. - Promise.resolve(@repositoryForDirectorySync(directory)) - - # Returns either: - # * {GitRepository} if the given directory has a Git repository. - # * `null` if the given directory does not have a Git repository. - repositoryForDirectorySync: (directory) -> - # Only one GitRepository should be created for each .git folder. Therefore, - # we must check directory and its parent directories to find the nearest - # .git folder. - gitDir = findGitDirectorySync(directory) - unless gitDir - return null - - gitDirPath = gitDir.getPath() - repo = @pathToRepository[gitDirPath] - unless repo - repo = GitRepository.open(gitDirPath, {@project, @config}) - return null unless repo - repo.onDidDestroy(=> delete @pathToRepository[gitDirPath]) - @pathToRepository[gitDirPath] = repo - repo.refreshIndex() - repo.refreshStatus() - repo diff --git a/src/git-repository-provider.js b/src/git-repository-provider.js new file mode 100644 index 000000000..096e70c73 --- /dev/null +++ b/src/git-repository-provider.js @@ -0,0 +1,107 @@ +const fs = require('fs'); +const { Directory } = require('pathwatcher'); +const GitRepository = require('./git-repository'); + +// Returns the .gitdir path in the agnostic Git symlink .git file given, or +// null if the path is not a valid gitfile. +// +// * `gitFile` {String} path of gitfile to parse +const gitFileRegex = RegExp('^gitdir: (.+)'); +function pathFromGitFile(gitFile) { + try { + const gitFileBuff = fs.readFileSync(gitFile, 'utf8'); + return gitFileBuff != null ? gitFileBuff.match(gitFileRegex)[1] : undefined; + } catch (error) {} +} + +// Checks whether a valid `.git` directory is contained within the given +// directory or one of its ancestors. If so, a Directory that corresponds to the +// `.git` folder will be returned. Otherwise, returns `null`. +// +// * `directory` {Directory} to explore whether it is part of a Git repository. +function findGitDirectorySync(directory) { + // TODO: Fix node-pathwatcher/src/directory.coffee so the following methods + // can return cached values rather than always returning new objects: + // getParent(), getFile(), getSubdirectory(). + let gitDir = directory.getSubdirectory('.git'); + const gitDirPath = pathFromGitFile( + typeof gitDir.getPath === 'function' ? gitDir.getPath() : undefined + ); + if (gitDirPath) { + gitDir = new Directory(directory.resolve(gitDirPath)); + } + if ( + (typeof gitDir.existsSync === 'function' ? gitDir.existsSync() : undefined) && + isValidGitDirectorySync(gitDir) + ) { + return gitDir; + } else if (directory.isRoot()) { + return null; + } else { + return findGitDirectorySync(directory.getParent()); + } +} + +// Returns a boolean indicating whether the specified directory represents a Git +// repository. +// +// * `directory` {Directory} whose base name is `.git`. +function isValidGitDirectorySync(directory) { + // To decide whether a directory has a valid .git folder, we use + // the heuristic adopted by the valid_repository_path() function defined in + // node_modules/git-utils/deps/libgit2/src/repository.c. + return ( + directory.getSubdirectory('objects').existsSync() && + directory.getFile('HEAD').existsSync() && + directory.getSubdirectory('refs').existsSync() + ); +} + +// Provider that conforms to the atom.repository-provider@0.1.0 service. +class GitRepositoryProvider { + constructor(project, config) { + // Keys are real paths that end in `.git`. + // Values are the corresponding GitRepository objects. + this.project = project; + this.config = config; + this.pathToRepository = {}; + } + + // Returns a {Promise} that resolves with either: + // * {GitRepository} if the given directory has a Git repository. + // * `null` if the given directory does not have a Git repository. + repositoryForDirectory(directory) { + // TODO: Currently, this method is designed to be async, but it relies on a + // synchronous API. It should be rewritten to be truly async. + return Promise.resolve(this.repositoryForDirectorySync(directory)); + } + + // Returns either: + // * {GitRepository} if the given directory has a Git repository. + // * `null` if the given directory does not have a Git repository. + repositoryForDirectorySync(directory) { + // Only one GitRepository should be created for each .git folder. Therefore, + // we must check directory and its parent directories to find the nearest + // .git folder. + const gitDir = findGitDirectorySync(directory); + if (!gitDir) { + return null; + } + + const gitDirPath = gitDir.getPath(); + let repo = this.pathToRepository[gitDirPath]; + if (!repo) { + repo = GitRepository.open(gitDirPath, { project: this.project, config: this.config }); + if (!repo) { + return null; + } + repo.onDidDestroy(() => delete this.pathToRepository[gitDirPath]); + this.pathToRepository[gitDirPath] = repo; + repo.refreshIndex(); + repo.refreshStatus(); + } + return repo; + } +} + +module.exports = GitRepositoryProvider; From 3cfd2f8398fbf05bcaa84ab2f39942026a05aed2 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sun, 14 Jan 2018 02:08:55 +0100 Subject: [PATCH 2/3] Add Async implementation for repositoryForDirectory --- src/git-repository-provider.js | 155 ++++++++++++++++++++++++--------- 1 file changed, 114 insertions(+), 41 deletions(-) diff --git a/src/git-repository-provider.js b/src/git-repository-provider.js index 096e70c73..9785a88ee 100644 --- a/src/git-repository-provider.js +++ b/src/git-repository-provider.js @@ -1,44 +1,93 @@ -const fs = require('fs'); -const { Directory } = require('pathwatcher'); -const GitRepository = require('./git-repository'); +const fs = require('fs') +const { Directory } = require('pathwatcher') +const GitRepository = require('./git-repository') + +const GIT_FILE_REGEX = RegExp('^gitdir: (.+)') // Returns the .gitdir path in the agnostic Git symlink .git file given, or // null if the path is not a valid gitfile. // // * `gitFile` {String} path of gitfile to parse -const gitFileRegex = RegExp('^gitdir: (.+)'); -function pathFromGitFile(gitFile) { +function pathFromGitFileSync (gitFile) { try { - const gitFileBuff = fs.readFileSync(gitFile, 'utf8'); - return gitFileBuff != null ? gitFileBuff.match(gitFileRegex)[1] : undefined; + const gitFileBuff = fs.readFileSync(gitFile, 'utf8') + return gitFileBuff != null ? gitFileBuff.match(GIT_FILE_REGEX)[1] : null } catch (error) {} } +// Returns a {Promise} that resolves to the .gitdir path in the agnostic +// Git symlink .git file given, or null if the path is not a valid gitfile. +// +// * `gitFile` {String} path of gitfile to parse +function pathFromGitFile (gitFile) { + return new Promise(resolve => { + fs.readFile(gitFile, 'utf8', (err, gitFileBuff) => { + if (err == null && gitFileBuff != null) { + const result = gitFileBuff.toString().match(GIT_FILE_REGEX) + resolve(result != null ? result[1] : null) + } else { + resolve(null) + } + }) + }) +} + // Checks whether a valid `.git` directory is contained within the given // directory or one of its ancestors. If so, a Directory that corresponds to the // `.git` folder will be returned. Otherwise, returns `null`. // // * `directory` {Directory} to explore whether it is part of a Git repository. -function findGitDirectorySync(directory) { +function findGitDirectorySync (directory) { // TODO: Fix node-pathwatcher/src/directory.coffee so the following methods // can return cached values rather than always returning new objects: // getParent(), getFile(), getSubdirectory(). - let gitDir = directory.getSubdirectory('.git'); - const gitDirPath = pathFromGitFile( - typeof gitDir.getPath === 'function' ? gitDir.getPath() : undefined - ); - if (gitDirPath) { - gitDir = new Directory(directory.resolve(gitDirPath)); + let gitDir = directory.getSubdirectory('.git') + if (typeof gitDir.getPath === 'function') { + const gitDirPath = pathFromGitFileSync(gitDir.getPath()) + if (gitDirPath) { + gitDir = new Directory(directory.resolve(gitDirPath)) + } } if ( - (typeof gitDir.existsSync === 'function' ? gitDir.existsSync() : undefined) && + typeof gitDir.existsSync === 'function' && + gitDir.existsSync() && isValidGitDirectorySync(gitDir) ) { - return gitDir; + return gitDir } else if (directory.isRoot()) { - return null; + return null } else { - return findGitDirectorySync(directory.getParent()); + return findGitDirectorySync(directory.getParent()) + } +} + +// Checks whether a valid `.git` directory is contained within the given +// directory or one of its ancestors. If so, a Directory that corresponds to the +// `.git` folder will be returned. Otherwise, returns `null`. +// +// Returns a {Promise} that resolves to +// * `directory` {Directory} to explore whether it is part of a Git repository. +async function findGitDirectory (directory) { + // TODO: Fix node-pathwatcher/src/directory.coffee so the following methods + // can return cached values rather than always returning new objects: + // getParent(), getFile(), getSubdirectory(). + let gitDir = directory.getSubdirectory('.git') + if (typeof gitDir.getPath === 'function') { + const gitDirPath = await pathFromGitFile(gitDir.getPath()) + if (gitDirPath) { + gitDir = new Directory(directory.resolve(gitDirPath)) + } + } + if ( + typeof gitDir.exists === 'function' && + (await gitDir.exists()) && + isValidGitDirectory(gitDir) + ) { + return gitDir + } else if (directory.isRoot()) { + return null + } else { + return await findGitDirectory(directory.getParent()) } } @@ -46,7 +95,7 @@ function findGitDirectorySync(directory) { // repository. // // * `directory` {Directory} whose base name is `.git`. -function isValidGitDirectorySync(directory) { +function isValidGitDirectorySync (directory) { // To decide whether a directory has a valid .git folder, we use // the heuristic adopted by the valid_repository_path() function defined in // node_modules/git-utils/deps/libgit2/src/repository.c. @@ -54,54 +103,78 @@ function isValidGitDirectorySync(directory) { directory.getSubdirectory('objects').existsSync() && directory.getFile('HEAD').existsSync() && directory.getSubdirectory('refs').existsSync() - ); + ) +} + +// Returns a {Promise} that resolves to a {Boolean} indicating whether the +// specified directory represents a Git repository. +// +// * `directory` {Directory} whose base name is `.git`. +async function isValidGitDirectory (directory) { + // To decide whether a directory has a valid .git folder, we use + // the heuristic adopted by the valid_repository_path() function defined in + // node_modules/git-utils/deps/libgit2/src/repository.c. + return ( + (await directory.getSubdirectory('objects').exists()) && + (await directory.getFile('HEAD').exists()) && + (await directory.getSubdirectory('refs').exists()) + ) } // Provider that conforms to the atom.repository-provider@0.1.0 service. class GitRepositoryProvider { - constructor(project, config) { + constructor (project, config) { // Keys are real paths that end in `.git`. // Values are the corresponding GitRepository objects. - this.project = project; - this.config = config; - this.pathToRepository = {}; + this.project = project + this.config = config + this.pathToRepository = {} } // Returns a {Promise} that resolves with either: // * {GitRepository} if the given directory has a Git repository. // * `null` if the given directory does not have a Git repository. - repositoryForDirectory(directory) { - // TODO: Currently, this method is designed to be async, but it relies on a - // synchronous API. It should be rewritten to be truly async. - return Promise.resolve(this.repositoryForDirectorySync(directory)); + async repositoryForDirectory (directory) { + // Only one GitRepository should be created for each .git folder. Therefore, + // we must check directory and its parent directories to find the nearest + // .git folder. + const gitDir = await findGitDirectory(directory) + return this.repositoryForGitDirectory(gitDir) } // Returns either: // * {GitRepository} if the given directory has a Git repository. // * `null` if the given directory does not have a Git repository. - repositoryForDirectorySync(directory) { + repositoryForDirectorySync (directory) { // Only one GitRepository should be created for each .git folder. Therefore, // we must check directory and its parent directories to find the nearest // .git folder. - const gitDir = findGitDirectorySync(directory); + const gitDir = findGitDirectorySync(directory) + return this.repositoryForGitDirectory(gitDir) + } + + // Returns either: + // * {GitRepository} if the given Git directory has a Git repository. + // * `null` if the given directory does not have a Git repository. + repositoryForGitDirectory (gitDir) { if (!gitDir) { - return null; + return null } - const gitDirPath = gitDir.getPath(); - let repo = this.pathToRepository[gitDirPath]; + const gitDirPath = gitDir.getPath() + let repo = this.pathToRepository[gitDirPath] if (!repo) { - repo = GitRepository.open(gitDirPath, { project: this.project, config: this.config }); + repo = GitRepository.open(gitDirPath, { project: this.project, config: this.config }) if (!repo) { - return null; + return null } - repo.onDidDestroy(() => delete this.pathToRepository[gitDirPath]); - this.pathToRepository[gitDirPath] = repo; - repo.refreshIndex(); - repo.refreshStatus(); + repo.onDidDestroy(() => delete this.pathToRepository[gitDirPath]) + this.pathToRepository[gitDirPath] = repo + repo.refreshIndex() + repo.refreshStatus() } - return repo; + return repo } } -module.exports = GitRepositoryProvider; +module.exports = GitRepositoryProvider From 16b8c293a19f2d39166ecfd6e9bd76070a600754 Mon Sep 17 00:00:00 2001 From: Lukas Geiger Date: Sun, 14 Jan 2018 15:20:58 +0100 Subject: [PATCH 3/3] :white_check_mark: Add tests for repositoryForDirectorySync Since `repositoryForDirectory` and `repositoryForDirectorySync` don't share the same implementation anymore. --- spec/git-repository-provider-spec.js | 88 ++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 6 deletions(-) diff --git a/spec/git-repository-provider-spec.js b/spec/git-repository-provider-spec.js index 24993fe9b..2e679e755 100644 --- a/spec/git-repository-provider-spec.js +++ b/spec/git-repository-provider-spec.js @@ -82,6 +82,88 @@ describe('GitRepositoryProvider', () => { }) }) + describe('when specified a Directory without exists()', () => { + let directory + + beforeEach(() => { + // An implementation of Directory that does not implement existsSync(). + const subdirectory = {} + directory = { + getSubdirectory () {}, + isRoot () { return true } + } + spyOn(directory, 'getSubdirectory').andReturn(subdirectory) + }) + + it('returns a Promise that resolves to null', async () => { + const repo = await provider.repositoryForDirectory(directory) + expect(repo).toBe(null) + expect(directory.getSubdirectory).toHaveBeenCalledWith('.git') + }) + }) + }) + + describe('.repositoryForDirectorySync(directory)', () => { + describe('when specified a Directory with a Git repository', () => { + it('resolves with a GitRepository', async () => { + const directory = new Directory(path.join(__dirname, 'fixtures', 'git', 'master.git')) + const result = provider.repositoryForDirectorySync(directory) + expect(result).toBeInstanceOf(GitRepository) + expect(provider.pathToRepository[result.getPath()]).toBeTruthy() + expect(result.getType()).toBe('git') + + // Refresh should be started + await new Promise(resolve => result.onDidChangeStatuses(resolve)) + }) + + it('resolves with the same GitRepository for different Directory objects in the same repo', () => { + const firstRepo = provider.repositoryForDirectorySync( + new Directory(path.join(__dirname, 'fixtures', 'git', 'master.git')) + ) + const secondRepo = provider.repositoryForDirectorySync( + new Directory(path.join(__dirname, 'fixtures', 'git', 'master.git', 'objects')) + ) + + expect(firstRepo).toBeInstanceOf(GitRepository) + expect(firstRepo).toBe(secondRepo) + }) + }) + + describe('when specified a Directory without a Git repository', () => { + it('resolves with null', () => { + const directory = new Directory(temp.mkdirSync('dir')) + const repo = provider.repositoryForDirectorySync(directory) + expect(repo).toBe(null) + }) + }) + + describe('when specified a Directory with an invalid Git repository', () => { + it('resolves with null', () => { + const dirPath = temp.mkdirSync('dir') + fs.writeFileSync(path.join(dirPath, '.git', 'objects'), '') + fs.writeFileSync(path.join(dirPath, '.git', 'HEAD'), '') + fs.writeFileSync(path.join(dirPath, '.git', 'refs'), '') + + const directory = new Directory(dirPath) + const repo = provider.repositoryForDirectorySync(directory) + expect(repo).toBe(null) + }) + }) + + describe('when specified a Directory with a valid gitfile-linked repository', () => { + it('returns a Promise that resolves to a GitRepository', () => { + const gitDirPath = path.join(__dirname, 'fixtures', 'git', 'master.git') + const workDirPath = temp.mkdirSync('git-workdir') + fs.writeFileSync(path.join(workDirPath, '.git'), `gitdir: ${gitDirPath}\n`) + + const directory = new Directory(workDirPath) + const result = provider.repositoryForDirectorySync(directory) + expect(result).toBeInstanceOf(GitRepository) + expect(provider.pathToRepository[result.getPath()]).toBeTruthy() + expect(result.getType()).toBe('git') + }) + }) + describe('when specified a Directory without existsSync()', () => { let directory @@ -100,12 +182,6 @@ describe('GitRepositoryProvider', () => { expect(repo).toBe(null) expect(directory.getSubdirectory).toHaveBeenCalledWith('.git') }) - - it('returns a Promise that resolves to null for the async implementation', async () => { - const repo = await provider.repositoryForDirectory(directory) - expect(repo).toBe(null) - expect(directory.getSubdirectory).toHaveBeenCalledWith('.git') - }) }) }) })