Skip to content
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

Fix warnings #150

Merged
merged 5 commits into from
Apr 7, 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
13 changes: 8 additions & 5 deletions src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,9 +405,6 @@ pub fn incremental_build(
}
pb.finish();
if !compile_errors.is_empty() {
if helpers::contains_ascii_characters(&compile_warnings) {
println!("{}", &compile_warnings);
}
if show_progress {
println!(
"{}{} {}Compiled {} modules in {:.2}s",
Expand All @@ -418,7 +415,12 @@ pub fn incremental_build(
default_timing.unwrap_or(compile_duration).as_secs_f64()
);
}
println!("{}", &compile_errors);
if helpers::contains_ascii_characters(&compile_warnings) {
println!("{}", &compile_warnings);
}
if helpers::contains_ascii_characters(&compile_errors) {
println!("{}", &compile_errors);
}
// mark the original files as dirty again, because we didn't complete a full build
for (module_name, module) in build_state.modules.iter_mut() {
if tracked_dirty_modules.contains(module_name) {
Expand All @@ -437,8 +439,9 @@ pub fn incremental_build(
default_timing.unwrap_or(compile_duration).as_secs_f64()
);
}

if helpers::contains_ascii_characters(&compile_warnings) {
log::warn!("{}", &compile_warnings);
println!("{}", &compile_warnings);
}
Ok(())
}
Expand Down
51 changes: 11 additions & 40 deletions src/build/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use super::packages;
use crate::config;
use crate::helpers;
use ahash::{AHashMap, AHashSet};
use anyhow::{anyhow, Result};
use anyhow::anyhow;
use console::style;
use log::{debug, trace};
use rayon::prelude::*;
Expand All @@ -22,7 +22,7 @@ pub fn compile(
inc: impl Fn() + std::marker::Sync,
set_length: impl Fn(u64),
build_dev_deps: bool,
) -> Result<(String, String, usize)> {
) -> anyhow::Result<(String, String, usize)> {
let mut compiled_modules = AHashSet::<String>::new();
let dirty_modules = build_state
.modules
Expand Down Expand Up @@ -148,7 +148,7 @@ pub fn compile(
"cmi",
);

let cmi_digest = helpers::compute_file_hash(&cmi_path);
let cmi_digest = helpers::compute_file_hash(&Path::new(&cmi_path));

let package = build_state
.get_package(&module.package_name)
Expand Down Expand Up @@ -189,7 +189,7 @@ pub fn compile(
&build_state.workspace_root,
build_dev_deps,
);
let cmi_digest_after = helpers::compute_file_hash(&cmi_path);
let cmi_digest_after = helpers::compute_file_hash(&Path::new(&cmi_path));

// we want to compare both the hash of interface and the implementation
// compile assets to verify that nothing changed. We also need to checke the interface
Expand Down Expand Up @@ -509,15 +509,14 @@ fn compile_file(
project_root: &str,
workspace_root: &Option<String>,
build_dev_deps: bool,
) -> Result<Option<String>> {
) -> Result<Option<String>, 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),
sourcetype => Err(anyhow!(
sourcetype => Err(format!(
"Tried to compile a file that is not a source file ({}). Path to AST: {}. ",
sourcetype,
ast_path
sourcetype, ast_path
)),
}?;
let module_name = helpers::file_path_to_module_name(implementation_file_path, &package.namespace);
Expand All @@ -544,12 +543,11 @@ fn compile_file(
Ok(x) if !x.status.success() => {
let stderr = String::from_utf8_lossy(&x.stderr);
let stdout = String::from_utf8_lossy(&x.stdout);
Err(anyhow!(stderr.to_string() + &stdout))
Err(stderr.to_string() + &stdout)
}
Err(e) => Err(anyhow!(
Err(e) => Err(format!(
"Could not compile file. Error: {}. Path to AST: {:?}",
e,
ast_path
e, ast_path
)),
Ok(x) => {
let err = std::str::from_utf8(&x.stderr)
Expand Down Expand Up @@ -598,36 +596,9 @@ fn compile_file(
ocaml_build_path_abs.to_string() + "/" + &module_name + ".cmi",
);
}
match &module.source_type {
SourceType::SourceFile(SourceFile {
interface: Some(Interface { path, .. }),
..
})
| SourceType::SourceFile(SourceFile {
implementation: Implementation { path, .. },
..
}) => {
// we need to copy the source file to the build directory.
// editor tools expects the source file in lib/bs for finding the current package
// 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_build_path()).join(path),
)
.expect("copying source file failed");

let _ = std::fs::copy(
std::path::Path::new(&package.path).join(path),
std::path::Path::new(&package.get_build_path())
.join(std::path::Path::new(path).file_name().unwrap()),
)
.expect("copying source file failed");
}
_ => (),
}

if helpers::contains_ascii_characters(&err) {
if package.is_pinned_dep {
if package.is_pinned_dep || package.is_local_dep {
// supress warnings of external deps
Ok(Some(err))
} else {
Expand Down
3 changes: 3 additions & 0 deletions src/build/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub struct Package {
pub path: String,
pub dirs: Option<AHashSet<PathBuf>>,
pub is_pinned_dep: bool,
pub is_local_dep: bool,
pub is_root: bool,
}

Expand Down Expand Up @@ -410,6 +411,7 @@ fn make_package(config: config::Config, package_path: &str, is_pinned_dep: bool,
.to_string(),
dirs: None,
is_pinned_dep,
is_local_dep: !package_path.contains("node_modules"),
is_root,
}
}
Expand Down Expand Up @@ -912,6 +914,7 @@ mod test {
dirs: None,
is_pinned_dep: false,
is_root: false,
is_local_dep: false,
}
}
#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/build/parse.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,9 @@ pub fn generate_asts(
// probably better to do this in a different function
// specific to compiling mlmaps
let compile_path = package.get_mlmap_compile_path();
let mlmap_hash = helpers::compute_file_hash(&compile_path);
let mlmap_hash = helpers::compute_file_hash(&Path::new(&compile_path));
namespaces::compile_mlmap(package, module_name, &build_state.bsc_path);
let mlmap_hash_after = helpers::compute_file_hash(&compile_path);
let mlmap_hash_after = helpers::compute_file_hash(&Path::new(&compile_path));

let suffix = package
.namespace
Expand Down
2 changes: 1 addition & 1 deletion src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ pub fn format_namespaced_module_name(module_name: &str) -> String {
}
}

pub fn compute_file_hash(path: &str) -> Option<blake3::Hash> {
pub fn compute_file_hash(path: &Path) -> Option<blake3::Hash> {
match fs::read(path) {
Ok(str) => Some(blake3::hash(&str)),
Err(_) => None,
Expand Down