Skip to content
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

fix: When checking for changes to the cache, check for external packages. #574

Merged
merged 3 commits into from
Jun 15, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 2 additions & 0 deletions kclvm/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions kclvm/cmd/src/test_data/cache/main/kcl.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "main"
edition = "0.0.1"
version = "0.0.1"

[dependencies]
5 changes: 5 additions & 0 deletions kclvm/cmd/src/test_data/cache/main/main.k
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import kcl1
The_first_kcl_program = kcl1.kcl1
kcl1_schema = kcl1.Kcl1 {
name = "kcl1",
}
2 changes: 2 additions & 0 deletions kclvm/cmd/src/test_data/cache/main/main.k.v1
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import kcl1
The_first_kcl_program = kcl1.kcl1
5 changes: 5 additions & 0 deletions kclvm/cmd/src/test_data/cache/main/main.k.v2
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import kcl1
The_first_kcl_program = kcl1.kcl1
kcl1_schema = kcl1.Kcl1 {
name = "kcl1",
}
6 changes: 6 additions & 0 deletions kclvm/cmd/src/test_data/cache/v1/kcl1/kcl.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "kcl1"
edition = "0.0.1"
version = "0.0.1"

[dependencies]
1 change: 1 addition & 0 deletions kclvm/cmd/src/test_data/cache/v1/kcl1/main.k
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
kcl1 = 1
6 changes: 6 additions & 0 deletions kclvm/cmd/src/test_data/cache/v2/kcl1/kcl.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "kcl1"
edition = "0.0.1"
version = "0.0.1"

[dependencies]
4 changes: 4 additions & 0 deletions kclvm/cmd/src/test_data/cache/v2/kcl1/main.k
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
schema Kcl1:
name: str

kcl1 = 1
45 changes: 45 additions & 0 deletions kclvm/cmd/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,51 @@ fn test_run_command_with_konfig() {
}
}

#[test]
fn test_load_cache_with_different_pkg() {
let main_path = PathBuf::from("./src/test_data/cache/main/main.k");
let main_v1_path = PathBuf::from("./src/test_data/cache/main/main.k.v1");
let main_v2_path = PathBuf::from("./src/test_data/cache/main/main.k.v2");
let kcl1_v1_path = PathBuf::from("./src/test_data/cache/v1/kcl1");
let kcl1_v2_path = PathBuf::from("./src/test_data/cache/v2/kcl1");

// Copy the content from main.k.v1 to main.k
fs::copy(main_v1_path, &main_path).unwrap();
let matches = app().get_matches_from(&[
ROOT_CMD,
"run",
main_path.to_str().unwrap(),
"-E",
format!("kcl1={}", kcl1_v1_path.display()).as_str(),
]);

let matches = matches.subcommand_matches("run").unwrap();
let mut buf = Vec::new();
run_command(matches, &mut buf).unwrap();
assert_eq!(
String::from_utf8(buf).unwrap(),
"The_first_kcl_program: 1\n"
);

// Copy the content from main.k.v2 to main.k
fs::copy(main_v2_path, &main_path).unwrap();
let matches = app().get_matches_from(&[
ROOT_CMD,
"run",
main_path.to_str().unwrap(),
"-E",
format!("kcl1={}", kcl1_v2_path.display()).as_str(),
]);

let matches = matches.subcommand_matches("run").unwrap();
let mut buf = Vec::new();
run_command(matches, &mut buf).unwrap();
assert_eq!(
String::from_utf8(buf).unwrap(),
"The_first_kcl_program: 1\nkcl1_schema:\n name: kcl1\n"
);
}

/// rust crate [`gag`]: https://crates.io/crates/gag
/// allows redirecting stderr or stdout either to a file or to nothing,
/// but it only works on unix systems.
Expand Down
1 change: 1 addition & 0 deletions kclvm/config/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ anyhow = "1.0"

