Rewatch paths - windows support

.github/workflows/ci.yml

@@ -98,6 +98,9 @@ jobs:
       - name: Install npm packages
         run: yarn install
 
+      - name: Install testrepo deps
+        run: cd rewatch/testrepo && yarn install
+
       - name: Install dependencies (Linux)
         if: runner.os == 'Linux'
         uses: awalsh128/[email protected]
@@ -133,6 +136,8 @@ jobs:
       - name: Copy rewatch binary
         run: |
           cp rewatch/target/${{ matrix.rust-target }}/release/rewatch${{ runner.os == 'Windows' && '.exe' || '' }} rewatch
+          mkdir -p rewatch/target/release
+          cp rewatch/target/${{ matrix.rust-target }}/release/rewatch${{ runner.os == 'Windows' && '.exe' || '' }} rewatch/target/release 
           ./scripts/copyExes.js --rewatch
         shell: bash
 
@@ -354,6 +359,9 @@ jobs:
         if: runner.os != 'Windows'
         run: make -C tests/gentype_tests/typescript-react-example clean test
 
+      - name: Run rewatch tests
+        run: make test-rewatch
+
       - name: Run syntax benchmarks
         if: matrix.benchmarks
         run: ./_build/install/default/bin/syntax_benchmarks | tee tests/benchmark-output.json
@@ -507,7 +515,6 @@ jobs:
         working-directory: ${{ steps.tmp-dir.outputs.path }}
 
       - name: Install ReScript package in rewatch/testrepo
-        if: runner.os == 'Linux'
         run: |
           COMMIT_SHA="${{ github.event.pull_request.head.sha || github.sha }}"
           yarn add "rescript@https://pkg.pr.new/rescript-lang/rescript@${COMMIT_SHA::7}"
@@ -516,8 +523,7 @@ jobs:
 
       - name: Run rewatch integration tests
         # Currently failing on Windows and intermittently on macOS
-        if: runner.os == 'Linux'
-        run: make test-rewatch-ci
+        run: make test-rewatch-integration
 
   publish:
     needs:

Makefile

@@ -49,7 +49,7 @@ test-gentype:
 test-rewatch:
 	bash ./rewatch/tests/suite-ci.sh
 
-test-rewatch-ci:
+test-rewatch-integration:
 	bash ./rewatch/tests/suite-ci.sh node_modules/.bin/rewatch
 
 test-all: test test-gentype test-analysis test-tools test-rewatch

rewatch/src/build.rs

@@ -21,7 +21,7 @@ use serde::Serialize;
 use std::fmt;
 use std::fs::File;
 use std::io::{stdout, Write};
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 use std::time::{Duration, Instant};
 
 use self::compile::compiler_args;
@@ -55,15 +55,14 @@ pub struct CompilerArgs {
 }
 
 pub fn get_compiler_args(
-    path: &str,
+    path: &Path,
     rescript_version: Option<String>,
-    bsc_path: Option<String>,
+    bsc_path: &Option<PathBuf>,
     build_dev_deps: bool,
 ) -> Result<String> {
     let filename = &helpers::get_abs_path(path);
-    let package_root = helpers::get_abs_path(
-        &helpers::get_nearest_config(&std::path::PathBuf::from(path)).expect("Couldn't find package root"),
-    );
+    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()))?;
@@ -73,17 +72,13 @@ pub fn get_compiler_args(
     } else {
         let bsc_path = match bsc_path {
             Some(bsc_path) => helpers::get_abs_path(&bsc_path),
-            None => helpers::get_bsc(&package_root, workspace_root.to_owned()),
+            None => helpers::get_bsc(&package_root, &workspace_root),
         };
         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))
-        .unwrap()
-        .to_string_lossy()
-        .to_string();
+    let relative_filename = filename.strip_prefix(PathBuf::from(&package_root)).unwrap();
 
     let file_path = PathBuf::from(&package_root).join(filename);
     let contents = helpers::read_file(&file_path).expect("Error reading file");
@@ -97,18 +92,18 @@ pub fn get_compiler_args(
         workspace_root.as_ref().unwrap_or(&package_root),
         &contents,
     );
