fix(apm): no longer overrides apm

Summary: Fixes #39

Test Plan: new tests

Reviewers: juan, bengotow

Reviewed By: bengotow

Differential Revision: https://phab.nylas.com/D2481

spec/command-installer-spec.es6

@@ -0,0 +1,75 @@
+import CommandInstaller from '../src/command-installer'
+import fs from 'fs-plus'
+
+describe("CommandInstaller", () => {
+  beforeEach(() => {
+    this.resourcePath = "/resourcePath";
+    this.callback = jasmine.createSpy('callback')
+
+    spyOn(CommandInstaller, "symlinkCommand").andCallFake((sourcePath, destinationPath, callback) => {
+      callback()
+    })
+  });
+
+  it("Installs N1 if it doesn't already exist", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/N1")
+      fn(new Error("not found"), undefined)
+    })
+    CommandInstaller.installN1Command(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+    expect(this.callback.calls[0].args[0]).toBeUndefined()
+  });
+
+  it("Leaves the N1 link alone if exists and is already correct", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/N1")
+      fn(null, this.resourcePath + "/N1.sh")
+    })
+    CommandInstaller.installN1Command(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).not.toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+  });
+
+  it("Overrides the N1 link if it exists but is not correct", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/N1")
+      fn(null, this.resourcePath + "/totally/wrong/path")
+    })
+    CommandInstaller.installN1Command(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+  });
+
+  it("Installs apm if it doesn't already exist", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/apm")
+      fn(new Error("not found"), undefined)
+    })
+    CommandInstaller.installApmCommand(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+    expect(this.callback.calls[0].args[0]).toBeUndefined()
+  });
+
+  it("Leaves the apm link alone if exists and is already correct", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/apm")
+      fn(null, this.resourcePath + "/apm/node_modules/.bin/apm")
+    })
+    CommandInstaller.installApmCommand(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).not.toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+  });
+
+  it("Leaves the apm link alone it exists and is not correct since it likely refers to Atom's apm", () => {
+    spyOn(fs, "readlink").andCallFake((path, fn) => {
+      expect(path).toBe("/usr/local/bin/apm")
+      fn(null, this.resourcePath + "/pointing/to/Atom/apm")
+    })
+    CommandInstaller.installApmCommand(this.resourcePath, false, this.callback)
+    expect(CommandInstaller.symlinkCommand).not.toHaveBeenCalled()
+    expect(this.callback).toHaveBeenCalled()
+  });
+});

src/command-installer.coffee

@@ -5,27 +5,6 @@ fs = require 'fs-plus'
 mkdirp = require 'mkdirp'
 runas = require 'runas'
 
-symlinkCommand = (sourcePath, destinationPath, callback) ->
-  fs.unlink destinationPath, (error) ->
-    if error? and error?.code != 'ENOENT'
-      callback(error)
-    else
-      mkdirp path.dirname(destinationPath), (error) ->
-        if error?
-          callback(error)
-        else
-          fs.symlink sourcePath, destinationPath, callback
-
-symlinkCommandWithPrivilegeSync = (sourcePath, destinationPath) ->
-  if runas('/bin/rm', ['-f', destinationPath], admin: true) != 0
-    throw new Error("Failed to remove '#{destinationPath}'")
-
-  if runas('/bin/mkdir', ['-p', path.dirname(destinationPath)], admin: true) != 0
-    throw new Error("Failed to create directory '#{destinationPath}'")
-
-  if runas('/bin/ln', ['-s', sourcePath, destinationPath], admin: true) != 0
-    throw new Error("Failed to symlink '#{sourcePath}' to '#{destinationPath}'")
-
 module.exports =
   getInstallDirectory: ->
     "/usr/local/bin"
@@ -51,28 +30,53 @@ module.exports =
 
   installN1Command: (resourcePath, askForPrivilege, callback) ->
     commandPath = path.join(resourcePath, 'N1.sh')
-    @createSymlink commandPath, askForPrivilege, callback
+    @createSymlink commandPath, askForPrivilege, callback, {override: true}
 
   installApmCommand: (resourcePath, askForPrivilege, callback) ->
     commandPath = path.join(resourcePath, 'apm', 'node_modules', '.bin', 'apm')
-    @createSymlink commandPath, askForPrivilege, callback
+    @createSymlink commandPath, askForPrivilege, callback, {override: false}
 
-  createSymlink: (commandPath, askForPrivilege, callback) ->
+  createSymlink: (commandPath, askForPrivilege, callback, {override}) ->
     return unless process.platform is 'darwin'
 
     commandName = path.basename(commandPath, path.extname(commandPath))
     destinationPath = path.join(@getInstallDirectory(), commandName)
 
-    fs.readlink destinationPath, (error, realpath) ->
+    fs.readlink destinationPath, (error, realpath) =>
       if realpath is commandPath
         callback()
         return
+      else if realpath and realpath isnt commandPath and not override
+        callback()
+        return
+      else
+        @symlinkCommand commandPath, destinationPath, (error) =>
+          if askForPrivilege and error?.code is 'EACCES'
+            try
+              error = null
+              @symlinkCommandWithPrivilegeSync(commandPath, destinationPath)
+            catch error
 
-      symlinkCommand commandPath, destinationPath, (error) ->
+          callback?(error)
-        if askForPrivilege and error?.code is 'EACCES'
+
-          try
+  symlinkCommand: (sourcePath, destinationPath, callback) ->
-            error = null
+    fs.unlink destinationPath, (error) ->
-            symlinkCommandWithPrivilegeSync(commandPath, destinationPath)
+      if error? and error?.code != 'ENOENT'
-          catch error
+        callback(error)
+      else
+        mkdirp path.dirname(destinationPath), (error) ->
+          if error?
+            callback(error)
+          else
+            fs.symlink sourcePath, destinationPath, callback
+
+  symlinkCommandWithPrivilegeSync: (sourcePath, destinationPath) ->
+    if runas('/bin/rm', ['-f', destinationPath], admin: true) != 0
+      throw new Error("Failed to remove '#{destinationPath}'")
+
+    if runas('/bin/mkdir', ['-p', path.dirname(destinationPath)], admin: true) != 0
+      throw new Error("Failed to create directory '#{destinationPath}'")
+
+    if runas('/bin/ln', ['-s', sourcePath, destinationPath], admin: true) != 0
+      throw new Error("Failed to symlink '#{sourcePath}' to '#{destinationPath}'")
 
-        callback?(error)