Skip to content

Read bs-dev-dependencies if --dev was passed. #7650

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

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@



# Changelog

> **Tags:**
Expand Down Expand Up @@ -44,6 +47,7 @@
- Fix inside comment printing for empty dict. https://github.com/rescript-lang/rescript/pull/7654
- Fix I/O error message when trying to extract extra info from non-existing file. https://github.com/rescript-lang/rescript/pull/7656
- Fix fatal error when JSX expression used without configuring JSX in rescript.json. https://github.com/rescript-lang/rescript/pull/7656
- Rewatch: Only allow access to `"bs-dev-dependencies"` from `"type": "dev"` source files. https://github.com/rescript-lang/rescript/pull/7650

# 12.0.0-beta.1

Expand Down
16 changes: 8 additions & 8 deletions rewatch/src/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ pub mod packages;
pub mod parse;
pub mod read_compile_state;

use self::compile::compiler_args;
use self::parse::parser_args;
use crate::build::compile::{mark_modules_with_deleted_deps_dirty, mark_modules_with_expired_deps_dirty};
use crate::helpers::emojis::*;
use crate::helpers::{self, get_workspace_root};
Expand All @@ -26,9 +28,6 @@ use std::path::{Path, PathBuf};
use std::process::Stdio;
use std::time::{Duration, Instant};

use self::compile::compiler_args;
use self::parse::parser_args;

