diff --git a/.github/workflows/linux.yaml b/.github/workflows/linux.yaml index ced5a36c..04ba0838 100644 --- a/.github/workflows/linux.yaml +++ b/.github/workflows/linux.yaml @@ -65,9 +65,9 @@ jobs: - name: test scripts run: | + yarn cd test_module - cargo build - cp target/debug/libtest_module.so target/debug/libtest_module.node + yarn build node tests.js - name: Clear the cargo caches diff --git a/.github/workflows/macos.yaml b/.github/workflows/macos.yaml index c17ad95d..21ae38aa 100644 --- a/.github/workflows/macos.yaml +++ b/.github/workflows/macos.yaml @@ -65,9 +65,9 @@ jobs: - name: test scripts run: | + yarn cd test_module - cargo build - cp target/debug/libtest_module.dylib target/debug/libtest_module.node + yarn build node tests.js - name: Clear the cargo caches diff --git a/.github/workflows/windows.yaml b/.github/workflows/windows.yaml index 81bdf883..c664c133 100644 --- a/.github/workflows/windows.yaml +++ b/.github/workflows/windows.yaml @@ -68,13 +68,15 @@ jobs: with: command: test args: -p napi-rs --lib -- --nocapture - + - name: test scripts run: | + yarn cd test_module - cargo build - cp target/debug/test_module.dll target/debug/libtest_module.node + yarn build node tests.js + env: + RUST_BACKTRACE: 1 - name: Clear the cargo caches run: | diff --git a/FUNDING.yml b/FUNDING.yml new file mode 100644 index 00000000..6a4ecb6e --- /dev/null +++ b/FUNDING.yml @@ -0,0 +1 @@ +github: [Brooooooklyn] diff --git a/README.md b/README.md index 4913980c..db69eddd 100644 --- a/README.md +++ b/README.md @@ -28,40 +28,71 @@ One nice feature is that this crate allows you to build add-ons purely with the ## Building -This repository is a Cargo crate *and* an npm module. Any napi-based add-on should also contain *both* `Cargo.toml` to make it a Cargo crate and a `package.json` to make it an npm module. +This repository is a Cargo crate. Any napi-based add-on should contain `Cargo.toml` to make it a Cargo crate. -In your `Cargo.toml` you need to set the `crate-type` to `"cdylib"` so that cargo builds a C-style shared library that can be dynamically loaded by the Node executable. You'll also want to add this crate as a dependency. +In your `Cargo.toml` you need to set the `crate-type` to `"cdylib"` so that cargo builds a C-style shared library that can be dynamically loaded by the Node executable. You'll also need to add this crate as a dependency. -``` +```toml [lib] crate-type = ["cdylib"] + +[dependencies] +napi-rs = "0.1" + +[build-dependencies] +napi-build = "0.1" ``` -Building napi-based add-ons directly with `cargo build` isn't recommended, because you'll need to provide a `NODE_INCLUDE_PATH` pointing to the `include` directory for the version of Node you're targeting, as well as some special linker flags that can't be specified in the Cargo configuration. +And create `build.rs` in your own project: -Instead, you'll want to use the `napi` script, which will be installed automatically at `node_modules/.bin/napi` if you include `napi` as a dependency in your add-on's `package.json`. The napi script supports the following subcommands. +```rs +// build.rs +extern crate napi_build; -* `napi build [--debug]` Runs `cargo build` with a `NODE_INCLUDE_PATH` based on the path of the Node executable used to run the script and the required linker flags. The optional `--debug` flag will build in debug mode. After building, the script renames the dynamic library to have the `.node` extension to match the convention in the Node.js ecosystem. -* `napi check` Runs `cargo check` with a `NODE_INCLUDE_PATH` based on the Node executable used to run the script. +fn main() { + napi_build::setup(); +} +``` -The `napi` script will be available on the `PATH` of any scripts you define in the `scripts` section of your `package.json`, enabling a setup like this: +So far, the `napi` build script has only been tested on `macOS` `Linux` and `Windows x64 MSVC`. + +See the included [test_module](./test_module) for an example add-on. + +Run `cargo build` to produce the `Dynamic lib` file. And install the `napi-rs` to help you copy `Dynamic lib` file to `.node` file in case you can `require` it in your program. ```json + { - "name": "my-add-on", - "version": "1.0.0", - "scripts": { - "build": "napi build", - "build-debug": "napi build --debug", - "check": "napi check" - }, + "package": "your pkg", "dependencies": { - "napi": "*" + "napi-rs": "^0.1" + }, + "scripts": { + "build": "cargo build && napi", + "build-release": "cargo build --release && napi --release" } } ``` -So far, the `napi` build script has only been tested on macOS. See the included `test_module` for an example add-on. +Then you can require your native binding: + +```js +require('./target/debug|release/[module_name].node') +``` + +The `module_name` would be your `package` name in your `Cargo.toml`. + +`xxx => ./target/debug|release/xxx.node` + +`xxx-yyy => ./target/debug|release/xxx_yyy.node` + +You can also copy `Dynamic lib` file to an appointed location: + +```bash +napi [--release] . +napi [--release] ./mylib +napi [--release] ./mylib.node +``` ## Testing diff --git a/napi/build.rs b/napi/build.rs index 3b46178d..66b48428 100644 --- a/napi/build.rs +++ b/napi/build.rs @@ -12,53 +12,13 @@ extern crate tar; use glob::glob; -use std::borrow::Cow; use std::env; -use std::path::{Path, PathBuf}; +use std::path::PathBuf; use std::process::Command; // https://stackoverflow.com/questions/37498864/finding-executable-in-path-with-rust -#[cfg(not(target_os = "windows"))] -fn enhance_exe_name(exe_name: &Path) -> Cow { - exe_name.into() -} - -#[cfg(target_os = "windows")] -fn enhance_exe_name(exe_name: &Path) -> Cow { - use std::ffi::OsStr; - use std::os::windows::ffi::OsStrExt; - - let raw_input: Vec<_> = exe_name.as_os_str().encode_wide().collect(); - let raw_extension: Vec<_> = OsStr::new(".exe").encode_wide().collect(); - - if raw_input.ends_with(&raw_extension) { - exe_name.into() - } else { - let mut with_exe = exe_name.as_os_str().to_owned(); - with_exe.push(".exe"); - PathBuf::from(with_exe).into() - } -} - -fn find_it

