Skip to content

Commit

Permalink
fix: switch file mode option to executable
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshuaKGoldberg committed Jan 15, 2025
1 parent f7f1dac commit df00504
Show file tree
Hide file tree
Showing 13 changed files with 154 additions and 51 deletions.
65 changes: 65 additions & 0 deletions packages/create-fs/src/createWritingFileSystem.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import { describe, expect, it, vi } from "vitest";

import { createWritingFileSystem } from "./createWritingFileSystem.js";

const mockMkdir = vi.fn();
const mockWriteFile = vi.fn();

vi.mock("node:fs/promises", () => ({
get mkdir() {
return mockMkdir;
},
get writeFile() {
return mockWriteFile;
},
}));

const contents = "abc123";
const directoryPath = "path/to/file";
const filePath = "path/to/file";

describe("createWritingFileSystem", () => {
describe("writeDirectory", () => {
it("writes with recursive: true", async () => {
const system = createWritingFileSystem();

await system.writeDirectory(directoryPath);

expect(mockMkdir).toHaveBeenCalledWith(directoryPath, {
recursive: true,
});
});
});

describe("writeFile", () => {
it("writes with mode 0x644 when options does not exist", async () => {
const system = createWritingFileSystem();

await system.writeFile(filePath, contents);

expect(mockWriteFile).toHaveBeenCalledWith(filePath, contents, {
mode: 0x644,
});
});

it("writes with mode 0x644 when options.executable is false", async () => {
const system = createWritingFileSystem();

await system.writeFile(filePath, contents, { executable: false });

expect(mockWriteFile).toHaveBeenCalledWith(filePath, contents, {
mode: 0x644,
});
});

it("writes with mode 0x755 when options.executable is true", async () => {
const system = createWritingFileSystem();

await system.writeFile(filePath, contents, { executable: true });

expect(mockWriteFile).toHaveBeenCalledWith(filePath, contents, {
mode: 0x755,
});
});
});
});
11 changes: 9 additions & 2 deletions packages/create-fs/src/createWritingFileSystem.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,21 @@
import * as fs from "node:fs/promises";

import { createReadingFileSystem } from "./createReadingFileSystem.js";
import { CreatedFileOptions } from "./types/files.js";

