From 0947e29bdeb79bc162fc897e1ccfb301d2723a39 Mon Sep 17 00:00:00 2001 From: "arduano@localhost" <> Date: Mon, 20 May 2024 20:55:58 +1000 Subject: [PATCH 1/4] Propagate ctx around lambdas for access to current file path We need to be able to access ctx for certain builtins, e.g. import which needs to know the directory of the current file to know how to handle string-based import paths. --- nixjs-rt/src/builtins.ts | 15 ++++++++------- nixjs-rt/src/legacyTests.test.ts | 8 ++++---- nixjs-rt/src/lib.ts | 14 +++++++------- src/eval/emit_js.rs | 2 +- 4 files changed, 20 insertions(+), 19 deletions(-) diff --git a/nixjs-rt/src/builtins.ts b/nixjs-rt/src/builtins.ts index 8e6d4a3..6bc66a3 100644 --- a/nixjs-rt/src/builtins.ts +++ b/nixjs-rt/src/builtins.ts @@ -17,10 +17,11 @@ import { NixTypeClass, Path, TRUE, + toPath, } from "./lib"; import { dirOf, isAbsolutePath, normalizePath } from "./utils"; -type BuiltinsRecord = Record NixType>; +type BuiltinsRecord = Record NixType>; function builtinBasicTypeMismatchError( fnName: string, @@ -71,7 +72,7 @@ export function getBuiltins() { throw new Error("unimplemented"); }, - all: (pred) => { + all: (pred, ctx) => { const lambdaStrict = pred.toStrict(); if (!(lambdaStrict instanceof Lambda)) { throw builtinBasicTypeMismatchError("all", lambdaStrict, Lambda); @@ -84,7 +85,7 @@ export function getBuiltins() { } for (const element of listStrict.values) { - const result = lambdaStrict.apply(element); + const result = lambdaStrict.apply(element, ctx); if (!result.asBoolean()) { return FALSE; } @@ -94,7 +95,7 @@ export function getBuiltins() { }); }, - any: (pred) => { + any: (pred, ctx) => { const lambdaStrict = pred.toStrict(); if (!(lambdaStrict instanceof Lambda)) { throw builtinBasicTypeMismatchError("any", lambdaStrict, Lambda); @@ -107,7 +108,7 @@ export function getBuiltins() { } for (const element of listStrict.values) { - const result = lambdaStrict.apply(element); + const result = lambdaStrict.apply(element, ctx); if (result.asBoolean()) { return TRUE; } @@ -216,7 +217,7 @@ export function getBuiltins() { throw new Error("unimplemented"); }, - dirOf: (arg) => { + dirOf: (path) => { throw new Error("unimplemented"); }, @@ -350,7 +351,7 @@ export function getBuiltins() { return listStrict.values[0]; }, - import: (path) => { + import: (path, ctx) => { const pathStrict = path.toStrict(); if (!(pathStrict instanceof Path || pathStrict instanceof NixString)) { diff --git a/nixjs-rt/src/legacyTests.test.ts b/nixjs-rt/src/legacyTests.test.ts index a6b9add..ac48c0b 100644 --- a/nixjs-rt/src/legacyTests.test.ts +++ b/nixjs-rt/src/legacyTests.test.ts @@ -21,9 +21,9 @@ import { evalCtx, keyVals, toAttrpath } from "./testUtils"; // Apply: test("calling a lambda should return its value", () => { - expect(new Lambda((_) => new NixInt(1n)).apply(EMPTY_ATTRSET)).toStrictEqual( - new NixInt(1n), - ); + expect( + new Lambda((_) => new NixInt(1n)).apply(EMPTY_ATTRSET, evalCtx()), + ).toStrictEqual(new NixInt(1n)); }); // Arithmetic: @@ -496,7 +496,7 @@ test("parameter lambda", () => { expect( n .paramLambda(evalCtx(), "foo", (evalCtx) => evalCtx.lookup("foo")) - .apply(n.TRUE), + .apply(n.TRUE, evalCtx()), ).toBe(n.TRUE); }); diff --git a/nixjs-rt/src/lib.ts b/nixjs-rt/src/lib.ts index 1184e7d..e4b8c2a 100644 --- a/nixjs-rt/src/lib.ts +++ b/nixjs-rt/src/lib.ts @@ -142,7 +142,7 @@ export abstract class NixType { return _nixBoolFromJs(this.asBoolean() && rhs.asBoolean()); } - apply(param: NixType): NixType { + apply(param: NixType, ctx: EvalCtx): NixType { throw invalidTypeError( this, err`Attempt to call something which is not a function but is ${errType(this)}`, @@ -1063,8 +1063,8 @@ export class Lazy extends NixType { return this.toStrict().and(rhs); } - override apply(param: NixType): NixType { - return this.toStrict().apply(param); + override apply(param: NixType, ctx: EvalCtx): NixType { + return this.toStrict().apply(param, ctx); } override asBoolean(): boolean { @@ -1176,15 +1176,15 @@ export class Lazy extends NixType { } export class Lambda extends NixType { - body: (param: NixType) => NixType; + body: (param: NixType, ctx: EvalCtx) => NixType; - constructor(body: (param: NixType) => NixType) { + constructor(body: (param: NixType, ctx: EvalCtx) => NixType) { super(); this.body = body; } - override apply(param: NixType): NixType { - return this.body(param); + override apply(param: NixType, ctx: EvalCtx): NixType { + return this.body(param, ctx); } toJs(): any { diff --git a/src/eval/emit_js.rs b/src/eval/emit_js.rs index 7641090..717725d 100644 --- a/src/eval/emit_js.rs +++ b/src/eval/emit_js.rs @@ -49,7 +49,7 @@ fn emit_apply(apply: &ast::Apply, out_src: &mut String) -> Result<(), String> { .expect("Unexpected lambda application without arguments."), out_src, )?; - out_src.push(')'); + out_src.push_str(", ctx)"); Ok(()) } From 09e395fd8ac24773096b813c340fbbfcb3ff0512 Mon Sep 17 00:00:00 2001 From: "arduano@localhost" <> Date: Mon, 20 May 2024 21:01:17 +1000 Subject: [PATCH 2/4] Improve error handling for toStrict when calling `toStrict`, there was no try/catch for propagating the error back to Rust correctly. However, `call_js_function` didn't have the ability to correctly pass in the current `this` object to the function, so I made another function to handle it. In the future we should do a full cleanup of helper functions in general. --- src/eval/execution.rs | 2 +- src/eval/helpers.rs | 41 ++++++++++++++++++++++++++++++++--------- src/eval/types.rs | 33 +++++++++++++++++---------------- 3 files changed, 50 insertions(+), 26 deletions(-) diff --git a/src/eval/execution.rs b/src/eval/execution.rs index 1e66b5a..c60a474 100644 --- a/src/eval/execution.rs +++ b/src/eval/execution.rs @@ -192,7 +192,7 @@ fn nix_value_from_module( .expect("`n.recursiveStrict` is not a function."); let strict_nix_value = call_js_function(scope, &to_strict_fn, nixjs_rt_obj, &[nix_value])?; - js_value_to_nix(scope, &nixrt, &strict_nix_value) + js_value_to_nix(scope, &nixjs_rt_obj, &strict_nix_value) } fn create_eval_ctx<'s>( diff --git a/src/eval/helpers.rs b/src/eval/helpers.rs index 185bde2..81f0f07 100644 --- a/src/eval/helpers.rs +++ b/src/eval/helpers.rs @@ -72,15 +72,38 @@ pub fn call_js_function<'s>( args: &[v8::Local], ) -> Result, NixError> { let try_scope = &mut v8::TryCatch::new(scope); - let recv = v8::undefined(try_scope).into(); - let Some(strict_nix_value) = js_function.call(try_scope, recv, args) else { - // TODO: Again, the stack trace needs to be source-mapped. - if let Some(error) = try_scope.exception() { - let error = js_error_to_rust(try_scope, nixrt, error); - return Err(error); - } else { - return Err("Unknown evaluation error.".into()); - } + let this = v8::undefined(try_scope).into(); + let Some(strict_nix_value) = js_function.call(try_scope, this, args) else { + let exception = try_scope.exception(); + return Err(map_js_exception_value_to_rust(try_scope, nixrt, exception)); }; Ok(strict_nix_value) } + +pub fn call_js_instance_mehod<'s>( + scope: &mut v8::HandleScope<'s>, + js_function: &v8::Local, + this: v8::Local, + nixrt: v8::Local, + args: &[v8::Local], +) -> Result, NixError> { + let try_scope = &mut v8::TryCatch::new(scope); + let Some(strict_nix_value) = js_function.call(try_scope, this, args) else { + let exception = try_scope.exception(); + return Err(map_js_exception_value_to_rust(try_scope, nixrt, exception)); + }; + Ok(strict_nix_value) +} + +pub fn map_js_exception_value_to_rust<'s>( + scope: &mut v8::HandleScope<'s>, + nixrt: v8::Local, + exception: Option>, +) -> NixError { + // TODO: Again, the stack trace needs to be source-mapped. + if let Some(error) = exception { + js_error_to_rust(scope, nixrt, error) + } else { + "Unknown evaluation error.".into() + } +} diff --git a/src/eval/types.rs b/src/eval/types.rs index f9ba0de..aea49b3 100644 --- a/src/eval/types.rs +++ b/src/eval/types.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use super::{ error::NixError, - helpers::{is_nixrt_type, try_get_js_object_key}, + helpers::{call_js_instance_mehod, is_nixrt_type, try_get_js_object_key}, }; #[derive(Debug, PartialEq)] @@ -34,7 +34,7 @@ pub type EvalResult = Result; pub fn js_value_to_nix( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> EvalResult { if js_value.is_function() { @@ -75,7 +75,7 @@ pub fn js_value_to_nix( fn from_js_int( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "NixInt")? { @@ -92,7 +92,7 @@ fn from_js_int( fn from_js_string( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "NixString")? { @@ -111,7 +111,7 @@ fn from_js_string( fn from_js_lazy( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "Lazy")? { @@ -123,9 +123,10 @@ fn from_js_lazy( "Expected `toStrict` to be a method on the Lazy object. Internal conversion error: {err:?}" ) })?; - let strict_value = to_strict_method - .call(scope, *js_value, &[]) - .ok_or_else(|| "Could not convert the lazy value to strict.".to_string())?; + + let strict_value = + call_js_instance_mehod(scope, &to_strict_method, *js_value, *nixrt, &[])?; + return Ok(Some(js_value_to_nix(scope, nixrt, &strict_value)?)); } Ok(None) @@ -133,7 +134,7 @@ fn from_js_lazy( fn from_js_bool( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "NixBool")? { @@ -152,7 +153,7 @@ fn from_js_bool( fn from_js_float( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "NixFloat")? { @@ -176,7 +177,7 @@ fn from_js_float( fn from_js_attrset( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "Attrset")? { @@ -206,7 +207,7 @@ fn from_js_attrset( fn from_js_list( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if is_nixrt_type(scope, nixrt, js_value, "NixList")? { @@ -226,7 +227,7 @@ fn from_js_list( fn from_js_path( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if !is_nixrt_type(scope, nixrt, js_value, "Path")? { @@ -240,7 +241,7 @@ fn from_js_path( fn from_js_lambda( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_value: &v8::Local, ) -> Result, NixError> { if !is_nixrt_type(scope, nixrt, js_value, "Lambda")? { @@ -251,7 +252,7 @@ fn from_js_lambda( fn js_value_as_nix_array( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_array: &v8::Local, ) -> EvalResult { let length = js_array.length(); @@ -268,7 +269,7 @@ fn js_value_as_nix_array( fn js_map_as_attrset( scope: &mut v8::HandleScope<'_>, - nixrt: &v8::Local, + nixrt: &v8::Local, js_map: &v8::Local, ) -> EvalResult { let mut map: HashMap = HashMap::new(); From 0ea6c3d379ee50e8755274bcc3aad5fb1cbad8e7 Mon Sep 17 00:00:00 2001 From: "arduano@localhost" <> Date: Mon, 20 May 2024 20:56:15 +1000 Subject: [PATCH 3/4] Propagate current path through the rust code Propagating current path (workdir) through the rust code. --- src/cmd/eval.rs | 5 ++++- src/eval/execution.rs | 18 ++++++------------ src/tests/mod.rs | 8 +++++--- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/src/cmd/eval.rs b/src/cmd/eval.rs index 950dcc3..677c1fe 100644 --- a/src/cmd/eval.rs +++ b/src/cmd/eval.rs @@ -28,7 +28,10 @@ pub fn handle_cmd(parsed_args: &ArgMatches) -> Result<(), NixError> { let expr = parsed_args .get_one::("expr") .ok_or("You must use the '--expr' option. Nothing else is implemented :)")?; - print_value(&execution::evaluate(expr)?); + + let current_dir = std::env::current_dir().map_err(|_| "Couldn't get the current directory")?; + + print_value(&execution::evaluate(expr, ¤t_dir)?); println!(); Ok(()) } diff --git a/src/eval/execution.rs b/src/eval/execution.rs index c60a474..b7303cb 100644 --- a/src/eval/execution.rs +++ b/src/eval/execution.rs @@ -1,4 +1,3 @@ -use std::env::current_dir; use std::path::Path; use std::sync::Once; @@ -13,7 +12,7 @@ use super::types::js_value_to_nix; static INIT_V8: Once = Once::new(); -pub fn evaluate(nix_expr: &str) -> EvalResult { +pub fn evaluate(nix_expr: &str, workdir: &Path) -> EvalResult { initialize_v8(); // Declare the V8 execution context let isolate = &mut v8::Isolate::new(Default::default()); @@ -53,7 +52,7 @@ pub fn evaluate(nix_expr: &str) -> EvalResult { let root_nix_fn = nix_expr_to_js_function(scope, nix_expr)?; - nix_value_from_module(scope, root_nix_fn, nixjs_rt_obj) + nix_value_from_module(scope, root_nix_fn, nixjs_rt_obj, workdir) } fn nix_expr_to_js_function<'s>( @@ -170,20 +169,15 @@ fn initialize_v8() { fn nix_value_from_module( scope: &mut v8::ContextScope, - nix_value: v8::Local, + nix_module_fn: v8::Local, nixjs_rt_obj: v8::Local, + workdir: &Path, ) -> EvalResult { let nixrt: v8::Local = nixjs_rt_obj.into(); - let eval_ctx = create_eval_ctx( - scope, - &nixrt, - ¤t_dir().map_err(|err| { - format!("Failed to determine the current working directory. Error: {err}") - })?, - )?; + let eval_ctx = create_eval_ctx(scope, &nixrt, workdir)?; - let nix_value = call_js_function(scope, &nix_value, nixjs_rt_obj, &[eval_ctx.into()])?; + let nix_value = call_js_function(scope, &nix_module_fn, nixjs_rt_obj, &[eval_ctx.into()])?; let to_strict_fn: v8::Local = try_get_js_object_key(scope, &nixrt, "recursiveStrict")? diff --git a/src/tests/mod.rs b/src/tests/mod.rs index 5ae6b1e..31f1aa4 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -15,17 +15,19 @@ mod literals; mod operators; fn eval_ok(nix_expr: &str) -> Value { - match evaluate(nix_expr) { + let workdir = std::env::current_dir().unwrap(); + match evaluate(nix_expr, &workdir) { Ok(val) => val, Err(err) => panic!( - "eval '{}' shouldn't fail.\nError message: {}", + "eval '{}' shouldn't fail.\nError message: {:?}", nix_expr, err ), } } fn eval_err(nix_expr: &str) -> NixErrorKind { - evaluate(nix_expr) + let workdir = std::env::current_dir().unwrap(); + evaluate(nix_expr, &workdir) .expect_err(&format!("eval '{}' expected an error", nix_expr)) .kind } From 13e6ff9941fe5339de7b592d19071cdca00c8956 Mon Sep 17 00:00:00 2001 From: "arduano@localhost" <> Date: Mon, 20 May 2024 21:01:18 +1000 Subject: [PATCH 4/4] More robust import tests Everything I've done in previous PRs fixed issues with these tests. They test importing both using paths and strings, importing from the same folder, from a parent folder, from a child folder. --- nixjs-rt/src/builtins.ts | 9 +++- src/tests/builtins.rs | 52 ++++++++++++++++++- src/tests/import_tests/basic.nix | 3 ++ .../import_tests/child-folder-import.nix | 4 ++ src/tests/import_tests/nested/basic.nix | 3 ++ .../nested/parent-folder-import.nix | 4 ++ src/tests/import_tests/same-folder-import.nix | 4 ++ 7 files changed, 76 insertions(+), 3 deletions(-) create mode 100644 src/tests/import_tests/basic.nix create mode 100644 src/tests/import_tests/child-folder-import.nix create mode 100644 src/tests/import_tests/nested/basic.nix create mode 100644 src/tests/import_tests/nested/parent-folder-import.nix create mode 100644 src/tests/import_tests/same-folder-import.nix diff --git a/nixjs-rt/src/builtins.ts b/nixjs-rt/src/builtins.ts index 6bc66a3..02ccf63 100644 --- a/nixjs-rt/src/builtins.ts +++ b/nixjs-rt/src/builtins.ts @@ -549,7 +549,14 @@ export function getBuiltins() { throw new Error("unimplemented"); }, - toString: (arg) => { + toString: (arg: NixType) => { + if (arg instanceof NixString) { + return arg; + } else if (arg instanceof Path) { + return new NixString(arg.path); + } + + // TODO: Expand on this throw new Error("unimplemented"); }, diff --git a/src/tests/builtins.rs b/src/tests/builtins.rs index fe0a8f8..91e9580 100644 --- a/src/tests/builtins.rs +++ b/src/tests/builtins.rs @@ -488,11 +488,59 @@ mod import { #[test] fn eval() { assert_eq!( - eval_ok("(builtins.import ./flake.nix).description"), - Value::Str("A reimplementation or nix in Rust.".into()) + eval_ok("(builtins.import ./src/tests/import_tests/basic.nix).data"), + Value::Str("imported!".into()) ); } + #[test] + fn eval_same_folder_import() { + assert_eq!( + eval_ok("(builtins.import ./src/tests/import_tests/same-folder-import.nix).dataPath"), + Value::Str("imported!".into()) + ); + assert_eq!( + eval_ok("(builtins.import ./src/tests/import_tests/same-folder-import.nix).dataString"), + Value::Str("imported!".into()) + ); + } + + #[test] + fn eval_child_folder_import() { + assert_eq!( + eval_ok("(builtins.import ./src/tests/import_tests/child-folder-import.nix).dataPath"), + Value::Str("imported!".into()) + ); + assert_eq!( + eval_ok( + "(builtins.import ./src/tests/import_tests/child-folder-import.nix).dataString" + ), + Value::Str("imported!".into()) + ); + } + + #[test] + fn eval_parent_folder_import() { + assert_eq!( + eval_ok("(builtins.import ./src/tests/import_tests/nested/parent-folder-import.nix).dataPath"), + Value::Str("imported!".into()) + ); + assert_eq!( + eval_ok("(builtins.import ./src/tests/import_tests/nested/parent-folder-import.nix).dataString"), + Value::Str("imported!".into()) + ); + } + + #[test] + fn eval_relative_string() { + assert_eq!( + eval_err(r#"builtins.import "./foo.nix""#), + NixErrorKind::Other { + codename: "builtins-import-non-absolute-path".to_owned() + } + ) + } + #[test] fn eval_lazy() { assert_eq!( diff --git a/src/tests/import_tests/basic.nix b/src/tests/import_tests/basic.nix new file mode 100644 index 0000000..55c15c7 --- /dev/null +++ b/src/tests/import_tests/basic.nix @@ -0,0 +1,3 @@ +{ + data = "imported!"; +} diff --git a/src/tests/import_tests/child-folder-import.nix b/src/tests/import_tests/child-folder-import.nix new file mode 100644 index 0000000..f6aa112 --- /dev/null +++ b/src/tests/import_tests/child-folder-import.nix @@ -0,0 +1,4 @@ +{ + dataPath = (builtins.import ./nested/basic.nix).data; + dataString = (builtins.import (builtins.toString ./nested/basic.nix)).data; +} diff --git a/src/tests/import_tests/nested/basic.nix b/src/tests/import_tests/nested/basic.nix new file mode 100644 index 0000000..55c15c7 --- /dev/null +++ b/src/tests/import_tests/nested/basic.nix @@ -0,0 +1,3 @@ +{ + data = "imported!"; +} diff --git a/src/tests/import_tests/nested/parent-folder-import.nix b/src/tests/import_tests/nested/parent-folder-import.nix new file mode 100644 index 0000000..be7a8f9 --- /dev/null +++ b/src/tests/import_tests/nested/parent-folder-import.nix @@ -0,0 +1,4 @@ +{ + dataPath = (builtins.import ../basic.nix).data; + dataString = (builtins.import (builtins.toString ../basic.nix)).data; +} diff --git a/src/tests/import_tests/same-folder-import.nix b/src/tests/import_tests/same-folder-import.nix new file mode 100644 index 0000000..5040c08 --- /dev/null +++ b/src/tests/import_tests/same-folder-import.nix @@ -0,0 +1,4 @@ +{ + dataPath = (builtins.import ./basic.nix).data; + dataString = (builtins.import (builtins.toString ./basic.nix)).data; +}