fn is_dirty(module: &Module) -> bool {
match module.source_type {
SourceType::SourceFile(SourceFile {
Expand Down Expand Up @@ -56,14 +55,18 @@ pub struct CompilerArgs {
pub parser_args: Vec<String>,
}

pub fn get_compiler_args(path: &Path, build_dev_deps: bool) -> Result<String> {
pub fn get_compiler_args(path: &Path) -> Result<String> {
let filename = &helpers::get_abs_path(path);
let package_root =
helpers::get_abs_path(&helpers::get_nearest_config(&path).expect("Couldn't find package root"));
let workspace_root = get_workspace_root(&package_root).map(|p| helpers::get_abs_path(&p));
let root_rescript_config =
packages::read_config(&workspace_root.to_owned().unwrap_or(package_root.to_owned()))?;
let rescript_config = packages::read_config(&package_root)?;
let is_type_dev = match filename.strip_prefix(&package_root) {
Err(_) => false,
Ok(relative_path) => root_rescript_config.find_is_type_dev_for_path(relative_path),
};

// make PathBuf from package root and get the relative path for filename
let relative_filename = filename.strip_prefix(PathBuf::from(&package_root)).unwrap();
Expand Down Expand Up @@ -97,7 +100,7 @@ pub fn get_compiler_args(path: &Path, build_dev_deps: bool) -> Result<String> {
&package_root,
&workspace_root,
&None,
build_dev_deps,
is_type_dev,
true,
);

Expand Down Expand Up @@ -281,7 +284,6 @@ pub fn incremental_build(
show_progress: bool,
only_incremental: bool,
create_sourcedirs: bool,
build_dev_deps: bool,
snapshot_output: bool,
) -> Result<(), IncrementalBuildError> {
logs::initialize(&build_state.packages);
Expand Down Expand Up @@ -393,7 +395,6 @@ pub fn incremental_build(
show_progress,
|| pb.inc(1),
|size| pb.set_length(size),
build_dev_deps,
)
.map_err(|e| IncrementalBuildError {
kind: IncrementalBuildErrorKind::CompileError(Some(e.to_string())),
Expand Down Expand Up @@ -500,7 +501,6 @@ pub fn build(
show_progress,
false,
create_sourcedirs,
build_dev_deps,
snapshot_output,
) {
Ok(_) => {
Expand Down
1 change: 1 addition & 0 deletions rewatch/src/build/build_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ pub struct Module {
pub last_compiled_cmi: Option<SystemTime>,
pub last_compiled_cmt: Option<SystemTime>,
pub deps_dirty: bool,
pub is_type_dev: bool,
}

impl Module {
Expand Down
19 changes: 9 additions & 10 deletions rewatch/src/build/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ pub fn compile(
show_progress: bool,
inc: impl Fn() + std::marker::Sync,
set_length: impl Fn(u64),
build_dev_deps: bool,
) -> anyhow::Result<(String, String, usize)> {
let mut compiled_modules = AHashSet::<String>::new();
let dirty_modules = build_state
Expand Down Expand Up @@ -170,7 +169,6 @@ pub fn compile(
&build_state.packages,
&build_state.project_root,
&build_state.workspace_root,
build_dev_deps,
);
Some(result)
}
Expand All @@ -186,7 +184,6 @@ pub fn compile(
&build_state.packages,
&build_state.project_root,
&build_state.workspace_root,
build_dev_deps,
);
let cmi_digest_after = helpers::compute_file_hash(Path::new(&cmi_path));

Expand Down Expand Up @@ -360,13 +357,13 @@ pub fn compiler_args(
// if packages are known, we pass a reference here
// this saves us a scan to find their paths
packages: &Option<&AHashMap<String, packages::Package>>,
build_dev_deps: bool,
// Is the file listed as "type":"dev"?
is_type_dev: bool,
is_local_dep: bool,
) -> Vec<String> {
let bsc_flags = config::flatten_flags(&config.bsc_flags);

let dependency_paths =
get_dependency_paths(config, project_root, workspace_root, packages, build_dev_deps);
let dependency_paths = get_dependency_paths(config, project_root, workspace_root, packages, is_type_dev);

let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace());

Expand Down Expand Up @@ -504,7 +501,7 @@ fn get_dependency_paths(
project_root: &Path,
workspace_root: &Option<PathBuf>,
packages: &Option<&AHashMap<String, packages::Package>>,
build_dev_deps: bool,
is_file_type_dev: bool,
) -> Vec<Vec<String>> {
let normal_deps = config
.bs_dependencies
Expand All @@ -513,7 +510,9 @@ fn get_dependency_paths(
.into_iter()
.map(DependentPackage::Normal)
.collect();
let dev_deps = if build_dev_deps {

// We can only access dev dependencies for source_files that are marked as "type":"dev"
let dev_deps = if is_file_type_dev {
config
.bs_dev_dependencies
.clone()
Expand Down Expand Up @@ -569,7 +568,6 @@ fn compile_file(
packages: &AHashMap<String, packages::Package>,
project_root: &Path,
workspace_root: &Option<PathBuf>,
build_dev_deps: bool,
) -> Result<Option<String>, String> {
let ocaml_build_path_abs = package.get_ocaml_build_path();
let build_path_abs = package.get_build_path();
Expand All @@ -583,6 +581,7 @@ fn compile_file(
}?;
let module_name = helpers::file_path_to_module_name(implementation_file_path, &package.namespace);
let has_interface = module.get_interface().is_some();
let is_type_dev = module.is_type_dev;
let to_mjs_args = compiler_args(
&package.config,
&root_package.config,
Expand All @@ -593,7 +592,7 @@ fn compile_file(
project_root,
workspace_root,
&Some(packages),
build_dev_deps,
is_type_dev,
package.is_local_dep,
);

Expand Down
46 changes: 37 additions & 9 deletions rewatch/src/build/packages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::time::SystemTime;
#[derive(Debug, Clone)]
pub struct SourceFileMeta {
pub modified: SystemTime,
pub is_type_dev: bool,
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -112,6 +113,13 @@ impl Package {
.expect("namespace should be set for mlmap module");
self.get_build_path().join(format!("{}.cmi", suffix))
}

pub fn is_source_file_type_dev(&self, path: &Path) -> bool {
self.source_files
.as_ref()
.and_then(|sf| sf.get(path).map(|sfm| sfm.is_type_dev))
.unwrap_or(false)
}
}

impl PartialEq for Package {
Expand All @@ -138,6 +146,7 @@ pub fn read_folders(
package_dir: &Path,
path: &Path,
recurse: bool,
is_type_dev: bool,
) -> Result<AHashMap<PathBuf, SourceFileMeta>, Box<dyn error::Error>> {
let mut map: AHashMap<PathBuf, SourceFileMeta> = AHashMap::new();
let path_buf = PathBuf::from(path);
Expand All @@ -147,6 +156,7 @@ pub fn read_folders(
path.to_owned(),
SourceFileMeta {
modified: meta.modified().unwrap(),
is_type_dev,
},
)
});
Expand All @@ -159,7 +169,7 @@ pub fn read_folders(
let path_ext = entry_path_buf.extension().and_then(|x| x.to_str());
let new_path = path_buf.join(&name);
if metadata.file_type().is_dir() && recurse {
match read_folders(filter, package_dir, &new_path, recurse) {
match read_folders(filter, package_dir, &new_path, recurse, is_type_dev) {
Ok(s) => map.extend(s),
Err(e) => log::error!("Could not read directory: {}", e),
}
Expand All @@ -174,6 +184,7 @@ pub fn read_folders(
path,
SourceFileMeta {
modified: metadata.modified().unwrap(),
is_type_dev,
},
);
}
Expand Down Expand Up @@ -297,11 +308,16 @@ fn read_dependencies(
project_root: &Path,
workspace_root: &Option<PathBuf>,
show_progress: bool,
build_dev_deps: bool,
) -> Vec<Dependency> {
return parent_config
.bs_dependencies
.to_owned()
.unwrap_or_default()
let mut dependencies = parent_config.bs_dependencies.to_owned().unwrap_or_default();

// Concatenate dev dependencies if build_dev_deps is true
if build_dev_deps && let Some(dev_deps) = parent_config.bs_dev_dependencies.to_owned() {
dependencies.extend(dev_deps);
}

dependencies
.iter()
.filter_map(|package_name| {
if registered_dependencies_set.contains(package_name) {
Expand Down Expand Up @@ -360,7 +376,8 @@ fn read_dependencies(
&canonical_path,
project_root,
workspace_root,
show_progress
show_progress,
build_dev_deps
);

Dependency {
Expand All @@ -371,7 +388,7 @@ fn read_dependencies(
dependencies,
}
})
.collect::<Vec<Dependency>>();
.collect()
}

fn flatten_dependencies(dependencies: Vec<Dependency>) -> Vec<Dependency> {
Expand Down Expand Up @@ -461,6 +478,7 @@ fn read_packages(
project_root: &Path,
workspace_root: &Option<PathBuf>,
show_progress: bool,
build_dev_deps: bool,
) -> Result<AHashMap<String, Package>> {
let root_config = read_config(project_root)?;

Expand All @@ -477,6 +495,7 @@ fn read_packages(
project_root,
workspace_root,
show_progress,
build_dev_deps,
));
dependencies.iter().for_each(|d| {
if !map.contains_key(&d.name) {
Expand Down Expand Up @@ -515,9 +534,14 @@ pub fn get_source_files(
};

let path_dir = Path::new(&source.dir);
let is_type_dev = type_
.as_ref()
.map(|t| t.as_str() == "dev")
.unwrap_or(false)
.clone();
match (build_dev_deps, type_) {
(false, Some(type_)) if type_ == "dev" => (),
_ => match read_folders(filter, package_dir, path_dir, recurse) {
_ => match read_folders(filter, package_dir, path_dir, recurse, is_type_dev) {
Ok(files) => map.extend(files),

Err(_e) => log::error!(
Expand Down Expand Up @@ -596,7 +620,7 @@ pub fn make(
show_progress: bool,
build_dev_deps: bool,
) -> Result<AHashMap<String, Package>> {
let map = read_packages(root_folder, workspace_root, show_progress)?;
let map = read_packages(root_folder, workspace_root, show_progress, build_dev_deps)?;

/* Once we have the deduplicated packages, we can add the source files for each - to minimize
* the IO */
Expand Down Expand Up @@ -720,6 +744,8 @@ pub fn parse_packages(build_state: &mut BuildState) {
compile_dirty: false,
last_compiled_cmt: None,
last_compiled_cmi: None,
// Not sure if this is correct
is_type_dev: false,
},
);
});
Expand Down Expand Up @@ -772,6 +798,7 @@ pub fn parse_packages(build_state: &mut BuildState) {
compile_dirty: true,
last_compiled_cmt: None,
last_compiled_cmi: None,
is_type_dev: metadata.is_type_dev,
});
} else {
// remove last character of string: resi -> res, rei -> re, mli -> ml
Expand Down Expand Up @@ -833,6 +860,7 @@ pub fn parse_packages(build_state: &mut BuildState) {
compile_dirty: true,
last_compiled_cmt: None,
last_compiled_cmi: None,
is_type_dev: metadata.is_type_dev,
});
}
}
Expand Down
3 changes: 0 additions & 3 deletions rewatch/src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,9 +214,6 @@ pub enum Command {
/// Path to a rescript file (.res or .resi)
#[command()]
path: String,

#[command(flatten)]
dev: DevArg,
},
/// Use the legacy build system.
///
Expand Down
Loading