Skip to content

Commit

Permalink
Create a more specialized trait for module resolution (#109)
Browse files Browse the repository at this point in the history
  • Loading branch information
@rtimush
rtimush authored Oct 30, 2023
1 parent e24f74a commit b118932
Show file tree
Hide file tree
Showing 8 changed files with 162 additions and 139 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock
View file

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

1 change: 1 addition & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ vendored-openssl = ["git2/vendored-openssl"]
vendored-libgit2 = ["git2/vendored-libgit2"]

[dependencies]
anyhow = "1.0.75"
clap = { version = "4.3.2", features = ["derive"] }
derive-new = "0.5.9"
env_logger = "0.10.0"
Expand Down
10 changes: 7 additions & 3 deletions src/cache.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ use crate::{
use crate::proto_repository::ProtoRepository;

pub trait RepositoryCache {
fn clone_or_update(&self, entry: &Coordinate) -> Result<Box<dyn ProtoRepository>, CacheError>;
type Repository: ProtoRepository;

fn clone_or_update(&self, entry: &Coordinate) -> Result<Self::Repository, CacheError>;
}

pub struct ProtofetchGitCache {
Expand All @@ -32,7 +34,9 @@ pub enum CacheError {
}

impl RepositoryCache for ProtofetchGitCache {
fn clone_or_update(&self, entry: &Coordinate) -> Result<Box<dyn ProtoRepository>, CacheError> {
type Repository = ProtoGitRepository;

fn clone_or_update(&self, entry: &Coordinate) -> Result<Self::Repository, CacheError> {
let repo = match self.get_entry(entry) {
None => self.clone_repo(entry)?,
Some(path) => {
Expand All @@ -44,7 +48,7 @@ impl RepositoryCache for ProtofetchGitCache {
}
};

Ok(Box::new(ProtoGitRepository::new(repo)))
Ok(ProtoGitRepository::new(repo))
}
}

Expand Down
142 changes: 60 additions & 82 deletions src/fetch.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use crate::{
lock::{LockFile, LockedDependency},
Dependency, DependencyName, Descriptor, RevisionSpecification,
},
proto_repository::ProtoRepository,
resolver::ModuleResolver,
};
use log::{debug, error, info};
use thiserror::Error;
Expand All @@ -30,11 +32,16 @@ pub enum FetchError {
ProtoRepoError(#[from] crate::proto_repository::ProtoRepoError),
#[error("IO error: {0}")]
IO(#[from] std::io::Error),
#[error(transparent)]
Resolver(anyhow::Error),
}

pub fn lock(descriptor: &Descriptor, cache: &impl RepositoryCache) -> Result<LockFile, FetchError> {
pub fn lock(
descriptor: &Descriptor,
resolver: &impl ModuleResolver,
) -> Result<LockFile, FetchError> {
fn go(
cache: &impl RepositoryCache,
resolver: &impl ModuleResolver,
resolved: &mut BTreeMap<DependencyName, (RevisionSpecification, LockedDependency)>,
dependencies: &[Dependency],
) -> Result<(), FetchError> {
Expand All @@ -43,19 +50,23 @@ pub fn lock(descriptor: &Descriptor, cache: &impl RepositoryCache) -> Result<Loc
match resolved.get(&dependency.name) {
None => {
log::info!("Resolving {:?}", dependency.coordinate);
let repository = cache.clone_or_update(&dependency.coordinate)?;
let commit_hash = repository.resolve_commit_hash(&dependency.specification)?;
let mut descriptor =
repository.extract_descriptor(&dependency.name, &commit_hash)?;
let dependencies = descriptor
let mut resolved_module = resolver
.resolve(
&dependency.coordinate,
&dependency.specification,
&dependency.name,
)
.map_err(FetchError::Resolver)?;
let dependencies = resolved_module
.descriptor
.dependencies
.iter()
.map(|dep| dep.name.clone())
.collect();

let locked = LockedDependency {
name: dependency.name.clone(),
commit_hash,
commit_hash: resolved_module.commit_hash,
coordinate: dependency.coordinate.clone(),
dependencies,
rules: dependency.rules.clone(),
Expand All @@ -65,7 +76,7 @@ pub fn lock(descriptor: &Descriptor, cache: &impl RepositoryCache) -> Result<Loc
dependency.name.clone(),
(dependency.specification.clone(), locked),
);
children.append(&mut descriptor.dependencies);
children.append(&mut resolved_module.descriptor.dependencies);
}
Some((resolved_specification, resolved)) => {
if resolved.coordinate != dependency.coordinate {
Expand All @@ -88,15 +99,15 @@ pub fn lock(descriptor: &Descriptor, cache: &impl RepositoryCache) -> Result<Loc
}

if !children.is_empty() {
go(cache, resolved, &children)?;
go(resolver, resolved, &children)?;
}

Ok(())
}

let mut resolved = BTreeMap::new();

go(cache, &mut resolved, &descriptor.dependencies)?;
go(resolver, &mut resolved, &descriptor.dependencies)?;

Ok(LockFile {
module_name: descriptor.name.clone(),
Expand Down Expand Up @@ -152,73 +163,51 @@ pub fn fetch_sources<Cache: RepositoryCache>(

#[cfg(test)]
mod tests {
use std::rc::Rc;
use anyhow::anyhow;

use crate::{
model::protofetch::{Coordinate, Protocol, Revision, RevisionSpecification, Rules},
proto_repository::{ProtoRepoError, ProtoRepository},
resolver::ResolvedModule,
};

use super::*;

use pretty_assertions::assert_eq;

struct FakeRepositoryCache {
entries: BTreeMap<Coordinate, Rc<FakeRepository>>,
#[derive(Default)]
struct FakeModuleResolver {
entries: BTreeMap<Coordinate, BTreeMap<RevisionSpecification, ResolvedModule>>,
}

#[derive(Clone, Default)]
struct FakeRepository {
specifications: BTreeMap<RevisionSpecification, String>,
descriptors: BTreeMap<String, Descriptor>,
}

impl FakeRepository {
fn push(&mut self, revision: &str, commit_hash: &str, descriptor: Descriptor) {
self.specifications.insert(
impl FakeModuleResolver {
fn push(&mut self, name: &str, revision: &str, commit_hash: &str, descriptor: Descriptor) {
self.entries.entry(coordinate(name)).or_default().insert(
RevisionSpecification {
revision: Revision::pinned(revision),
branch: None,
},
commit_hash.to_string(),
ResolvedModule {
commit_hash: commit_hash.to_string(),
descriptor,
},
);
self.descriptors.insert(commit_hash.to_string(), descriptor);
}
}

impl RepositoryCache for FakeRepositoryCache {
fn clone_or_update(
&self,
entry: &Coordinate,
) -> Result<Box<dyn ProtoRepository>, CacheError> {
Ok(Box::new(self.entries.get(entry).unwrap().clone()))
}
}

impl ProtoRepository for Rc<FakeRepository> {
fn extract_descriptor(
&self,
_: &DependencyName,
commit_hash: &str,
) -> Result<Descriptor, ProtoRepoError> {
Ok(self.descriptors.get(commit_hash).unwrap().clone())
}

fn create_worktrees(
&self,
_: &str,
_: &DependencyName,
_: &str,
_: &Path,
) -> Result<(), ProtoRepoError> {
Ok(())
}

fn resolve_commit_hash(
impl ModuleResolver for FakeModuleResolver {
fn resolve(
&self,
coordinate: &Coordinate,
specification: &RevisionSpecification,
) -> Result<String, ProtoRepoError> {
Ok(self.specifications.get(specification).unwrap().clone())
_: &DependencyName,
) -> anyhow::Result<ResolvedModule> {
Ok(self
.entries
.get(coordinate)
.ok_or_else(|| anyhow!("Coordinate not found: {}", coordinate))?
.get(specification)
.ok_or_else(|| anyhow!("Specification not found: {}", specification))?
.clone())
}
}

Expand Down Expand Up @@ -259,8 +248,9 @@ mod tests {

#[test]
fn resolve_transitive() {
let mut foo = FakeRepository::default();
foo.push(
let mut resolver = FakeModuleResolver::default();
resolver.push(
"foo",
"1.0.0",
"c1",
Descriptor {
Expand All @@ -271,8 +261,8 @@ mod tests {
},
);

let mut bar = FakeRepository::default();
bar.push(
resolver.push(
"bar",
"2.0.0",
"c2",
Descriptor {
Expand All @@ -283,21 +273,14 @@ mod tests {
},
);

let cache = FakeRepositoryCache {
entries: BTreeMap::from([
(coordinate("foo"), Rc::new(foo)),
(coordinate("bar"), Rc::new(bar)),
]),
};

let lock_file = lock(
&Descriptor {
name: "root".to_owned(),
description: None,
proto_out_dir: None,
dependencies: vec![dependency("foo", "1.0.0")],
},
&cache,
&resolver,
)
.unwrap();

Expand All @@ -315,8 +298,9 @@ mod tests {

#[test]
fn resolve_transitive_root_priority() {
let mut foo = FakeRepository::default();
foo.push(
let mut resolver = FakeModuleResolver::default();
resolver.push(
"foo",
"1.0.0",
"c1",
Descriptor {
Expand All @@ -327,8 +311,8 @@ mod tests {
},
);

let mut bar = FakeRepository::default();
bar.push(
resolver.push(
"bar",
"1.0.0",
"c3",
Descriptor {
Expand All @@ -338,7 +322,8 @@ mod tests {
dependencies: Vec::new(),
},
);
bar.push(
resolver.push(
"bar",
"2.0.0",
"c2",
Descriptor {
Expand All @@ -349,21 +334,14 @@ mod tests {
},
);

let cache = FakeRepositoryCache {
entries: BTreeMap::from([
(coordinate("foo"), Rc::new(foo)),
(coordinate("bar"), Rc::new(bar)),
]),
};

let lock_file = lock(
&Descriptor {
name: "root".to_owned(),
description: None,
proto_out_dir: None,
dependencies: vec![dependency("foo", "1.0.0"), dependency("bar", "1.0.0")],
},
&cache,
&resolver,
)
.unwrap();

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ mod fetch;
mod model;
mod proto;
mod proto_repository;
mod resolver;

pub use api::{Protofetch, ProtofetchBuilder};
Loading

0 comments on commit b118932

Please sign in to comment.