fix: apps:rename and apps:destroy incorrectly handles git remotes (#3110

)

* fix: apps:rename and apps:destroy incorrecty handle git remotes

* Removed debugger statement

packages/cli/src/commands/apps/destroy.ts

@@ -1,7 +1,6 @@
 import color from '@heroku-cli/color'
 import {Command, flags} from '@heroku-cli/command'
 import {Args, ux} from '@oclif/core'
-import {uniq} from 'lodash'
 import confirmCommand from '../../lib/confirmCommand'
 import * as git from '../../lib/ci/git'
 
@@ -31,20 +30,18 @@ export default class Destroy extends Command {
     ux.action.start(`Destroying ${color.app(app)} (including all add-ons)`)
     await this.heroku.delete(`/apps/${app}`)
 
+    /**
+     * It is possible to have as many git remotes as
+     * you want, and they can all point to the same url.
+     * The only requirement is that the "name" is unique.
+     */
     if (git.inGitRepo()) {
       // delete git remotes pointing to this app
-      await git.listRemotes()
-        .then(remotes => {
-          const transformed = remotes
-            .filter(r => git.gitUrl(app) === r[1] || git.sshGitUrl(app) === r[1])
-            .map(r => r[0])
-
-          const uniqueRemotes = uniq(transformed)
-
-          uniqueRemotes.forEach(element => {
-            git.rmRemote(element)
-          })
-        })
+      const remotes = await git.listRemotes()
+      await Promise.all([
+        remotes.get(git.gitUrl(app))?.map(({name}) => git.rmRemote(name)),
+        remotes.get(git.sshGitUrl(app))?.map(({name}) => git.rmRemote(name)),
+      ])
     }
 
     ux.action.stop()

packages/cli/src/commands/apps/rename.ts

@@ -42,16 +42,30 @@ export default class AppsRename extends Command {
     }
 
     if (git.inGitRepo()) {
-    // delete git remotes pointing to this app
-      await _(await git.listRemotes())
-        .filter(r => git.gitUrl(oldApp) === r[1] || git.sshGitUrl(oldApp) === r[1])
-        .map(r => r[0])
-        .uniq()
-        .map(r => {
-          return git.rmRemote(r)
-            .then(() => git.createRemote(r, gitUrl))
-            .then(() => ux.log(`Git remote ${r} updated`))
-        }).value()
+      /**
+       * It is possible to have as many git remotes as
+       * you want, and they can all point to the same url.
+       * The only requirement is that the "name" is unique.
+       */
+      const remotes = await git.listRemotes()
+      const httpsUrl = git.gitUrl(oldApp)
+      const sshUrl = git.sshGitUrl(oldApp)
+      const targetRemotesBySSHUrl = remotes.get(sshUrl)
+      const targetRemotesByHttpsUrl = remotes.get(httpsUrl)
+
+      const doRename = async (remotes: {name: string}[] | undefined, url: string) => {
+        for (const remote of remotes ?? []) {
+          const {name} = remote
+          await git.rmRemote(name)
+          await git.createRemote(name, url.replace(oldApp, newApp))
+          ux.log(`Git remote ${name} updated`)
+        }
+      }
+
+      await Promise.all([
+        doRename(targetRemotesByHttpsUrl, httpsUrl),
+        doRename(targetRemotesBySSHUrl, sshUrl),
+      ])
     }
 
     ux.warn("Don't forget to update git remotes for all other local checkouts of the app.")

packages/cli/src/lib/ci/git.ts

@@ -17,6 +17,8 @@ function runGit(...args: string[]): Promise <string> {
   return new Promise((resolve, reject) => {
     git.on('exit', (exitCode: number) => {
       if (exitCode === 0) {
+        // not all git commands write data to stdout
+        resolve(exitCode.toString(10))
         return
       }
 
@@ -92,8 +94,27 @@ function gitUrl(app?: string) {
   return `https://${vars.httpGitHost}/${app}.git`
 }
 
-async function listRemotes() {
-  return runGit('remote', '-v').then(remotes => remotes.trim().split('\n').map(r => r.split(/\s/)))
+/**
+ * Lists remotes by their url and returns an
+ * array of objects containing the name and kind
+ *
+ * @return A map of remotes whose key is the url
+ * and value is an array of objects containing
+ * the 'name' (heroku, heroku-dev, etc.) and 'kind' (fetch, push, etc.)
+ */
+async function listRemotes(): Promise<Map<string, {name: string, kind: string}[]>> {
+  const gitRemotes = await runGit('remote', '-v')
+  const lines = gitRemotes.trim().split('\n')
+  const remotes = lines.map(line => line.trim().split(/\s+/)).map(([name, url, kind]) => ({name, url, kind}))
+  const remotesByUrl = new Map<string, {name: string, kind: string}[]>()
+
+  remotes.forEach(remote => {
+    const {url, ...nameAndKind} = remote
+    const entry = remotesByUrl.get(url) ?? []
+    entry.push(nameAndKind)
+    remotesByUrl.set(url, entry)
+  })
+  return remotesByUrl
 }
 
 function inGitRepo() {
@@ -105,25 +126,22 @@ function inGitRepo() {
   }
 }
 
-function rmRemote(remote: string) {
-  return runGit('remote', 'rm', remote)
+async function rmRemote(remote: string) {
+  await runGit('remote', 'rm', remote)
 }
 
-function hasGitRemote(remote: string) {
-  return runGit('remote')
-    .then(remotes => remotes.split('\n'))
-    .then(remotes => remotes.find(r => r === remote))
+async function hasGitRemote(remote: string) {
+  const remotes = await runGit('remote')
+  return remotes.split('\n').find(r => r === remote)
 }
 
-function createRemote(remote: string, url: string) {
-  return hasGitRemote(remote)
-    .then(exists => {
-      if (!exists) {
-        return runGit('remote', 'add', remote, url)
-      }
+async function createRemote(remote: string, url: string) {
+  const exists = await hasGitRemote(remote)
+  if (!exists) {
+    return runGit('remote', 'add', remote, url)
+  }
 
-      return null
-    })
+  return null
 }
 
 export {