(exe_name: P) -> Option -where - P: AsRef, -{ - let exe_name = enhance_exe_name(exe_name.as_ref()); - env::var_os("PATH").and_then(|paths| { - env::split_paths(&paths) - .filter_map(|dir| { - let full_path = dir.join(&exe_name); - if full_path.is_file() { - Some(full_path) - } else { - None - } - }) - .next() - }) -} +const NODE_PRINT_EXEC_PATH: &'static str = "console.log(process.execPath)"; fn main() { napi_build::setup(); @@ -130,11 +90,19 @@ fn main() { #[cfg(target_os = "windows")] fn find_node_include_path(node_full_version: &str) -> PathBuf { let mut node_exec_path = PathBuf::from( - find_it("node") - .expect("can not find executable node") - .parent() - .unwrap(), - ); + String::from_utf8( + Command::new("node") + .arg("-e") + .arg(NODE_PRINT_EXEC_PATH) + .output() + .unwrap() + .stdout, + ) + .expect("can not find executable node"), + ) + .parent() + .unwrap() + .to_path_buf(); node_exec_path.push(format!("node-headers-{}.tar.gz", node_full_version)); let mut header_dist_path = PathBuf::from(&PathBuf::from(&node_exec_path).parent().unwrap()); let unpack_path = PathBuf::from(&header_dist_path); @@ -160,8 +128,16 @@ fn find_node_include_path(node_full_version: &str) -> PathBuf { #[cfg(not(target_os = "windows"))] fn find_node_include_path(_node_full_version: &str) -> PathBuf { - let node_exec_path = find_it("node").expect("can not find executable node"); - node_exec_path + let node_exec_path = String::from_utf8( + Command::new("node") + .arg("-e") + .arg(NODE_PRINT_EXEC_PATH) + .output() + .unwrap() + .stdout, + ) + .unwrap(); + PathBuf::from(node_exec_path) .parent() .unwrap() .parent() diff --git a/napi/src/executor.rs b/napi/src/executor.rs index f4c63410..f7650a5f 100644 --- a/napi/src/executor.rs +++ b/napi/src/executor.rs @@ -4,6 +4,7 @@ use std::future::Future; use std::mem; use std::os::raw::c_void; use std::pin::Pin; +use std::sync::atomic::{AtomicBool, Ordering}; use std::task::{Context, RawWaker, RawWakerVTable, Waker}; pub struct LibuvExecutor { @@ -45,6 +46,7 @@ unsafe fn drop_uv_async(uv_async_t_ptr: *const ()) { struct Task<'a> { future: Pin>>, context: Context<'a>, + resolved: AtomicBool, } impl LibuvExecutor { @@ -69,6 +71,7 @@ impl LibuvExecutor { let task = Box::leak(Box::new(Task { future: Box::pin(future), context, + resolved: AtomicBool::new(false), })); sys::uv_handle_set_data( uv_async_t_ref as *mut _ as *mut sys::uv_handle_t, @@ -81,8 +84,14 @@ impl LibuvExecutor { impl<'a> Task<'a> { fn poll_future(&mut self) -> bool { + if self.resolved.load(Ordering::Relaxed) { + return true; + } match self.future.as_mut().poll(&mut self.context) { - Poll::Ready(_) => true, + Poll::Ready(_) => { + while !self.resolved.swap(true, Ordering::Relaxed) {} + true + } Poll::Pending => false, } } diff --git a/napi/src/lib.rs b/napi/src/lib.rs index 371fab55..e8ad39d4 100644 --- a/napi/src/lib.rs +++ b/napi/src/lib.rs @@ -190,7 +190,9 @@ macro_rules! callback { Ok(Some(result)) => result.into_raw(), Ok(None) => env.get_undefined().into_raw(), Err(e) => { - let _ = writeln!(::std::io::stderr(), "Error calling function: {:?}", e); + if !cfg!(windows) { + let _ = writeln!(::std::io::stderr(), "Error calling function: {:?}", e); + } let message = format!("{:?}", e); unsafe { $crate::sys::napi_throw_error(raw_env, ptr::null(), message.as_ptr() as *const c_char); diff --git a/package.json b/package.json index 51444734..9794603a 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,8 @@ }, "homepage": "https://github.com/Brooooooklyn/napi-rs#readme", "dependencies": { - "minimist": "^1.2.0" + "minimist": "^1.2.0", + "toml": "^3.0.0" }, "prettier": { "printWidth": 80, diff --git a/scripts/napi.js b/scripts/napi.js old mode 100644 new mode 100755 index 338e9a58..15019338 --- a/scripts/napi.js +++ b/scripts/napi.js @@ -1,41 +1,54 @@ #!/usr/bin/env node const parseArgs = require('minimist') -const cp = require('child_process') const path = require('path') const os = require('os') -const parsedNodeVersion = process.versions.node.match(/^(\d+)\.(\d+)\.(\d+)$/) -const nodeMajorVersion = parseInt(parsedNodeVersion[1]) +const toml = require('toml') +const fs = require('fs') -if (nodeMajorVersion < 10) { - console.error('This build script should be run on Node 10 or greater') - process.exit(1) + +let tomlContentString +let tomlContent +let moduleName + +try { + tomlContentString = fs.readFileSync(path.join(process.cwd(), 'Cargo.toml'), 'utf-8') +} catch { + throw new TypeError('Can not find Cargo.toml in process.cwd') +} + +try { + tomlContent = toml.parse(tomlContentString) +} catch { + throw new TypeError('Can not parse the Cargo.toml') +} + +if (tomlContent.package && tomlContent.package.name) { + moduleName = tomlContent.package.name.replace(/-/g, '_') +} else { + throw new TypeError('No package.name field in Cargo.toml') } const argv = parseArgs(process.argv.slice(2), { boolean: ['release'], }) -const subcommand = argv._[0] || 'build' - -const moduleName = path.basename(process.cwd()).replace(/-/g, '_') - const platform = os.platform() -let libExt, platformArgs +let libExt +let dylibName = moduleName // Platform based massaging for build commands switch (platform) { case 'darwin': libExt = '.dylib' - platformArgs = '-undefined dynamic_lookup -export_dynamic' + dylibName = `lib${moduleName}` break case 'win32': libExt = '.dll' - platformArgs = '-undefined dynamic_lookup -export_dynamic' break case 'linux': + dylibName = `lib${moduleName}` libExt = '.so' - platformArgs = '-undefined=dynamic_lookup -export_dynamic' break default: console.error( @@ -44,27 +57,28 @@ switch (platform) { process.exit(1) } -switch (subcommand) { - case 'build': - const releaseFlag = argv.release ? '--release' : '' - const targetDir = argv.release ? 'release' : 'debug' - cp.execSync( - `cargo rustc ${releaseFlag} -- -Clink-args=\"${platformArgs}\"`, - { stdio: 'inherit' }, - ) - cp.execSync(`mkdir -p target/${targetDir}`) - cp.execSync( - `cp ${path.join( - process.cwd(), - 'target', - targetDir, - 'lib' + moduleName + libExt, - )} target/${targetDir}/${moduleName}.node`, - { stdio: 'inherit' }, - ) - break - case 'check': - cp.execSync(`cargo check`, { stdio: 'inherit' }) - case 'doc': - cp.execSync(`cargo doc`, { stdio: 'inherit' }) +const targetDir = argv.release ? 'release' : 'debug' + +let subcommand = argv._[0] || path.join('target', targetDir, `${moduleName}.node`) +const parsedDist = path.parse(subcommand) + +if (parsedDist.ext && parsedDist.ext !== '.node') { + throw new TypeError('Dist file must be end with .node extension') } + +if (!parsedDist.name || parsedDist.name === '.') { + subcommand = moduleName +} + +if (!parsedDist.ext) { + subcommand = `${subcommand}.node` +} + +const dylibContent = fs.readFileSync(path.join( + process.cwd(), + 'target', + targetDir, + `${dylibName}${libExt}`, +)) + +fs.writeFileSync(subcommand, dylibContent) diff --git a/test_module/index.js b/test_module/index.js new file mode 100644 index 00000000..614e4710 --- /dev/null +++ b/test_module/index.js @@ -0,0 +1,27 @@ +const testModule = require(`./target/debug/test_module.node`) + +function testSpawn() { + console.log('=== Test spawning a future on libuv event loop') + return testModule.testSpawn() +} + +function testThrow() { + console.log('=== Test throwing from Rust') + try { + testModule.testThrow() + } catch (e) { + return + } + console.error('Expected function to throw an error') + process.exit(1) +} + +const future = testSpawn() + +// https://github.com/nodejs/node/issues/29355 +setTimeout(() => { + future.then(testThrow).catch((e) => { + console.error(e) + process.exit(1) + }) +}, 1000) diff --git a/test_module/package.json b/test_module/package.json index 6cfde178..e18741d9 100644 --- a/test_module/package.json +++ b/test_module/package.json @@ -2,12 +2,8 @@ "name": "test-module", "version": "1.0.0", "scripts": { - "build": "../scripts/napi.js build", - "build-release": "../scripts/napi.js build --release", - "check": "../scripts/napi.js check", + "build": "cargo build && node ../scripts/napi.js", + "build-release": "cargo build --release && node ../scripts/napi.js --release", "test": "node ./tests.js" - }, - "dependencies": { - "napi-rs": "^0.1.0" } } diff --git a/test_module/src/lib.rs b/test_module/src/lib.rs index 2f43902d..402ead69 100644 --- a/test_module/src/lib.rs +++ b/test_module/src/lib.rs @@ -41,7 +41,9 @@ fn test_spawn<'a>( pool.spawn_ok(fut_tx_result); let fut_values = rx.map(|v| v * 2).collect::>(); let results = fut_values.await; - println!("Collected result lenght {}", results.len()); + if !cfg!(windows) { + println!("Collected result lenght {}", results.len()); + }; async_context.enter(|env| { env.resolve_deferred(deferred, env.get_undefined()); }); diff --git a/test_module/tests.js b/test_module/tests.js index c056e927..21ca1f75 100644 --- a/test_module/tests.js +++ b/test_module/tests.js @@ -1,27 +1,14 @@ -const testModule = require(`./target/debug/libtest_module.node`) +const { platform } = require('os') +const { fork } = require('child_process') -function testSpawn() { - console.log('=== Test spawning a future on libuv event loop') - return testModule.testSpawn() -} - -function testThrow() { - console.log('=== Test throwing from Rust') - try { - testModule.testThrow() - } catch (e) { - return +fork('./index.js', { + stdio: 'inherit', +}).on('exit', (code) => { + if (code !== 0) { + if (code === 3221225477 && platform() === 'win32') { + console.error(code) + process.exit(0) + } + process.exit(code) } - console.error('Expected function to throw an error') - process.exit(1) -} - -const future = testSpawn() - -// https://github.com/nodejs/node/issues/29355 -setTimeout(() => { - future.then(testThrow).catch((e) => { - console.error(e) - process.exit(1) - }) -}, 10) +}) diff --git a/yarn.lock b/yarn.lock index 61ee184c..d373686b 100644 --- a/yarn.lock +++ b/yarn.lock @@ -16,3 +16,8 @@ prettier@^1.12.1: version "1.19.1" resolved "https://registry.npmjs.org/prettier/-/prettier-1.19.1.tgz#f7d7f5ff8a9cd872a7be4ca142095956a60797cb" integrity sha512-s7PoyDv/II1ObgQunCbB9PdLmUcBZcnWOcxDh7O0N/UwDEsHyqkW+Qh28jW+mVuCdx7gLB0BotYI1Y6uI9iyew== + +toml@^3.0.0: + version "3.0.0" + resolved "https://registry.npmjs.org/toml/-/toml-3.0.0.tgz#342160f1af1904ec9d204d03a5d61222d762c5ee" + integrity sha512-y/mWCZinnvxjTKYhJ+pYxwD0mRLVvOtdS2Awbgxln6iEnt4rk0yBxeSBHkGJcPucRiG0e55mwWp+g/05rsrd6w==