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

refactor: refine tman & ten_rust data structure #444

Merged
merged 1 commit into from
Dec 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
26 changes: 17 additions & 9 deletions core/src/ten_manager/src/cmd/cmd_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ use inquire::Confirm;
use semver::VersionReq;

use ten_rust::pkg_info::{
dependencies::{DependencyRelationship, PkgDependency},
dependencies::PkgDependency,
find_to_be_replaced_local_pkgs, find_untracked_local_packages,
get_pkg_info_from_path,
pkg_type::PkgType,
Expand All @@ -47,7 +47,7 @@ use crate::{
package_info::tman_get_all_existed_pkgs_info_of_app,
solver::{
introducer::extract_introducer_relations_from_raw_solver_results,
solve::solve_all,
solve::{solve_all, DependencyRelationship},
solver_error::{parse_error_statement, print_conflict_info},
solver_result::{
extract_solver_results_from_raw_solver_results,
Expand Down Expand Up @@ -292,16 +292,20 @@ fn write_pkgs_into_lock(pkgs: &Vec<&PkgInfo>, app_dir: &Path) -> Result<()> {

fn parse_pkg_name_version(
pkg_name_version: &str,
) -> Result<(String, Option<String>, VersionReq)> {
) -> Result<(String, String, VersionReq)> {
let parts: Vec<&str> = pkg_name_version.split('@').collect();
if parts.len() == 2 {
Ok((
parts[0].to_string(),
Some(parts[1].to_string()),
parts[1].to_string(),
VersionReq::parse(parts[1])?,
))
} else {
Ok((pkg_name_version.to_string(), None, VersionReq::STAR))
Ok((
pkg_name_version.to_string(),
VersionReq::STAR.to_string(),
VersionReq::STAR,
))
}
}

Expand Down Expand Up @@ -584,12 +588,16 @@ pub async fn execute_cmd(
);

let extra_dependency_relationship = DependencyRelationship {
pkg_type: app_pkg_.basic_info.type_and_name.pkg_type,
name: app_pkg_.basic_info.type_and_name.name.clone(),
type_and_name: PkgTypeAndName {
pkg_type: app_pkg_.basic_info.type_and_name.pkg_type,
name: app_pkg_.basic_info.type_and_name.name.clone(),
},
version: app_pkg_.basic_info.version.clone(),
dependency: PkgDependency {
pkg_type: desired_pkg_type_,
name: desired_pkg_src_name_.clone(),
type_and_name: PkgTypeAndName {
pkg_type: desired_pkg_type_,
name: desired_pkg_src_name_.clone(),
},
version_req: desired_pkg_src_version_.clone(),
version_req_str: desired_pkg_src_version_str_,
},
Expand Down
64 changes: 18 additions & 46 deletions core/src/ten_manager/src/dep_and_candidate/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
// Refer to the "LICENSE" file in the root directory for more information.
//
use std::collections::{HashMap, HashSet};
use std::hash::{Hash, Hasher};

use anyhow::{anyhow, Result};
use semver::{Version, VersionReq};
Expand Down Expand Up @@ -52,62 +51,35 @@ impl MergedVersionReq {
}
}

struct FoundDependency {
pkg_type: PkgType,
name: String,
version_reqs: MergedVersionReq,
}

impl Hash for FoundDependency {
fn hash<H: Hasher>(&self, state: &mut H) {
self.pkg_type.hash(state);
self.name.hash(state);
}
}

impl PartialEq for FoundDependency {
fn eq(&self, other: &Self) -> bool {
self.pkg_type == other.pkg_type && self.name == other.name
}
}

impl Eq for FoundDependency {}

/// If the contents of merged_dependencies are the same before and after the
/// merge, it means merged_dependencies has not changed, so return false.
/// Otherwise, it means there has been a change, so return true. This allows the
/// caller to know that there may be new content and that some actions may need
/// to be taken.
fn merge_dependency_to_dependencies(
merged_dependencies: &mut HashSet<FoundDependency>,
merged_dependencies: &mut HashMap<PkgTypeAndName, MergedVersionReq>,
dependency: &PkgDependency,
) -> Result<bool> {
let searched_target = FoundDependency {
pkg_type: dependency.pkg_type,
name: dependency.name.clone(),
version_reqs: MergedVersionReq::default(),
let searched_target = PkgTypeAndName {
pkg_type: dependency.type_and_name.pkg_type,
name: dependency.type_and_name.name.clone(),
};

let mut changed = true;

// Note that FoundDependency will only use pkg_identity as the hash key.
if let Some(mut existed_dependency) =
merged_dependencies.take(&searched_target)
if let Some(existed_dependency) =
merged_dependencies.get_mut(&searched_target)
{
changed = existed_dependency
.version_reqs
.merge(&dependency.version_req)?;

// Put back.
merged_dependencies.insert(existed_dependency);
changed = existed_dependency.merge(&dependency.version_req)?;
} else {
// This is the first time seeing this dependency.

merged_dependencies.insert(FoundDependency {
pkg_type: dependency.pkg_type,
name: dependency.name.clone(),
version_reqs: MergedVersionReq::new(&dependency.version_req),
});
merged_dependencies.insert(
PkgTypeAndName {
pkg_type: dependency.type_and_name.pkg_type,
name: dependency.type_and_name.name.clone(),
},
MergedVersionReq::new(&dependency.version_req),
);
}

Ok(changed)
Expand All @@ -133,7 +105,7 @@ async fn process_dependencies_to_get_candidates(
tman_config: &TmanConfig,
support: &PkgSupport,
input_dependencies: &Vec<PkgDependency>,
merged_dependencies: &mut HashSet<FoundDependency>,
merged_dependencies: &mut HashMap<PkgTypeAndName, MergedVersionReq>,
all_candidates: &mut HashMap<
PkgTypeAndName,
HashMap<PkgBasicInfo, PkgInfo>,
Expand Down Expand Up @@ -162,8 +134,8 @@ async fn process_dependencies_to_get_candidates(
// criteria.
let results = get_package_list(
tman_config,
dependency.pkg_type,
&dependency.name,
dependency.type_and_name.pkg_type,
&dependency.type_and_name.name,
&criteria,
)
.await?;
Expand Down Expand Up @@ -310,7 +282,7 @@ pub async fn get_all_candidates_from_deps(
extra_dependencies: &Vec<PkgDependency>,
locked_pkgs: Option<&HashMap<PkgTypeAndName, PkgInfo>>,
) -> Result<HashMap<PkgTypeAndName, HashMap<PkgBasicInfo, PkgInfo>>> {
let mut merged_dependencies = HashSet::<FoundDependency>::new();
let mut merged_dependencies = HashMap::new();
let mut processed_pkgs = HashSet::<PkgBasicInfo>::new();

// If there is extra dependencies (ex: specified in the command line),
Expand Down
13 changes: 8 additions & 5 deletions core/src/ten_manager/src/manifest_lock/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,13 @@ impl<'a> From<&'a ManifestLockItem> for PkgInfo {
.map(|deps| {
deps.into_iter()
.map(|dep| PkgDependency {
pkg_type: PkgType::from_str(&dep.pkg_type).unwrap(),
name: dep.name,
type_and_name: PkgTypeAndName {
pkg_type: PkgType::from_str(&dep.pkg_type)
.unwrap(),
name: dep.name,
},
version_req: VersionReq::STAR,
version_req_str: None,
version_req_str: VersionReq::STAR.to_string(),
})
.collect()
})
Expand Down Expand Up @@ -305,8 +308,8 @@ pub struct ManifestLockItemDependencyItem {
impl From<PkgDependency> for ManifestLockItemDependencyItem {
fn from(pkg_dep: PkgDependency) -> Self {
ManifestLockItemDependencyItem {
pkg_type: pkg_dep.pkg_type.to_string(),
name: pkg_dep.name,
pkg_type: pkg_dep.type_and_name.pkg_type.to_string(),
name: pkg_dep.type_and_name.name,
}
}
}
13 changes: 3 additions & 10 deletions core/src/ten_manager/src/solver/introducer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,8 @@ pub fn extract_introducer_relations_from_raw_solver_results(
{
// The `version` declared in the `dependencies` section in
// manifest.json is always present.
requested_version_str = requested_dep_in_introducer
.version_req_str
.as_ref()
.unwrap()
.clone();
requested_version_str =
requested_dep_in_introducer.version_req_str.clone();
} else {
return Err(anyhow::anyhow!(
"Requested dependency [{}]{} not found in introducer, should not happen.", pkg_type_str, name
Expand Down Expand Up @@ -110,11 +107,7 @@ pub fn get_dependency_chain(
)
.unwrap();
chain.push((
requested_dep_in_introducer
.version_req_str
.as_ref()
.unwrap()
.clone(),
requested_dep_in_introducer.version_req_str.clone(),
conflict_pkg_identity.clone(),
));

Expand Down
26 changes: 17 additions & 9 deletions core/src/ten_manager/src/solver/solve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,9 @@ use clingo::{
SolveMode, Statistics, StatisticsType,
};

use semver::Version;
use ten_rust::pkg_info::{
dependencies::DependencyRelationship, pkg_basic_info::PkgBasicInfo,
dependencies::PkgDependency, pkg_basic_info::PkgBasicInfo,
pkg_type::PkgType, pkg_type_and_name::PkgTypeAndName, PkgInfo,
};

Expand All @@ -23,6 +24,13 @@ use crate::{
log::{tman_verbose_print, tman_verbose_println},
};

#[derive(Debug)]
pub struct DependencyRelationship {
pub type_and_name: PkgTypeAndName,
pub version: Version,
pub dependency: PkgDependency,
}

fn get_model(
tman_config: &TmanConfig,
model: &Model,
Expand Down Expand Up @@ -309,8 +317,8 @@ fn create_input_str_for_dependency_relationship(
{
input_str.push_str(&format!(
"depends_on_declared(\"{}\", \"{}\", \"{}\", \"{}\", \"{}\", \"{}\").\n",
dependency_relationship.pkg_type,
dependency_relationship.name,
dependency_relationship.type_and_name.pkg_type,
dependency_relationship.type_and_name.name,
dependency_relationship.version,
candidate.1.basic_info.type_and_name.pkg_type,
candidate.1.basic_info.type_and_name.name,
Expand All @@ -322,8 +330,8 @@ fn create_input_str_for_dependency_relationship(
return Err(TmanError::Custom(
format!(
"Failed to find candidates for {}:{}@{}",
dependency_relationship.dependency.pkg_type,
dependency_relationship.dependency.name,
dependency_relationship.dependency.type_and_name.pkg_type,
dependency_relationship.dependency.type_and_name.name,
dependency_relationship.version,
)
.to_string(),
Expand Down Expand Up @@ -383,8 +391,8 @@ fn create_input_str_for_pkg_info_dependencies(
return Err(TmanError::Custom(
format!(
"Failed to find candidates for [{}]{}({})",
dependency.pkg_type,
dependency.name,
dependency.type_and_name.pkg_type,
dependency.type_and_name.name,
dependency.version_req
)
.to_string(),
Expand All @@ -395,8 +403,8 @@ fn create_input_str_for_pkg_info_dependencies(
return Err(TmanError::Custom(
format!(
"Failed to find candidates for {}:{}@{}",
dependency.pkg_type,
dependency.name,
dependency.type_and_name.pkg_type,
dependency.type_and_name.name,
dependency.version_req
)
.to_string(),
Expand Down
2 changes: 1 addition & 1 deletion core/src/ten_rust/src/json/bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use std::ptr;
use crate::json;

#[no_mangle]
pub extern "C" fn ten_remove_json_comments(
pub unsafe extern "C" fn ten_remove_json_comments(
json_with_comments: *const c_char,
) -> *mut c_char {
if json_with_comments.is_null() {
Expand Down
Loading
Loading