Merge pull request #10352 from atom/dh-async-repo-relativize-symlinks

Support symlinked repo roots when relativizing

spec/git-repository-async-spec.js

@@ -427,7 +427,7 @@ describe('GitRepositoryAsync', () => {
       repo.onDidChangeStatus(statusHandler)
       editor.save()
 
-      waitsFor(() => statusHandler.callCount > 0)
+      waitsFor('the onDidChangeStatus handler to be called', () => statusHandler.callCount > 0)
       runs(() => {
         expect(statusHandler.callCount).toBeGreaterThan(0)
         expect(statusHandler).toHaveBeenCalledWith({path: editor.getPath(), pathStatus: 256})
@@ -443,7 +443,7 @@ describe('GitRepositoryAsync', () => {
       repo.onDidChangeStatus(statusHandler)
       editor.getBuffer().reload()
 
-      waitsFor(() => statusHandler.callCount > 0)
+      waitsFor('the onDidChangeStatus handler to be called', () => statusHandler.callCount > 0)
       runs(() => {
         expect(statusHandler.callCount).toBeGreaterThan(0)
         expect(statusHandler).toHaveBeenCalledWith({path: editor.getPath(), pathStatus: 256})
@@ -459,7 +459,7 @@ describe('GitRepositoryAsync', () => {
       repo.onDidChangeStatus(statusHandler)
       editor.getBuffer().emitter.emit('did-change-path')
 
-      waitsFor(() => statusHandler.callCount > 0)
+      waitsFor('the onDidChangeStatus handler to be called', () => statusHandler.callCount > 0)
       runs(() => {
         expect(statusHandler.callCount).toBeGreaterThan(0)
         expect(statusHandler).toHaveBeenCalledWith({path: editor.getPath(), pathStatus: 256})
@@ -469,7 +469,7 @@ describe('GitRepositoryAsync', () => {
         buffer.onDidChangePath(pathHandler)
         buffer.emitter.emit('did-change-path')
 
-        waitsFor(() => pathHandler.callCount > 0)
+        waitsFor('the onDidChangePath handler to be called', () => pathHandler.callCount > 0)
         runs(() => expect(pathHandler.callCount).toBeGreaterThan(0))
       })
     })
@@ -749,4 +749,50 @@ describe('GitRepositoryAsync', () => {
       expect(newLines).toBe(1)
     })
   })
+
+  describe('GitRepositoryAsync::relativizeToWorkingDirectory(_path)', () => {
+    let workingDirectory
+
+    beforeEach(() => {
+      workingDirectory = copyRepository()
+      repo = GitRepositoryAsync.open(workingDirectory)
+    })
+
+    it('relativizes the given path to the working directory of the repository', async () => {
+      let absolutePath = path.join(workingDirectory, 'a.txt')
+      expect(await repo.relativizeToWorkingDirectory(absolutePath)).toBe('a.txt')
+      absolutePath = path.join(workingDirectory, 'a/b/c.txt')
+      expect(await repo.relativizeToWorkingDirectory(absolutePath)).toBe('a/b/c.txt')
+      expect(await repo.relativizeToWorkingDirectory('a.txt')).toBe('a.txt')
+      expect(await repo.relativizeToWorkingDirectory('/not/in/workdir')).toBe('/not/in/workdir')
+      expect(await repo.relativizeToWorkingDirectory(null)).toBe(null)
+      expect(await repo.relativizeToWorkingDirectory()).toBe(undefined)
+      expect(await repo.relativizeToWorkingDirectory('')).toBe('')
+      expect(await repo.relativizeToWorkingDirectory(workingDirectory)).toBe('')
+    })
+
+    describe('when the opened path is a symlink', () => {
+      it('relativizes against both the linked path and real path', async () => {
+        // Symlinks require admin privs on windows so we just skip this there,
+        // done in git-utils as well
+        if (process.platform === 'win32') {
+          return
+        }
+
+        const linkDirectory = path.join(temp.mkdirSync('atom-working-dir-symlink'), 'link')
+        fs.symlinkSync(workingDirectory, linkDirectory)
+        const linkedRepo = GitRepositoryAsync.open(linkDirectory)
+        expect(await linkedRepo.relativizeToWorkingDirectory(path.join(workingDirectory, 'test1'))).toBe('test1')
+        expect(await linkedRepo.relativizeToWorkingDirectory(path.join(linkDirectory, 'test2'))).toBe('test2')
+        expect(await linkedRepo.relativizeToWorkingDirectory(path.join(linkDirectory, 'test2/test3'))).toBe('test2/test3')
+        expect(await linkedRepo.relativizeToWorkingDirectory('test2/test3')).toBe('test2/test3')
+      })
+
+      it('handles case insensitive filesystems', async () => {
+        repo.isCaseInsensitive = true
+        expect(await repo.relativizeToWorkingDirectory(path.join(workingDirectory.toUpperCase(), 'a.txt'))).toBe('a.txt')
+        expect(await repo.relativizeToWorkingDirectory(path.join(workingDirectory.toUpperCase(), 'a/b/c.txt'))).toBe('a/b/c.txt')
+      })
+    })
+  })
 })

spec/spec-helper.coffee

@@ -208,13 +208,15 @@ addCustomMatchers = (spec) ->
       element.style.display in ['block', 'inline-block', 'static', 'fixed']
 
 window.waitsForPromise = (args...) ->
+  label = null
   if args.length > 1
-    {shouldReject, timeout} = args[0]
+    {shouldReject, timeout, label} = args[0]
   else
     shouldReject = false
+  label ?= 'promise to be resolved or rejected'
   fn = _.last(args)
 
-  window.waitsFor timeout, (moveOn) ->
+  window.waitsFor label, timeout, (moveOn) ->
     promise = fn()
     if shouldReject
       promise.catch.call(promise, moveOn)

src/git-repository-async.js

@@ -171,23 +171,21 @@ export default class GitRepositoryAsync {
   //
   // Returns the relative {String} path.
   relativize (_path, workingDirectory) {
-    // Cargo-culted from git-utils. The original implementation also handles
-    // this.openedWorkingDirectory, which is set by git-utils when the
-    // repository is opened. Those branches of the if tree aren't included here
-    // yet, but if we determine we still need that here it should be simple to
-    // port.
-    //
     // The original implementation also handled null workingDirectory as it
     // pulled it from a sync function that could return null. We require it
     // to be passed here.
+    let openedWorkingDirectory
     if (!_path || !workingDirectory) {
       return _path
     }
 
-    // Depending on where the paths come from, they may have a '/private/'
-    // prefix. Standardize by stripping that out.
-    _path = _path.replace(/^\/private\//, '/')
-    workingDirectory = workingDirectory.replace(/^\/private\//, '/')
+    // If the opened directory and the workdir differ, this is a symlinked repo
+    // root, so we have to do all the checks below twice--once against the realpath
+    // and one against the opened path
+    const opened = this.openedPath.replace(/\/\.git$/, '')
+    if (path.relative(opened, workingDirectory) !== '') {
+      openedWorkingDirectory = opened
+    }
 
     if (process.platform === 'win32') {
       _path = _path.replace(/\\/g, '/')
@@ -197,22 +195,39 @@ export default class GitRepositoryAsync {
       }
     }
 
-    if (!/\/$/.test(workingDirectory)) {
-      workingDirectory = `${workingDirectory}/`
-    }
+    workingDirectory = workingDirectory.replace(/\/$/, '')
 
-    const originalPath = _path
     if (this.isCaseInsensitive) {
       _path = _path.toLowerCase()
       workingDirectory = workingDirectory.toLowerCase()
     }
 
+    // Depending on where the paths come from, they may have a '/private/'
+    // prefix. Standardize by stripping that out.
+    _path = _path.replace(/^\/private\//, '/')
+    workingDirectory = workingDirectory.replace(/^\/private\//, '/')
+
+    const originalPath = _path
     if (_path.indexOf(workingDirectory) === 0) {
-      return originalPath.substring(workingDirectory.length)
+      return originalPath.substring(workingDirectory.length + 1)
     } else if (_path === workingDirectory) {
       return ''
     }
 
+    if (openedWorkingDirectory) {
+      if (this.isCaseInsensitive) {
+        openedWorkingDirectory = openedWorkingDirectory.toLowerCase()
+      }
+      openedWorkingDirectory = openedWorkingDirectory.replace(/\/$/, '')
+      openedWorkingDirectory = openedWorkingDirectory.replace(/^\/private\//, '/')
+
+      if (_path.indexOf(openedWorkingDirectory) === 0) {
+        return originalPath.substring(openedWorkingDirectory.length + 1)
+      } else if (_path === openedWorkingDirectory) {
+        return ''
+      }
+    }
+
     return _path
   }
 
@@ -435,11 +450,8 @@ export default class GitRepositoryAsync {
   // value can be passed to {::isStatusModified} or {::isStatusNew} to get more
   // information.
   getDirectoryStatus (directoryPath) {
-    return this.getRepo()
-      .then(repo => {
-        const relativePath = this.relativize(directoryPath, repo.workdir())
-        return this._getStatus([relativePath])
-      })
+    return this.relativizeToWorkingDirectory(directoryPath)
+      .then(relativePath => this._getStatus([relativePath]))
       .then(statuses => {
         return Promise.all(statuses.map(s => s.statusBit())).then(bits => {
           return bits
@@ -1010,7 +1022,7 @@ export default class GitRepositoryAsync {
           opts.pathspec = paths
         }
 
-        return repo.getStatus(opts)
+        return repo.getStatusExt(opts)
       })
   }
 }