kclvm-version = {path = "../version"}
kclvm-utils = {path = "../utils"}
kclvm-ast = {path = "../ast"}
dirs = "5.0.0"
17 changes: 15 additions & 2 deletions kclvm/config/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use super::modfile::KCL_FILE_SUFFIX;
use crypto::digest::Digest;
use crypto::md5::Md5;
use fslock::LockFile;
use kclvm_utils::pkgpath::parse_external_pkg_name;
use serde::{de::DeserializeOwned, Serialize};
use std::collections::HashMap;
use std::error;
Expand Down Expand Up @@ -35,7 +36,13 @@ impl Default for CacheOption {
}

/// Load pkg cache.
pub fn load_pkg_cache<T>(root: &str, target: &str, pkgpath: &str, option: CacheOption) -> Option<T>
pub fn load_pkg_cache<T>(
root: &str,
target: &str,
pkgpath: &str,
option: CacheOption,
external_pkgs: &HashMap<String, String>,
) -> Option<T>
where
T: DeserializeOwned + Default,
{
Expand All @@ -48,7 +55,13 @@ where
} else {
// Compare the md5 using cache
let real_path = get_pkg_realpath_from_pkgpath(root, pkgpath);
if Path::new(&real_path).exists() {
// If the file exists and it is an internal package or an external package,
// Check the cache info.
let pkg_name = parse_external_pkg_name(pkgpath).ok()?;
if Path::new(&real_path).exists()
|| (external_pkgs.get(&pkg_name).is_some()
&& Path::new(external_pkgs.get(&pkg_name).unwrap()).exists())
Peefy marked this conversation as resolved.
Show resolved Hide resolved
{
let cache_info = read_info_cache(root, target, Some(&option.cache_dir));
let relative_path = real_path.replacen(root, ".", 1);
match cache_info.get(&relative_path) {
Expand Down
33 changes: 2 additions & 31 deletions kclvm/parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use kclvm_config::modfile::{
use kclvm_error::{ErrorKind, Message, Position, Style};
use kclvm_sema::plugin::PLUGIN_MODULE_PREFIX;
use kclvm_utils::path::PathPrefix;
use kclvm_utils::pkgpath::parse_external_pkg_name;
use kclvm_utils::pkgpath::rm_external_pkg_name;

use lexer::parse_token_streams;
use parser::Parser;
Expand Down Expand Up @@ -758,34 +760,3 @@ impl Loader {
std::path::Path::new(path).exists()
}
}

/// Remove the external package name prefix from the current import absolute path.
///
/// # Note
/// [`rm_external_pkg_name`] just remove the prefix of the import path,
/// so it can't distinguish whether the current path is an internal package or an external package.
///
/// # Error
/// An error is returned if an empty string is passed in.
pub fn rm_external_pkg_name(pkgpath: &str) -> Result<String, String> {
Ok(pkgpath
.to_string()
.trim_start_matches(parse_external_pkg_name(pkgpath)?.as_str())
.to_string())
}

/// Remove the external package name prefix from the current import absolute path.
///
/// # Note
/// [`rm_external_pkg_name`] just remove the prefix of the import path,
/// so it can't distinguish whether the current path is an internal package or an external package.
///
/// # Error
/// An error is returned if an empty string is passed in.
pub fn parse_external_pkg_name(pkgpath: &str) -> Result<String, String> {
let mut names = pkgpath.splitn(2, '.');
match names.next() {
Some(it) => Ok(it.to_string()),
None => Err(format!("Invalid external package name `{}`", pkgpath)),
}
}
12 changes: 10 additions & 2 deletions kclvm/runner/src/assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ pub(crate) struct KclvmAssembler {
entry_file: String,
single_file_assembler: KclvmLibAssembler,
target: String,
external_pkgs: HashMap<String, String>,
}

impl KclvmAssembler {
Expand All @@ -199,13 +200,15 @@ impl KclvmAssembler {
scope: ProgramScope,
entry_file: String,
single_file_assembler: KclvmLibAssembler,
external_pkgs: HashMap<String, String>,
) -> Self {
Self {
program,
scope,
entry_file,
single_file_assembler,
target: env!("KCLVM_DEFAULT_TARGET").to_string(),
external_pkgs,
}
}

Expand Down Expand Up @@ -326,8 +329,13 @@ impl KclvmAssembler {
assembler.assemble(&compile_prog, import_names, &code_file, &code_file_path)
} else {
// Read the lib path cache
let file_relative_path: Option<String> =
load_pkg_cache(root, &target, &pkgpath, CacheOption::default());
let file_relative_path: Option<String> = load_pkg_cache(
root,
&target,
&pkgpath,
CacheOption::default(),
&self.external_pkgs,
);
let file_abs_path = match file_relative_path {
Some(file_relative_path) => {
let path = if file_relative_path.starts_with('.') {
Expand Down
1 change: 1 addition & 0 deletions kclvm/runner/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ pub fn execute(
scope,
temp_entry_file.clone(),
KclvmLibAssembler::LLVM,
args.get_package_maps_from_external_pkg(),
)
.gen_libs();

Expand Down
3 changes: 2 additions & 1 deletion kclvm/runner/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ fn gen_assembler(entry_file: &str, test_kcl_case_path: &str) -> KclvmAssembler {
scope,
entry_file.to_string(),
KclvmLibAssembler::LLVM,
HashMap::new(),
)
}

Expand Down Expand Up @@ -386,7 +387,7 @@ fn test_clean_path_for_genlibs() {
.to_string(),
);
let scope = resolve_program(&mut prog);
let assembler = KclvmAssembler::new(prog, scope, String::new(), KclvmLibAssembler::LLVM);
let assembler = KclvmAssembler::new(prog, scope, String::new(), KclvmLibAssembler::LLVM, HashMap::new());

let temp_dir = tempdir().unwrap();
let temp_dir_path = temp_dir.path().to_str().unwrap();
Expand Down
1 change: 1 addition & 0 deletions kclvm/tools/src/LSP/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ kclvm-driver = {path = "../../../driver"}
kclvm-parser = {path = "../../../parser"}
kclvm-sema = {path = "../../../sema"}
kclvm-ast = {path = "../../../ast"}
kclvm-utils = {path = "../../../utils"}
compiler_base_session = {path = "../../../../compiler_base/session"}

lsp-server = { version = "0.6.0", default-features = false }
Expand Down
3 changes: 2 additions & 1 deletion kclvm/tools/src/LSP/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@ use kclvm_driver::kpm_metadata::fetch_metadata;
use kclvm_driver::{get_kcl_files, lookup_compile_unit};
use kclvm_error::Diagnostic;
use kclvm_error::Position as KCLPos;
use kclvm_parser::{load_program, rm_external_pkg_name, ParseSession};
use kclvm_parser::{load_program, ParseSession};
use kclvm_sema::resolver::{resolve_program, scope::ProgramScope};
use kclvm_utils::pkgpath::rm_external_pkg_name;
use lsp_types::Url;
use parking_lot::{RwLock, RwLockReadGuard};
use ra_ap_vfs::{FileId, Vfs};
Expand Down
1 change: 1 addition & 0 deletions kclvm/utils/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
pub mod path;
pub mod pkgpath;
32 changes: 32 additions & 0 deletions kclvm/utils/src/pkgpath.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
//! This file primarily offers utils for working with kcl package paths.

/// Remove the external package name prefix from the current import absolute path.
///
/// # Note
/// [`rm_external_pkg_name`] just remove the prefix of the import path,
/// so it can't distinguish whether the current path is an internal package or an external package.
///
/// # Error
/// An error is returned if an empty string is passed in.
pub fn rm_external_pkg_name(pkgpath: &str) -> Result<String, String> {
Ok(pkgpath
.to_string()
.trim_start_matches(parse_external_pkg_name(pkgpath)?.as_str())
.to_string())
}

/// Remove the external package name prefix from the current import absolute path.
///
/// # Note
/// [`rm_external_pkg_name`] just remove the prefix of the import path,
/// so it can't distinguish whether the current path is an internal package or an external package.
///
/// # Error
/// An error is returned if an empty string is passed in.
pub fn parse_external_pkg_name(pkgpath: &str) -> Result<String, String> {
let mut names = pkgpath.splitn(2, '.');
match names.next() {
Some(it) => Ok(it.to_string()),
None => Err(format!("Invalid external package name `{}`", pkgpath)),
}
}