From 64bd05d2d9adddeee1ee6049b805a33204106458 Mon Sep 17 00:00:00 2001 From: he1pa <18012015693@163.com> Date: Thu, 12 Sep 2024 20:20:20 +0800 Subject: [PATCH 1/3] fix: fix ${pkg_name:KCL_MOD} in entry Signed-off-by: he1pa <18012015693@163.com> --- kclvm/driver/src/lib.rs | 21 +++--- kclvm/parser/src/entry.rs | 72 +++++++++++++++++-- kclvm/runner/src/lib.rs | 7 +- kclvm/tools/src/LSP/src/compile.rs | 18 ++--- kclvm/tools/src/LSP/src/state.rs | 5 +- .../workspace/pkg_mod_test/base/base.k | 0 .../test_data/workspace/pkg_mod_test/kcl.mod | 2 + .../test_data/workspace/pkg_mod_test/pkg1/a.k | 3 + .../test_data/workspace/pkg_mod_test/pkg2/b.k | 0 .../workspace/pkg_mod_test/test/kcl.mod | 8 +++ .../workspace/pkg_mod_test/test/main.k | 3 + kclvm/tools/src/LSP/src/tests.rs | 12 ++-- 12 files changed, 115 insertions(+), 36 deletions(-) create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/base/base.k create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/kcl.mod create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg1/a.k create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg2/b.k create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/kcl.mod create mode 100644 kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/main.k diff --git a/kclvm/driver/src/lib.rs b/kclvm/driver/src/lib.rs index 91d43374d..1fa2f08aa 100644 --- a/kclvm/driver/src/lib.rs +++ b/kclvm/driver/src/lib.rs @@ -183,15 +183,13 @@ pub fn lookup_compile_workspace( work_dir: work_dir.clone(), ..Default::default() }; - match canonicalize_input_files(&files, work_dir, true) { - Ok(kcl_paths) => { - // 1. find the kcl.mod path - let metadata = - fill_pkg_maps_for_k_file(tool, file.into(), &mut load_opt) - .unwrap_or(None); - (kcl_paths, Some(load_opt), metadata) - } - Err(_) => default_res, + + let metadata = + fill_pkg_maps_for_k_file(tool, file.into(), &mut load_opt).unwrap_or(None); + if files.is_empty() { + default_res + } else { + (files, Some(load_opt), metadata) } } Err(_) => default_res, @@ -205,10 +203,7 @@ pub fn lookup_compile_workspace( if let Some(files) = mod_file.get_entries() { let work_dir = dir.to_string_lossy().to_string(); load_opt.work_dir = work_dir.clone(); - match canonicalize_input_files(&files, work_dir, true) { - Ok(kcl_paths) => (kcl_paths, Some(load_opt), metadata), - Err(_) => default_res, - } + (files, Some(load_opt), metadata) } else { default_res } diff --git a/kclvm/parser/src/entry.rs b/kclvm/parser/src/entry.rs index 6cec6f9f3..90495daf6 100644 --- a/kclvm/parser/src/entry.rs +++ b/kclvm/parser/src/entry.rs @@ -297,9 +297,10 @@ pub fn get_compile_entries_from_paths( } let mut result = Entries::default(); let mut k_code_queue = VecDeque::from(opts.k_code_list.clone()); - for s in file_paths { - let path = ModRelativePath::from(s.to_string()); + for file in file_paths { + let abs_path = abs_path(file, &opts.work_dir); + let path = ModRelativePath::from(abs_path.to_string()); // If the path is a [`ModRelativePath`] with prefix '${:KCL_MOD}', // calculate the real path and the package name. if let Some((pkg_name, pkg_path)) = path.get_root_pkg_name()?.and_then(|name| { @@ -325,11 +326,11 @@ pub fn get_compile_entries_from_paths( entry.push_k_code(k_code_queue.pop_front()); result.push_entry(entry); continue; - } else if let Some(root) = get_pkg_root(s) { + } else if let Some(root) = get_pkg_root(&abs_path) { // If the path is a normal path. let mut entry: Entry = Entry::new(kclvm_ast::MAIN_PKG.to_string(), root.clone()); entry.extend_k_files_and_codes( - get_main_files_from_pkg_path(s, &root, kclvm_ast::MAIN_PKG, opts)?, + get_main_files_from_pkg_path(&abs_path, &root, kclvm_ast::MAIN_PKG, opts)?, &mut k_code_queue, ); result.push_entry(entry); @@ -388,10 +389,71 @@ pub fn get_compile_entries_from_paths( } Ok(()) })?; - Ok(result) } +pub fn abs_path(file: &String, work_dir: &String) -> String { + let path = std::path::Path::new(file); + let is_absolute = path.is_absolute(); + // If the input file or path is a relative path and it is not a absolute path in the KCL module VFS, + // join with the work directory path and convert it to a absolute path. + let path = ModRelativePath::from(file.to_string()); + let abs_path = if !is_absolute + && !path + .is_relative_path() + .map_err(|err| err.to_string()) + .unwrap() + { + let filepath = std::path::Path::new(work_dir).join(file); + match filepath.canonicalize() { + Ok(path) => Some(path.adjust_canonicalization()), + Err(_) => Some(filepath.to_string_lossy().to_string()), + } + } else { + None + }; + abs_path.unwrap_or(file.clone()) +} + +/// fix relative path, ${:KCL_MOD} and ${KCL_MOD} to absolute path +pub fn fix_path(file_paths: &[String], opts: &LoadProgramOptions) -> Result> { + let mut fixed_files = vec![]; + let abs_paths: Vec = file_paths + .iter() + .map(|f| abs_path(f, &opts.work_dir)) + .collect(); + let root = + match kclvm_config::modfile::get_pkg_root_from_paths(&abs_paths, opts.work_dir.clone()) { + Ok(root) => root, + Err(e) => { + return Err(anyhow::anyhow!(format!("Get pkg root failed: {:?}", e))); + } + }; + + for file in abs_paths { + let path = ModRelativePath::from(file.clone()); + // If the path is a [`ModRelativePath`] with prefix '${:KCL_MOD}', + // calculate the real path and the package name. + if let Some((_, pkg_path)) = path.get_root_pkg_name()?.and_then(|name| { + opts.package_maps + .get(&name) + .map(|pkg_path: &String| (name, pkg_path)) + }) { + // Replace the mod relative path prefix '${:KCL_MOD}' with the real path. + let s = path.canonicalize_by_root_path(pkg_path)?; + fixed_files.push(s); + } else if path.is_relative_path()? && path.get_root_pkg_name()?.is_none() { + // Replace the mod relative path prefix '${KCL_MOD}' with the real path. + let s = path.canonicalize_by_root_path(&root)?; + fixed_files.push(s); + } else { + fixed_files.push(file); + } + } + + Ok(fixed_files) +} + /// Get files in the main package with the package root. fn get_main_files_from_pkg_path( pkg_path: &str, diff --git a/kclvm/runner/src/lib.rs b/kclvm/runner/src/lib.rs index dde68dcc3..67de31984 100644 --- a/kclvm/runner/src/lib.rs +++ b/kclvm/runner/src/lib.rs @@ -7,7 +7,7 @@ use kclvm_ast::{ MAIN_PKG, }; use kclvm_config::cache::KCL_CACHE_PATH_ENV_VAR; -use kclvm_driver::{canonicalize_input_files, expand_input_files}; +use kclvm_driver::expand_input_files; use kclvm_parser::{load_program, KCLModuleCache, ParseSessionRef}; use kclvm_query::apply_overrides; use kclvm_sema::resolver::{ @@ -350,11 +350,8 @@ fn build>( /// Expand and return the normalized file paths for the input file list. pub fn expand_files(args: &ExecProgramArgs) -> Result> { let k_files = &args.k_filename_list; - let work_dir = args.work_dir.clone().unwrap_or_default(); let k_files = expand_input_files(k_files); - let kcl_paths = - canonicalize_input_files(&k_files, work_dir, false).map_err(|err| anyhow!(err))?; - Ok(kcl_paths) + Ok(k_files) } /// Clean all the tmp files generated during lib generating and linking. diff --git a/kclvm/tools/src/LSP/src/compile.rs b/kclvm/tools/src/LSP/src/compile.rs index 18764dc84..70bc2c529 100644 --- a/kclvm/tools/src/LSP/src/compile.rs +++ b/kclvm/tools/src/LSP/src/compile.rs @@ -5,8 +5,7 @@ use kclvm_ast::ast::Program; use kclvm_driver::{lookup_compile_workspace, toolchain}; use kclvm_error::Diagnostic; use kclvm_parser::{ - entry::get_normalized_k_files_from_paths, load_program, KCLModuleCache, LoadProgramOptions, - ParseSessionRef, + entry::fix_path, load_program, KCLModuleCache, LoadProgramOptions, ParseSessionRef, }; use kclvm_sema::{ advanced_resolver::AdvancedResolver, @@ -36,20 +35,22 @@ pub fn compile( // Ignore the kcl plugin sematic check. let mut opts = opts.unwrap_or_default(); opts.load_plugins = true; - // Get input files - let files = match get_normalized_k_files_from_paths(files, &opts) { - Ok(file_list) => file_list, + + let fixed_paths = match fix_path(files, &opts) { + Ok(fixed_paths) => fixed_paths, Err(e) => { return ( IndexSet::new(), - Err(anyhow::anyhow!("Compile failed: {:?}", e)), + Err(anyhow::anyhow!("Fix paths error: {:?}", e)), ) } }; - let files: Vec<&str> = files.iter().map(|s| s.as_str()).collect(); + + let fixed_paths: Vec<&str> = fixed_paths.iter().map(|s| s.as_str()).collect(); + // Update opt.k_code_list if let Some(vfs) = ¶ms.vfs { - let mut k_code_list = match load_files_code_from_vfs(&files, vfs) { + let mut k_code_list = match load_files_code_from_vfs(&fixed_paths, vfs) { Ok(code_list) => code_list, Err(e) => { return ( @@ -80,6 +81,7 @@ pub fn compile( } } + let files: Vec<&str> = files.iter().map(|s| s.as_str()).collect(); // Parser let sess = ParseSessionRef::default(); let mut program = match load_program(sess.clone(), &files, Some(opts), params.module_cache) { diff --git a/kclvm/tools/src/LSP/src/state.rs b/kclvm/tools/src/LSP/src/state.rs index 4ee297c90..fa1dd834f 100644 --- a/kclvm/tools/src/LSP/src/state.rs +++ b/kclvm/tools/src/LSP/src/state.rs @@ -264,10 +264,13 @@ impl LanguageServerState { // If all workspaces do not contain the current file, get files workspace and store in temporary_workspace if !may_contain { + self.log_message(format!( + "Not contains in any workspace, compile: {:?}", + filename + )); let tool = Arc::clone(&self.tool); let (workspaces, failed) = lookup_compile_workspaces(&*tool.read(), &filename, true); - if workspaces.is_empty() { self.temporary_workspace.write().remove(&file.file_id); self.log_message(format!( diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/base/base.k b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/base/base.k new file mode 100644 index 000000000..e69de29bb diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/kcl.mod b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/kcl.mod new file mode 100644 index 000000000..83741247b --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/kcl.mod @@ -0,0 +1,2 @@ +[package] +name = "pkg_mod_test" \ No newline at end of file diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg1/a.k b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg1/a.k new file mode 100644 index 000000000..26dee57bd --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg1/a.k @@ -0,0 +1,3 @@ +import pkg2 +schema Name: + name: str \ No newline at end of file diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg2/b.k b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/pkg2/b.k new file mode 100644 index 000000000..e69de29bb diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/kcl.mod b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/kcl.mod new file mode 100644 index 000000000..37fddc5ec --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/kcl.mod @@ -0,0 +1,8 @@ +[package] +name = "pkg_mod" + +[dependencies] +pkg_mod_test = { path = "../../pkg_mod_test" } + +[profile] +entries = ["../base/base.k", "main.k", "${pkg_mod_test:KCL_MOD}/pkg1/a.k"] \ No newline at end of file diff --git a/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/main.k b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/main.k new file mode 100644 index 000000000..968858470 --- /dev/null +++ b/kclvm/tools/src/LSP/src/test_data/workspace/pkg_mod_test/test/main.k @@ -0,0 +1,3 @@ +import pkg_mod_test + +import pkg_mod_test.pkg2 diff --git a/kclvm/tools/src/LSP/src/tests.rs b/kclvm/tools/src/LSP/src/tests.rs index 6a91ca916..4ca3cbb89 100644 --- a/kclvm/tools/src/LSP/src/tests.rs +++ b/kclvm/tools/src/LSP/src/tests.rs @@ -2258,10 +2258,7 @@ fn kcl_workspace_init_kclsettings_test() { )); assert_eq!(expected, workspaces.keys().cloned().collect()); - assert_eq!( - vec![a.to_str().unwrap().to_string(),], - workspaces.values().next().unwrap().0 - ); + assert_eq!(vec!["a.k",], workspaces.values().next().unwrap().0); assert!(failed.is_none()); } @@ -2394,3 +2391,10 @@ fn init_workspace_sema_token_test() { let res = server.send_and_receive(r); assert!(res.result.is_some()); } + +#[test] +fn pkg_mod_test() { + let (_file, _program, diags, _gs) = + compile_test_file("src/test_data/workspace/pkg_mod_test/test/main.k"); + assert_eq!(diags.iter().filter(|diag| diag.is_error()).count(), 0); +} From 40e10276abb5ea8b8ced3aeff4e354aaea3295e0 Mon Sep 17 00:00:00 2001 From: he1pa <18012015693@163.com> Date: Thu, 12 Sep 2024 20:32:40 +0800 Subject: [PATCH 2/3] remove fn canonicalize_input_files() Signed-off-by: he1pa <18012015693@163.com> --- kclvm/api/src/service/service_impl.rs | 13 +---- kclvm/driver/src/lib.rs | 74 +-------------------------- kclvm/driver/src/tests.rs | 17 +----- 3 files changed, 3 insertions(+), 101 deletions(-) diff --git a/kclvm/api/src/service/service_impl.rs b/kclvm/api/src/service/service_impl.rs index 387173d32..c3c45b85a 100644 --- a/kclvm/api/src/service/service_impl.rs +++ b/kclvm/api/src/service/service_impl.rs @@ -5,10 +5,8 @@ use std::string::String; use crate::gpyrpc::*; -use anyhow::anyhow; use kcl_language_server::rename; use kclvm_config::settings::build_settings_pathbuf; -use kclvm_driver::canonicalize_input_files; use kclvm_loader::option::list_options; use kclvm_loader::{load_packages_with_cache, LoadPackageOptions}; use kclvm_parser::load_program; @@ -860,16 +858,7 @@ impl KclvmServiceImpl { ) -> anyhow::Result { let settings_files = args.files.iter().map(|f| f.as_str()).collect::>(); let settings_pathbuf = build_settings_pathbuf(&[], Some(settings_files), None)?; - let files = if !settings_pathbuf.settings().input().is_empty() { - canonicalize_input_files( - &settings_pathbuf.settings().input(), - args.work_dir.clone(), - false, - ) - .map_err(|e| anyhow!(e))? - } else { - vec![] - }; + let files = settings_pathbuf.settings().input(); Ok(settings_pathbuf .settings() .clone() diff --git a/kclvm/driver/src/lib.rs b/kclvm/driver/src/lib.rs index 1fa2f08aa..2eaba78ee 100644 --- a/kclvm/driver/src/lib.rs +++ b/kclvm/driver/src/lib.rs @@ -11,9 +11,8 @@ use glob::glob; use kclvm_config::{ modfile::{ get_pkg_root, load_mod_file, KCL_FILE_EXTENSION, KCL_FILE_SUFFIX, KCL_MOD_FILE, - KCL_MOD_PATH_ENV, KCL_WORK_FILE, + KCL_WORK_FILE, }, - path::ModRelativePath, settings::{build_settings_pathbuf, DEFAULT_SETTING_FILE}, workfile::load_work_file, }; @@ -59,77 +58,6 @@ pub fn expand_input_files(k_files: &[String]) -> Vec { res } -/// Normalize input files with the working directory and replace ${KCL_MOD} with the module root path. -pub fn canonicalize_input_files( - k_files: &[String], - work_dir: String, - check_exist: bool, -) -> Result, String> { - let mut kcl_paths = Vec::::new(); - // The first traversal changes the relative path to an absolute path - for file in k_files.iter() { - let path = Path::new(file); - - let is_absolute = path.is_absolute(); - let is_exist_maybe_symlink = path.exists(); - // If the input file or path is a relative path and it is not a absolute path in the KCL module VFS, - // join with the work directory path and convert it to a absolute path. - let path = ModRelativePath::from(file.to_string()); - let abs_path = if !is_absolute && !path.is_relative_path().map_err(|err| err.to_string())? { - let filepath = Path::new(&work_dir).join(file); - match filepath.canonicalize() { - Ok(path) => Some(path.adjust_canonicalization()), - Err(_) => { - if check_exist { - return Err(format!( - "Cannot find the kcl file, please check the file path {}", - file - )); - } - Some(filepath.to_string_lossy().to_string()) - } - } - } else { - None - }; - // If the input file or path is a symlink, convert it to a real path. - let real_path = if is_exist_maybe_symlink { - match PathBuf::from(file.to_string()).canonicalize() { - Ok(real_path) => Some(String::from(real_path.to_str().unwrap())), - Err(_) => { - if check_exist { - return Err(format!( - "Cannot find the kcl file, please check the file path {}", - file - )); - } - Some(file.to_string()) - } - } - } else { - None - }; - - kcl_paths.push(abs_path.unwrap_or(real_path.unwrap_or(file.to_string()))); - } - - // Get the root path of the project - let pkgroot = kclvm_config::modfile::get_pkg_root_from_paths(&kcl_paths, work_dir)?; - - // The second traversal replaces ${KCL_MOD} with the project root path - kcl_paths = kcl_paths - .iter() - .map(|file| { - if file.contains(KCL_MOD_PATH_ENV) { - file.replace(KCL_MOD_PATH_ENV, pkgroot.as_str()) - } else { - file.clone() - } - }) - .collect(); - Ok(kcl_paths) -} - /// Get compile workspace(files and options) from a single file input. /// 1. Lookup entry files in kcl.yaml /// 2. Lookup entry files in kcl.mod diff --git a/kclvm/driver/src/tests.rs b/kclvm/driver/src/tests.rs index 07844ad8b..632f7c5ee 100644 --- a/kclvm/driver/src/tests.rs +++ b/kclvm/driver/src/tests.rs @@ -9,24 +9,9 @@ use walkdir::WalkDir; use crate::arguments::parse_key_value_pair; use crate::toolchain::Toolchain; use crate::toolchain::{fill_pkg_maps_for_k_file, CommandToolchain, NativeToolchain}; -use crate::{canonicalize_input_files, expand_input_files, get_pkg_list}; +use crate::{expand_input_files, get_pkg_list}; use crate::{lookup_the_nearest_file_dir, toolchain}; -#[test] -fn test_canonicalize_input_files() { - let input_files = vec!["file1.k".to_string(), "file2.k".to_string()]; - let work_dir = ".".to_string(); - let expected_files = vec![ - Path::new(".").join("file1.k").to_string_lossy().to_string(), - Path::new(".").join("file2.k").to_string_lossy().to_string(), - ]; - assert_eq!( - canonicalize_input_files(&input_files, work_dir.clone(), false).unwrap(), - expected_files - ); - assert!(canonicalize_input_files(&input_files, work_dir, true).is_err()); -} - #[test] fn test_expand_input_files_with_kcl_mod() { let path = PathBuf::from("src/test_data/expand_file_pattern"); From 76c4b0e047b1ce89f6ff7f064663b0b43dde1175 Mon Sep 17 00:00:00 2001 From: he1pa <18012015693@163.com> Date: Thu, 12 Sep 2024 20:55:28 +0800 Subject: [PATCH 3/3] remove unwrap() Signed-off-by: he1pa <18012015693@163.com> --- kclvm/parser/src/entry.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/kclvm/parser/src/entry.rs b/kclvm/parser/src/entry.rs index 90495daf6..9f478fb38 100644 --- a/kclvm/parser/src/entry.rs +++ b/kclvm/parser/src/entry.rs @@ -398,12 +398,12 @@ pub fn abs_path(file: &String, work_dir: &String) -> String { // If the input file or path is a relative path and it is not a absolute path in the KCL module VFS, // join with the work directory path and convert it to a absolute path. let path = ModRelativePath::from(file.to_string()); - let abs_path = if !is_absolute - && !path - .is_relative_path() - .map_err(|err| err.to_string()) - .unwrap() - { + let is_relative_path = match path.is_relative_path() { + Ok(is_relative_path) => is_relative_path, + _ => return file.clone(), + }; + + let abs_path = if !is_absolute && !is_relative_path { let filepath = std::path::Path::new(work_dir).join(file); match filepath.canonicalize() { Ok(path) => Some(path.adjust_canonicalization()),