From a3b5b2004cef3f48055b796c83ff43a23e759551 Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Fri, 31 Mar 2023 11:22:39 +0800 Subject: [PATCH 1/6] feat: add sub commands '-E, --external' for 'kclvm_cli run' to specify the external package path. --- kclvm/cmd/src/lib.rs | 1 + kclvm/cmd/src/settings.rs | 54 ++++++++++++ kclvm/config/src/settings.rs | 5 +- kclvm/parser/src/lib.rs | 19 ++-- kclvm/parser/src/tests.rs | 86 +++++++++++++++++++ .../import_by_external_assign.k | 3 + .../import_by_external_config_expr.k | 3 + .../import_by_external_nested_vendor.k | 3 + .../import_by_external_vendor_subpkg.k | 3 + kclvm/runner/src/runner.rs | 5 ++ 10 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 kclvm/parser/testdata_without_kclmod/import_by_external_assign.k create mode 100644 kclvm/parser/testdata_without_kclmod/import_by_external_config_expr.k create mode 100644 kclvm/parser/testdata_without_kclmod/import_by_external_nested_vendor.k create mode 100644 kclvm/parser/testdata_without_kclmod/import_by_external_vendor_subpkg.k diff --git a/kclvm/cmd/src/lib.rs b/kclvm/cmd/src/lib.rs index 7014c96d3..966042478 100644 --- a/kclvm/cmd/src/lib.rs +++ b/kclvm/cmd/src/lib.rs @@ -57,6 +57,7 @@ pub fn app() -> clap::App<'static> { (@arg path_selector: ... -S --path_selector "Specify the path selector") (@arg overrides: ... -O --overrides +takes_value "Specify the configuration override path and value") (@arg target: --target +takes_value "Specify the target type") + (@arg package_map: ... -E --external +takes_value "Mapping of package name and path where the package is located") ) (@subcommand lint => (@arg input: ... "Sets the input file to use") diff --git a/kclvm/cmd/src/settings.rs b/kclvm/cmd/src/settings.rs index d556e2d06..552659a5f 100644 --- a/kclvm/cmd/src/settings.rs +++ b/kclvm/cmd/src/settings.rs @@ -5,6 +5,7 @@ use kclvm_config::settings::{build_settings_pathbuf, Config, SettingsFile, Setti use kclvm_driver::arguments::parse_key_value_pair; use kclvm_error::Handler; use kclvm_runtime::PanicInfo; +use std::collections::HashMap; /// Build settings from arg matches. pub(crate) fn must_build_settings(matches: &ArgMatches) -> SettingsPathBuf { @@ -30,11 +31,14 @@ pub(crate) fn build_settings(matches: &ArgMatches) -> Result { Some(files) => files.into_iter().collect::>(), None => vec![], }; + let setting_files = matches .values_of("setting") .map(|files| files.into_iter().collect::>()); let arguments = strings_from_matches(matches, "arguments"); + let package_maps = hashmaps_from_matches(matches, "package_map"); + build_settings_pathbuf( files.as_slice(), setting_files, @@ -47,6 +51,7 @@ pub(crate) fn build_settings(matches: &ArgMatches) -> Result { disable_none: bool_from_matches(matches, "disable_none"), verbose: u32_from_matches(matches, "verbose"), debug: bool_from_matches(matches, "debug"), + package_maps, ..Default::default() }), kcl_options: if arguments.is_some() { @@ -63,3 +68,52 @@ pub(crate) fn build_settings(matches: &ArgMatches) -> Result { }), ) } + +#[inline] +fn hashmaps_from_matches(matches: &ArgMatches, key: &str) -> Option> { + matches.values_of(key).map(|files| { + files + .into_iter() + .map(|s| { + let parts: Vec<&str> = s.split('=').collect(); + if parts.len() == 2 { + let key = parts[0].trim(); + let value = parts[1].trim(); + (key.to_string(), value.to_string()) + } else { + (String::default(), String::default()) + } + }) + .collect::>() + }) +} + +#[inline] +fn strings_from_matches(matches: &ArgMatches, key: &str) -> Option> { + matches.values_of(key).map(|files| { + files + .into_iter() + .map(|v| v.to_string()) + .collect::>() + }) +} + +#[inline] +fn bool_from_matches(matches: &ArgMatches, key: &str) -> Option { + let occurrences = matches.occurrences_of(key); + if occurrences > 0 { + Some(true) + } else { + None + } +} + +#[inline] +fn u32_from_matches(matches: &ArgMatches, key: &str) -> Option { + let occurrences = matches.occurrences_of(key); + if occurrences > 0 { + Some(occurrences as u32) + } else { + None + } +} diff --git a/kclvm/config/src/settings.rs b/kclvm/config/src/settings.rs index 8fb16d9f0..086b6754b 100644 --- a/kclvm/config/src/settings.rs +++ b/kclvm/config/src/settings.rs @@ -1,7 +1,7 @@ // Copyright 2021 The KCL Authors. All rights reserved. use anyhow::Result; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::{collections::HashMap, path::PathBuf}; /// Default settings file `kcl.yaml` pub const DEFAULT_SETTING_FILE: &str = "kcl.yaml"; @@ -56,6 +56,7 @@ pub struct Config { pub disable_none: Option, pub verbose: Option, pub debug: Option, + pub package_maps: Option>, } impl SettingsFile { @@ -71,6 +72,7 @@ impl SettingsFile { disable_none: Some(false), verbose: Some(0), debug: Some(false), + package_maps: Some(HashMap::default()), }), kcl_options: Some(vec![]), } @@ -157,6 +159,7 @@ pub fn merge_settings(settings: &[SettingsFile]) -> SettingsFile { set_if!(result_kcl_cli_configs, disable_none, kcl_cli_configs); set_if!(result_kcl_cli_configs, verbose, kcl_cli_configs); set_if!(result_kcl_cli_configs, debug, kcl_cli_configs); + set_if!(result_kcl_cli_configs, package_maps, kcl_cli_configs); } } if let Some(kcl_options) = &setting.kcl_options { diff --git a/kclvm/parser/src/lib.rs b/kclvm/parser/src/lib.rs index 233f3cad8..25df7e127 100644 --- a/kclvm/parser/src/lib.rs +++ b/kclvm/parser/src/lib.rs @@ -182,6 +182,7 @@ pub struct LoadProgramOptions { pub work_dir: String, pub k_code_list: Vec, pub vendor_dirs: Vec, + pub package_maps: HashMap, pub cmd_args: Vec, pub cmd_overrides: Vec, @@ -199,6 +200,7 @@ impl Default for LoadProgramOptions { work_dir: Default::default(), k_code_list: Default::default(), vendor_dirs: vec![get_vendor_home()], + package_maps: Default::default(), cmd_args: Default::default(), cmd_overrides: Default::default(), mode: ParseMode::ParseComments, @@ -576,15 +578,16 @@ impl Loader { /// Look for [`pkgpath`] in the external package's home. /// If found, return to the [`pkgroot`]， else return [`None`] fn is_external_pkg(&self, pkgpath: &str) -> Result, String> { - let root_path = match self.pkg_exists(self.opts.vendor_dirs.clone(), pkgpath) { - Some(path) => path, - None => return Ok(None), - }; + let pkg_name = self.parse_external_pkg_name(pkgpath)?; - let pathbuf = PathBuf::from(root_path); - let rootpkg = pathbuf - .join(self.parse_external_pkg_name(pkgpath)?) - .join(KCL_MOD_FILE); + let rootpkg = if let Some(root) = self.opts.package_maps.get(&pkg_name) { + PathBuf::from(root).join(KCL_MOD_FILE) + } else { + match self.pkg_exists(self.opts.vendor_dirs.clone(), pkgpath) { + Some(path) => PathBuf::from(path).join(pkg_name).join(KCL_MOD_FILE), + None => return Ok(None), + } + }; if rootpkg.exists() { return Ok(Some( diff --git a/kclvm/parser/src/tests.rs b/kclvm/parser/src/tests.rs index 0048c640d..38cb0df43 100644 --- a/kclvm/parser/src/tests.rs +++ b/kclvm/parser/src/tests.rs @@ -329,3 +329,89 @@ fn test_import_vendor_without_kclmod_and_same_name() { } } } + +#[test] +fn test_import_vendor_by_external_arguments() { + let vendor = set_vendor_home(); + let sm = SourceMap::new(FilePathMapping::empty()); + let sess = Arc::new(ParseSession::with_source_map(Arc::new(sm))); + + let external_dir = &PathBuf::from(".") + .join("testdata") + .join("test_vendor") + .canonicalize() + .unwrap(); + + let test_cases = vec![ + ( + "import_by_external_assign.k", + "assign", + vec!["__main__", "assign"], + ), + ( + "import_by_external_config_expr.k", + "config_expr", + vec!["__main__", "config_expr"], + ), + ( + "import_by_external_nested_vendor.k", + "nested_vendor", + vec![ + "__main__", + "nested_vendor", + "vendor_subpkg", + "sub.sub2", + "sub.sub1", + "sub.sub", + "sub", + ], + ), + ( + "import_by_external_vendor_subpkg.k", + "vendor_subpkg", + vec![ + "__main__", + "vendor_subpkg", + "sub.sub1", + "sub.sub2", + "sub.sub", + "sub", + ], + ), + ]; + + let dir = &PathBuf::from(".") + .join("testdata_without_kclmod") + .canonicalize() + .unwrap(); + + test_cases + .into_iter() + .for_each(|(test_case_name, dep_name, pkgs)| { + let mut opts = LoadProgramOptions::default(); + opts.package_maps.insert( + dep_name.to_string(), + external_dir.join(dep_name).display().to_string(), + ); + let test_case_path = dir.join(test_case_name).display().to_string(); + let m = load_program(sess.clone(), &[&test_case_path], None).unwrap(); + + assert_eq!(m.pkgs.len(), pkgs.len()); + m.pkgs.into_iter().for_each(|(name, modules)| { + assert!(pkgs.contains(&name.as_str())); + for pkg in pkgs.clone() { + if name == pkg { + if name == "__main__" { + assert_eq!(modules.len(), 1); + assert_eq!(modules.get(0).unwrap().filename, test_case_path); + } else { + modules.into_iter().for_each(|module| { + assert!(module.filename.contains(&vendor)); + }); + } + break; + } + } + }); + }); +} diff --git a/kclvm/parser/testdata_without_kclmod/import_by_external_assign.k b/kclvm/parser/testdata_without_kclmod/import_by_external_assign.k new file mode 100644 index 000000000..26a35b488 --- /dev/null +++ b/kclvm/parser/testdata_without_kclmod/import_by_external_assign.k @@ -0,0 +1,3 @@ +import assign as a + +t1 = a.a \ No newline at end of file diff --git a/kclvm/parser/testdata_without_kclmod/import_by_external_config_expr.k b/kclvm/parser/testdata_without_kclmod/import_by_external_config_expr.k new file mode 100644 index 000000000..66805c186 --- /dev/null +++ b/kclvm/parser/testdata_without_kclmod/import_by_external_config_expr.k @@ -0,0 +1,3 @@ +import config_expr as ce + +t = ce.config \ No newline at end of file diff --git a/kclvm/parser/testdata_without_kclmod/import_by_external_nested_vendor.k b/kclvm/parser/testdata_without_kclmod/import_by_external_nested_vendor.k new file mode 100644 index 000000000..94df0aed1 --- /dev/null +++ b/kclvm/parser/testdata_without_kclmod/import_by_external_nested_vendor.k @@ -0,0 +1,3 @@ +import nested_vendor as nv + +t = sv.ub_in_subpkg \ No newline at end of file diff --git a/kclvm/parser/testdata_without_kclmod/import_by_external_vendor_subpkg.k b/kclvm/parser/testdata_without_kclmod/import_by_external_vendor_subpkg.k new file mode 100644 index 000000000..3d1a29b76 --- /dev/null +++ b/kclvm/parser/testdata_without_kclmod/import_by_external_vendor_subpkg.k @@ -0,0 +1,3 @@ +import vendor_subpkg as vs + +t1 = vs.sub_in_subpkg \ No newline at end of file diff --git a/kclvm/runner/src/runner.rs b/kclvm/runner/src/runner.rs index c07cde4c0..2c4e85365 100644 --- a/kclvm/runner/src/runner.rs +++ b/kclvm/runner/src/runner.rs @@ -1,3 +1,5 @@ +use std::collections::HashMap; + use kclvm_ast::ast; use kclvm_config::{ modfile::get_vendor_home, @@ -23,6 +25,7 @@ pub type kclvm_value_ref_t = std::ffi::c_void; pub struct ExecProgramArgs { pub work_dir: Option, pub k_filename_list: Vec, + pub package_maps: HashMap, pub k_code_list: Vec, // -D key=value pub args: Vec, @@ -80,6 +83,7 @@ impl ExecProgramArgs { kclvm_parser::LoadProgramOptions { work_dir: self.work_dir.clone().unwrap_or_default(), vendor_dirs: vec![get_vendor_home()], + package_maps: self.package_maps.clone(), k_code_list: self.k_code_list.clone(), cmd_args: self.args.clone(), cmd_overrides: self.overrides.clone(), @@ -106,6 +110,7 @@ impl TryFrom for ExecProgramArgs { args.overrides.push(parse_override_spec(override_str)?); } args.path_selector = cli_configs.path_selector.unwrap_or_default(); + args.package_maps = cli_configs.package_maps.unwrap_or(HashMap::default()) } if let Some(options) = settings.kcl_options { args.args = options From 73c4406f7ae5473e7e75cc424be6f4329cd93e65 Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Fri, 7 Apr 2023 14:03:09 +0800 Subject: [PATCH 2/6] feat: add some comments --- kclvm/config/src/settings.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kclvm/config/src/settings.rs b/kclvm/config/src/settings.rs index 086b6754b..ce0299be9 100644 --- a/kclvm/config/src/settings.rs +++ b/kclvm/config/src/settings.rs @@ -56,6 +56,8 @@ pub struct Config { pub disable_none: Option, pub verbose: Option, pub debug: Option, + // kclvm needs a mapping between the package name and the package path + // to determine the source code path corresponding to different version package. pub package_maps: Option>, } From 9a6940290d10f479a806f34598ce2cce4824357e Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Fri, 7 Apr 2023 14:19:27 +0800 Subject: [PATCH 3/6] fix: make fmt --- kclvm/cmd/src/settings.rs | 49 ------------------------------------ kclvm/cmd/src/util.rs | 23 +++++++++++++++++ kclvm/config/src/settings.rs | 4 +-- 3 files changed, 25 insertions(+), 51 deletions(-) diff --git a/kclvm/cmd/src/settings.rs b/kclvm/cmd/src/settings.rs index 552659a5f..1d596afc5 100644 --- a/kclvm/cmd/src/settings.rs +++ b/kclvm/cmd/src/settings.rs @@ -68,52 +68,3 @@ pub(crate) fn build_settings(matches: &ArgMatches) -> Result { }), ) } - -#[inline] -fn hashmaps_from_matches(matches: &ArgMatches, key: &str) -> Option> { - matches.values_of(key).map(|files| { - files - .into_iter() - .map(|s| { - let parts: Vec<&str> = s.split('=').collect(); - if parts.len() == 2 { - let key = parts[0].trim(); - let value = parts[1].trim(); - (key.to_string(), value.to_string()) - } else { - (String::default(), String::default()) - } - }) - .collect::>() - }) -} - -#[inline] -fn strings_from_matches(matches: &ArgMatches, key: &str) -> Option> { - matches.values_of(key).map(|files| { - files - .into_iter() - .map(|v| v.to_string()) - .collect::>() - }) -} - -#[inline] -fn bool_from_matches(matches: &ArgMatches, key: &str) -> Option { - let occurrences = matches.occurrences_of(key); - if occurrences > 0 { - Some(true) - } else { - None - } -} - -#[inline] -fn u32_from_matches(matches: &ArgMatches, key: &str) -> Option { - let occurrences = matches.occurrences_of(key); - if occurrences > 0 { - Some(occurrences as u32) - } else { - None - } -} diff --git a/kclvm/cmd/src/util.rs b/kclvm/cmd/src/util.rs index 659924260..54d69c8c5 100644 --- a/kclvm/cmd/src/util.rs +++ b/kclvm/cmd/src/util.rs @@ -1,4 +1,5 @@ use clap::ArgMatches; +use std::collections::HashMap; #[inline] pub(crate) fn strings_from_matches(matches: &ArgMatches, key: &str) -> Option> { @@ -10,6 +11,28 @@ pub(crate) fn strings_from_matches(matches: &ArgMatches, key: &str) -> Option Option> { + matches.values_of(key).map(|files| { + files + .into_iter() + .map(|s| { + let parts: Vec<&str> = s.split('=').collect(); + if parts.len() == 2 { + let key = parts[0].trim(); + let value = parts[1].trim(); + (key.to_string(), value.to_string()) + } else { + (String::default(), String::default()) + } + }) + .collect::>() + }) +} + #[inline] pub(crate) fn string_from_matches(matches: &ArgMatches, key: &str) -> Option { matches.value_of(key).map(|v| v.to_string()) diff --git a/kclvm/config/src/settings.rs b/kclvm/config/src/settings.rs index ce0299be9..c4b7d8e3c 100644 --- a/kclvm/config/src/settings.rs +++ b/kclvm/config/src/settings.rs @@ -56,8 +56,8 @@ pub struct Config { pub disable_none: Option, pub verbose: Option, pub debug: Option, - // kclvm needs a mapping between the package name and the package path - // to determine the source code path corresponding to different version package. + // kclvm needs a mapping between the package name and the package path + // to determine the source code path corresponding to different version package. pub package_maps: Option>, } From 591fcf3544458550a26f29b6ed7d655d1b117a1e Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Fri, 7 Apr 2023 18:01:32 +0800 Subject: [PATCH 4/6] feat: add invalid arguments check --- kclvm/cmd/src/settings.rs | 2 +- kclvm/cmd/src/util.rs | 19 +++++++++---------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/kclvm/cmd/src/settings.rs b/kclvm/cmd/src/settings.rs index 1d596afc5..3b9b406bc 100644 --- a/kclvm/cmd/src/settings.rs +++ b/kclvm/cmd/src/settings.rs @@ -37,7 +37,7 @@ pub(crate) fn build_settings(matches: &ArgMatches) -> Result { .map(|files| files.into_iter().collect::>()); let arguments = strings_from_matches(matches, "arguments"); - let package_maps = hashmaps_from_matches(matches, "package_map"); + let package_maps = hashmaps_from_matches(matches, "package_map").transpose()?; build_settings_pathbuf( files.as_slice(), diff --git a/kclvm/cmd/src/util.rs b/kclvm/cmd/src/util.rs index 54d69c8c5..20f3a33ba 100644 --- a/kclvm/cmd/src/util.rs +++ b/kclvm/cmd/src/util.rs @@ -1,4 +1,7 @@ +use anyhow::bail; +use anyhow::Result; use clap::ArgMatches; +use kclvm_driver::arguments::parse_key_value_pair; use std::collections::HashMap; #[inline] @@ -15,21 +18,17 @@ pub(crate) fn strings_from_matches(matches: &ArgMatches, key: &str) -> Option Option> { +) -> Option>> { matches.values_of(key).map(|files| { files .into_iter() - .map(|s| { - let parts: Vec<&str> = s.split('=').collect(); - if parts.len() == 2 { - let key = parts[0].trim(); - let value = parts[1].trim(); - (key.to_string(), value.to_string()) - } else { - (String::default(), String::default()) + .map(|s| match parse_key_value_pair(s) { + Ok(pair) => Ok((pair.key, pair.value)), + Err(err) => { + bail!("Invalid arguments format '-E, --external', use'kclvm_cli run --help' for more help.") } }) - .collect::>() + .collect::>>() }) } From 6726cb693d41ecd2a1c430705df8f7fc0509111d Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Mon, 10 Apr 2023 15:33:02 +0800 Subject: [PATCH 5/6] fix: add #[serde_skip] for package_maps. --- kclvm/runner/src/runner.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/kclvm/runner/src/runner.rs b/kclvm/runner/src/runner.rs index 2c4e85365..67334f77d 100644 --- a/kclvm/runner/src/runner.rs +++ b/kclvm/runner/src/runner.rs @@ -25,6 +25,8 @@ pub type kclvm_value_ref_t = std::ffi::c_void; pub struct ExecProgramArgs { pub work_dir: Option, pub k_filename_list: Vec, + // -E key=value + #[serde(skip)] pub package_maps: HashMap, pub k_code_list: Vec, // -D key=value From 139032711dc3ffee233260620e071513d2fc3ba2 Mon Sep 17 00:00:00 2001 From: zong-zhe Date: Mon, 10 Apr 2023 16:36:40 +0800 Subject: [PATCH 6/6] fix: fix test cases on windows --- kclvm/parser/src/tests.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kclvm/parser/src/tests.rs b/kclvm/parser/src/tests.rs index 38cb0df43..7b2143a7f 100644 --- a/kclvm/parser/src/tests.rs +++ b/kclvm/parser/src/tests.rs @@ -143,6 +143,8 @@ fn set_vendor_home() -> String { /// The testing will set environment variables, /// so can not to execute test cases concurrently. fn test_in_order() { + test_import_vendor_by_external_arguments(); + println!("{:?} PASS", "test_import_vendor_by_external_arguments"); test_import_vendor_without_vendor_home(); println!("{:?} PASS", "test_import_vendor_without_vendor_home"); test_import_vendor_without_kclmod(); @@ -330,7 +332,6 @@ fn test_import_vendor_without_kclmod_and_same_name() { } } -#[test] fn test_import_vendor_by_external_arguments() { let vendor = set_vendor_home(); let sm = SourceMap::new(FilePathMapping::empty());