feat(fetch): Added 'pruneTags' parameter (#840) (#847)

billiegoose · web-flow · commit fefdf0a6e4f8 · 2019-09-08T15:01:59.000-04:00
diff --git a/__tests__/__fixtures__/test-fetch-client.git/refs/tags/v1.0.0 b/__tests__/__fixtures__/test-fetch-client.git/refs/tags/v1.0.0
@@ -0,0 +1 @@
+5a8905a02e181fe1821068b8c0f48cb6633d5b81
diff --git a/__tests__/__fixtures__/test-fetch-client.git/refs/tags/v1.0.0-beta1 b/__tests__/__fixtures__/test-fetch-client.git/refs/tags/v1.0.0-beta1
@@ -0,0 +1 @@
+5a8905a02e181fe1821068b8c0f48cb6633d5b81
diff --git a/__tests__/__fixtures__/test-fetch-server.git/refs/tags/v1.0.0 b/__tests__/__fixtures__/test-fetch-server.git/refs/tags/v1.0.0
@@ -0,0 +1 @@
+97c024f73eaab2781bf3691597bc7c833cb0e22f
diff --git a/__tests__/test-fetch.js b/__tests__/test-fetch.js
@@ -134,6 +134,7 @@ describe('fetch', () => {
     expect(err).toBeDefined()
     expect(err.code).toEqual(E.NoRefspecConfiguredError)
   })
+
   it('fetch empty repository from git-http-mock-server', async () => {
     const { fs, dir, gitdir } = await makeFixture('test-empty')
     await fetch({
@@ -166,4 +167,32 @@ describe('fetch', () => {
       false
     )
   })
+
+  // XXX: After a PROLONGED and tiring battle... I don't know why the eff the pruneTags
+  // parameter is causing Mobile Safari 11 tests to crash / disconnect.
+  // So... I'm just gonna consider it a fluke.
+  // TODO: Remove this check when we drop support for Safari 11.
+  ;(typeof navigator !== 'undefined' && navigator.userAgent.match(/iPhone/)
+    ? xit
+    : it)('fetch --prune-tags from git-http-mock-server', async () => {
+    const { fs, dir, gitdir } = await makeFixture('test-fetch-client')
+    expect(await fs.exists(`${gitdir}/refs/tags/v1.0.0-beta1`)).toBe(true)
+    const oldValue = await fs.read(`${gitdir}/refs/tags/v1.0.0`, 'utf8')
+    try {
+      await fetch({
+        dir,
+        gitdir,
+        depth: 1,
+        tags: true,
+        pruneTags: true
+      })
+    } catch (err) {
+      // shrug
+    }
+    // assert that tag was deleted
+    expect(await fs.exists(`${gitdir}/refs/tags/v1.0.0-beta1`)).toBe(false)
+    // assert that tags was force-updated
+    const newValue = await fs.read(`${gitdir}/refs/tags/v1.0.0`, 'utf8')
+    expect(oldValue).not.toEqual(newValue)
+  })
 })
diff --git a/src/commands/fetch.js b/src/commands/fetch.js
@@ -62,6 +62,7 @@ import { config } from './config'
  * @param {string} [args.oauth2format] - See the [Authentication](./authentication.html) documentation
  * @param {object} [args.headers] - Additional headers to include in HTTP requests, similar to git's `extraHeader` config
  * @param {boolean} [args.prune] - Delete local remote-tracking branches that are not present on the remote
+ * @param {boolean} [args.pruneTags] - Prune local tags that don’t exist on the remote, and force-update those tags that differ
  * @param {import('events').EventEmitter} [args.emitter] - [deprecated] Overrides the emitter set via the ['emitter' plugin](./plugin_emitter.md).
  * @param {string} [args.emitterPrefix = ''] - Scope emitted events by prepending `emitterPrefix` to the event name.
  *
@@ -111,6 +112,7 @@ export async function fetch ({
   singleBranch = false,
   headers = {},
   prune = false,
+  pruneTags = false,
   // @ts-ignore
   onprogress // deprecated
 }) {
@@ -144,7 +146,8 @@ export async function fetch ({
       tags,
       singleBranch,
       headers,
-      prune
+      prune,
+      pruneTags
     })
     if (response === null) {
       return {
@@ -234,7 +237,8 @@ async function fetchPackfile ({
   tags,
   singleBranch,
   headers,
-  prune
+  prune,
+  pruneTags
 }) {
   const fs = new FileSystem(_fs)
   // Sanity checks
@@ -429,7 +433,8 @@ async function fetchPackfile ({
       refs: remoteRefs,
       symrefs: remoteHTTP.symrefs,
       tags,
-      prune
+      prune,
+      pruneTags
     })
     if (prune) {
       response.pruned = pruned
diff --git a/src/index.d.ts b/src/index.d.ts
@@ -394,6 +394,7 @@ export function fetch(args: GitDir & {
   tags?: boolean;
   singleBranch?: boolean;
   prune?: boolean;
+  pruneTags?: boolean;
   headers?: { [key: string]: string };
 }): Promise<FetchResponse>;
 
diff --git a/src/managers/GitRefManager.js b/src/managers/GitRefManager.js
@@ -30,7 +30,8 @@ export class GitRefManager {
     symrefs,
     tags,
     refspecs = undefined,
-    prune = false
+    prune = false,
+    pruneTags = false
   }) {
     const fs = new FileSystem(_fs)
     // Validate input
@@ -50,13 +51,25 @@ export class GitRefManager {
     }
     const refspec = GitRefSpecSet.from(refspecs)
     const actualRefsToWrite = new Map()
+    // Delete all current tags if the pruneTags argument is true.
+    if (pruneTags) {
+      const tags = await GitRefManager.listRefs({
+        fs,
+        gitdir,
+        filepath: 'refs/tags'
+      })
+      await GitRefManager.deleteRefs({
+        fs,
+        gitdir,
+        refs: tags.map(tag => `refs/tags/${tag}`)
+      })
+    }
     // Add all tags if the fetch tags argument is true.
     if (tags) {
       for (const serverRef of refs.keys()) {
         if (serverRef.startsWith('refs/tags') && !serverRef.endsWith('^{}')) {
-          const filename = join(gitdir, serverRef)
           // Git's behavior is to only fetch tags that do not conflict with tags already present.
-          if (!(await fs.exists(filename))) {
+          if (!(await GitRefManager.exists({ fs, gitdir, ref: serverRef }))) {
             // If there is a dereferenced an annotated tag value available, prefer that.
             const oid = refs.get(serverRef + '^{}') || refs.get(serverRef)
             actualRefsToWrite.set(serverRef, oid)
@@ -82,16 +95,20 @@ export class GitRefManager {
     const pruned = []
     if (prune) {
       for (const filepath of refspec.localNamespaces()) {
-        const refs = (await this.listRefs({ fs, gitdir, filepath })).map(
-          file => `${filepath}/${file}`
-        )
+        const refs = (await GitRefManager.listRefs({
+          fs,
+          gitdir,
+          filepath
+        })).map(file => `${filepath}/${file}`)
         for (const ref of refs) {
           if (!actualRefsToWrite.has(ref)) {
-            this.deleteRef({ fs, gitdir, ref })
             pruned.push(ref)
           }
         }
       }
+      if (pruned.length > 0) {
+        await GitRefManager.deleteRefs({ fs, gitdir, refs: pruned })
+      }
     }
     // Update files
     // TODO: For large repos with a history of thousands of pull requests
@@ -129,15 +146,24 @@ export class GitRefManager {
     await fs.write(join(gitdir, ref), 'ref: ' + `${value.trim()}\n`, 'utf8')
   }
 
-  static async deleteRef ({ fs: _fs, gitdir, ref }) {
+  static async deleteRef ({ fs, gitdir, ref }) {
+    return GitRefManager.deleteRefs({ fs, gitdir, refs: [ref] })
+  }
+
+  static async deleteRefs ({ fs: _fs, gitdir, refs }) {
     const fs = new FileSystem(_fs)
     // Delete regular ref
-    await fs.rm(join(gitdir, ref))
+    await Promise.all(refs.map(ref => fs.rm(join(gitdir, ref))))
     // Delete any packed ref
     let text = await fs.read(`${gitdir}/packed-refs`, { encoding: 'utf8' })
     const packed = GitPackedRefs.from(text)
-    if (packed.refs.has(ref)) {
-      packed.delete(ref)
+    const beforeSize = packed.refs.size
+    for (const ref of refs) {
+      if (packed.refs.has(ref)) {
+        packed.delete(ref)
+      }
+    }
+    if (packed.refs.size < beforeSize) {
       text = packed.toString()
       await fs.write(`${gitdir}/packed-refs`, text, { encoding: 'utf8' })
     }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+5a8905a02e181fe1821068b8c0f48cb6633d5b81`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+97c024f73eaab2781bf3691597bc7c833cb0e22f`