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

Create a more specialized trait for module resolution #109

Merged
merged 1 commit into from
Oct 30, 2023
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
7 changes: 7 additions & 0 deletions Cargo.lock

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
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
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
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
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