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
This commit is contained in:
eric sciple 2026-01-09 13:42:23 -06:00 committed by GitHub
parent 064fe7f331
commit de0fac2e45
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 226 additions and 95 deletions

View File

@ -87,6 +87,17 @@ jobs:
- name: Verify fetch filter
run: __test__/verify-fetch-filter.sh
# Fetch tags
- name: Checkout with fetch-tags
uses: ./
with:
ref: test-data/v2/basic
path: fetch-tags-test
fetch-tags: true
- name: Verify fetch-tags
shell: bash
run: __test__/verify-fetch-tags.sh
# Sparse checkout
- name: Sparse checkout
uses: ./

View File

@ -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,45 +122,7 @@ describe('Test fetchDepth and fetchTags options', () => {
const refSpec = ['refspec1', 'refspec2']
const options = {
filter: 'filterValue',
fetchDepth: 0,
fetchTags: true
}
await git.fetch(refSpec, options)
expect(mockExec).toHaveBeenCalledWith(
expect.any(String),
[
'-c',
'protocol.version=2',
'fetch',
'--prune',
'--no-recurse-submodules',
'--filter=filterValue',
'origin',
'refspec1',
'refspec2'
],
expect.any(Object)
)
})
it('should call execGit with the correct arguments when fetchDepth is 0 and fetchTags is false', async () => {
jest.spyOn(exec, 'exec').mockImplementation(mockExec)
const workingDirectory = 'test'
const lfs = false
const doSparseCheckout = false
git = await commandManager.createCommandManager(
workingDirectory,
lfs,
doSparseCheckout
)
const refSpec = ['refspec1', 'refspec2']
const options = {
filter: 'filterValue',
fetchDepth: 0,
fetchTags: false
fetchDepth: 0
}
await git.fetch(refSpec, options)
@ -183,7 +145,45 @@ describe('Test fetchDepth and fetchTags options', () => {
)
})
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 0 and refSpec includes tags', async () => {
jest.spyOn(exec, 'exec').mockImplementation(mockExec)
const workingDirectory = 'test'
const lfs = false
const doSparseCheckout = false
git = await commandManager.createCommandManager(
workingDirectory,
lfs,
doSparseCheckout
)
const refSpec = ['refspec1', 'refspec2', '+refs/tags/*:refs/tags/*']
const options = {
filter: 'filterValue',
fetchDepth: 0
}
await git.fetch(refSpec, options)
expect(mockExec).toHaveBeenCalledWith(
expect.any(String),
[
'-c',
'protocol.version=2',
'fetch',
'--no-tags',
'--prune',
'--no-recurse-submodules',
'--filter=filterValue',
'origin',
'refspec1',
'refspec2',
'+refs/tags/*:refs/tags/*'
],
expect.any(Object)
)
})
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)
)

View File

@ -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'
)
})
})

9
__test__/verify-fetch-tags.sh Executable file
View File

@ -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"

68
dist/index.js vendored
View File

