Skip to content

Gentype fix #149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Mar 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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");
}
Expand Down
8 changes: 6 additions & 2 deletions src/build/clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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);
});
Expand Down
59 changes: 30 additions & 29 deletions src/build/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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>>>();

Expand All @@ -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();

Expand Down Expand Up @@ -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
Expand All @@ -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()
Expand All @@ -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),
Expand All @@ -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)
Expand All @@ -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 {
Expand All @@ -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");

Expand Down Expand Up @@ -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",
Expand All @@ -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!(
Expand Down Expand Up @@ -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());
Expand Down
12 changes: 8 additions & 4 deletions src/build/deps.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
4 changes: 2 additions & 2 deletions src/build/logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
26 changes: 7 additions & 19 deletions src/build/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}

Expand All @@ -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);

Expand Down
Loading