Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/build/src/core/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
dist
21 changes: 21 additions & 0 deletions packages/build/tests/plugins/fixtures/cache_utils/build.mjs
Original file line number Diff line number Diff line change
@@ -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>'),
]);
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module.exports = {
onPreBuild: async ({ utils }) => {
await utils.cache.restore('dist');
},
onPostBuild: async ({ utils }) => {
await utils.cache.save('dist');
},
};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
name: custom-cache-plugin
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
[build]
command = "node build.mjs"

[[plugins]]
package = "/netlify-plugin-cache"
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"type": "commonjs"
}
74 changes: 74 additions & 0 deletions packages/build/tests/plugins/snapshots/tests.js.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Binary file modified packages/build/tests/plugins/snapshots/tests.js.snap
Binary file not shown.
10 changes: 10 additions & 0 deletions packages/build/tests/plugins/tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the comment, but wouldn't it be better to do some explicit matching here so we do not even need the comment?

Something like

t.true(output.includes("content inside dist/.dot\nhello.txt"))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point I hate those snapshots anyway πŸ˜‚

})

test('Can run list util', async (t) => {
const output = await new Fixture('./fixtures/functions_list').runWithBuild()
t.snapshot(normalizeOutput(output))
Expand Down
47 changes: 30 additions & 17 deletions packages/cache-utils/src/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
39 changes: 23 additions & 16 deletions packages/cache-utils/tests/dir.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
2 changes: 1 addition & 1 deletion packages/testing/src/dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down