Separate args

packages/cache/__tests__/tar.test.ts

@@ -59,13 +59,13 @@ test('zstd extract tar', async () => {
   expect(execMock).toHaveBeenCalledWith(
     `"${tarPath}"`,
     [
-      '--use-compress-program',
-      IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30',
       '-xf',
       IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath,
       '-P',
       '-C',
-      IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace
+      IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace,
+      '--use-compress-program',
+      IS_WINDOWS ? 'zstd -d --long=30' : 'unzstd --long=30'
     ]
       .concat(IS_WINDOWS ? ['--force-local'] : [])
       .concat(IS_MAC ? ['--delay-directory-restore'] : []),
@@ -89,12 +89,12 @@ test('gzip extract tar', async () => {
   expect(execMock).toHaveBeenCalledWith(
     `"${tarPath}"`,
     [
-      '-z',
       '-xf',
       IS_WINDOWS ? archivePath.replace(/\\/g, '/') : archivePath,
       '-P',
       '-C',
-      IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace
+      IS_WINDOWS ? workspace?.replace(/\\/g, '/') : workspace,
+      '-z'
     ]
       .concat(IS_WINDOWS ? ['--force-local'] : [])
       .concat(IS_MAC ? ['--delay-directory-restore'] : []),
@@ -180,11 +180,15 @@ test('zstd create tar with windows BSDtar', async () => {
     const archiveFolder = getTempDir()
     const workspace = process.env['GITHUB_WORKSPACE']
     const sourceDirectories = ['~/.npm/cache', `${workspace}/dist`]
-    const tarFilename = "cache.tar"
+    const tarFilename = 'cache.tar'
 
     await fs.promises.mkdir(archiveFolder, {recursive: true})
 
-    await tar.createTar(archiveFolder, sourceDirectories, CompressionMethod.Zstd)
+    await tar.createTar(
+      archiveFolder,
+      sourceDirectories,
+      CompressionMethod.Zstd
+    )
 
     const tarPath = SystemTarPathOnWindows
 
@@ -203,9 +207,10 @@ test('zstd create tar with windows BSDtar', async () => {
         workspace?.replace(/\\/g, '/'),
         '--files-from',
         'manifest.txt',
-        "&&",
+        '&&',
-        "zstd -T0 --long=30 -o",
+        'zstd -T0 --long=30 -o',
-        CacheFilename.Zstd.replace(/\\/g, '/')
+        CacheFilename.Zstd.replace(/\\/g, '/'),
+        tarFilename.replace(/\\/g, '/')
       ],
       {
         cwd: archiveFolder

packages/cache/src/internal/tar.ts

@@ -8,16 +8,13 @@ import {CompressionMethod, SystemTarPathOnWindows} from './constants'
 const IS_WINDOWS = process.platform === 'win32'
 
 // Function also mutates the args array. For non-mutation call with passing an empty array.
-async function getTarPath(args: string[]): Promise<string> {
+async function getTarPath(): Promise<string> {
   switch (process.platform) {
     case 'win32': {
       const gnuTar = await utils.getGnuTarPathOnWindows()
-      const systemTar = `${process.env['windir']}\\System32\\tar.exe`
+      const systemTar = SystemTarPathOnWindows
       if (gnuTar) {
         // Use GNUtar as default on windows
-        if (args.length > 0) {
-          args.push('--force-local')
-        }
         return gnuTar
       } else if (existsSync(systemTar)) {
         return systemTar
@@ -28,9 +25,6 @@ async function getTarPath(args: string[]): Promise<string> {
       const gnuTar = await io.which('gtar', false)
       if (gnuTar) {
         // fix permission denied errors when extracting BSD tar archive with GNU tar - https://github.com/actions/cache/issues/527
-        if (args.length > 0) {
-          args.push('--delay-directory-restore')
-        }
         return gnuTar
       }
       break
@@ -41,9 +35,100 @@ async function getTarPath(args: string[]): Promise<string> {
   return await io.which('tar', true)
 }
 
+async function getTarArgs(
+  compressionMethod: CompressionMethod,
+  type: string,
+  archivePath = ''
+): Promise<string[]> {
+  const args = []
+  const manifestFilename = 'manifest.txt'
+  const cacheFileName = utils.getCacheFileName(compressionMethod)
+  const tarFile = 'cache.tar'
+  const tarPath = await getTarPath()
+  const workingDirectory = getWorkingDirectory()
+  const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows
+
+  // Method specific args
+  switch (type) {
+    case 'create':
+      args.push(
+        '--posix',
+        '-cf',
+        BSD_TAR_WINDOWS
+          ? tarFile
+          : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        '--exclude',
+        BSD_TAR_WINDOWS
+          ? tarFile
+          : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        '-P',
+        '-C',
+        workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        '--files-from',
+        manifestFilename
+      )
+      break
+    case 'extract':
+      args.push(
+        '-xf',
+        BSD_TAR_WINDOWS
+          ? tarFile
+          : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        '-P',
+        '-C',
+        workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/')
+      )
+      break
+    // TODO: Correct the below code especially archivePath for BSD_TAR_WINDOWS
+    case 'list':
+      args.push(
+        '-tf',
+        BSD_TAR_WINDOWS
+          ? tarFile
+          : archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+        '-P'
+      )
+      break
+  }
+
+  // Platform specific args
+  switch (process.platform) {
+    case 'win32': {
+      const gnuTar = await utils.getGnuTarPathOnWindows()
+      if (gnuTar) {
+        // Use GNUtar as default on windows
+        args.push('--force-local')
+      }
+      break
+    }
+    case 'darwin': {
+      const gnuTar = await io.which('gtar', false)
+      if (gnuTar) {
+        // fix permission denied errors when extracting BSD tar archive with GNU tar - https://github.com/actions/cache/issues/527
+        args.push('--delay-directory-restore')
+      }
+      break
+    }
+  }
+
+  return args
+}
+
 async function execTar(args: string[], cwd?: string): Promise<void> {
   try {
-    await exec(`"${await getTarPath(args)}"`, args, {cwd})
+    await exec(`"${await getTarPath()}"`, args, {cwd})
+  } catch (error) {
+    throw new Error(`Tar failed with error: ${error?.message}`)
+  }
+}
+
+async function execCommand(
+  command: string,
+  args: string[],
+  cwd?: string
+): Promise<void> {
+  try {
+    await exec(command, args, {cwd})
   } catch (error) {
     throw new Error(`Tar failed with error: ${error?.message}`)
   }
@@ -61,12 +146,19 @@ async function getCompressionProgram(
   // unzstd is equivalent to 'zstd -d'
   // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit.
   // Using 30 here because we also support 32-bit self-hosted runners.
-  const tarPath = await getTarPath([])
+  const tarPath = await getTarPath()
+  const cacheFileName = utils.getCacheFileName(compressionMethod)
+  const tarFile = 'cache.tar'
   const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows
   switch (compressionMethod) {
     case CompressionMethod.Zstd:
       if (BSD_TAR_WINDOWS) {
-        return ['-a'] // auto-detect compression
+        return [
+          'zstd -d --long=30 -o',
+          tarFile,
+          cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+          '&&'
+        ]
       }
       return [
         '--use-compress-program',
@@ -74,7 +166,12 @@ async function getCompressionProgram(
       ]
     case CompressionMethod.ZstdWithoutLong:
       if (BSD_TAR_WINDOWS) {
-        return ['-a'] // auto-detect compression
+        return [
+          'zstd -d -o',
+          tarFile,
+          cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+          '&&'
+        ]
       }
       return ['--use-compress-program', IS_WINDOWS ? 'zstd -d' : 'unzstd']
     default:
@@ -86,14 +183,20 @@ export async function listTar(
   archivePath: string,
   compressionMethod: CompressionMethod
 ): Promise<void> {
-  const args = [
+  const tarPath = await getTarPath()
-    ...(await getCompressionProgram(compressionMethod)),
+  const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows
-    '-tf',
+  const compressionArgs = await getCompressionProgram(compressionMethod)
-    archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+  const tarArgs = await getTarArgs(compressionMethod, 'list', archivePath)
-    '-P'
+  // TODO: Add a test for BSD tar on windows
-  ]
+  if (BSD_TAR_WINDOWS) {
+    const command = compressionArgs[0]
+    const args = compressionArgs.slice(1).concat(tarArgs)
+    await execCommand(command, args)
+  } else {
+    const args = tarArgs.concat(compressionArgs)
     await execTar(args)
   }
+}
 
 export async function extractTar(
   archivePath: string,
@@ -101,17 +204,20 @@ export async function extractTar(
 ): Promise<void> {
   // Create directory to extract tar into
   const workingDirectory = getWorkingDirectory()
+  const tarPath = await getTarPath()
+  const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows
   await io.mkdirP(workingDirectory)
-  const args = [
+  const tarArgs = await getTarArgs(compressionMethod, 'extract', archivePath)
-    ...(await getCompressionProgram(compressionMethod)),
+  const compressionArgs = await getCompressionProgram(compressionMethod)
-    '-xf',
+  if (BSD_TAR_WINDOWS) {
-    archivePath.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+    const command = compressionArgs[0]
-    '-P',
+    const args = compressionArgs.slice(1).concat(tarArgs)
-    '-C',
+    await execCommand(command, args)
-    workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/')
+  } else {
-  ]
+    const args = tarArgs.concat(compressionArgs)
     await execTar(args)
   }
+}
 
 export async function createTar(
   archiveFolder: string,
@@ -122,7 +228,7 @@ export async function createTar(
   const manifestFilename = 'manifest.txt'
   const cacheFileName = utils.getCacheFileName(compressionMethod)
   const tarFile = 'cache.tar'
-  const tarPath = await getTarPath([])
+  const tarPath = await getTarPath()
   const BSD_TAR_WINDOWS = IS_WINDOWS && tarPath === SystemTarPathOnWindows
   writeFileSync(
     path.join(archiveFolder, manifestFilename),
@@ -135,11 +241,16 @@ export async function createTar(
   // --long=#: Enables long distance matching with # bits. Maximum is 30 (1GB) on 32-bit OS and 31 (2GB) on 64-bit.
   // Using 30 here because we also support 32-bit self-hosted runners.
   // Long range mode is added to zstd in v1.3.2 release, so we will not use --long in older version of zstd.
-  async function getCompressionProgram(): Promise<string[]> {
+  function getCompressionProgram(): string[] {
     switch (compressionMethod) {
       case CompressionMethod.Zstd:
         if (BSD_TAR_WINDOWS) {
-          return ['&&', 'zstd -T0 --long=30 -o']
+          return [
+            '&&',
+            'zstd -T0 --long=30 -o',
+            cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+            tarFile
+          ]
         }
         return [
           '--use-compress-program',
@@ -147,33 +258,20 @@ export async function createTar(
         ]
       case CompressionMethod.ZstdWithoutLong:
         if (BSD_TAR_WINDOWS) {
-          return ['&&', 'zstd -T0 -o']
+          return [
+            '&&',
+            'zstd -T0 -o',
+            cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
+            tarFile
+          ]
         }
         return ['--use-compress-program', IS_WINDOWS ? 'zstd -T0' : 'zstdmt']
       default:
         return ['-z']
     }
   }
-  const args = [
+  const tarArgs = await getTarArgs(compressionMethod, 'create')
-    '--posix',
+  const compressionArgs = getCompressionProgram()
-    '-cf',
+  const args = tarArgs.concat(compressionArgs)
-    BSD_TAR_WINDOWS
-      ? tarFile
-      : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-    '--exclude',
-    BSD_TAR_WINDOWS
-      ? tarFile
-      : cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-    '-P',
-    '-C',
-    workingDirectory.replace(new RegExp(`\\${path.sep}`, 'g'), '/'),
-    '--files-from',
-    manifestFilename,
-    ...(await getCompressionProgram())
-  ].concat(
-    BSD_TAR_WINDOWS
-      ? [cacheFileName.replace(new RegExp(`\\${path.sep}`, 'g'), '/')]
-      : []
-  )
   await execTar(args, archiveFolder)
 }