-    let is_interface = filename.ends_with('i');
+    let is_interface = filename.to_string_lossy().ends_with('i');
     let has_interface = if is_interface {
         true
     } else {
-        let mut interface_filename = filename.to_string();
+        let mut interface_filename = filename.to_string_lossy().to_string();
         interface_filename.push('i');
         PathBuf::from(&interface_filename).exists()
     };
     let compiler_args = compiler_args(
         &rescript_config,
         &root_rescript_config,
-        &ast_path.to_string_lossy(),
+        &ast_path,
         &rescript_version,
         &relative_filename,
         is_interface,
@@ -131,15 +126,15 @@ pub fn initialize_build(
     default_timing: Option<Duration>,
     filter: &Option<regex::Regex>,
     show_progress: bool,
-    path: &str,
-    bsc_path: Option<String>,
+    path: &Path,
+    bsc_path: &Option<PathBuf>,
     build_dev_deps: bool,
 ) -> Result<BuildState> {
     let project_root = helpers::get_abs_path(path);
     let workspace_root = helpers::get_workspace_root(&project_root);
     let bsc_path = match bsc_path {
         Some(bsc_path) => helpers::get_abs_path(&bsc_path),
-        None => helpers::get_bsc(&project_root, workspace_root.to_owned()),
+        None => helpers::get_bsc(&project_root, &workspace_root),
     };
     let root_config_name = packages::read_package_name(&project_root)?;
     let rescript_version = helpers::get_rescript_version(&bsc_path);
@@ -445,19 +440,18 @@ 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_build_path()).join("build.ninja"))
-            .expect("Unable to write file");
+        let mut f = File::create(package.get_build_path().join("build.ninja")).expect("Unable to write file");
         f.write_all(b"").expect("unable to write to ninja file");
     }
 }
 
 pub fn build(
     filter: &Option<regex::Regex>,
-    path: &str,
+    path: &Path,
     show_progress: bool,
     no_timing: bool,
     create_sourcedirs: bool,
-    bsc_path: Option<String>,
+    bsc_path: &Option<PathBuf>,
     build_dev_deps: bool,
 ) -> Result<BuildState> {
     let default_timing: Option<std::time::Duration> = if no_timing {

rewatch/src/build/build_types.rs

@@ -1,6 +1,6 @@
 use crate::build::packages::{Namespace, Package};
 use ahash::{AHashMap, AHashSet};
-use std::{fmt::Display, time::SystemTime};
+use std::{fmt::Display, path::PathBuf, time::SystemTime};
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum ParseState {
@@ -19,7 +19,7 @@ pub enum CompileState {
 }
 #[derive(Debug, Clone, PartialEq)]
 pub struct Interface {
-    pub path: String,
+    pub path: PathBuf,
     pub parse_state: ParseState,
     pub compile_state: CompileState,
     pub last_modified: SystemTime,
@@ -28,7 +28,7 @@ pub struct Interface {
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Implementation {
-    pub path: String,
+    pub path: PathBuf,
     pub parse_state: ParseState,
     pub compile_state: CompileState,
     pub last_modified: SystemTime,
@@ -91,12 +91,12 @@ pub struct BuildState {
     pub modules: AHashMap<String, Module>,
     pub packages: AHashMap<String, Package>,
     pub module_names: AHashSet<String>,
-    pub project_root: String,
+    pub project_root: PathBuf,
     pub root_config_name: String,
     pub deleted_modules: AHashSet<String>,
     pub rescript_version: String,
-    pub bsc_path: String,
-    pub workspace_root: Option<String>,
+    pub bsc_path: PathBuf,
+    pub workspace_root: Option<PathBuf>,
     pub deps_initialized: bool,
 }
 
@@ -109,12 +109,12 @@ impl BuildState {
         self.modules.get(module_name)
     }
     pub fn new(
-        project_root: String,
+        project_root: PathBuf,
         root_config_name: String,
         packages: AHashMap<String, Package>,
-        workspace_root: Option<String>,
+        workspace_root: Option<PathBuf>,
         rescript_version: String,
-        bsc_path: String,
+        bsc_path: PathBuf,
     ) -> Self {
         Self {
             module_names: AHashSet::new(),
@@ -136,20 +136,22 @@ impl BuildState {
     }
 }
 
+#[derive(Debug)]
 pub struct AstModule {
     pub module_name: String,
     pub package_name: String,
     pub namespace: Namespace,
     pub last_modified: SystemTime,
-    pub ast_file_path: String,
+    pub ast_file_path: PathBuf,
     pub is_root: bool,
     pub suffix: String,
 }
 
+#[derive(Debug)]
 pub struct CompileAssetsState {
-    pub ast_modules: AHashMap<String, AstModule>,
+    pub ast_modules: AHashMap<PathBuf, AstModule>,
     pub cmi_modules: AHashMap<String, SystemTime>,
     pub cmt_modules: AHashMap<String, SystemTime>,
-    pub ast_rescript_file_locations: AHashSet<String>,
-    pub rescript_file_locations: AHashSet<String>,
+    pub ast_rescript_file_locations: AHashSet<PathBuf>,
+    pub rescript_file_locations: AHashSet<PathBuf>,
 }

rewatch/src/build/clean.rs

@@ -7,9 +7,10 @@ use anyhow::Result;
 use console::style;
 use rayon::prelude::*;
 use std::io::Write;
+use std::path::{Path, PathBuf};
 use std::time::Instant;
 
-fn remove_ast(package: &packages::Package, source_file: &str) {
+fn remove_ast(package: &packages::Package, source_file: &Path) {
     let _ = std::fs::remove_file(helpers::get_compiler_asset(
         package,
         &packages::Namespace::NoNamespace,
@@ -18,7 +19,7 @@ fn remove_ast(package: &packages::Package, source_file: &str) {
     ));
 }
 
-fn remove_iast(package: &packages::Package, source_file: &str) {
+fn remove_iast(package: &packages::Package, source_file: &Path) {
     let _ = std::fs::remove_file(helpers::get_compiler_asset(
         package,
         &packages::Namespace::NoNamespace,
@@ -27,15 +28,14 @@ fn remove_iast(package: &packages::Package, source_file: &str) {
     ));
 }
 
-fn remove_mjs_file(source_file: &str, suffix: &String) {
-    let _ = std::fs::remove_file(helpers::change_extension(
-        source_file,
+fn remove_mjs_file(source_file: &Path, suffix: &String) {
+    let _ = std::fs::remove_file(source_file.with_extension(
         // suffix.to_string includes the ., so we need to remove it
         &suffix.to_string()[1..],
     ));
 }
 
-fn remove_compile_asset(package: &packages::Package, source_file: &str, extension: &str) {
+fn remove_compile_asset(package: &packages::Package, source_file: &Path, extension: &str) {
     let _ = std::fs::remove_file(helpers::get_compiler_asset(
         package,
         &package.namespace,
@@ -50,7 +50,7 @@ fn remove_compile_asset(package: &packages::Package, source_file: &str, extensio
     ));
 }
 
-pub fn remove_compile_assets(package: &packages::Package, source_file: &str) {
+pub fn remove_compile_assets(package: &packages::Package, source_file: &Path) {
     // optimization
     // only issue cmti if there is an interfacce file
     for extension in &["cmj", "cmi", "cmt", "cmti"] {
@@ -79,23 +79,20 @@ pub fn clean_mjs_files(build_state: &BuildState) {
                         .filter_map(|spec| {
                             if spec.in_source {
                                 Some((
-                                    std::path::PathBuf::from(package.path.to_string())
-                                        .join(&source_file.implementation.path)
-                                        .to_string_lossy()
-                                        .to_string(),
+                                    package.path.join(&source_file.implementation.path),
                                     root_package.config.get_suffix(spec),
                                 ))
                             } else {
                                 None
                             }
                         })
-                        .collect::<Vec<(String, String)>>(),
+                        .collect::<Vec<(PathBuf, String)>>(),
                 )
             }
             _ => None,
         })
         .flatten()
-        .collect::<Vec<(String, String)>>();
+        .collect::<Vec<(PathBuf, String)>>();
 
     rescript_file_locations
         .par_iter()
@@ -118,7 +115,7 @@ pub fn cleanup_previous_build(
     let diff = compile_assets_state
         .ast_rescript_file_locations
         .difference(&compile_assets_state.rescript_file_locations)
-        .collect::<Vec<&String>>();
+        .collect::<Vec<&PathBuf>>();
 
     let diff_len = diff.len();
 
@@ -133,7 +130,7 @@ pub fn cleanup_previous_build(
                 ..
             } = compile_assets_state
                 .ast_modules
-                .get(&res_file_location.to_string())
+                .get(*res_file_location)
                 .expect("Could not find module name for ast file");
 
             let package = build_state
@@ -173,18 +170,14 @@ pub fn cleanup_previous_build(
                 .get_mut(module_name)
                 .expect("Could not find module for ast file");
 
-            let compile_dirty = compile_assets_state.cmt_modules.get(module_name);
+            let cmt_last_modified = 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);
-
-                if let Some(last_modified) = last_modified {
-                    if compile_dirty > last_modified && !deleted_interfaces.contains(module_name) {
-                        module.compile_dirty = false;
-                    }
+            if let Some(cmt_last_modified) = cmt_last_modified {
+                if cmt_last_modified > ast_last_modified && !deleted_interfaces.contains(module_name) {
+                    module.compile_dirty = false;
                 }
             }
 
@@ -338,7 +331,12 @@ pub fn cleanup_after_build(build_state: &BuildState) {
     });
 }
 
-pub fn clean(path: &str, show_progress: bool, bsc_path: Option<String>, build_dev_deps: bool) -> Result<()> {
+pub fn clean(
+    path: &Path,
+    show_progress: bool,
+    bsc_path: &Option<PathBuf>,
+    build_dev_deps: bool,
+) -> Result<()> {
     let project_root = helpers::get_abs_path(path);
     let workspace_root = helpers::get_workspace_root(&project_root);
     let packages = packages::make(
@@ -352,7 +350,7 @@ pub fn clean(path: &str, show_progress: bool, bsc_path: Option<String>, build_de
     let root_config_name = packages::read_package_name(&project_root)?;
     let bsc_path = match bsc_path {
         Some(bsc_path) => helpers::get_abs_path(&bsc_path),
-        None => helpers::get_bsc(&project_root, workspace_root.to_owned()),
+        None => helpers::get_bsc(&project_root, &workspace_root),
     };
 
     let rescript_version = helpers::get_rescript_version(&bsc_path);