From 25cf3715b6c710616d6364f60ea06f6d0bc19c34 Mon Sep 17 00:00:00 2001 From: Ignacio Aldama Vicente Date: Thu, 2 Nov 2023 11:03:16 +0100 Subject: [PATCH 1/6] fix: fix CoW in Windows --- .gitignore | 1 + Cargo.toml | 1 + __test__/main.spec.ts | 158 ++++++++++++++++++++++++--------------- __test__/threads.spec.ts | 4 +- infinite_clone_test.mjs | 1 - package.json | 2 +- src/lib.rs | 119 ++++++++++++++++------------- 7 files changed, 168 insertions(+), 118 deletions(-) diff --git a/.gitignore b/.gitignore index 6fb7c71..761607e 100644 --- a/.gitignore +++ b/.gitignore @@ -200,4 +200,5 @@ Cargo.lock @reflink sandbox +__reflink-tests-* *.json.bak \ No newline at end of file diff --git a/Cargo.toml b/Cargo.toml index 50fc7ac..be8ead7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,6 +7,7 @@ version = "0.0.0" crate-type = ["cdylib"] [dependencies] +copy_on_write = "0.1.0" futures = "0.3.28" # Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix napi = { version = "2.12.2", default-features = false, features = ["napi4"] } diff --git a/__test__/main.spec.ts b/__test__/main.spec.ts index eaaa31f..4c87c2a 100644 --- a/__test__/main.spec.ts +++ b/__test__/main.spec.ts @@ -1,79 +1,91 @@ -import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'; +import { afterAll, describe, expect, it } from 'vitest'; import { join, resolve } from 'path'; -import { reflinkFileSync, reflinkFile } from '../index.js'; import { mkdir, rm, writeFile } from 'fs/promises'; import { readFileSync } from 'fs'; import { randomUUID, createHash } from 'crypto'; +import { rimraf } from 'rimraf'; +import { reflinkFileSync, reflinkFile } from '../index.js'; -const sandboxDir = join(process.cwd(), `__reflink-tests-${randomUUID()}`); +const sandboxDir = () => join(process.cwd(), `__reflink-tests-${randomUUID()}`); const sandboxFiles = [ { - path: join(sandboxDir, 'file1.txt'), + path: 'file1.txt', content: 'Hello World!', sha: createHash('sha256').update('Hello World!').digest('hex'), }, { - path: join(sandboxDir, 'file2.txt'), + path: 'file2.txt', content: 'Hello World!', sha: createHash('sha256').update('Hello World!').digest('hex'), }, { - path: join(sandboxDir, 'file3.txt'), + path: 'file3.txt', content: 'Hello World!', sha: createHash('sha256').update('Hello World!').digest('hex'), }, ]; -describe('reflink', () => { - beforeAll(async () => { - await mkdir(sandboxDir, { recursive: true }); - }); +const sandboxDirectories: string[] = []; - afterAll(async () => { - await rm(sandboxDir, { recursive: true, force: true }); - }); +async function prepare(dir: string) { + await mkdir(dir, { recursive: true }); + + sandboxDirectories.push(dir); - beforeEach(async () => { - // remove the sandbox directory and recreate it - await rm(sandboxDir, { recursive: true, force: true }); - await mkdir(sandboxDir, { recursive: true }); + return Promise.all( + sandboxFiles.map(async (file) => { + await writeFile(join(dir, file.path), file.content); + return { + ...file, + path: join(dir, file.path), + }; + }) + ); +} - // create the files again +describe('reflink', () => { + afterAll(async () => { await Promise.all( - sandboxFiles.map(async (file) => { - await writeFile(file.path, file.content); + sandboxDirectories.map(async (dir) => { + await rimraf(dir).catch(() => {}); }) ); }); - it('should correctly clone a file (sync)', () => { - const file = sandboxFiles[0]; + it('should correctly clone a file (sync)', async () => { + const dir = sandboxDir(); + const files = await prepare(dir); + const file = files[0]; - reflinkFileSync(file.path, join(sandboxDir, 'file1-copy.txt')); + reflinkFileSync(file.path, join(dir, 'file1-copy.txt')); - const content = readFileSync(join(sandboxDir, 'file1-copy.txt'), 'utf-8'); + const content = readFileSync(join(dir, 'file1-copy.txt'), 'utf-8'); expect(content).toBe(file.content); }); it('should correctly clone a file (async)', async () => { - const file = sandboxFiles[0]; + const dir = sandboxDir(); + const files = await prepare(dir); + const file = files[0]; - await reflinkFile(file.path, join(sandboxDir, 'file1-copy.txt')); + await reflinkFile(file.path, join(dir, 'file1-copy.txt')); - const content = readFileSync(join(sandboxDir, 'file1-copy.txt'), 'utf-8'); + const content = readFileSync(join(dir, 'file1-copy.txt'), 'utf-8'); expect(content).toBe(file.content); }); it('should keep the same content in source file after editing the cloned file', async () => { - const file = sandboxFiles[0]; + const dir = sandboxDir(); + const files = await prepare(dir); + const file = files[0]; - await reflinkFile(file.path, join(sandboxDir, 'file1-copy.txt')); + await reflinkFile(file.path, join(dir, 'file1-copy.txt')); await writeFile( - join(sandboxDir, 'file1-copy.txt'), + join(dir, 'file1-copy.txt'), file.content + '\nAdded content!' ); @@ -82,65 +94,84 @@ describe('reflink', () => { expect(originalContent).toBe(file.content); }); - it('should fail if the source file does not exist (sync)', () => { + it('should fail if the source file does not exist (sync)', async () => { + const dir = sandboxDir(); + await prepare(dir); + expect(() => { reflinkFileSync( - join(sandboxDir, 'file-does-not-exist.txt'), - join(sandboxDir, 'file1-copy.txt') + join(dir, 'file-does-not-exist.txt'), + join(dir, 'file1-copy.txt') ); }).toThrow(); }); it('should fail if the source file does not exist (async)', async () => { + const dir = sandboxDir(); + await prepare(dir); + await expect( reflinkFile( - join(sandboxDir, 'file-does-not-exist.txt'), - join(sandboxDir, 'file1-copy.txt') + join(dir, 'file-does-not-exist.txt'), + join(dir, 'file1-copy.txt') ) ).rejects.toThrow(); }); - it('should fail if the destination file already exists (sync)', () => { + it('should fail if the destination file already exists (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); + expect(() => { reflinkFileSync(sandboxFiles[0].path, sandboxFiles[1].path); }).toThrow(); }); it('should fail if the destination file already exists (async)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); await expect( reflinkFile(sandboxFiles[0].path, sandboxFiles[1].path) ).rejects.toThrow(); }); - it('should fail if the source file is a directory (sync)', () => { + it('should fail if the source file is a directory (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); expect(() => { - reflinkFileSync(sandboxDir, sandboxFiles[1].path); + reflinkFileSync(dir, sandboxFiles[1].path); }).toThrow(); }); it('should fail if the source file is a directory (async)', async () => { - await expect( - reflinkFile(sandboxDir, sandboxFiles[1].path) - ).rejects.toThrow(); + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); + await expect(reflinkFile(dir, sandboxFiles[1].path)).rejects.toThrow(); }); - it('should fail if the source and destination files are the same (sync)', () => { + it('should fail if the source and destination files are the same (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); expect(() => { reflinkFileSync(sandboxFiles[0].path, sandboxFiles[0].path); }).toThrow(); }); it('should fail if the source and destination files are the same (async)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); await expect( reflinkFile(sandboxFiles[0].path, sandboxFiles[0].path) ).rejects.toThrow(); }); - it('should fail if the destination parent directory does not exist (sync)', () => { + it('should fail if the destination parent directory does not exist (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); expect(() => { reflinkFileSync( sandboxFiles[0].path, - join(sandboxDir, 'does-not-exist', 'file1-copy.txt') + join(dir, 'does-not-exist', 'file1-copy.txt') ); }).toThrow(); }); @@ -190,8 +221,11 @@ describe('reflink', () => { }); it('should correctly clone 1000 files (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); + const files = Array.from({ length: 1000 }, (_, i) => ({ - path: join(sandboxDir, `file${i}.txt`), + path: join(dir, `file${i}.txt`), content: 'Hello World!', })); @@ -201,22 +235,21 @@ describe('reflink', () => { await Promise.all( files.map(async (file, i) => - reflinkFileSync(file.path, join(sandboxDir, `file${i}-copy.txt`)) + reflinkFileSync(file.path, join(dir, `file${i}-copy.txt`)) ) ); files.forEach((file, i) => { - const content = readFileSync( - join(sandboxDir, `file${i}-copy.txt`), - 'utf-8' - ); + const content = readFileSync(join(dir, `file${i}-copy.txt`), 'utf-8'); expect(content).toBe(file.content); }); }); it('should correctly clone 1000 files (async)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); const files = Array.from({ length: 1000 }, (_, i) => ({ - path: join(sandboxDir, `file${i}.txt`), + path: join(dir, `file${i}.txt`), content: 'Hello World!', hash: createHash('sha256').update('Hello World!').digest('hex'), })); @@ -227,15 +260,12 @@ describe('reflink', () => { await Promise.all( files.map(async (file, i) => - reflinkFile(file.path, join(sandboxDir, `file${i}-copy.txt`)) + reflinkFile(file.path, join(dir, `file${i}-copy.txt`)) ) ); files.forEach((file, i) => { - const content = readFileSync( - join(sandboxDir, `file${i}-copy.txt`), - 'utf-8' - ); + const content = readFileSync(join(dir, `file${i}-copy.txt`), 'utf-8'); const hash = createHash('sha256').update(content).digest('hex'); expect(content).toBe(file.content); expect(hash).toBe(file.hash); @@ -243,13 +273,15 @@ describe('reflink', () => { }); it('should keep the same hash when cloning a file more than 3,000 times', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); const srcFile = { path: resolve('./package.json'), content: readFileSync(join('./package.json'), 'utf-8'), }; const destFiles = Array.from({ length: 3_000 }, (_, i) => ({ - path: join(sandboxDir, `file1-copy-${i}.txt`), + path: join(dir, `file1-copy-${i}.txt`), hash: createHash('sha256').update(srcFile.content).digest('hex'), })); @@ -276,13 +308,15 @@ describe('reflink', () => { }); it('should clone "sample.pyc" file correctly (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); const srcFile = { path: resolve(join('fixtures', 'sample.pyc')), content: readFileSync(join('fixtures', 'sample.pyc')), }; const destFile = { - path: join(sandboxDir, 'sample.pyc'), + path: join(dir, 'sample.pyc'), hash: createHash('sha256').update(srcFile.content).digest('hex'), }; @@ -299,13 +333,15 @@ describe('reflink', () => { * The issue with empty cloned files doesnt seem related to ASCII characters */ it.skip('should clone "ascii-file.js" file correctly (sync)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); const srcFile = { path: resolve(join('fixtures', 'ascii-file.js')), content: readFileSync(join('fixtures', 'ascii-file.js')), }; const destFile = { - path: join(sandboxDir, 'ascii-file.js'), + path: join(dir, 'ascii-file.js'), hash: createHash('sha256').update(srcFile.content).digest('hex'), }; @@ -325,13 +361,15 @@ describe('reflink', () => { }); it('should clone "sample.pyc" file correctly (async)', async () => { + const dir = sandboxDir(); + const sandboxFiles = await prepare(dir); const srcFile = { path: resolve(join('fixtures', 'sample.pyc')), content: readFileSync(join('fixtures', 'sample.pyc')), }; const destFile = { - path: join(sandboxDir, 'sample.pyc'), + path: join(dir, 'sample.pyc'), hash: createHash('sha256').update(srcFile.content).digest('hex'), }; diff --git a/__test__/threads.spec.ts b/__test__/threads.spec.ts index 9c9e848..69fde8d 100644 --- a/__test__/threads.spec.ts +++ b/__test__/threads.spec.ts @@ -25,7 +25,7 @@ describe('reflink worker', () => { content: readFileSync(join(process.cwd(), 'fixtures', 'ascii-file.js')), }; - const destFiles = Array.from({ length: 100 }, () => ({ + const destFiles = Array.from({ length: 1000 }, () => ({ path: join(TEST_DIR, `dest-${randomUUID()}.js`), })); @@ -65,7 +65,7 @@ describe('reflink worker', () => { } }); - it('clone the same file to different location simultaneously (sync)', async () => { + it('clone the same file to different location simultaneously (async)', async () => { const src = { path: join(process.cwd(), 'fixtures', 'ascii-file.js'), content: readFileSync(join(process.cwd(), 'fixtures', 'ascii-file.js')), diff --git a/infinite_clone_test.mjs b/infinite_clone_test.mjs index 4a861b2..040762e 100644 --- a/infinite_clone_test.mjs +++ b/infinite_clone_test.mjs @@ -31,7 +31,6 @@ async function main() { for (let i = 0; i < 1000; i++) { const destPath = path.join('./sandbox', `file1-copy-${i}.txt`); - // Assume reflinkFile is your function that performs the file cloning operation await reflinkFile(srcFile.path, destPath); const destContent = await fs.readFile(destPath, 'utf-8'); diff --git a/package.json b/package.json index 4c474d1..1d01d2b 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ "build": "napi build --platform --release", "build:debug": "napi build --platform", "prepublishOnly": "napi prepublish -t npm", - "pretest": "yarn build", + "pretest": "pnpm build", "test": "cargo t && vitest", "bench": "node benchmark.mjs", "universal": "napi universal", diff --git a/src/lib.rs b/src/lib.rs index de22532..eb6bb1e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,88 +2,99 @@ #[macro_use] extern crate napi_derive; - +use copy_on_write::reflink_file_sync as reflink; use napi::{bindgen_prelude::AsyncTask, Env, Error, JsNumber, Result, Task}; use std::path::PathBuf; -use reflink_copy; pub struct AsyncReflink { - src: PathBuf, - dst: PathBuf, + src: PathBuf, + dst: PathBuf, } #[napi] impl Task for AsyncReflink { - type Output = (); - type JsValue = JsNumber; - - fn compute(&mut self) -> Result { - match reflink_copy::reflink(&self.src, &self.dst) { - Ok(_) => { - Ok(()) - }, - Err(err) => return Err(Error::from_reason(format!( - "{}, reflink '{}' -> '{}'", - err.to_string(), - self.src.display(), - self.dst.display() - ))), - } + type Output = (); + type JsValue = JsNumber; + + fn compute(&mut self) -> Result { + // Convert PathBuf to &str + let src_str = self + .src + .to_str() + .ok_or_else(|| Error::from_reason("Invalid UTF-8 sequence in source path".to_string()))?; + let dst_str = self.dst.to_str().ok_or_else(|| { + Error::from_reason("Invalid UTF-8 sequence in destination path".to_string()) + })?; + + // Call reflink with string slices + match reflink(src_str, dst_str) { + Ok(_) => Ok(()), + Err(err) => Err(Error::from_reason(format!( + "{}, reflink '{}' -> '{}'", + err.to_string(), + src_str, + dst_str + ))), } + } - fn resolve(&mut self, env: Env, _: ()) -> Result { - env.create_int32(0) - } + fn resolve(&mut self, env: Env, _: ()) -> Result { + env.create_int32(0) + } } // Async version #[napi(js_name = "reflinkFile")] pub fn reflink_task(src: String, dst: String) -> AsyncTask { - let src_path = PathBuf::from(src); - let dst_path = PathBuf::from(dst); - AsyncTask::new(AsyncReflink { src: src_path, dst: dst_path }) + let src_path = PathBuf::from(src); + let dst_path = PathBuf::from(dst); + AsyncTask::new(AsyncReflink { + src: src_path, + dst: dst_path, + }) } // Sync version #[napi(js_name = "reflinkFileSync")] pub fn reflink_sync(env: Env, src: String, dst: String) -> Result { - let src_path = PathBuf::from(src); - let dst_path = PathBuf::from(dst); - match reflink_copy::reflink(&src_path, &dst_path) { - Ok(_) => Ok(env.create_int32(0)?), - Err(err) => Err(Error::from_reason(format!( - "{}, reflink '{}' -> '{}'", - err.to_string(), - src_path.display(), - dst_path.display() - ))), - } + // Call reflink with string slices + match reflink(&src, &dst) { + Ok(_) => Ok(env.create_int32(0)?), + Err(err) => Err(Error::from_reason(format!( + "{}, reflink '{}' -> '{}'", + err.to_string(), + src, + dst + ))), + } } #[test] pub fn test_pyc_file() { - let src = std::path::Path::new("fixtures/sample.pyc"); - let dst = std::path::Path::new("fixtures/sample.pyc.reflink"); + let src = "fixtures/sample.pyc"; + let dst = "fixtures/sample.pyc.reflink"; - // Remove the destination file if it already exists - if dst.exists() { - std::fs::remove_file(&dst).unwrap(); - } + let dst_path = std::path::Path::new(dst); + + // Remove the destination file if it already exists + if dst_path.exists() { + std::fs::remove_file(&dst_path).unwrap(); + } - // Run the reflink operation - let result = reflink_copy::reflink(&src, &dst); - assert!(result.is_ok()); + // Run the reflink operation + let result = reflink(src, dst); + assert!(result.is_ok()); - println!("Reflinked '{}' -> '{}'", src.display(), dst.display()); + println!("Reflinked '{}' -> '{}'", src, dst); - // Further validation: compare the contents of both files to make sure they are identical - let src_contents = std::fs::read(&src).expect("Failed to read source file"); - let dst_contents = std::fs::read(&dst).expect("Failed to read destination file"); + // Further validation: compare the contents of both files to make sure they are identical + let src_contents = std::fs::read(src).expect("Failed to read source file"); + let dst_contents = std::fs::read(dst).expect("Failed to read destination file"); - assert_eq!(src_contents, dst_contents); + assert_eq!(src_contents, dst_contents); - // Remove the destination file - std::fs::remove_file(&dst).unwrap(); + // Remove the destination file + std::fs::remove_file(&dst_path).unwrap(); - println!("File contents match, reflink operation successful") -} \ No newline at end of file + println!("File contents match, reflink operation successful") +} From 4b02d74c1a43e92d93910f5d73e94440cf84724d Mon Sep 17 00:00:00 2001 From: Ignacio Aldama Vicente Date: Thu, 2 Nov 2023 11:22:54 +0100 Subject: [PATCH 2/6] style: remove unused package --- Cargo.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index be8ead7..32542ed 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -12,7 +12,6 @@ futures = "0.3.28" # Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix napi = { version = "2.12.2", default-features = false, features = ["napi4"] } napi-derive = "2.12.2" -reflink-copy = { version = "0.1.10" } [build-dependencies] napi-build = "2.0.1" From 62364976d6a180e521eb6456e156b61b5c223965 Mon Sep 17 00:00:00 2001 From: Ignacio Aldama Vicente Date: Thu, 2 Nov 2023 12:05:32 +0100 Subject: [PATCH 3/6] test: clone less times in parallel --- __test__/threads.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/__test__/threads.spec.ts b/__test__/threads.spec.ts index 69fde8d..0d1f690 100644 --- a/__test__/threads.spec.ts +++ b/__test__/threads.spec.ts @@ -25,7 +25,7 @@ describe('reflink worker', () => { content: readFileSync(join(process.cwd(), 'fixtures', 'ascii-file.js')), }; - const destFiles = Array.from({ length: 1000 }, () => ({ + const destFiles = Array.from({ length: 100 }, () => ({ path: join(TEST_DIR, `dest-${randomUUID()}.js`), })); From c3c6f2b4e5401f3a056a485c9da6aa711a8aefdf Mon Sep 17 00:00:00 2001 From: Ignacio Aldama Vicente Date: Thu, 2 Nov 2023 14:59:52 +0100 Subject: [PATCH 4/6] style: remove identation change --- src/lib.rs | 122 ++++++++++++++++++++++++++--------------------------- 1 file changed, 59 insertions(+), 63 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index eb6bb1e..5879b1e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,94 +7,90 @@ use napi::{bindgen_prelude::AsyncTask, Env, Error, JsNumber, Result, Task}; use std::path::PathBuf; pub struct AsyncReflink { - src: PathBuf, - dst: PathBuf, + src: PathBuf, + dst: PathBuf, } #[napi] impl Task for AsyncReflink { - type Output = (); - type JsValue = JsNumber; - - fn compute(&mut self) -> Result { - // Convert PathBuf to &str - let src_str = self - .src - .to_str() - .ok_or_else(|| Error::from_reason("Invalid UTF-8 sequence in source path".to_string()))?; - let dst_str = self.dst.to_str().ok_or_else(|| { - Error::from_reason("Invalid UTF-8 sequence in destination path".to_string()) - })?; - - // Call reflink with string slices - match reflink(src_str, dst_str) { - Ok(_) => Ok(()), - Err(err) => Err(Error::from_reason(format!( - "{}, reflink '{}' -> '{}'", - err.to_string(), - src_str, - dst_str - ))), + type Output = (); + type JsValue = JsNumber; + + fn compute(&mut self) -> Result { + let src_str = self.src.to_str().ok_or_else(|| { + Error::from_reason("Invalid UTF-8 sequence in source path".to_string()) + })?; + + let dst_str = self.dst.to_str().ok_or_else(|| { + Error::from_reason("Invalid UTF-8 sequence in destination path".to_string()) + })?; + + match reflink(src_str, dst_str) { + Ok(_) => { + Ok(()) + }, + Err(err) => return Err(Error::from_reason(format!( + "{}, reflink '{}' -> '{}'", + err.to_string(), + self.src.display(), + self.dst.display() + ))), + } } - } - fn resolve(&mut self, env: Env, _: ()) -> Result { - env.create_int32(0) - } + fn resolve(&mut self, env: Env, _: ()) -> Result { + env.create_int32(0) + } } // Async version #[napi(js_name = "reflinkFile")] pub fn reflink_task(src: String, dst: String) -> AsyncTask { - let src_path = PathBuf::from(src); - let dst_path = PathBuf::from(dst); - AsyncTask::new(AsyncReflink { - src: src_path, - dst: dst_path, - }) + let src_path = PathBuf::from(src); + let dst_path = PathBuf::from(dst); + AsyncTask::new(AsyncReflink { src: src_path, dst: dst_path }) } // Sync version #[napi(js_name = "reflinkFileSync")] pub fn reflink_sync(env: Env, src: String, dst: String) -> Result { - // Call reflink with string slices - match reflink(&src, &dst) { - Ok(_) => Ok(env.create_int32(0)?), - Err(err) => Err(Error::from_reason(format!( - "{}, reflink '{}' -> '{}'", - err.to_string(), - src, - dst - ))), - } + match reflink(&src, &dst) { + Ok(_) => Ok(env.create_int32(0)?), + Err(err) => Err(Error::from_reason(format!( + "{}, reflink '{}' -> '{}'", + err.to_string(), + src, + dst + ))), + } } #[test] pub fn test_pyc_file() { - let src = "fixtures/sample.pyc"; - let dst = "fixtures/sample.pyc.reflink"; + let src = "fixtures/sample.pyc"; + let dst = "fixtures/sample.pyc.reflink"; - let dst_path = std::path::Path::new(dst); + let dst_path = std::path::Path::new(dst); - // Remove the destination file if it already exists - if dst_path.exists() { - std::fs::remove_file(&dst_path).unwrap(); - } + // Remove the destination file if it already exists + if dst_path.exists() { + std::fs::remove_file(&dst).unwrap(); + } - // Run the reflink operation - let result = reflink(src, dst); - assert!(result.is_ok()); + // Run the reflink operation + let result = reflink(&src, &dst); + assert!(result.is_ok()); - println!("Reflinked '{}' -> '{}'", src, dst); + println!("Reflinked '{}' -> '{}'", src, dst); - // Further validation: compare the contents of both files to make sure they are identical - let src_contents = std::fs::read(src).expect("Failed to read source file"); - let dst_contents = std::fs::read(dst).expect("Failed to read destination file"); + // Further validation: compare the contents of both files to make sure they are identical + let src_contents = std::fs::read(&src).expect("Failed to read source file"); + let dst_contents = std::fs::read(&dst).expect("Failed to read destination file"); - assert_eq!(src_contents, dst_contents); + assert_eq!(src_contents, dst_contents); - // Remove the destination file - std::fs::remove_file(&dst_path).unwrap(); + // Remove the destination file + std::fs::remove_file(&dst).unwrap(); - println!("File contents match, reflink operation successful") -} + println!("File contents match, reflink operation successful") +} \ No newline at end of file From 37e9d1f5a808f249c136fd894014657deedbd4c7 Mon Sep 17 00:00:00 2001 From: Nacho Aldama Date: Thu, 2 Nov 2023 21:49:44 +0100 Subject: [PATCH 5/6] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Khải --- src/lib.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 5879b1e..2c25e7b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -2,7 +2,7 @@ #[macro_use] extern crate napi_derive; -use copy_on_write::reflink_file_sync as reflink; +use copy_on_write::reflink_file_sync; use napi::{bindgen_prelude::AsyncTask, Env, Error, JsNumber, Result, Task}; use std::path::PathBuf; @@ -25,7 +25,7 @@ impl Task for AsyncReflink { Error::from_reason("Invalid UTF-8 sequence in destination path".to_string()) })?; - match reflink(src_str, dst_str) { + match reflink_file_sync(src_str, dst_str) { Ok(_) => { Ok(()) }, @@ -54,7 +54,7 @@ pub fn reflink_task(src: String, dst: String) -> AsyncTask { // Sync version #[napi(js_name = "reflinkFileSync")] pub fn reflink_sync(env: Env, src: String, dst: String) -> Result { - match reflink(&src, &dst) { + match reflink_file_sync(src, dst) { Ok(_) => Ok(env.create_int32(0)?), Err(err) => Err(Error::from_reason(format!( "{}, reflink '{}' -> '{}'", @@ -73,15 +73,15 @@ pub fn test_pyc_file() { let dst_path = std::path::Path::new(dst); // Remove the destination file if it already exists - if dst_path.exists() { + if dst_path.try_exists().unwrap() { std::fs::remove_file(&dst).unwrap(); } // Run the reflink operation - let result = reflink(&src, &dst); + let result = reflink_file_sync(src, dst); assert!(result.is_ok()); - println!("Reflinked '{}' -> '{}'", src, dst); + println!("Reflinked {src:?} -> {dst:?}"); // Further validation: compare the contents of both files to make sure they are identical let src_contents = std::fs::read(&src).expect("Failed to read source file"); From 295cdfff811766ca42950e2ac2e05a1e58cd3cc0 Mon Sep 17 00:00:00 2001 From: Ignacio Aldama Vicente Date: Fri, 3 Nov 2023 09:03:12 +0100 Subject: [PATCH 6/6] types: fix types issues --- Cargo.toml | 2 +- src/lib.rs | 25 +++++++------------------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 32542ed..e098c85 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,7 +7,7 @@ version = "0.0.0" crate-type = ["cdylib"] [dependencies] -copy_on_write = "0.1.0" +copy_on_write = "0.1.1" futures = "0.3.28" # Default enable napi4 feature, see https://nodejs.org/api/n-api.html#node-api-version-matrix napi = { version = "2.12.2", default-features = false, features = ["napi4"] } diff --git a/src/lib.rs b/src/lib.rs index 2c25e7b..053f774 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,11 +4,10 @@ extern crate napi_derive; use copy_on_write::reflink_file_sync; use napi::{bindgen_prelude::AsyncTask, Env, Error, JsNumber, Result, Task}; -use std::path::PathBuf; pub struct AsyncReflink { - src: PathBuf, - dst: PathBuf, + src: String, + dst: String, } #[napi] @@ -17,23 +16,15 @@ impl Task for AsyncReflink { type JsValue = JsNumber; fn compute(&mut self) -> Result { - let src_str = self.src.to_str().ok_or_else(|| { - Error::from_reason("Invalid UTF-8 sequence in source path".to_string()) - })?; - - let dst_str = self.dst.to_str().ok_or_else(|| { - Error::from_reason("Invalid UTF-8 sequence in destination path".to_string()) - })?; - - match reflink_file_sync(src_str, dst_str) { + match reflink_file_sync(&self.src, &self.dst) { Ok(_) => { Ok(()) }, Err(err) => return Err(Error::from_reason(format!( "{}, reflink '{}' -> '{}'", err.to_string(), - self.src.display(), - self.dst.display() + self.src, + self.dst ))), } } @@ -46,15 +37,13 @@ impl Task for AsyncReflink { // Async version #[napi(js_name = "reflinkFile")] pub fn reflink_task(src: String, dst: String) -> AsyncTask { - let src_path = PathBuf::from(src); - let dst_path = PathBuf::from(dst); - AsyncTask::new(AsyncReflink { src: src_path, dst: dst_path }) + AsyncTask::new(AsyncReflink { src, dst }) } // Sync version #[napi(js_name = "reflinkFileSync")] pub fn reflink_sync(env: Env, src: String, dst: String) -> Result { - match reflink_file_sync(src, dst) { + match reflink_file_sync(&src, &dst) { Ok(_) => Ok(env.create_int32(0)?), Err(err) => Err(Error::from_reason(format!( "{}, reflink '{}' -> '{}'",