Gentype fix (#149)

Author: jfrolich

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");
     }

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<String>) -> 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);
     });

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::<Vec<Vec<String>>>();
 
@@ -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<String>,
     build_dev_deps: bool,
 ) -> Result<Option<String>> {
+    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());

src/build/deps.rs

@@ -9,8 +9,10 @@ fn get_dep_modules(
     namespace: Option<String>,
     package_modules: &AHashSet<String>,
     valid_modules: &AHashSet<String>,
+    package: &packages::Package,
 ) -> AHashSet<String> {
     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<String>
                 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 {

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"

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);