Skip to content

Commit b6b2c64

Browse files
rolandpeelenrolandpeelen
committed
Dedupe file names
1 parent d78d42b commit b6b2c64

5 files changed

Lines changed: 114 additions & 61 deletions

File tree

‎rewatch/src/build/clean.rs‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,23 @@ use std::io::Write;
1414
use std::path::{Path, PathBuf};
1515
use std::time::Instant;
1616

17+
fn remove_ast_cache(package: &packages::Package, source_file: &Path, extension: &str) {
18+
let ast_cache_path = helpers::get_ocaml_ast_cache_path_for_extension(package, source_file, extension);
19+
let _ = std::fs::remove_file(&ast_cache_path);
20+
21+
let legacy_ast_cache_path =
22+
helpers::get_compiler_asset(package, &packages::Namespace::NoNamespace, source_file, extension);
23+
if legacy_ast_cache_path != ast_cache_path {
24+
let _ = std::fs::remove_file(legacy_ast_cache_path);
25+
}
26+
}
27+
1728
fn remove_ast(package: &packages::Package, source_file: &Path) {
18-
let _ = std::fs::remove_file(helpers::get_compiler_asset(
19-
package,
20-
&packages::Namespace::NoNamespace,
21-
source_file,
22-
"ast",
23-
));
29+
remove_ast_cache(package, source_file, "ast");
2430
}
2531

2632
fn remove_iast(package: &packages::Package, source_file: &Path) {
27-
let _ = std::fs::remove_file(helpers::get_compiler_asset(
28-
package,
29-
&packages::Namespace::NoNamespace,
30-
source_file,
31-
"iast",
32-
));
33+
remove_ast_cache(package, source_file, "iast");
3334
}
3435

3536
fn remove_mjs_file(source_file: &Path, suffix: &str) {

‎rewatch/src/build/parse.rs‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -383,10 +383,9 @@ fn generate_ast(
383383
}
384384
};
385385
if let Ok((ast_path, _)) = &result {
386-
let _ = std::fs::copy(
387-
Path::new(&build_path_abs).join(ast_path),
388-
package.get_ocaml_build_path().join(ast_path.file_name().unwrap()),
389-
);
386+
let cache_path = helpers::get_ocaml_ast_cache_path(&package, filename);
387+
helpers::create_path_for_path(cache_path.parent().expect("AST cache path should have a parent"));
388+
let _ = std::fs::copy(Path::new(&build_path_abs).join(ast_path), cache_path);
390389
}
391390
result
392391
}

‎rewatch/src/build/read_compile_state.rs‎

Lines changed: 52 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,17 @@ use std::fs;
77
use std::path::{Path, PathBuf};
88
use std::time::SystemTime;
99

10+
type CompileAsset = (
11+
PathBuf,
12+
SystemTime,
13+
String,
14+
String,
15+
PathBuf,
16+
bool,
17+
packages::Namespace,
18+
bool,
19+
);
20+
1021
pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result<CompileAssetsState> {
1122
let mut ast_modules: AHashMap<PathBuf, AstModule> = AHashMap::new();
1223
let mut cmi_modules: AHashMap<String, SystemTime> = AHashMap::new();
@@ -45,53 +56,12 @@ pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result<CompileAssets
4556
.packages
4657
.par_iter()
4758
.map(|(_, package)| {
48-
let read_dir = fs::read_dir(package.get_ocaml_build_path()).unwrap();
49-
read_dir
50-
.filter_map(|entry| match entry {
51-
Ok(entry) => {
52-
let path = entry.path();
53-
let extension = path.extension().and_then(|e| e.to_str());
54-
match extension {
55-
Some(ext) => match ext {
56-
"iast" | "ast" | "cmi" | "cmt" => Some((
57-
path.to_owned(),
58-
entry.metadata().unwrap().modified().unwrap(),
59-
ext.to_owned(),
60-
package.name.to_owned(),
61-
package.path.to_owned(),
62-
package.config.is_multi_entry_enabled(),
63-
package.namespace.to_owned(),
64-
package.is_root,
65-
)),
66-
_ => None,
67-
},
68-
None => None,
69-
}
70-
}
71-
Err(_) => None,
72-
})
73-
.collect::<Vec<(
74-
PathBuf,
75-
SystemTime,
76-
String,
77-
String,
78-
PathBuf,
79-
bool,
80-
packages::Namespace,
81-
bool,
82-
)>>()
59+
let mut package_assets = Vec::new();
60+
collect_compile_assets(package, &package.get_ocaml_build_path(), &mut package_assets);
61+
package_assets
8362
})
8463
.flatten()
85-
.collect::<Vec<(
86-
PathBuf,
87-
SystemTime,
88-
String,
89-
String,
90-
PathBuf,
91-
bool,
92-
packages::Namespace,
93-
bool,
94-
)>>();
64+
.collect::<Vec<CompileAsset>>();
9565

9666
let root_config = build_state.get_root_config();
9767

@@ -183,3 +153,40 @@ fn get_res_path_from_ast(ast_file: &Path) -> Option<PathBuf> {
183153
}
184154
None
185155
}
156+
157+
fn collect_compile_assets(package: &packages::Package, dir: &Path, assets: &mut Vec<CompileAsset>) {
158+
let Ok(read_dir) = fs::read_dir(dir) else {
159+
return;
160+
};
161+
162+
for entry in read_dir.flatten() {
163+
let path = entry.path();
164+
let Ok(metadata) = entry.metadata() else {
165+
continue;
166+
};
167+
168+
if metadata.is_dir() {
169+
collect_compile_assets(package, &path, assets);
170+
continue;
171+
}
172+
173+
let Some(extension) = path.extension().and_then(|extension| extension.to_str()) else {
174+
continue;
175+
};
176+
let extension = extension.to_owned();
177+
178+
match extension.as_str() {
179+
"iast" | "ast" | "cmi" | "cmt" => assets.push((
180+
path,
181+
metadata.modified().unwrap(),
182+
extension,
183+
package.name.to_owned(),
184+
package.path.to_owned(),
185+
package.config.is_multi_entry_enabled(),
186+
package.namespace.to_owned(),
187+
package.is_root,
188+
)),
189+
_ => {}
190+
}
191+
}
192+
}

