From d31c2dd88dc1f685a699fca39223eb3ca0913180 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Fri, 9 Dec 2022 07:56:15 +0000 Subject: [PATCH 01/11] Fix issues --- packages/cache/src/cache.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 3840e439..c395d448 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -92,21 +92,29 @@ export async function restoreCache( let archivePath = '' try { try { + console.log('before first get cache entry') // path are needed to compute version cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { compressionMethod }) + console.log('after first get cache entry') + console.log(cacheEntry) } catch (error) { // This is to support the old cache entry created // by the old version of the cache action on windows. + console.log('in first catch block') if ( process.platform === 'win32' && compressionMethod !== CompressionMethod.Gzip ) { + console.log( + "Couldn't find cache entry with zstd compression, falling back to gzip compression" + ) compressionMethod = CompressionMethod.Gzip cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { compressionMethod }) + console.log(cacheEntry) if (!cacheEntry?.archiveLocation) { throw error } @@ -148,6 +156,8 @@ export async function restoreCache( return cacheEntry.cacheKey } catch (error) { + console.log('In second catch block') + console.log(error) const typedError = error as Error if (typedError.name === ValidationError.name) { throw error @@ -183,7 +193,8 @@ export async function saveCache( checkPaths(paths) checkKey(key) - const compressionMethod = await utils.getCompressionMethod() + // const compressionMethod = await utils.getCompressionMethod() + const compressionMethod = CompressionMethod.Gzip let cacheId = -1 const cachePaths = await utils.resolvePaths(paths) From 7a532d03f43c81fc08c18339e3eabf3e2c54ead1 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Fri, 9 Dec 2022 09:33:59 +0000 Subject: [PATCH 02/11] Reconfigure catch block --- packages/cache/__tests__/restoreCache.test.ts | 2 +- packages/cache/src/cache.ts | 30 ++++++------------- packages/cache/src/internal/tar.ts | 13 ++++++-- 3 files changed, 21 insertions(+), 24 deletions(-) diff --git a/packages/cache/__tests__/restoreCache.test.ts b/packages/cache/__tests__/restoreCache.test.ts index 82b259b9..9cf45799 100644 --- a/packages/cache/__tests__/restoreCache.test.ts +++ b/packages/cache/__tests__/restoreCache.test.ts @@ -174,7 +174,7 @@ test('restore with zstd as default but gzip compressed cache found on windows', const getCacheMock = jest.spyOn(cacheHttpClient, 'getCacheEntry') getCacheMock .mockImplementationOnce(async () => { - throw new Error('Cache not found.') + return Promise.resolve(null) }) .mockImplementationOnce(async () => { return Promise.resolve(cacheEntry) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index c395d448..8d8bbd2c 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -91,18 +91,13 @@ export async function restoreCache( let compressionMethod = await utils.getCompressionMethod() let archivePath = '' try { - try { - console.log('before first get cache entry') - // path are needed to compute version - cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { - compressionMethod - }) - console.log('after first get cache entry') - console.log(cacheEntry) - } catch (error) { + // path are needed to compute version + cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { + compressionMethod + }) + if (!cacheEntry?.archiveLocation) { // This is to support the old cache entry created // by the old version of the cache action on windows. - console.log('in first catch block') if ( process.platform === 'win32' && compressionMethod !== CompressionMethod.Gzip @@ -114,19 +109,15 @@ export async function restoreCache( cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { compressionMethod }) - console.log(cacheEntry) if (!cacheEntry?.archiveLocation) { - throw error + return undefined } } else { - throw error + // Cache not found + return undefined } } - if (!cacheEntry?.archiveLocation) { - // Cache not found - return undefined - } archivePath = path.join( await utils.createTempDirectory(), utils.getCacheFileName(compressionMethod) @@ -156,8 +147,6 @@ export async function restoreCache( return cacheEntry.cacheKey } catch (error) { - console.log('In second catch block') - console.log(error) const typedError = error as Error if (typedError.name === ValidationError.name) { throw error @@ -193,8 +182,7 @@ export async function saveCache( checkPaths(paths) checkKey(key) - // const compressionMethod = await utils.getCompressionMethod() - const compressionMethod = CompressionMethod.Gzip + const compressionMethod = await utils.getCompressionMethod() let cacheId = -1 const cachePaths = await utils.resolvePaths(paths) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index ae55d321..f2f75918 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -127,6 +127,8 @@ async function getArgs( type: string, archivePath = '' ): Promise { + let args: string + const tarPath = await getTarPath() const tarArgs = await getTarArgs( tarPath, @@ -142,11 +144,18 @@ async function getArgs( tarPath.type === ArchiveToolType.BSD && compressionMethod !== CompressionMethod.Gzip && IS_WINDOWS + if (BSD_TAR_ZSTD && type !== 'create') { - return [...compressionArgs, ...tarArgs].join(' ') + args = [...compressionArgs, ...tarArgs].join(' ') } else { - return [...tarArgs, ...compressionArgs].join(' ') + args = [...tarArgs, ...compressionArgs].join(' ') } + + if (BSD_TAR_ZSTD) { + args = ['cmd /c "', args, '"'].join(' ') + } + + return args } function getWorkingDirectory(): string { From 0c23c38c68eb4cc62c95231b67c8656a0110b5d9 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Fri, 9 Dec 2022 11:06:42 +0000 Subject: [PATCH 03/11] Add debug logging for gzip fall back --- packages/cache/src/cache.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index 8d8bbd2c..d15975c3 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -102,9 +102,6 @@ export async function restoreCache( process.platform === 'win32' && compressionMethod !== CompressionMethod.Gzip ) { - console.log( - "Couldn't find cache entry with zstd compression, falling back to gzip compression" - ) compressionMethod = CompressionMethod.Gzip cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, { compressionMethod @@ -112,6 +109,10 @@ export async function restoreCache( if (!cacheEntry?.archiveLocation) { return undefined } + + core.debug( + "Couldn't find cache entry with zstd compression, falling back to gzip compression" + ) } else { // Cache not found return undefined From 0690c105158181754fb962f4f23edfeb42543d55 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Fri, 9 Dec 2022 11:47:00 +0000 Subject: [PATCH 04/11] Fix test --- packages/cache/__tests__/tar.test.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index c68d0be8..efd38d85 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -95,6 +95,7 @@ test('zstd extract tar with windows BSDtar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( [ + 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), @@ -104,7 +105,8 @@ test('zstd extract tar with windows BSDtar', async () => { TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workspace?.replace(/\\/g, '/') + workspace?.replace(/\\/g, '/'), + '"' // end cmd /c ].join(' ') ) } @@ -233,6 +235,7 @@ test('zstd create tar with windows BSDtar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( [ + 'cmd /c "', `"${tarPath}"`, '--posix', '-cf', @@ -247,7 +250,8 @@ test('zstd create tar with windows BSDtar', async () => { '&&', 'zstd -T0 --long=30 -o', CacheFilename.Zstd.replace(/\\/g, '/'), - TarFilename.replace(/\\/g, '/') + TarFilename.replace(/\\/g, '/'), + '"' // end cmd /c ].join(' '), undefined, // args { @@ -338,6 +342,7 @@ test('zstd list tar with windows BSDtar', async () => { expect(execMock).toHaveBeenCalledTimes(1) expect(execMock).toHaveBeenCalledWith( [ + 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), @@ -345,7 +350,8 @@ test('zstd list tar with windows BSDtar', async () => { `"${tarPath}"`, '-tf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P' + '-P', + '"' // end cmd /c ].join(' ') ) } From d175a181a08ea8fe5433c7b0b147f5e611cd5a03 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 06:53:11 +0000 Subject: [PATCH 05/11] Add end to end test for cache using bsd on windows and address review comments --- .github/workflows/cache-windows-test.yml | 90 ++++++++++++++++++++++ packages/cache/__tests__/tar.test.ts | 95 +++++++++++++++++------- packages/cache/src/internal/tar.ts | 56 +++++++------- 3 files changed, 182 insertions(+), 59 deletions(-) create mode 100644 .github/workflows/cache-windows-test.yml diff --git a/.github/workflows/cache-windows-test.yml b/.github/workflows/cache-windows-test.yml new file mode 100644 index 00000000..3868f296 --- /dev/null +++ b/.github/workflows/cache-windows-test.yml @@ -0,0 +1,90 @@ +name: cache-windows-bsd-unit-tests +on: + push: + branches: + - main + paths-ignore: + - '**.md' + pull_request: + paths-ignore: + - '**.md' + +jobs: + build: + name: Build + + runs-on: windows-latest + + steps: + - name: Checkout + uses: actions/checkout@v2 + + - shell: bash + run: | + rm "C:\Program Files\Git\usr\bin\tar.exe" + + - name: Set Node.js 12.x + uses: actions/setup-node@v1 + with: + node-version: 12.x + + # In order to save & restore cache from a shell script, certain env variables need to be set that are only available in the + # node context. This runs a local action that gets and sets the necessary env variables that are needed + - name: Set env variables + uses: ./packages/cache/__tests__/__fixtures__/ + + # Need root node_modules because certain npm packages like jest are configured for the entire repository and it won't be possible + # without these to just compile the cache package + - name: Install root npm packages + run: npm ci + + - name: Compile cache package + run: | + npm ci + npm run tsc + working-directory: packages/cache + + - name: Generate files in working directory + shell: bash + run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} test-cache + + - name: Generate files outside working directory + shell: bash + run: packages/cache/__tests__/create-cache-files.sh ${{ runner.os }} ~/test-cache + + # We're using node -e to call the functions directly available in the @actions/cache package + - name: Save cache using saveCache() + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').saveCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}'))" + + - name: Delete cache folders before restoring + shell: bash + run: | + rm -rf test-cache + rm -rf ~/test-cache + + - name: Restore cache using restoreCache() with http-client + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').restoreCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}',[],{useAzureSdk: false}))" + + - name: Verify cache restored with http-client + shell: bash + run: | + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} test-cache + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} ~/test-cache + + - name: Delete cache folders before restoring + shell: bash + run: | + rm -rf test-cache + rm -rf ~/test-cache + + - name: Restore cache using restoreCache() with Azure SDK + run: | + node -e "Promise.resolve(require('./packages/cache/lib/cache').restoreCache(['test-cache','~/test-cache'],'test-${{ runner.os }}-${{ github.run_id }}'))" + + - name: Verify cache restored with Azure SDK + shell: bash + run: | + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} test-cache + packages/cache/__tests__/verify-cache-files.sh ${{ runner.os }} ~/test-cache diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index efd38d85..3e837960 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -73,7 +73,9 @@ test('zstd extract tar', async () => { '--use-compress-program', IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -92,22 +94,31 @@ test('zstd extract tar with windows BSDtar', async () => { await tar.extractTar(archivePath, CompressionMethod.Zstd) expect(mkdirMock).toHaveBeenCalledWith(workspace) - expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + expect(execMock).toHaveBeenCalledTimes(2) + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&', + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ].join(' '), + undefined, + {cwd: undefined} + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ `"${tarPath}"`, '-xf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), '-P', '-C', - workspace?.replace(/\\/g, '/'), - '"' // end cmd /c - ].join(' ') + workspace?.replace(/\\/g, '/') + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -137,7 +148,9 @@ test('gzip extract tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -164,7 +177,9 @@ test('gzip extract GNU tar on windows with GNUtar in path', async () => { workspace?.replace(/\\/g, '/'), '--force-local', '-z' - ].join(' ') + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -232,10 +247,11 @@ test('zstd create tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows - expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + expect(execMock).toHaveBeenCalledTimes(2) + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', `"${tarPath}"`, '--posix', '-cf', @@ -246,12 +262,20 @@ test('zstd create tar with windows BSDtar', async () => { '-C', workspace?.replace(/\\/g, '/'), '--files-from', - ManifestFilename, - '&&', + ManifestFilename + ].join(' '), + undefined, // args + { + cwd: archiveFolder + } + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ 'zstd -T0 --long=30 -o', CacheFilename.Zstd.replace(/\\/g, '/'), - TarFilename.replace(/\\/g, '/'), - '"' // end cmd /c + TarFilename.replace(/\\/g, '/') ].join(' '), undefined, // args { @@ -324,7 +348,9 @@ test('zstd list tar', async () => { '--use-compress-program', IS_WINDOWS ? '"zstd -d --long=30"' : 'unzstd --long=30' ]) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -340,19 +366,28 @@ test('zstd list tar with windows BSDtar', async () => { const tarPath = SystemTarPathOnWindows expect(execMock).toHaveBeenCalledTimes(1) - expect(execMock).toHaveBeenCalledWith( + + expect(execMock).toHaveBeenNthCalledWith( + 1, [ - 'cmd /c "', 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&', + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') + ].join(' '), + undefined, + {cwd: undefined} + ) + + expect(execMock).toHaveBeenNthCalledWith( + 2, + [ `"${tarPath}"`, '-tf', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '-P', - '"' // end cmd /c - ].join(' ') + '-P' + ].join(' '), + undefined, + {cwd: undefined} ) } }) @@ -378,7 +413,9 @@ test('zstdWithoutLong list tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) @@ -402,6 +439,8 @@ test('gzip list tar', async () => { .concat(IS_WINDOWS ? ['--force-local'] : []) .concat(IS_MAC ? ['--delay-directory-restore'] : []) .concat(['-z']) - .join(' ') + .join(' '), + undefined, + {cwd: undefined} ) }) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index f2f75918..97d36d3a 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -122,12 +122,12 @@ async function getTarArgs( return args } -async function getArgs( +async function getCommands( compressionMethod: CompressionMethod, type: string, archivePath = '' -): Promise { - let args: string +): Promise { + let args const tarPath = await getTarPath() const tarArgs = await getTarArgs( @@ -146,16 +146,16 @@ async function getArgs( IS_WINDOWS if (BSD_TAR_ZSTD && type !== 'create') { - args = [...compressionArgs, ...tarArgs].join(' ') + args = [...compressionArgs, ...tarArgs] } else { - args = [...tarArgs, ...compressionArgs].join(' ') + args = [...tarArgs, ...compressionArgs] } if (BSD_TAR_ZSTD) { - args = ['cmd /c "', args, '"'].join(' ') + return args } - return args + return [args.join(' ')] } function getWorkingDirectory(): string { @@ -182,8 +182,7 @@ async function getDecompressionProgram( ? [ 'zstd -d --long=30 -o', TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] : [ '--use-compress-program', @@ -194,8 +193,7 @@ async function getDecompressionProgram( ? [ 'zstd -d -o', TarFilename, - archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), - '&&' + archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] : ['--use-compress-program', IS_WINDOWS ? '"zstd -d"' : 'unzstd'] default: @@ -221,7 +219,6 @@ async function getCompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - '&&', 'zstd -T0 --long=30 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename @@ -233,7 +230,6 @@ async function getCompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - '&&', 'zstd -T0 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename @@ -244,16 +240,22 @@ async function getCompressionProgram( } } +async function execCommands(commands: string[], cwd?: string): Promise { + for (const command of commands) { + try { + await exec(command, undefined, {cwd}) + } catch (error) { + throw new Error(`${command[0]} failed with error: ${error?.message}`) + } + } +} + export async function listTar( archivePath: string, compressionMethod: CompressionMethod ): Promise { - const args = await getArgs(compressionMethod, 'list', archivePath) - try { - await exec(args) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'list', archivePath) + await execCommands(commands) } export async function extractTar( @@ -263,12 +265,8 @@ export async function extractTar( // Create directory to extract tar into const workingDirectory = getWorkingDirectory() await io.mkdirP(workingDirectory) - const args = await getArgs(compressionMethod, 'extract', archivePath) - try { - await exec(args) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'extract', archivePath) + await execCommands(commands) } export async function createTar( @@ -281,10 +279,6 @@ export async function createTar( path.join(archiveFolder, ManifestFilename), sourceDirectories.join('\n') ) - const args = await getArgs(compressionMethod, 'create') - try { - await exec(args, undefined, {cwd: archiveFolder}) - } catch (error) { - throw new Error(`Tar failed with error: ${error?.message}`) - } + const commands = await getCommands(compressionMethod, 'create') + await execCommands(commands, archiveFolder) } From bbf5659dfa310b4cf317f6d00b36dddb8e4a5f7a Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 07:04:15 +0000 Subject: [PATCH 06/11] Fix test --- packages/cache/src/internal/tar.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 97d36d3a..6d48e7b8 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -146,9 +146,9 @@ async function getCommands( IS_WINDOWS if (BSD_TAR_ZSTD && type !== 'create') { - args = [...compressionArgs, ...tarArgs] + args = [[...compressionArgs].join(' '), [...tarArgs].join(' ')] } else { - args = [...tarArgs, ...compressionArgs] + args = [[...tarArgs].join(' '), [...compressionArgs].join(' ')] } if (BSD_TAR_ZSTD) { From 6bc5dc5a23f73476158e721ec19cae1e75e31e33 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 07:25:36 +0000 Subject: [PATCH 07/11] Fix test --- packages/cache/__tests__/tar.test.ts | 2 +- packages/cache/src/internal/tar.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index 3e837960..a6a79a3d 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -365,7 +365,7 @@ test('zstd list tar with windows BSDtar', async () => { await tar.listTar(archivePath, CompressionMethod.Zstd) const tarPath = SystemTarPathOnWindows - expect(execMock).toHaveBeenCalledTimes(1) + expect(execMock).toHaveBeenCalledTimes(2) expect(execMock).toHaveBeenNthCalledWith( 1, diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 6d48e7b8..69731381 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -180,7 +180,7 @@ async function getDecompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - 'zstd -d --long=30 -o', + 'zstd -d --long=30 --force -o', TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] @@ -191,7 +191,7 @@ async function getDecompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - 'zstd -d -o', + 'zstd -d --force -o', TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] @@ -219,7 +219,7 @@ async function getCompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - 'zstd -T0 --long=30 -o', + 'zstd -T0 --long=30 --force -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename ] @@ -230,7 +230,7 @@ async function getCompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - 'zstd -T0 -o', + 'zstd -T0 --force -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename ] @@ -245,7 +245,9 @@ async function execCommands(commands: string[], cwd?: string): Promise { try { await exec(command, undefined, {cwd}) } catch (error) { - throw new Error(`${command[0]} failed with error: ${error?.message}`) + throw new Error( + `${command.split(' ')[0]} failed with error: ${error?.message}` + ) } } } From d7ae8cd1efdfc798552e0c2d5bbb18734236d49e Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 07:51:54 +0000 Subject: [PATCH 08/11] Fix tests --- packages/cache/__tests__/tar.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index a6a79a3d..a33f4fab 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -99,7 +99,7 @@ test('zstd extract tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 1, [ - 'zstd -d --long=30 -o', + 'zstd -d --long=30 --force -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ].join(' '), @@ -273,7 +273,7 @@ test('zstd create tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 2, [ - 'zstd -T0 --long=30 -o', + 'zstd -T0 --long=30 --force -o', CacheFilename.Zstd.replace(/\\/g, '/'), TarFilename.replace(/\\/g, '/') ].join(' '), @@ -370,7 +370,7 @@ test('zstd list tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 1, [ - 'zstd -d --long=30 -o', + 'zstd -d --long=30 --force -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ].join(' '), From cad074ceef5b3c14783d2d6cef0a05b58680d7eb Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 10:58:58 +0000 Subject: [PATCH 09/11] Add better comments --- packages/cache/src/cache.ts | 5 ++--- packages/cache/src/internal/cacheHttpClient.ts | 2 ++ packages/cache/src/internal/tar.ts | 10 +++++++++- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/cache/src/cache.ts b/packages/cache/src/cache.ts index d15975c3..2ebf44ca 100644 --- a/packages/cache/src/cache.ts +++ b/packages/cache/src/cache.ts @@ -96,8 +96,7 @@ export async function restoreCache( compressionMethod }) if (!cacheEntry?.archiveLocation) { - // This is to support the old cache entry created - // by the old version of the cache action on windows. + // This is to support the old cache entry created by gzip on windows. if ( process.platform === 'win32' && compressionMethod !== CompressionMethod.Gzip @@ -111,7 +110,7 @@ export async function restoreCache( } core.debug( - "Couldn't find cache entry with zstd compression, falling back to gzip compression" + "Couldn't find cache entry with zstd compression, falling back to gzip compression." ) } else { // Cache not found diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index c66d1a73..d196b9d5 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -104,6 +104,7 @@ export async function getCacheEntry( httpClient.getJson(getCacheApiUrl(resource)) ) if (response.statusCode === 204) { + // Cache not found return null } if (!isSuccessStatusCode(response.statusCode)) { @@ -113,6 +114,7 @@ export async function getCacheEntry( const cacheResult = response.result const cacheDownloadUrl = cacheResult?.archiveLocation if (!cacheDownloadUrl) { + // Cache achiveLocation not found throw new Error('Cache not found.') } core.setSecret(cacheDownloadUrl) diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 69731381..0af6a87a 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -14,7 +14,7 @@ import { const IS_WINDOWS = process.platform === 'win32' -// Function also mutates the args array. For non-mutation call with passing an empty array. +// Returns tar path and type: BSD or GNU async function getTarPath(): Promise { switch (process.platform) { case 'win32': { @@ -43,6 +43,7 @@ async function getTarPath(): Promise { default: break } + // Default assumption is GNU tar is present in path return { path: await io.which('tar', true), type: ArchiveToolType.GNU @@ -60,6 +61,7 @@ async function getTarArgs( const cacheFileName = utils.getCacheFileName(compressionMethod) const tarFile = 'cache.tar' const workingDirectory = getWorkingDirectory() + // Speficic args for BSD tar on windows for workaround const BSD_TAR_ZSTD = tarPath.type === ArchiveToolType.BSD && compressionMethod !== CompressionMethod.Gzip && @@ -122,6 +124,7 @@ async function getTarArgs( return args } +// Returns commands to run tar and compression program async function getCommands( compressionMethod: CompressionMethod, type: string, @@ -201,6 +204,7 @@ async function getDecompressionProgram( } } +// Used for creating the archive // -T#: Compress using # working thread. If # is 0, attempt to detect and use the number of physical CPU cores. // zstdmt is equivalent to 'zstd -T0' // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit. @@ -240,6 +244,7 @@ async function getCompressionProgram( } } +// Executes all commands as separate processes async function execCommands(commands: string[], cwd?: string): Promise { for (const command of commands) { try { @@ -252,6 +257,7 @@ async function execCommands(commands: string[], cwd?: string): Promise { } } +// List the contents of a tar export async function listTar( archivePath: string, compressionMethod: CompressionMethod @@ -260,6 +266,7 @@ export async function listTar( await execCommands(commands) } +// Extract a tar export async function extractTar( archivePath: string, compressionMethod: CompressionMethod @@ -271,6 +278,7 @@ export async function extractTar( await execCommands(commands) } +// Create a tar export async function createTar( archiveFolder: string, sourceDirectories: string[], From e7e19845caa32f4d2b519c8fe4f5e15d57637c98 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 17:44:34 +0530 Subject: [PATCH 10/11] Update packages/cache/src/internal/cacheHttpClient.ts Co-authored-by: Bishal Prasad --- packages/cache/src/internal/cacheHttpClient.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cache/src/internal/cacheHttpClient.ts b/packages/cache/src/internal/cacheHttpClient.ts index d196b9d5..77debfed 100644 --- a/packages/cache/src/internal/cacheHttpClient.ts +++ b/packages/cache/src/internal/cacheHttpClient.ts @@ -114,7 +114,7 @@ export async function getCacheEntry( const cacheResult = response.result const cacheDownloadUrl = cacheResult?.archiveLocation if (!cacheDownloadUrl) { - // Cache achiveLocation not found + // Cache achiveLocation not found. This should never happen, and hence bail out. throw new Error('Cache not found.') } core.setSecret(cacheDownloadUrl) From 2d3c79e6fe27dc9096e36f52834b6bdc9fbd9167 Mon Sep 17 00:00:00 2001 From: Sampark Sharma Date: Mon, 12 Dec 2022 12:18:07 +0000 Subject: [PATCH 11/11] Address review comments --- .github/workflows/cache-windows-test.yml | 1 + packages/cache/__tests__/tar.test.ts | 6 +++--- packages/cache/src/internal/tar.ts | 8 ++++---- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/cache-windows-test.yml b/.github/workflows/cache-windows-test.yml index 3868f296..c7f7a5a9 100644 --- a/.github/workflows/cache-windows-test.yml +++ b/.github/workflows/cache-windows-test.yml @@ -78,6 +78,7 @@ jobs: run: | rm -rf test-cache rm -rf ~/test-cache + rm -f cache.tar - name: Restore cache using restoreCache() with Azure SDK run: | diff --git a/packages/cache/__tests__/tar.test.ts b/packages/cache/__tests__/tar.test.ts index a33f4fab..a6a79a3d 100644 --- a/packages/cache/__tests__/tar.test.ts +++ b/packages/cache/__tests__/tar.test.ts @@ -99,7 +99,7 @@ test('zstd extract tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 1, [ - 'zstd -d --long=30 --force -o', + 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ].join(' '), @@ -273,7 +273,7 @@ test('zstd create tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 2, [ - 'zstd -T0 --long=30 --force -o', + 'zstd -T0 --long=30 -o', CacheFilename.Zstd.replace(/\\/g, '/'), TarFilename.replace(/\\/g, '/') ].join(' '), @@ -370,7 +370,7 @@ test('zstd list tar with windows BSDtar', async () => { expect(execMock).toHaveBeenNthCalledWith( 1, [ - 'zstd -d --long=30 --force -o', + 'zstd -d --long=30 -o', TarFilename.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ].join(' '), diff --git a/packages/cache/src/internal/tar.ts b/packages/cache/src/internal/tar.ts index 0af6a87a..0da4e8df 100644 --- a/packages/cache/src/internal/tar.ts +++ b/packages/cache/src/internal/tar.ts @@ -183,7 +183,7 @@ async function getDecompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - 'zstd -d --long=30 --force -o', + 'zstd -d --long=30 -o', TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] @@ -194,7 +194,7 @@ async function getDecompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - 'zstd -d --force -o', + 'zstd -d -o', TarFilename, archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/') ] @@ -223,7 +223,7 @@ async function getCompressionProgram( case CompressionMethod.Zstd: return BSD_TAR_ZSTD ? [ - 'zstd -T0 --long=30 --force -o', + 'zstd -T0 --long=30 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename ] @@ -234,7 +234,7 @@ async function getCompressionProgram( case CompressionMethod.ZstdWithoutLong: return BSD_TAR_ZSTD ? [ - 'zstd -T0 --force -o', + 'zstd -T0 -o', cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'), TarFilename ]