@ -653,7 +653,6 @@ const fs = __importStar(__nccwpck_require__(7147));
const fshelper = __importStar(__nccwpck_require__(7219));
const io = __importStar(__nccwpck_require__(7436));
const path = __importStar(__nccwpck_require__(1017));
const refHelper = __importStar(__nccwpck_require__(8601));
const regexpHelper = __importStar(__nccwpck_require__(3120));
const retryHelper = __importStar(__nccwpck_require__(2155));
const git_version_1 = __nccwpck_require__(3142);
@ -831,9 +830,9 @@ class GitCommandManager {
fetch(refSpec, options) {
return __awaiter(this, void 0, void 0, function* () {
const args = ['-c', 'protocol.version=2', 'fetch'];
if (!refSpec.some(x => x === refHelper.tagsRefSpec) && !options.fetchTags) {
args.push('--no-tags');
}
// Always use --no-tags for explicit control over tag fetching
// Tags are fetched explicitly via refspec when needed
args.push('--no-tags');
args.push('--prune', '--no-recurse-submodules');
if (options.showProgress) {
args.push('--progress');
@ -1539,13 +1538,26 @@ function getSource(settings) {
if (!(yield refHelper.testRef(git, settings.ref, settings.commit))) {
refSpec = refHelper.getRefSpec(settings.ref, settings.commit);
yield git.fetch(refSpec, fetchOptions);
// Verify the ref now matches. For branches, the targeted fetch above brings
// in the specific commit. For tags (fetched by ref), this will fail if
// the tag was moved after the workflow was triggered.
if (!(yield refHelper.testRef(git, settings.ref, settings.commit))) {
throw new Error(`The ref '${settings.ref}' does not point to the expected commit '${settings.commit}'. ` +
`The ref may have been updated after the workflow was triggered.`);
}
}
}
else {
fetchOptions.fetchDepth = settings.fetchDepth;
fetchOptions.fetchTags = settings.fetchTags;
const refSpec = refHelper.getRefSpec(settings.ref, settings.commit);
const refSpec = refHelper.getRefSpec(settings.ref, settings.commit, settings.fetchTags);
yield git.fetch(refSpec, fetchOptions);
// For tags, verify the ref still points to the expected commit.
// Tags are fetched by ref (not commit), so if a tag was moved after the
// workflow was triggered, we would silently check out the wrong commit.
if (!(yield refHelper.testRef(git, settings.ref, settings.commit))) {
throw new Error(`The ref '${settings.ref}' does not point to the expected commit '${settings.commit}'. ` +
`The ref may have been updated after the workflow was triggered.`);
}
}
core.endGroup();
// Checkout info
@ -2284,53 +2296,67 @@ function getRefSpecForAllHistory(ref, commit) {
}
return result;
}
function getRefSpec(ref, commit) {
function getRefSpec(ref, commit, fetchTags) {
if (!ref && !commit) {
throw new Error('Args ref and commit cannot both be empty');
}
const upperRef = (ref || '').toUpperCase();
const result = [];
// When fetchTags is true, always include the tags refspec
if (fetchTags) {
result.push(exports.tagsRefSpec);
}
// SHA
if (commit) {
// refs/heads
if (upperRef.startsWith('REFS/HEADS/')) {
const branch = ref.substring('refs/heads/'.length);
return [`+${commit}:refs/remotes/origin/${branch}`];
result.push(`+${commit}:refs/remotes/origin/${branch}`);
}
// refs/pull/
else if (upperRef.startsWith('REFS/PULL/')) {
const branch = ref.substring('refs/pull/'.length);
return [`+${commit}:refs/remotes/pull/${branch}`];
result.push(`+${commit}:refs/remotes/pull/${branch}`);
}
// refs/tags/
else if (upperRef.startsWith('REFS/TAGS/')) {
return [`+${commit}:${ref}`];
if (!fetchTags) {
result.push(`+${ref}:${ref}`);
}
}
// Otherwise no destination ref
else {
return [commit];
result.push(commit);
}
}
// Unqualified ref, check for a matching branch or tag
else if (!upperRef.startsWith('REFS/')) {
return [
`+refs/heads/${ref}*:refs/remotes/origin/${ref}*`,
`+refs/tags/${ref}*:refs/tags/${ref}*`
];
result.push(`+refs/heads/${ref}*:refs/remotes/origin/${ref}*`);
if (!fetchTags) {
result.push(`+refs/tags/${ref}*:refs/tags/${ref}*`);
}
}
// refs/heads/
else if (upperRef.startsWith('REFS/HEADS/')) {
const branch = ref.substring('refs/heads/'.length);
return [`+${ref}:refs/remotes/origin/${branch}`];
result.push(`+${ref}:refs/remotes/origin/${branch}`);
}
// refs/pull/
else if (upperRef.startsWith('REFS/PULL/')) {
const branch = ref.substring('refs/pull/'.length);
return [`+${ref}:refs/remotes/pull/${branch}`];
result.push(`+${ref}:refs/remotes/pull/${branch}`);
}
// refs/tags/
else {
return [`+${ref}:${ref}`];
else if (upperRef.startsWith('REFS/TAGS/')) {
if (!fetchTags) {
result.push(`+${ref}:${ref}`);
}
}
// Other refs
else {
result.push(`+${ref}:${ref}`);
}
return result;
}
/**
* Tests whether the initial fetch created the ref at the expected commit
@ -2366,7 +2392,9 @@ function testRef(git, ref, commit) {
// refs/tags/
else if (upperRef.startsWith('REFS/TAGS/')) {
const tagName = ref.substring('refs/tags/'.length);
return ((yield git.tagExists(tagName)) && commit === (yield git.revParse(ref)));
// Use ^{commit} to dereference annotated tags to their underlying commit
return ((yield git.tagExists(tagName)) &&
commit === (yield git.revParse(`${ref}^{commit}`)));
}
// Unexpected
else {

View File

@ -37,7 +37,6 @@ export interface IGitCommandManager {
options: {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
}
): Promise<void>
@ -280,14 +279,13 @@ class GitCommandManager {
options: {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
}
): Promise<void> {
const args = ['-c', 'protocol.version=2', 'fetch']
if (!refSpec.some(x => x === refHelper.tagsRefSpec) && !options.fetchTags) {
args.push('--no-tags')
}
// Always use --no-tags for explicit control over tag fetching
// Tags are fetched explicitly via refspec when needed
args.push('--no-tags')
args.push('--prune', '--no-recurse-submodules')
if (options.showProgress) {

View File

@ -159,7 +159,6 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
const fetchOptions: {
filter?: string
fetchDepth?: number
fetchTags?: boolean
showProgress?: boolean
} = {}
@ -182,12 +181,35 @@ export async function getSource(settings: IGitSourceSettings): Promise<void> {
if (!(await refHelper.testRef(git, settings.ref, settings.commit))) {
refSpec = refHelper.getRefSpec(settings.ref, settings.commit)
await git.fetch(refSpec, fetchOptions)
// Verify the ref now matches. For branches, the targeted fetch above brings
// in the specific commit. For tags (fetched by ref), this will fail if
// the tag was moved after the workflow was triggered.
if (!(await refHelper.testRef(git, settings.ref, settings.commit))) {
throw new Error(
`The ref '${settings.ref}' does not point to the expected commit '${settings.commit}'. ` +
`The ref may have been updated after the workflow was triggered.`
)
}
}
} else {
fetchOptions.fetchDepth = settings.fetchDepth
fetchOptions.fetchTags = settings.fetchTags
const refSpec = refHelper.getRefSpec(settings.ref, settings.commit)
const refSpec = refHelper.getRefSpec(
settings.ref,
settings.commit,
settings.fetchTags
)
await git.fetch(refSpec, fetchOptions)
// For tags, verify the ref still points to the expected commit.
// Tags are fetched by ref (not commit), so if a tag was moved after the
// workflow was triggered, we would silently check out the wrong commit.
if (!(await refHelper.testRef(git, settings.ref, settings.commit))) {
throw new Error(
`The ref '${settings.ref}' does not point to the expected commit '${settings.commit}'. ` +
`The ref may have been updated after the workflow was triggered.`
)
}
}
core.endGroup()

View File

@ -76,55 +76,75 @@ export function getRefSpecForAllHistory(ref: string, commit: string): string[] {
return result
}
export function getRefSpec(ref: string, commit: string): string[] {
export function getRefSpec(
ref: string,
commit: string,
fetchTags?: boolean
): string[] {
if (!ref && !commit) {
throw new Error('Args ref and commit cannot both be empty')
}
const upperRef = (ref || '').toUpperCase()
const result: string[] = []
// When fetchTags is true, always include the tags refspec
if (fetchTags) {
result.push(tagsRefSpec)
}
// SHA
if (commit) {
// refs/heads
if (upperRef.startsWith('REFS/HEADS/')) {
const branch = ref.substring('refs/heads/'.length)
return [`+${commit}:refs/remotes/origin/${branch}`]
result.push(`+${commit}:refs/remotes/origin/${branch}`)
}
// refs/pull/
else if (upperRef.startsWith('REFS/PULL/')) {
const branch = ref.substring('refs/pull/'.length)
return [`+${commit}:refs/remotes/pull/${branch}`]
result.push(`+${commit}:refs/remotes/pull/${branch}`)
}
// refs/tags/
else if (upperRef.startsWith('REFS/TAGS/')) {
return [`+${commit}:${ref}`]
if (!fetchTags) {
result.push(`+${ref}:${ref}`)
}
}
// Otherwise no destination ref
else {
return [commit]
result.push(commit)
}
}
// Unqualified ref, check for a matching branch or tag
else if (!upperRef.startsWith('REFS/')) {
return [
`+refs/heads/${ref}*:refs/remotes/origin/${ref}*`,
`+refs/tags/${ref}*:refs/tags/${ref}*`
]
result.push(`+refs/heads/${ref}*:refs/remotes/origin/${ref}*`)
if (!fetchTags) {
result.push(`+refs/tags/${ref}*:refs/tags/${ref}*`)
}
}
// refs/heads/
else if (upperRef.startsWith('REFS/HEADS/')) {
const branch = ref.substring('refs/heads/'.length)
return [`+${ref}:refs/remotes/origin/${branch}`]
result.push(`+${ref}:refs/remotes/origin/${branch}`)
}
// refs/pull/
else if (upperRef.startsWith('REFS/PULL/')) {
const branch = ref.substring('refs/pull/'.length)
return [`+${ref}:refs/remotes/pull/${branch}`]
result.push(`+${ref}:refs/remotes/pull/${branch}`)
}
// refs/tags/
else {
return [`+${ref}:${ref}`]
else if (upperRef.startsWith('REFS/TAGS/')) {
if (!fetchTags) {
result.push(`+${ref}:${ref}`)
}
}
// Other refs
else {
result.push(`+${ref}:${ref}`)
}
return result
}
/**
@ -170,8 +190,10 @@ export async function testRef(
// refs/tags/
else if (upperRef.startsWith('REFS/TAGS/')) {
const tagName = ref.substring('refs/tags/'.length)
// Use ^{commit} to dereference annotated tags to their underlying commit
return (
(await git.tagExists(tagName)) && commit === (await git.revParse(ref))
(await git.tagExists(tagName)) &&
commit === (await git.revParse(`${ref}^{commit}`))
)
}
// Unexpected