diff --git a/src/build.rs b/src/build.rs index 20efb5c..9b707f5 100644 --- a/src/build.rs +++ b/src/build.rs @@ -78,9 +78,10 @@ pub fn get_compiler_args( }; helpers::get_rescript_version(&bsc_path) }; + // make PathBuf from package root and get the relative path for filename let relative_filename = PathBuf::from(&filename) - .strip_prefix(PathBuf::from(&package_root).parent().unwrap()) + .strip_prefix(PathBuf::from(&package_root)) .unwrap() .to_string_lossy() .to_string(); @@ -108,7 +109,7 @@ pub fn get_compiler_args( let compiler_args = compiler_args( &rescript_config, &root_rescript_config, - &ast_path, + &ast_path.to_string_lossy(), &rescript_version, &relative_filename, is_interface, @@ -451,7 +452,7 @@ pub fn incremental_build( pub fn write_build_ninja(build_state: &BuildState) { for package in build_state.packages.values() { // write empty file: - let mut f = File::create(std::path::Path::new(&package.get_bs_build_path()).join("build.ninja")) + let mut f = File::create(std::path::Path::new(&package.get_build_path()).join("build.ninja")) .expect("Unable to write file"); f.write_all(b"").expect("unable to write to ninja file"); } diff --git a/src/build/clean.rs b/src/build/clean.rs index 302a27e..e88e652 100644 --- a/src/build/clean.rs +++ b/src/build/clean.rs @@ -158,7 +158,11 @@ pub fn cleanup_previous_build( .get_mut(module_name) .expect("Could not find module for ast file"); - let compile_dirty = compile_assets_state.cmi_modules.get(module_name); + let compile_dirty = compile_assets_state.cmt_modules.get(module_name); + // if there is a new AST but it has not been compiled yet, we mark the module as compile dirty + // we do this by checking if the cmt file is newer than the AST file. We always compile the + // interface AND implementation. For some reason the CMI file is not always rewritten if it + // doesn't have any changes, that's why we just look at the CMT file. if let Some(compile_dirty) = compile_dirty { let last_modified = Some(ast_last_modified); @@ -356,7 +360,7 @@ pub fn clean(path: &str, show_progress: bool, bsc_path: Option) -> Resul let path = std::path::Path::new(&path_str); let _ = std::fs::remove_dir_all(path); - let path_str = package.get_bs_build_path(); + let path_str = package.get_ocaml_build_path(); let path = std::path::Path::new(&path_str); let _ = std::fs::remove_dir_all(path); }); diff --git a/src/build/compile.rs b/src/build/compile.rs index 17fad80..b780908 100644 --- a/src/build/compile.rs +++ b/src/build/compile.rs @@ -162,7 +162,7 @@ pub fn compile( let result = compile_file( package, root_package, - &package.get_iast_path(&path), + &helpers::get_ast_path(&path).to_string_lossy(), module, &build_state.rescript_version, true, @@ -179,7 +179,7 @@ pub fn compile( let result = compile_file( package, root_package, - &package.get_ast_path(&source_file.implementation.path), + &helpers::get_ast_path(&source_file.implementation.path).to_string_lossy(), module, &build_state.rescript_version, false, @@ -379,16 +379,16 @@ pub fn compiler_args( .par_iter() .map(|package_name| { let canonicalized_path = if let Some(packages) = packages { - packages - .get(package_name) - .expect("expect package") - .path - .to_string() + let package = packages.get(package_name).expect("expect package"); + package.path.to_string() } else { packages::read_dependency(package_name, project_root, project_root, workspace_root) .expect("cannot find dep") }; - vec!["-I".to_string(), packages::get_build_path(&canonicalized_path)] + vec![ + "-I".to_string(), + packages::get_ocaml_build_path(&canonicalized_path), + ] }) .collect::>>(); @@ -415,9 +415,6 @@ pub fn compiler_args( packages::Namespace::NoNamespace => vec![], }; - let jsx_args = root_config.get_jsx_args(); - let jsx_module_args = root_config.get_jsx_module_args(); - let jsx_mode_args = root_config.get_jsx_mode_args(); let uncurried_args = root_config.get_uncurried_args(version); let gentype_arg = root_config.get_gentype_arg(); @@ -478,15 +475,12 @@ pub fn compiler_args( vec![ namespace_args, read_cmi_args, - vec!["-I".to_string(), ".".to_string()], + vec!["-I".to_string(), "../ocaml".to_string()], deps.concat(), - gentype_arg, - jsx_args, - jsx_module_args, - jsx_mode_args, uncurried_args, bsc_flags.to_owned(), warning_args, + gentype_arg, // vec!["-warn-error".to_string(), "A".to_string()], // ^^ this one fails for bisect-ppx // this is the default @@ -497,6 +491,7 @@ pub fn compiler_args( // "-I".to_string(), // abs_node_modules_path.to_string() + "/rescript/ocaml", // ], + vec!["-bs-v".to_string(), format!("{}", version)], vec![ast_path.to_string()], ] .concat() @@ -515,6 +510,7 @@ fn compile_file( workspace_root: &Option, build_dev_deps: bool, ) -> Result> { + let ocaml_build_path_abs = package.get_ocaml_build_path(); let build_path_abs = package.get_build_path(); let implementation_file_path = match &module.source_type { SourceType::SourceFile(ref source_file) => Ok(&source_file.implementation.path), @@ -539,7 +535,6 @@ fn compile_file( &Some(packages), build_dev_deps, ); - let to_mjs = Command::new(bsc_path) .current_dir(helpers::canonicalize_string_path(&build_path_abs.to_owned()).unwrap()) .args(to_mjs_args) @@ -566,35 +561,41 @@ fn compile_file( // perhaps we can do this copying somewhere else if !is_interface { let _ = std::fs::copy( - build_path_abs.to_string() + "/" + &module_name + ".cmi", - std::path::Path::new(&package.get_bs_build_path()) + std::path::Path::new(&package.get_build_path()) .join(dir) // because editor tooling doesn't support namespace entries yet // we just remove the @ for now. This makes sure the editor support // doesn't break .join(module_name.to_owned() + ".cmi"), + ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmi", ); let _ = std::fs::copy( - build_path_abs.to_string() + "/" + &module_name + ".cmj", - std::path::Path::new(&package.get_bs_build_path()) + std::path::Path::new(&package.get_build_path()) .join(dir) .join(module_name.to_owned() + ".cmj"), + ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmj", ); let _ = std::fs::copy( - build_path_abs.to_string() + "/" + &module_name + ".cmt", - std::path::Path::new(&package.get_bs_build_path()) + std::path::Path::new(&package.get_build_path()) .join(dir) // because editor tooling doesn't support namespace entries yet // we just remove the @ for now. This makes sure the editor support // doesn't break .join(module_name.to_owned() + ".cmt"), + ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmt", ); } else { let _ = std::fs::copy( - build_path_abs.to_string() + "/" + &module_name + ".cmti", - std::path::Path::new(&package.get_bs_build_path()) + std::path::Path::new(&package.get_build_path()) .join(dir) .join(module_name.to_owned() + ".cmti"), + ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmti", + ); + let _ = std::fs::copy( + std::path::Path::new(&package.get_build_path()) + .join(dir) + .join(module_name.to_owned() + ".cmi"), + ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmi", ); } match &module.source_type { @@ -611,7 +612,7 @@ fn compile_file( // and in lib/ocaml when referencing modules in other packages let _ = std::fs::copy( std::path::Path::new(&package.path).join(path), - std::path::Path::new(&package.get_bs_build_path()).join(path), + std::path::Path::new(&package.get_build_path()).join(path), ) .expect("copying source file failed"); @@ -672,7 +673,7 @@ pub fn mark_modules_with_expired_deps_dirty(build_state: &mut BuildState) { let dependent_module = build_state.modules.get(dependent).unwrap(); match dependent_module.source_type { SourceType::SourceFile(_) => { - match (module.last_compiled_cmt, module.last_compiled_cmi) { + match (module.last_compiled_cmt, module.last_compiled_cmt) { (None, None) | (Some(_), None) | (None, Some(_)) => { // println!( // "🛑 {} is a dependent of {} but has no cmt/cmi", @@ -686,7 +687,7 @@ pub fn mark_modules_with_expired_deps_dirty(build_state: &mut BuildState) { // we compare the last compiled time of the dependent module with the last // compile of the interface of the module it depends on, if the interface // didn't change it doesn't matter - match (dependent_module.last_compiled_cmt, module.last_compiled_cmi) { + match (dependent_module.last_compiled_cmt, module.last_compiled_cmt) { (Some(last_compiled_dependent), Some(last_compiled)) => { if last_compiled_dependent < last_compiled { // println!( @@ -719,7 +720,7 @@ pub fn mark_modules_with_expired_deps_dirty(build_state: &mut BuildState) { let dependent_module = build_state.modules.get(dependent_of_namespace).unwrap(); if let (Some(last_compiled_dependent), Some(last_compiled)) = - (dependent_module.last_compiled_cmt, module.last_compiled_cmi) + (dependent_module.last_compiled_cmt, module.last_compiled_cmt) { if last_compiled_dependent < last_compiled { modules_with_expired_deps.insert(dependent.to_string()); diff --git a/src/build/deps.rs b/src/build/deps.rs index 90e8365..59ddd3c 100644 --- a/src/build/deps.rs +++ b/src/build/deps.rs @@ -9,8 +9,10 @@ fn get_dep_modules( namespace: Option, package_modules: &AHashSet, valid_modules: &AHashSet, + package: &packages::Package, ) -> AHashSet { let mut deps = AHashSet::new(); + let ast_file = package.get_build_path() + "/" + ast_file; if let Ok(lines) = helpers::read_lines(ast_file.to_string()) { // we skip the first line with is some null characters // the following lines in the AST are the dependency modules @@ -74,23 +76,25 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet let package = build_state .get_package(&module.package_name) .expect("Package not found"); - let ast_path = package.get_ast_path(&source_file.implementation.path); + let ast_path = helpers::get_ast_path(&source_file.implementation.path); if module.deps_dirty || !build_state.deps_initialized { let mut deps = get_dep_modules( - &ast_path, + &ast_path.to_string_lossy(), package.namespace.to_suffix(), package.modules.as_ref().unwrap(), all_mod, + &package, ); if let Some(interface) = &source_file.interface { - let iast_path = package.get_iast_path(&interface.path); + let iast_path = helpers::get_ast_path(&interface.path); deps.extend(get_dep_modules( - &iast_path, + &iast_path.to_string_lossy(), package.namespace.to_suffix(), package.modules.as_ref().unwrap(), all_mod, + &package, )) } match &package.namespace { diff --git a/src/build/logs.rs b/src/build/logs.rs index 8f79afd..5f3b8b4 100644 --- a/src/build/logs.rs +++ b/src/build/logs.rs @@ -16,8 +16,8 @@ enum Location { fn get_log_file_path(package: &packages::Package, subfolder: Location) -> String { let build_folder = match subfolder { - Location::Bs => package.get_bs_build_path(), - Location::Ocaml => package.get_build_path(), + Location::Bs => package.get_build_path(), + Location::Ocaml => package.get_ocaml_build_path(), }; build_folder.to_owned() + "/.compiler.log" diff --git a/src/build/packages.rs b/src/build/packages.rs index 68713b0..cc3dae2 100644 --- a/src/build/packages.rs +++ b/src/build/packages.rs @@ -63,12 +63,16 @@ pub struct Package { } pub fn get_build_path(canonical_path: &str) -> String { + format!("{}/lib/bs", canonical_path) +} + +pub fn get_ocaml_build_path(canonical_path: &str) -> String { format!("{}/lib/ocaml", canonical_path) } impl Package { - pub fn get_bs_build_path(&self) -> String { - format!("{}/lib/bs", self.path) + pub fn get_ocaml_build_path(&self) -> String { + get_ocaml_build_path(&self.path) } pub fn get_build_path(&self) -> String { @@ -94,14 +98,6 @@ impl Package { .expect("namespace should be set for mlmap module") + ".cmi" } - - pub fn get_ast_path(&self, source_file: &str) -> String { - helpers::get_compiler_asset(self, &packages::Namespace::NoNamespace, source_file, "ast") - } - - pub fn get_iast_path(&self, source_file: &str) -> String { - helpers::get_compiler_asset(self, &packages::Namespace::NoNamespace, source_file, "iast") - } } impl PartialEq for Package { @@ -561,14 +557,6 @@ pub fn make( * the IO */ let result = extend_with_children(filter, map); - result.values().for_each(|package| { - if let Some(dirs) = &package.dirs { - dirs.iter().for_each(|dir| { - let _ = std::fs::create_dir_all(std::path::Path::new(&package.get_bs_build_path()).join(dir)); - }) - } - }); - Ok(result) } @@ -589,7 +577,7 @@ pub fn parse_packages(build_state: &mut BuildState) { build_state.module_names.extend(package_modules) } let build_path_abs = package.get_build_path(); - let bs_build_path = package.get_bs_build_path(); + let bs_build_path = package.get_ocaml_build_path(); helpers::create_build_path(&build_path_abs); helpers::create_build_path(&bs_build_path); diff --git a/src/build/parse.rs b/src/build/parse.rs index ddc9c94..92935ce 100644 --- a/src/build/parse.rs +++ b/src/build/parse.rs @@ -29,7 +29,12 @@ pub fn generate_asts( match &module.source_type { SourceType::MlMap(_mlmap) => { let path = package.get_mlmap_path(); - (module_name.to_owned(), Ok((path, None)), Ok(None), false) + ( + module_name.to_owned(), + Ok((Path::new(&path).to_path_buf(), None)), + Ok(None), + false, + ) } SourceType::SourceFile(source_file) => { @@ -69,13 +74,20 @@ pub fn generate_asts( } else { ( Ok(( - helpers::get_basename(&source_file.implementation.path).to_string() + ".ast", + Path::new( + &(helpers::get_basename(&source_file.implementation.path).to_string() + + ".ast"), + ) + .to_path_buf(), None, )), - Ok(source_file - .interface - .as_ref() - .map(|i| (helpers::get_basename(&i.path).to_string() + ".iast", None))), + Ok(source_file.interface.as_ref().map(|i| { + ( + Path::new(&(helpers::get_basename(&i.path).to_string() + ".iast")) + .to_path_buf(), + None, + ) + })), false, ) }; @@ -86,8 +98,8 @@ pub fn generate_asts( }) .collect::), String>, - Result)>, String>, + Result<(PathBuf, Option), String>, + Result)>, String>, bool, )>>() .into_iter() @@ -201,24 +213,23 @@ pub fn generate_asts( .namespace .to_suffix() .expect("namespace should be set for mlmap module"); - // copy the mlmap to the bs build path for editor tooling let base_build_path = package.get_build_path() + "/" + &suffix; - let base_bs_build_path = package.get_bs_build_path() + "/" + &suffix; + let base_ocaml_build_path = package.get_ocaml_build_path() + "/" + &suffix; let _ = std::fs::copy( base_build_path.to_string() + ".cmi", - base_bs_build_path.to_string() + ".cmi", + base_ocaml_build_path.to_string() + ".cmi", ); let _ = std::fs::copy( base_build_path.to_string() + ".cmt", - base_bs_build_path.to_string() + ".cmt", + base_ocaml_build_path.to_string() + ".cmt", ); let _ = std::fs::copy( base_build_path.to_string() + ".cmj", - base_bs_build_path.to_string() + ".cmj", + base_ocaml_build_path.to_string() + ".cmj", ); let _ = std::fs::copy( base_build_path.to_string() + ".mlmap", - base_bs_build_path.to_string() + ".mlmap", + base_ocaml_build_path.to_string() + ".mlmap", ); match (mlmap_hash, mlmap_hash_after) { (Some(digest), Some(digest_after)) => !digest.eq(&digest_after), @@ -250,11 +261,9 @@ pub fn parser_args( workspace_root: &Option, root_path: &str, contents: &str, -) -> (String, Vec) { +) -> (PathBuf, Vec) { let file = &filename.to_string(); - let path = PathBuf::from(filename); - let ast_extension = path_to_ast_extension(&path); - let ast_path = (helpers::get_basename(&file.to_string()).to_owned()) + ast_extension; + let ast_path = helpers::get_ast_path(file); let ppx_flags = config::flatten_ppx_flags( &if let Some(workspace_root) = workspace_root { format!("{}/node_modules", &workspace_root) @@ -272,7 +281,7 @@ pub fn parser_args( let file = "../../".to_string() + file; ( - ast_path.to_string(), + ast_path.to_owned(), [ vec!["-bs-v".to_string(), format!("{}", version)], ppx_flags, @@ -285,7 +294,7 @@ pub fn parser_args( "-absname".to_string(), "-bs-ast".to_string(), "-o".to_string(), - ast_path.to_string(), + ast_path.to_string_lossy().to_string(), file, ], ] @@ -300,7 +309,7 @@ fn generate_ast( version: &str, bsc_path: &str, workspace_root: &Option, -) -> Result<(String, Option), String> { +) -> Result<(PathBuf, Option), String> { let file_path = PathBuf::from(&package.path).join(filename); let contents = helpers::read_file(&file_path).expect("Error reading file"); @@ -315,6 +324,11 @@ fn generate_ast( &contents, ); + // generate the dir of the ast_path (it mirrors the source file dir) + helpers::create_build_path( + &(package.get_build_path() + "/" + &ast_path.parent().unwrap().to_string_lossy()), + ); + /* Create .ast */ let result = if let Some(res_to_ast) = Some( Command::new(bsc_path) @@ -342,26 +356,14 @@ fn generate_ast( )) }; if let Ok((ast_path, _)) = &result { - let dir = std::path::Path::new(filename).parent().unwrap(); let _ = std::fs::copy( - build_path_abs.to_string() + "/" + ast_path, - std::path::Path::new(&package.get_bs_build_path()) - .join(dir) - .join(ast_path), + Path::new(&build_path_abs).join(&ast_path), + std::path::Path::new(&package.get_ocaml_build_path()).join(ast_path.file_name().unwrap()), ); } result } -fn path_to_ast_extension(path: &Path) -> &str { - let extension = path.extension().unwrap().to_str().unwrap(); - if helpers::is_interface_ast_file(extension) { - ".iast" - } else { - ".ast" - } -} - fn include_ppx(flag: &str, contents: &str) -> bool { if flag.contains("bisect") { return std::env::var("BISECT_ENABLE").is_ok(); diff --git a/src/build/read_compile_state.rs b/src/build/read_compile_state.rs index b6cbc9a..27fd1a7 100644 --- a/src/build/read_compile_state.rs +++ b/src/build/read_compile_state.rs @@ -52,7 +52,7 @@ pub fn read(build_state: &mut BuildState) -> CompileAssetsState { .packages .par_iter() .map(|(_, package)| { - let read_dir = fs::read_dir(std::path::Path::new(&package.get_build_path())).unwrap(); + let read_dir = fs::read_dir(std::path::Path::new(&package.get_ocaml_build_path())).unwrap(); read_dir .filter_map(|entry| match entry { Ok(entry) => { diff --git a/src/helpers.rs b/src/helpers.rs index ea34c11..a41c609 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -184,6 +184,24 @@ pub fn string_ends_with_any(s: &Path, suffixes: &[&str]) -> bool { .any(|&suffix| s.extension().unwrap_or(&OsString::new()).to_str().unwrap_or("") == suffix) } +fn path_to_ast_extension(path: &Path) -> &str { + let extension = path.extension().unwrap().to_str().unwrap(); + if extension.ends_with("i") { + ".iast" + } else { + ".ast" + } +} + +pub fn get_ast_path(source_file: &str) -> PathBuf { + let source_path = Path::new(source_file); + + source_path.parent().unwrap().join( + file_path_to_compiler_asset_basename(source_file, &packages::Namespace::NoNamespace) + + path_to_ast_extension(source_path), + ) +} + pub fn get_compiler_asset( package: &packages::Package, namespace: &packages::Namespace, @@ -194,18 +212,15 @@ pub fn get_compiler_asset( "ast" | "iast" => &packages::Namespace::NoNamespace, _ => namespace, }; - package.get_build_path() + package.get_ocaml_build_path() + "/" + &file_path_to_compiler_asset_basename(source_file, namespace) + "." + extension } -pub fn canonicalize_string_path(path: &str) -> Option { - return Path::new(path) - .canonicalize() - .ok() - .map(|path| path.to_str().expect("Could not canonicalize").to_string()); +pub fn canonicalize_string_path(path: &str) -> Option { + return Path::new(path).canonicalize().ok(); } pub fn get_bs_compiler_asset( @@ -221,7 +236,7 @@ pub fn get_bs_compiler_asset( let dir = std::path::Path::new(&source_file).parent().unwrap(); - std::path::Path::new(&package.get_bs_build_path()) + std::path::Path::new(&package.get_build_path()) .join(dir) .join(file_path_to_compiler_asset_basename(source_file, namespace) + extension) .to_str() diff --git a/src/sourcedirs.rs b/src/sourcedirs.rs index ff20663..ed704aa 100644 --- a/src/sourcedirs.rs +++ b/src/sourcedirs.rs @@ -85,7 +85,7 @@ pub fn print(buildstate: &BuildState) { // Write sourcedirs.json write_sourcedirs_files( - package.get_bs_build_path(), + package.get_build_path(), &SourceDirs { dirs: &dirs.clone().into_iter().collect::>(), pkgs: &pkgs.clone().flatten().collect::>(), @@ -109,7 +109,7 @@ pub fn print(buildstate: &BuildState) { // Write sourcedirs.json write_sourcedirs_files( - root_package.get_bs_build_path(), + root_package.get_build_path(), &SourceDirs { dirs: &merged_dirs.into_iter().collect::>(), pkgs: &merged_pkgs.into_iter().collect::>(), diff --git a/tests/snapshots/rename-file-with-interface.txt b/tests/snapshots/rename-file-with-interface.txt index 0e37bc6..5ebe107 100644 --- a/tests/snapshots/rename-file-with-interface.txt +++ b/tests/snapshots/rename-file-with-interface.txt @@ -6,6 +6,6 @@ [4/7] 🧹 Cleaning up previous build... [4/7] 🧹 Cleaned 2/11 0.00s  [5/7] 🧱 Parsed 1 source files in 0.00s  [6/7] 🌴 Collected deps in 0.00s - [7/7] 🤺 Compiled 1 modules in 0.00s + [7/7] 🤺 Compiled 2 modules in 0.00s  ✨ Finished Compilation in 0.00s diff --git a/tests/snapshots/rename-interface-file.txt b/tests/snapshots/rename-interface-file.txt index 9ecd912..a51a6de 100644 --- a/tests/snapshots/rename-interface-file.txt +++ b/tests/snapshots/rename-interface-file.txt @@ -6,6 +6,6 @@ [4/7] 🧹 Cleaning up previous build... [4/7] 🧹 Cleaned 1/11 0.00s  [5/7] 🧱 Parsed 1 source files in 0.00s  [6/7] 🌴 Collected deps in 0.00s - [7/7] 🤺 Compiled 1 modules in 0.00s + [7/7] 🤺 Compiled 2 modules in 0.00s  ✨ Finished Compilation in 0.00s