‎rewatch/src/helpers.rs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,32 @@ pub fn get_ast_path(source_file: &Path) -> PathBuf {
430430
.join(format!("{basename}{extension}"))
431431
}
432432

433+
pub fn get_ocaml_ast_cache_path(package: &packages::Package, source_file: &Path) -> PathBuf {
434+
let source_file = source_file.strip_prefix(&package.path).unwrap_or(source_file);
435+
let ast_path = get_ast_path(source_file);
436+
get_ocaml_ast_cache_path_from_ast_path(package, &ast_path)
437+
}
438+
439+
pub fn get_ocaml_ast_cache_path_for_extension(
440+
package: &packages::Package,
441+
source_file: &Path,
442+
extension: &str,
443+
) -> PathBuf {
444+
let source_file = source_file.strip_prefix(&package.path).unwrap_or(source_file);
445+
let ast_path = get_ast_path(source_file).with_extension(extension);
446+
get_ocaml_ast_cache_path_from_ast_path(package, &ast_path)
447+
}
448+
449+
fn get_ocaml_ast_cache_path_from_ast_path(package: &packages::Package, ast_path: &Path) -> PathBuf {
450+
if package.config.is_multi_entry_enabled() {
451+
package.get_ocaml_build_path().join(ast_path)
452+
} else {
453+
package
454+
.get_ocaml_build_path()
455+
.join(ast_path.file_name().expect("AST path should have a file name"))
456+
}
457+
}
458+
433459
pub fn get_compiler_asset(
434460
package: &packages::Package,
435461
namespace: &packages::Namespace,

‎rewatch/tests/compile/10-duplicate-module-name.sh‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,27 @@ else
2626
rm -rf packages/main/src/lower-a packages/main/src/lower-b
2727
exit 1
2828
fi
29+
30+
lower_a_ast=packages/main/lib/ocaml/src/lower-a/lowercaseDuplicate.ast
31+
lower_b_ast=packages/main/lib/ocaml/src/lower-b/lowercaseDuplicate.ast
32+
if [ -f "$lower_a_ast" ] && [ -f "$lower_b_ast" ] && [ ! -f packages/main/lib/ocaml/lowercaseDuplicate.ast ];
33+
then
34+
success "Duplicate lowercase AST cache paths are unique"
35+
else
36+
error "Duplicate lowercase AST cache paths should preserve source directories"
37+
rm -rf packages/main/src/lower-a packages/main/src/lower-b
38+
exit 1
39+
fi
40+
2941
rm -rf packages/main/src/lower-a packages/main/src/lower-b
42+
rewatch build &> /dev/null
43+
if [ -f "$lower_a_ast" ] || [ -f "$lower_b_ast" ];
44+
then
45+
error "Removed lowercase file AST cache paths should be cleaned"
46+
exit 1
47+
else
48+
success "Removed lowercase file AST cache paths are cleaned"
49+
fi
3050

3151
mkdir -p packages/main/src/lower-private
3252
echo 'let value = 1' > packages/main/src/lower-private/lowerPrivate.res

0 commit comments

Comments
 (0)