From 5921bc8a76f21dbff8ef5088dc2dc3b479ab5309 Mon Sep 17 00:00:00 2001 From: eric sciple Date: Fri, 9 Jan 2026 13:42:23 -0600 Subject: Fix tag handling: preserve annotations and explicit fetch-tags (#2356) This PR fixes several issues with tag handling in the checkout action: 1. fetch-tags: true now works (fixes #1471) - Tags refspec is now included in getRefSpec() when fetchTags=true - Previously tags were only fetched during a separate fetch that was overwritten by the main fetch 2. Tag checkout preserves annotations (fixes #290) - Tags are fetched via refspec (+refs/tags/*:refs/tags/*) instead of --tags flag - This fetches the actual tag objects, preserving annotations 3. Tag checkout with fetch-tags: true no longer fails (fixes #1467) - When checking out a tag with fetchTags=true, only the wildcard refspec is used (specific tag refspec is redundant) Changes: - src/ref-helper.ts: getRefSpec() now accepts fetchTags parameter and prepends tags refspec when true - src/git-command-manager.ts: fetch() simplified to always use --no-tags, tags are fetched explicitly via refspec - src/git-source-provider.ts: passes fetchTags to getRefSpec() - Added E2E test for fetch-tags option Related #1471, #1467, #290 --- __test__/git-command-manager.test.ts | 41 ++++++++++++++++++----------------- __test__/ref-helper.test.ts | 42 +++++++++++++++++++++++++++++++++++- __test__/verify-fetch-tags.sh | 9 ++++++++ 3 files changed, 71 insertions(+), 21 deletions(-) create mode 100755 __test__/verify-fetch-tags.sh (limited to '__test__') diff --git a/__test__/git-command-manager.test.ts b/__test__/git-command-manager.test.ts index 23b8863..8a97d82 100644 --- a/__test__/git-command-manager.test.ts +++ b/__test__/git-command-manager.test.ts @@ -108,7 +108,7 @@ describe('Test fetchDepth and fetchTags options', () => { jest.restoreAllMocks() }) - it('should call execGit with the correct arguments when fetchDepth is 0 and fetchTags is true', async () => { + it('should call execGit with the correct arguments when fetchDepth is 0', async () => { jest.spyOn(exec, 'exec').mockImplementation(mockExec) const workingDirectory = 'test' const lfs = false @@ -122,8 +122,7 @@ describe('Test fetchDepth and fetchTags options', () => { const refSpec = ['refspec1', 'refspec2'] const options = { filter: 'filterValue', - fetchDepth: 0, - fetchTags: true + fetchDepth: 0 } await git.fetch(refSpec, options) @@ -134,6 +133,7 @@ describe('Test fetchDepth and fetchTags options', () => { '-c', 'protocol.version=2', 'fetch', + '--no-tags', '--prune', '--no-recurse-submodules', '--filter=filterValue', @@ -145,7 +145,7 @@ describe('Test fetchDepth and fetchTags options', () => { ) }) - it('should call execGit with the correct arguments when fetchDepth is 0 and fetchTags is false', async () => { + it('should call execGit with the correct arguments when fetchDepth is 0 and refSpec includes tags', async () => { jest.spyOn(exec, 'exec').mockImplementation(mockExec) const workingDirectory = 'test' @@ -156,11 +156,10 @@ describe('Test fetchDepth and fetchTags options', () => { lfs, doSparseCheckout ) - const refSpec = ['refspec1', 'refspec2'] + const refSpec = ['refspec1', 'refspec2', '+refs/tags/*:refs/tags/*'] const options = { filter: 'filterValue', - fetchDepth: 0, - fetchTags: false + fetchDepth: 0 } await git.fetch(refSpec, options) @@ -177,13 +176,14 @@ describe('Test fetchDepth and fetchTags options', () => { '--filter=filterValue', 'origin', 'refspec1', - 'refspec2' + 'refspec2', + '+refs/tags/*:refs/tags/*' ], expect.any(Object) ) }) - it('should call execGit with the correct arguments when fetchDepth is 1 and fetchTags is false', async () => { + it('should call execGit with the correct arguments when fetchDepth is 1', async () => { jest.spyOn(exec, 'exec').mockImplementation(mockExec) const workingDirectory = 'test' @@ -197,8 +197,7 @@ describe('Test fetchDepth and fetchTags options', () => { const refSpec = ['refspec1', 'refspec2'] const options = { filter: 'filterValue', - fetchDepth: 1, - fetchTags: false + fetchDepth: 1 } await git.fetch(refSpec, options) @@ -222,7 +221,7 @@ describe('Test fetchDepth and fetchTags options', () => { ) }) - it('should call execGit with the correct arguments when fetchDepth is 1 and fetchTags is true', async () => { + it('should call execGit with the correct arguments when fetchDepth is 1 and refSpec includes tags', async () => { jest.spyOn(exec, 'exec').mockImplementation(mockExec) const workingDirectory = 'test' @@ -233,11 +232,10 @@ describe('Test fetchDepth and fetchTags options', () => { lfs, doSparseCheckout ) - const refSpec = ['refspec1', 'refspec2'] + const refSpec = ['refspec1', 'refspec2', '+refs/tags/*:refs/tags/*'] const options = { filter: 'filterValue', - fetchDepth: 1, - fetchTags: true + fetchDepth: 1 } await git.fetch(refSpec, options) @@ -248,13 +246,15 @@ describe('Test fetchDepth and fetchTags options', () => { '-c', 'protocol.version=2', 'fetch', + '--no-tags', '--prune', '--no-recurse-submodules', '--filter=filterValue', '--depth=1', 'origin', 'refspec1', - 'refspec2' + 'refspec2', + '+refs/tags/*:refs/tags/*' ], expect.any(Object) ) @@ -338,7 +338,7 @@ describe('Test fetchDepth and fetchTags options', () => { ) }) - it('should call execGit with the correct arguments when fetchTags is true and showProgress is true', async () => { + it('should call execGit with the correct arguments when showProgress is true and refSpec includes tags', async () => { jest.spyOn(exec, 'exec').mockImplementation(mockExec) const workingDirectory = 'test' @@ -349,10 +349,9 @@ describe('Test fetchDepth and fetchTags options', () => { lfs, doSparseCheckout ) - const refSpec = ['refspec1', 'refspec2'] + const refSpec = ['refspec1', 'refspec2', '+refs/tags/*:refs/tags/*'] const options = { filter: 'filterValue', - fetchTags: true, showProgress: true } @@ -364,13 +363,15 @@ describe('Test fetchDepth and fetchTags options', () => { '-c', 'protocol.version=2', 'fetch', + '--no-tags', '--prune', '--no-recurse-submodules', '--progress', '--filter=filterValue', 'origin', 'refspec1', - 'refspec2' + 'refspec2', + '+refs/tags/*:refs/tags/*' ], expect.any(Object) ) diff --git a/__test__/ref-helper.test.ts b/__test__/ref-helper.test.ts index 5c8d76b..4943abd 100644 --- a/__test__/ref-helper.test.ts +++ b/__test__/ref-helper.test.ts @@ -152,7 +152,22 @@ describe('ref-helper tests', () => { it('getRefSpec sha + refs/tags/', async () => { const refSpec = refHelper.getRefSpec('refs/tags/my-tag', commit) expect(refSpec.length).toBe(1) - expect(refSpec[0]).toBe(`+${commit}:refs/tags/my-tag`) + expect(refSpec[0]).toBe(`+refs/tags/my-tag:refs/tags/my-tag`) + }) + + it('getRefSpec sha + refs/tags/ with fetchTags', async () => { + // When fetchTags is true, only include tags wildcard (specific tag is redundant) + const refSpec = refHelper.getRefSpec('refs/tags/my-tag', commit, true) + expect(refSpec.length).toBe(1) + expect(refSpec[0]).toBe('+refs/tags/*:refs/tags/*') + }) + + it('getRefSpec sha + refs/heads/ with fetchTags', async () => { + // When fetchTags is true, include both the branch refspec and tags wildcard + const refSpec = refHelper.getRefSpec('refs/heads/my/branch', commit, true) + expect(refSpec.length).toBe(2) + expect(refSpec[0]).toBe('+refs/tags/*:refs/tags/*') + expect(refSpec[1]).toBe(`+${commit}:refs/remotes/origin/my/branch`) }) it('getRefSpec sha only', async () => { @@ -168,6 +183,14 @@ describe('ref-helper tests', () => { expect(refSpec[1]).toBe('+refs/tags/my-ref*:refs/tags/my-ref*') }) + it('getRefSpec unqualified ref only with fetchTags', async () => { + // When fetchTags is true, skip specific tag pattern since wildcard covers all + const refSpec = refHelper.getRefSpec('my-ref', '', true) + expect(refSpec.length).toBe(2) + expect(refSpec[0]).toBe('+refs/tags/*:refs/tags/*') + expect(refSpec[1]).toBe('+refs/heads/my-ref*:refs/remotes/origin/my-ref*') + }) + it('getRefSpec refs/heads/ only', async () => { const refSpec = refHelper.getRefSpec('refs/heads/my/branch', '') expect(refSpec.length).toBe(1) @@ -187,4 +210,21 @@ describe('ref-helper tests', () => { expect(refSpec.length).toBe(1) expect(refSpec[0]).toBe('+refs/tags/my-tag:refs/tags/my-tag') }) + + it('getRefSpec refs/tags/ only with fetchTags', async () => { + // When fetchTags is true, only include tags wildcard (specific tag is redundant) + const refSpec = refHelper.getRefSpec('refs/tags/my-tag', '', true) + expect(refSpec.length).toBe(1) + expect(refSpec[0]).toBe('+refs/tags/*:refs/tags/*') + }) + + it('getRefSpec refs/heads/ only with fetchTags', async () => { + // When fetchTags is true, include both the branch refspec and tags wildcard + const refSpec = refHelper.getRefSpec('refs/heads/my/branch', '', true) + expect(refSpec.length).toBe(2) + expect(refSpec[0]).toBe('+refs/tags/*:refs/tags/*') + expect(refSpec[1]).toBe( + '+refs/heads/my/branch:refs/remotes/origin/my/branch' + ) + }) }) diff --git a/__test__/verify-fetch-tags.sh b/__test__/verify-fetch-tags.sh new file mode 100755 index 0000000..74cff1e --- /dev/null +++ b/__test__/verify-fetch-tags.sh @@ -0,0 +1,9 @@ +#!/bin/sh + +# Verify tags were fetched +TAG_COUNT=$(git -C ./fetch-tags-test tag | wc -l) +if [ "$TAG_COUNT" -eq 0 ]; then + echo "Expected tags to be fetched, but found none" + exit 1 +fi +echo "Found $TAG_COUNT tags" -- cgit v1.2.3