export function createWritingFileSystem() {
return {
...createReadingFileSystem(),
writeDirectory: async (directoryPath: string) =>
void (await fs.mkdir(directoryPath, { recursive: true })),
writeFile: async (filePath: string, contents: string) => {
await fs.writeFile(filePath, contents);
writeFile: async (
filePath: string,
contents: string,
options?: CreatedFileOptions,
) => {
await fs.writeFile(filePath, contents, {
mode: options?.executable ? 0x755 : 0x644,
});
},
};
}
20 changes: 10 additions & 10 deletions packages/create-fs/src/intake/intakeFromDirectory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,18 @@ describe("intakeFromDirectory", () => {
mockStat
.mockResolvedValueOnce({
isDirectory: () => false,
mode: 123,
mode: 0x644,
})
.mockResolvedValueOnce({
isDirectory: () => false,
mode: 456,
mode: 0x755,
});

const directory = await intakeFromDirectory("from");

expect(directory).toEqual({
"included-a": ["contents-a", { mode: 123 }],
"included-b": ["contents-b", { mode: 456 }],
"included-a": ["contents-a", { executable: false }],
"included-b": ["contents-b", { executable: true }],
});
expect(mockReaddir.mock.calls).toEqual([["from"]]);
expect(mockStat.mock.calls).toEqual([
Expand All @@ -65,20 +65,20 @@ describe("intakeFromDirectory", () => {
mockStat
.mockResolvedValueOnce({
isDirectory: () => false,
mode: 123,
mode: 0x644,
})
.mockResolvedValueOnce({
isDirectory: () => false,
mode: 456,
mode: 0x755,
});

const directory = await intakeFromDirectory("from", {
exclude: /excluded/,
});

expect(directory).toEqual({
"included-a": ["contents-a", { mode: 123 }],
"included-b": ["contents-b", { mode: 456 }],
"included-a": ["contents-a", { executable: false }],
"included-b": ["contents-b", { executable: true }],
});
expect(mockReaddir.mock.calls).toEqual([["from"]]);
expect(mockStat.mock.calls).toEqual([
Expand All @@ -98,7 +98,7 @@ describe("intakeFromDirectory", () => {
})
.mockResolvedValueOnce({
isDirectory: () => false,
mode: 123,
mode: 0x644,
});

const directory = await intakeFromDirectory("from", {
Expand All @@ -107,7 +107,7 @@ describe("intakeFromDirectory", () => {

expect(directory).toEqual({
middle: {
included: ["contents", { mode: 123 }],
included: ["contents", { executable: false }],
},
});
expect(mockReaddir.mock.calls).toEqual([["from"], ["from/middle"]]);
Expand Down
6 changes: 5 additions & 1 deletion packages/create-fs/src/intake/intakeFromDirectory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as fs from "node:fs/promises";
import path from "node:path";

import { CreatedDirectory } from "../types/files.js";
import { isModeExecutable } from "./isModeExecutable.js";

export interface IntakeFromDirectorySettings {
exclude?: RegExp;
Expand All @@ -24,7 +25,10 @@ export async function intakeFromDirectory(

directory[child] = stat.isDirectory()
? await intakeFromDirectory(childPath, settings)
: [(await fs.readFile(childPath)).toString(), { mode: stat.mode }];
: [
(await fs.readFile(childPath)).toString(),
{ executable: isModeExecutable(stat.mode) },
];
}

return directory;
Expand Down
13 changes: 13 additions & 0 deletions packages/create-fs/src/intake/isModeExecutable.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { describe, expect, test } from "vitest";

import { isModeExecutable } from "./isModeExecutable.js";

describe("isModeExecutable", () => {
test.each([
["0x755", true],
["0x777", true],
["0x644", false],
])("%s is %j", (mode, expected) => {
expect(isModeExecutable(parseInt(mode, 16))).toBe(expected);
});
});
3 changes: 3 additions & 0 deletions packages/create-fs/src/intake/isModeExecutable.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export function isModeExecutable(mode: number) {
return (mode & 0o1) !== 0;
}
5 changes: 2 additions & 3 deletions packages/create-fs/src/types/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ export type CreatedFileEntry =

export interface CreatedFileOptions {
/**
* File mode (permission and sticky bits) per chmod().
* @example 0o777 for an executable file.
* Whether to set executable permissions (e.g. 0x755) instead of non-executable (e.g. 0x644).
*/
mode?: number;
executable?: boolean;
}
2 changes: 1 addition & 1 deletion packages/create-fs/src/types/system.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ export type WriteFile = (
) => Promise<void>;

export interface WriteFileOptions {
mode?: number;
executable?: boolean;
}

export interface WritingFileSystem extends ReadingFileSystem {
Expand Down
42 changes: 27 additions & 15 deletions packages/create-testers/src/diffCreatedDirectory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,30 +34,42 @@ describe("diffCreatedDirectory", () => {
],
[{ a: "b\n" }, {}, undefined],
[{ a: "" }, { a: [""] }, undefined],
[{ a: "" }, { a: ["", { mode: undefined }] }, undefined],
[{ a: "" }, { a: ["", { mode: 123 }] }, undefined],
[{ a: [""] }, { a: ["", { mode: 123 }] }, undefined],
[{ a: "" }, { a: ["", { executable: undefined }] }, undefined],
[{ a: "" }, { a: ["", { executable: true }] }, undefined],
[{ a: [""] }, { a: ["", { executable: true }] }, undefined],
[
{ a: ["", { mode: undefined }] },
{ a: ["", { mode: undefined }] },
{ a: ["", { executable: undefined }] },
{ a: ["", { executable: undefined }] },
undefined,
],
[{ a: ["", { mode: undefined }] }, { a: ["", { mode: 123 }] }, undefined],
[{ a: ["", { mode: 123 }] }, { a: ["", { mode: 123 }] }, undefined],
[{ a: ["", { mode: 123 }] }, { a: [""] }, undefined],
[{ a: ["", { mode: 123 }] }, { a: ["", {}] }, undefined],
[{ a: ["", { mode: 123 }] }, { a: ["", { mode: undefined }] }, undefined],
[
{ a: ["", { mode: 123 }] },
{ a: ["", { mode: 456 }] },
{ a: ["", { executable: undefined }] },
{ a: ["", { executable: true }] },
undefined,
],
[
{ a: ["", { executable: true }] },
{ a: ["", { executable: true }] },
undefined,
],
[{ a: ["", { executable: true }] }, { a: [""] }, undefined],
[{ a: ["", { executable: true }] }, { a: ["", {}] }, undefined],
[
{ a: ["", { executable: true }] },
{ a: ["", { executable: undefined }] },
undefined,
],
[
{ a: ["", { executable: true }] },
{ a: ["", { executable: false }] },
{
a: [
undefined,
{
mode: `@@ -1,1 +1,1 @@
-7b
executable: `@@ -1,1 +1,1 @@
-true
\\ No newline at end of file
+1c8
+false
\\ No newline at end of file
`,
},
Expand Down
14 changes: 7 additions & 7 deletions packages/create-testers/src/diffCreatedDirectory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export type DiffedCreatedFileEntry =
| string;

export interface DiffedCreatedFileOptions {
mode?: string;
executable?: string;
}

export type ProcessText = (text: string, filePath: string) => string;
Expand Down Expand Up @@ -177,17 +177,17 @@ function diffCreatedFileOptions(
pathToFile: string,
): DiffedCreatedFileOptions | undefined {
if (
actual?.mode === undefined ||
created?.mode === undefined ||
actual.mode === created.mode
actual?.executable === undefined ||
created?.executable === undefined ||
actual.executable === created.executable
) {
return undefined;
}

return {
mode: diffCreatedFileText(
actual.mode.toString(16),
created.mode.toString(16),
executable: diffCreatedFileText(
actual.executable.toString(),
created.executable.toString(),
pathToFile,
(text) => text,
),
Expand Down
12 changes: 6 additions & 6 deletions packages/create/src/mergers/mergeFileEntries.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ describe("mergeFileEntries", () => {
[["a", {}], ["a", {}], "a"],
[["a", {}], ["a", {}], "a"],
[
["a", { mode: 0o123 }],
["a", { mode: 0o123 }],
["a", { mode: 0o123 }],
["a", { executable: true }],
["a", { executable: true }],
["a", { executable: true }],
],
[
["a", { mode: 0o123 }],
["a", { mode: 0o456 }],
new Error(`Conflicting created file modes at path: 'test/path'.`),
["a", { executable: true }],
["a", { executable: false }],
new Error(`Conflicting created file executable at path: 'test/path'.`),
],
["a", "b", new Error("Conflicting created files at path: 'test/path'.")],
[
Expand Down
8 changes: 4 additions & 4 deletions packages/create/src/mergers/mergeFileEntries.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,15 @@ export function mergeFileEntries(
throw new Error(`Conflicting created files at path: '${path.join("/")}'.`);
}

if (firstSettings?.mode !== secondSettings?.mode) {
if (firstSettings?.executable !== secondSettings?.executable) {
throw new Error(
`Conflicting created file modes at path: '${path.join("/")}'.`,
`Conflicting created file executable at path: '${path.join("/")}'.`,
);
}

const mode = firstSettings?.mode ?? secondSettings?.mode;
const executable = firstSettings?.executable ?? secondSettings?.executable;

return mode ? [firstFile as string, { mode }] : firstFile;
return executable ? [firstFile as string, { executable }] : firstFile;
}

function isBlankEntry(entry: CreatedFileEntry | undefined) {
Expand Down
4 changes: 2 additions & 2 deletions packages/site/src/content/docs/engine/runtime/creations.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ Each property in a `files` object may be one of the following:
- `object`: A directory, whose properties recursively are file creations
- `string`: A file to be created
- `[string, CreatedFileOptions]`: A file to be created with [`fsPromises.writeFile` options](https://nodejs.org/api/fs.html#fspromiseswritefilefile-data-options):
- `mode`: Integer mode, such as `0o777` to make executable
- `executable`: Whether to add permissions to make the file executable

For example, this Block generates an executable `.husky/pre-commit` file:

Expand All @@ -90,7 +90,7 @@ export const blockPreCommit = base.createBlock({
produce() {
return {
".husky": {
"pre-commit": ["npx lint-staged\n", { mode: 0o777 }],
"pre-commit": ["npx lint-staged\n", { executable: true }],
},
};
},
Expand Down

0 comments on commit df00504

Please sign in to comment.