fix: fixes an issue where the cache could not save .dot directories (#4843)

Lukas Holzer · web-flow · commit ccb3a005e69a · 2023-02-01T16:07:03.000Z
Fixes https://github.com/netlify/pillar-workflow/issues/1017
diff --git a/packages/build/src/core/constants.js b/packages/build/src/core/constants.js
@@ -21,7 +21,6 @@ export const getConstants = async function ({
 }) {
   const isLocal = mode !== 'buildbot'
   const normalizedCacheDir = getCacheDir({ cacheDir, cwd: buildDir })
-
   const constants = {
     // Path to the Netlify configuration file
     CONFIG_PATH: configPath,
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/.gitignore b/packages/build/tests/plugins/fixtures/cache_utils/.gitignore
@@ -0,0 +1 @@
+dist
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/build.mjs b/packages/build/tests/plugins/fixtures/cache_utils/build.mjs
@@ -0,0 +1,21 @@
+import { mkdir, readdir, writeFile } from 'fs/promises';
+import { existsSync } from 'fs';
+
+
+if (existsSync('dist/.dot')) {
+  console.log('content inside dist/.dot:');
+  console.log((await readdir('dist/.dot')).join('\n'));
+}
+if (existsSync('dist/other')) {
+  console.log('content inside dist/other:');
+  console.log((await readdir('dist/other')).join('\n'));
+}
+
+console.log('Generate files');
+await mkdir('dist/.dot', { recursive: true });
+await mkdir('dist/other', { recursive: true });
+
+await Promise.all([
+  writeFile('dist/.dot/hello.txt', ''),
+  writeFile('dist/other/index.html', '<h1>hello world</h1>'),
+]);
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/netlify-plugin-cache/index.js b/packages/build/tests/plugins/fixtures/cache_utils/netlify-plugin-cache/index.js
@@ -0,0 +1,8 @@
+module.exports = {
+  onPreBuild: async ({ utils }) => {
+    await utils.cache.restore('dist');
+  },
+  onPostBuild: async ({ utils }) => {
+    await utils.cache.save('dist');
+  },
+};
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/netlify-plugin-cache/manifest.yaml b/packages/build/tests/plugins/fixtures/cache_utils/netlify-plugin-cache/manifest.yaml
@@ -0,0 +1 @@
+name: custom-cache-plugin
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/netlify.toml b/packages/build/tests/plugins/fixtures/cache_utils/netlify.toml
@@ -0,0 +1,5 @@
+[build]
+command = "node build.mjs"
+
+[[plugins]]
+package = "/netlify-plugin-cache"
diff --git a/packages/build/tests/plugins/fixtures/cache_utils/package.json b/packages/build/tests/plugins/fixtures/cache_utils/package.json
@@ -0,0 +1,3 @@
+{
+  "type": "commonjs"
+}
diff --git a/packages/build/tests/plugins/snapshots/tests.js.md b/packages/build/tests/plugins/snapshots/tests.js.md
@@ -1943,6 +1943,80 @@ Generated by [AVA](https://avajs.dev).
     (Netlify Build completed in 1ms)␊
     Build step duration: Netlify Build completed in 1ms`
 
+## Cache utils are caching .dot directories as well
+
+> Snapshot 1
+
+    `␊
+      Netlify Build                                                 ␊
+    ────────────────────────────────────────────────────────────────␊
+    ␊
+    > Version␊
+      @netlify/build 1.0.0␊
+    ␊
+    > Flags␊
+      debug: true␊
+      repositoryRoot: packages/build/tests/plugins/fixtures/cache_utils␊
+      testOpts:␊
+        pluginsListUrl: test␊
+        silentLingeringProcesses: true␊
+    ␊
+    > Current directory␊
+      packages/build/tests/plugins/fixtures/cache_utils␊
+    ␊
+    > Config file␊
+      packages/build/tests/plugins/fixtures/cache_utils/netlify.toml␊
+    ␊
+    > Resolved config␊
+      build:␊
+        command: node build.mjs␊
+        commandOrigin: config␊
+        publish: packages/build/tests/plugins/fixtures/cache_utils␊
+        publishOrigin: default␊
+      plugins:␊
+        - inputs: {}␊
+          origin: config␊
+          package: /external/path␊
+    ␊
+    > Context␊
+      production␊
+    ␊
+    > Loading plugins␊
+       - /external/path from netlify.toml␊
+    ␊
+      1. /external/path (onPreBuild event)                   ␊
+    ────────────────────────────────────────────────────────────────␊
+    ␊
+    ␊
+    (/external/path onPreBuild completed in 1ms)␊
+    Build step duration: /external/path onPreBuild completed in 1ms␊
+    ␊
+      2. build.command from netlify.toml                            ␊
+    ────────────────────────────────────────────────────────────────␊
+    ␊
+    $ node build.mjs␊
+    content inside dist/.dot:␊
+    hello.txt␊
+    content inside dist/other:␊
+    index.html␊
+    Generate files␊
+    ␊
+    (build.command completed in 1ms)␊
+    Build step duration: build.command completed in 1ms␊
+    ␊
+      3. /external/path (onPostBuild event)                  ␊
+    ────────────────────────────────────────────────────────────────␊
+    ␊
+    ␊
+    (/external/path onPostBuild completed in 1ms)␊
+    Build step duration: /external/path onPostBuild completed in 1ms␊
+    ␊
+      Netlify Build Complete                                        ␊
+    ────────────────────────────────────────────────────────────────␊
+    ␊
+    (Netlify Build completed in 1ms)␊
+    Build step duration: Netlify Build completed in 1ms`
+
 ## Can run list util
 
 > Snapshot 1
diff --git a/packages/build/tests/plugins/snapshots/tests.js.snap b/packages/build/tests/plugins/snapshots/tests.js.snap
diff --git a/packages/build/tests/plugins/tests.js b/packages/build/tests/plugins/tests.js
@@ -198,6 +198,16 @@ test('Can run utils', async (t) => {
   }
 })
 
+test('Cache utils are caching .dot directories as well', async (t) => {
+  // cleanup cache first
+  await removeDir([`${FIXTURES_DIR}/cache_utils/dist`, `${FIXTURES_DIR}/cache_utils/.netlify`])
+  // generate cache
+  await new Fixture('./fixtures/cache_utils').runWithBuild()
+  // should have cached files in the output message
+  const output = await new Fixture('./fixtures/cache_utils').runWithBuild()
+  t.snapshot(normalizeOutput(output))
+})
+
 test('Can run list util', async (t) => {
   const output = await new Fixture('./fixtures/functions_list').runWithBuild()
   t.snapshot(normalizeOutput(output))
diff --git a/packages/cache-utils/src/fs.ts b/packages/cache-utils/src/fs.ts
@@ -2,67 +2,80 @@ import { promises as fs } from 'fs'
 import { basename, dirname } from 'path'
 
 import cpy from 'cpy'
-import { globby } from 'globby'
+import { Options, globby } from 'globby'
 import { isNotJunk } from 'junk'
 import { moveFile } from 'move-file'
 
 /**
  * Move or copy a cached file/directory from/to a local one
+ * @param src The src directory or file to cache
+ * @param dest The destination location
+ * @param move If the file should be moved, moving is faster but removes the source files locally
  */
-export const moveCacheFile = async function (src: string, dest: string, move: boolean) {
+export const moveCacheFile = async function (src: string, dest: string, move = false) {
   // Moving is faster but removes the source files locally
   if (move) {
     return moveFile(src, dest, { overwrite: false })
   }
 
-  const glob = await getSrcGlob(src)
-  if (glob) {
-    return cpy(glob.srcGlob, dirname(dest), { cwd: glob.cwd, parents: true, overwrite: false })
+  const { srcGlob, ...options } = await getSrcGlob(src)
+  if (srcGlob) {
+    return cpy(srcGlob, dirname(dest), { ...options, parents: true, overwrite: false })
   }
 }
 
 /**
  * Non-existing files and empty directories are always skipped
  */
 export const hasFiles = async function (src: string): Promise<boolean> {
-  const glob = await getSrcGlob(src)
-  if (!glob) {
-    return false
-  }
-
-  return glob.srcGlob !== undefined && !(await isEmptyDir({ srcGlob: glob.srcGlob, cwd: glob.cwd, isDir: glob.isDir }))
+  const { srcGlob, isDir, ...options } = await getSrcGlob(src)
+  return srcGlob !== undefined && !(await isEmptyDir(srcGlob, isDir, options))
 }
 
 /** Replicates what `cpy` is doing under the hood. */
-const isEmptyDir = async function ({ srcGlob, cwd, isDir }) {
+const isEmptyDir = async function (globPattern: string, isDir: boolean, options: Options) {
   if (!isDir) {
     return false
   }
 
-  const files = await globby(srcGlob, { cwd })
+  const files = await globby(globPattern, options)
   const filteredFiles = files.filter((file) => isNotJunk(basename(file)))
   return filteredFiles.length === 0
 }
 
+type GlobOptions = {
+  srcGlob?: string
+  isDir: boolean
+  cwd: string
+  dot?: boolean
+}
+
 /**
  * Get globbing pattern with files to move/copy
  */
-const getSrcGlob = async function (src: string): Promise<null | { srcGlob: string; cwd: string; isDir: boolean }> {
+const getSrcGlob = async function (src: string): Promise<GlobOptions> {
   const srcStat = await getStat(src)
 
   if (srcStat === undefined) {
-    return null
+    return { srcGlob: undefined, isDir: false, cwd: '' }
   }
 
   const isDir = srcStat.isDirectory()
   const srcBasename = basename(src)
   const cwd = dirname(src)
 
+  const baseOptions: GlobOptions = {
+    srcGlob: srcBasename,
+    isDir,
+    cwd,
+    dot: true, // collect .dot directories as well
+  }
+
   if (isDir) {
-    return { srcGlob: `${srcBasename}/**`, cwd, isDir }
+    return { ...baseOptions, srcGlob: `${srcBasename}/**` }
   }
 
-  return { srcGlob: srcBasename, cwd, isDir }
+  return baseOptions
 }
 
 const getStat = async (src: string) => {
diff --git a/packages/cache-utils/tests/digests.test.ts b/packages/cache-utils/tests/digests.test.ts
diff --git a/packages/cache-utils/tests/dir.test.ts b/packages/cache-utils/tests/dir.test.ts
@@ -1,26 +1,33 @@
 import { promises as fs } from 'fs'
 
 import { pathExists } from 'path-exists'
-import { expect, test, vi } from 'vitest'
+import { SpyInstance, afterEach, beforeEach, expect, test, vi } from 'vitest'
 
 import { restore, save } from '../src/main.js'
 
 import { createTmpDir, removeFiles } from './helpers/main.js'
 
-test('Should allow changing the cache directory', async () => {
-  const [cacheDir, srcDir] = await Promise.all([createTmpDir(), createTmpDir()])
-  const cwdSpy = vi.spyOn(process, 'cwd').mockImplementation(() => srcDir)
-  try {
-    const srcFile = `${srcDir}/test`
-    await fs.writeFile(srcFile, '')
-    expect(await save(srcFile, { cacheDir })).toBe(true)
-    const cachedFiles = await fs.readdir(cacheDir)
-    expect(cachedFiles.length).toBe(1)
-    await removeFiles(srcFile)
-    expect(await restore(srcFile, { cacheDir })).toBe(true)
-    expect(await pathExists(srcFile)).toBe(true)
-  } finally {
-    await removeFiles([cacheDir, srcDir])
-  }
+let cacheDir, srcDir: string
+let cwdSpy: SpyInstance<[], string>
+
+beforeEach(async () => {
+  cacheDir = await createTmpDir()
+  srcDir = await createTmpDir()
+  cwdSpy = vi.spyOn(process, 'cwd').mockImplementation(() => srcDir)
+})
+
+afterEach(async () => {
+  await removeFiles([cacheDir, srcDir])
   cwdSpy.mockRestore()
 })
+
+test('Should allow changing the cache directory', async () => {
+  const srcFile = `${srcDir}/test`
+  await fs.writeFile(srcFile, '')
+  expect(await save(srcFile, { cacheDir })).toBe(true)
+  const cachedFiles = await fs.readdir(cacheDir)
+  expect(cachedFiles.length).toBe(1)
+  await removeFiles(srcFile)
+  expect(await restore(srcFile, { cacheDir })).toBe(true)
+  expect(await pathExists(srcFile)).toBe(true)
+})
diff --git a/packages/testing/src/dir.ts b/packages/testing/src/dir.ts
@@ -27,7 +27,7 @@ const createGit = (cwd: string) => {
 // Removing a directory sometimes fails on Windows in CI due to Windows
 // directory locking.
 // This results in `EBUSY: resource busy or locked, rmdir /path/to/dir`
-export const removeDir = async function (dir) {
+export const removeDir = async function (dir: string | string[]) {
   try {
     await del(dir, { force: true })
   } catch {
