Skip to content
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

Fix cross-device mv by switching to copy #846

Closed
wants to merge 33 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
53116ae
sm-polyfilling
seanmorris Dec 6, 2023
e985f2e
Updating mv to work across devices
seanmorris Dec 6, 2023
25471e2
Refactoring.
seanmorris Dec 6, 2023
aa23eb5
Tweaks.
seanmorris Dec 6, 2023
fe314c0
Separating polyfills.
seanmorris Dec 12, 2023
71b5108
Separating polyfills.
seanmorris Dec 12, 2023
b1dd043
Switching from move to copy.
seanmorris Dec 12, 2023
430cd8a
Switching from move to copy.
seanmorris Dec 12, 2023
0575d89
Lint
seanmorris Dec 12, 2023
a45592e
Comment.
seanmorris Dec 13, 2023
c774621
More readable copy function
seanmorris Dec 15, 2023
fd8f768
Simplifying.
seanmorris Dec 15, 2023
6d8c576
Comments.
seanmorris Dec 15, 2023
06c2315
Guard clause.
seanmorris Dec 15, 2023
b760a1d
Unit test for copy, extending timeout for networking test
seanmorris Dec 15, 2023
d112c05
Explicit recursive copy.
seanmorris Dec 15, 2023
5097fb1
Adding directory copy test.
seanmorris Dec 15, 2023
b182c56
Expanding timeout for e2e
seanmorris Dec 15, 2023
396874a
Expanding timeout for e2e
seanmorris Dec 15, 2023
21b86e2
Extending test timeout.
seanmorris Dec 15, 2023
d9d8201
Fixing copy for web
seanmorris Dec 16, 2023
25b50f8
Fixing copy for web
seanmorris Dec 16, 2023
9e8453b
Reverting.
seanmorris Dec 16, 2023
cd3902f
PR comments.
seanmorris Dec 16, 2023
0a3ef1e
Options object.
seanmorris Dec 16, 2023
577d824
options object
seanmorris Dec 16, 2023
100c4db
options object
seanmorris Dec 16, 2023
ca027d4
Typescript fix.
seanmorris Dec 16, 2023
6f97978
Typescript fix.
seanmorris Dec 16, 2023
36c17b8
Unit test
seanmorris Dec 16, 2023
bcb225d
More readablity.
seanmorris Dec 16, 2023
8da6708
File to Dir test.
seanmorris Dec 16, 2023
cc576fc
doc comment
seanmorris Dec 16, 2023
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
3 changes: 3 additions & 0 deletions nx.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
}
}
},
"affected": {
"defaultBase": "trunk"
},
"pluginsConfig": {
"@nx/js": {
"analyzeSourceFiles": true
Expand Down
78 changes: 74 additions & 4 deletions packages/php-wasm/node/src/test/php.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
$pipes
);

// Yields back to JS event loop to capture and process the
// Yields back to JS event loop to capture and process the
// child_process output. This is fine. Regular PHP scripts
// typically wait for the child process to finish.
sleep(1);
Expand Down Expand Up @@ -122,7 +122,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
// This test fails
if (!['7.0', '7.1', '7.2', '7.3'].includes(phpVersion)) {
/*
There is a race condition in this variant of the test which
There is a race condition in this variant of the test which
causes the following failure (but only sometimes):

src/test/php.spec.ts > PHP 8.2 > proc_open() > cat – stdin=pipe, stdout=file, stderr=file
Expand All @@ -142,7 +142,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
);
fwrite($pipes[0], 'WordPress\n');

// Yields back to JS event loop to capture and process the
// Yields back to JS event loop to capture and process the
// child_process output. This is fine. Regular PHP scripts
// typically wait for the child process to finish.
sleep(1);
Expand Down Expand Up @@ -173,7 +173,7 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
$pipes
);

// Yields back to JS event loop to capture and process the
// Yields back to JS event loop to capture and process the
// child_process output. This is fine. Regular PHP scripts
// typically wait for the child process to finish.
sleep(1);
Expand Down Expand Up @@ -324,6 +324,76 @@ describe.each(SupportedPHPVersions)('PHP %s', (phpVersion) => {
);
});

it('cp() should copy a file', () => {
php.mkdir(testDirPath);
const file1 = testDirPath + '/1.txt';
const file2 = testDirPath + '/2.txt';
php.writeFile(file1, '1');
php.cp(file1, file2);
expect(php.fileExists(file1)).toEqual(true);
expect(php.fileExists(file2)).toEqual(true);
expect(php.readFileAsText(file2)).toEqual('1');
});

it('cp() should copy a directory', () => {
const testDirPath1 = testDirPath;
const testDirPath2 = '/__test987654322';
php.mkdir(testDirPath1);
php.writeFile(testDirPath1 + '/1.txt', '1');
php.writeFile(testDirPath1 + '/2.txt', '2');
php.cp(testDirPath1, testDirPath2, { recursive: true });
expect(php.fileExists(testDirPath2 + '/1.txt')).toEqual(true);
expect(php.fileExists(testDirPath2 + '/2.txt')).toEqual(true);
expect(php.readFileAsText(testDirPath2 + '/1.txt')).toEqual('1');
expect(php.readFileAsText(testDirPath2 + '/2.txt')).toEqual('2');
});

it('cp() should copy a file into a directory', () => {
const testDirPath1 = testDirPath;
const testDirPath2 = '/__test987654322';
php.mkdir(testDirPath1);
php.mkdir(testDirPath2);
php.writeFile(testDirPath1 + '/1.txt', '1');
php.cp(testDirPath1 + '/1.txt', testDirPath2);
expect(php.fileExists(testDirPath2 + '/1.txt')).toEqual(true);
expect(php.readFileAsText(testDirPath2 + '/1.txt')).toEqual('1');
});

it('cp() should replace target file if it exists', () => {
php.mkdir(testDirPath);
const file1 = testDirPath + '/1.txt';
const file2 = testDirPath + '/2.txt';
php.writeFile(file1, '1');
php.writeFile(file2, '2');
php.cp(file1, file2);
expect(php.fileExists(file1)).toEqual(true);
expect(php.fileExists(file2)).toEqual(true);
expect(php.readFileAsText(file2)).toEqual('1');
});

it('cp() should throw a useful error when source file does not exist', () => {
php.mkdir(testDirPath);
const file1 = testDirPath + '/1.txt';
const file2 = testDirPath + '/2.txt';
expect(() => {
php.cp(file1, file2);
}).toThrowError(
`Could not copy "${testDirPath}/1.txt": There is no such file or directory OR the parent directory does not exist.`
);
});

it('cp() should throw a useful error when target directory does not exist', () => {
php.mkdir(testDirPath);
const file1 = testDirPath + '/1.txt';
const file2 = testDirPath + '/nowhere/2.txt';
php.writeFile(file1, '1');
expect(() => {
php.cp(file1, file2);
}).toThrowError(
`Could not copy "${testDirPath}/1.txt": There is no such file or directory OR the parent directory does not exist.`
);
});

it('mkdir() should create a directory', () => {
php.mkdir(testDirPath);
expect(php.fileExists(testDirPath)).toEqual(true);
Expand Down
74 changes: 73 additions & 1 deletion packages/php-wasm/universal/src/lib/base-php.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,14 @@ import {
SpawnHandler,
PHPEventListener,
PHPEvent,
CpOptions,
} from './universal-php';
import {
getFunctionsMaybeMissingFromAsyncify,
improveWASMErrorReporting,
UnhandledRejectionsTarget,
} from './wasm-error-reporting';
import { Semaphore } from '@php-wasm/util';
import { Semaphore, joinPaths, basename } from '@php-wasm/util';

const STRING = 'string';
const NUMBER = 'number';
Expand Down Expand Up @@ -616,6 +617,77 @@ export abstract class BasePHP implements IsomorphicLocalPHP {
this[__private__dont__use].FS.rename(fromPath, toPath);
}

/** @inheritDoc */
@rethrowFileSystemError('Could not copy "{path}"')
cp(
sourcePath: string,
destinationPath: string,
options: CpOptions = { recursive: false }
) {
const FS = this[__private__dont__use].FS;

const sourceStat = FS.stat(sourcePath);

// The FILEMODE tells us things about the file, like
// permissions, whether its a file, link, or directory
// as well as its access and exec permissions.
//
// The 15th bit of the FILEMODE is the sticky-bit.
// Emscripten directories will have the sticky-bit set.
//
// We'll use a binary literal (0b...) with a logical
// and (&) to check for that.
const stickyBit = 0b100000000000000;
const sourceIsDir = sourceStat.mode & stickyBit;

let destinationExists: boolean;
let destinationIsDir = false;

try {
const destinationStat = FS.stat(destinationPath);
destinationExists = true;
// Similar check to above but testing if DESTINATION is a directory.
destinationIsDir = !!(destinationStat.mode & stickyBit);
} catch {
destinationExists = false;
}

if (!sourceIsDir) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What will happen if I copy a directory tree like wordpress/wp-content/plugins onto another directory, say /www that already has wordpress/wp-content/plugins in it? It seems like it would duplicate some directories?

const file = this.readFileAsBuffer(sourcePath);

if (destinationIsDir) {
FS.writeFile(
joinPaths(destinationPath, basename(sourcePath)),
file
);
} else {
FS.writeFile(destinationPath, file);
}

return;
}

if (!options.recursive) {
throw new Error(
`Cannot use non-recurive copy on directory: ${sourcePath}`
);
}

if (!destinationExists) {
FS.mkdir(destinationPath);
}

const files = this.listFiles(sourcePath);

files.forEach((file: string) =>
this.cp(
joinPaths(sourcePath, file),
joinPaths(destinationPath, file),
options
)
);
}

/** @inheritDoc */
@rethrowFileSystemError('Could not remove directory "{path}"')
rmdir(path: string, options: RmDirOptions = { recursive: true }) {
Expand Down
1 change: 1 addition & 0 deletions packages/php-wasm/universal/src/lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ export type {
PHPRunOptions,
UniversalPHP,
ListFilesOptions,
CpOptions,
RmDirOptions,
PHPEvent,
PHPEventListener,
Expand Down
27 changes: 27 additions & 0 deletions packages/php-wasm/universal/src/lib/universal-php.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,25 @@ export interface IsomorphicLocalPHP extends RequestHandler {
*/
mv(oldPath: string, newPath: string): void;

/**
* Copies a file or directory in the PHP filesystem to a
* new location. The target directory's parent must be a
* valid location in the filesystem prior to copying.
*
* If the target path is a file that already exists,
* it will be overwritten.
*
* If the target path is a directory that already exists,
* the file or directory will be copied into it.
*
* If the target path's parent directory does not exist,
* an error will be thrown.
*
* @param oldPath The file or directory to be copied.
* @param newPath The new, full path to copy the file or directory to.
*/
cp(oldPath: string, newPath: string, options?: CpOptions): void;

/**
* Removes a directory from the PHP filesystem.
*
Expand Down Expand Up @@ -542,6 +561,14 @@ export interface FileInfo {
data: Uint8Array;
}

export interface CpOptions {
/**
* If true, recursively copies the directory and all its contents.
* Default: false.
*/
recursive?: boolean;
}

export interface RmDirOptions {
/**
* If true, recursively removes the directory and all its contents.
Expand Down
6 changes: 6 additions & 0 deletions packages/php-wasm/web/src/lib/web-php-endpoint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type {
PHPRequest,
PHPResponse,
PHPRunOptions,
CpOptions,
RmDirOptions,
SpawnHandler,
PHPEventListener,
Expand Down Expand Up @@ -89,6 +90,11 @@ export class WebPHPEndpoint implements IsomorphicLocalPHP {
return _private.get(this)!.php.mv(fromPath, toPath);
}

/** @inheritDoc @php-wasm/universal!IsomorphicLocalPHP.cp */
cp(fromPath: string, toPath: string, options?: CpOptions) {
return _private.get(this)!.php.cp(fromPath, toPath, options);
}

/** @inheritDoc @php-wasm/universal!IsomorphicLocalPHP.rmdir */
rmdir(path: string, options?: RmDirOptions) {
return _private.get(this)!.php.rmdir(path, options);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ export async function installAsset(

// Move asset folder to target path
const assetFolderPath = `${targetPath}/${assetFolderName}`;
await playground.mv(tmpAssetPath, assetFolderPath);
await playground.cp(tmpAssetPath, assetFolderPath, { recursive: true });
await cleanup();

return {
Expand Down
Loading