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

Support registry fallback chains #35

Merged
merged 11 commits into from
Nov 10, 2021
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@

# Insta uncommitted snapshots
*.snap.new

*.DS_Store
18 changes: 13 additions & 5 deletions src/commands/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use crate::installation::InstallationContext;
use crate::lockfile::{LockPackage, Lockfile};
use crate::manifest::Manifest;
use crate::package_id::PackageId;
use crate::package_source::{PackageSource, PackageSourceMap, Registry, TestRegistry};
use crate::package_source::{
PackageSource, PackageSourceId, PackageSourceMap, Registry, TestRegistry,
};
use crate::resolution::resolve;

use super::GlobalOptions;
Expand All @@ -28,11 +30,12 @@ impl InstallSubcommand {
.unwrap_or_else(|| Lockfile::from_manifest(&manifest));

let default_registry: Box<dyn PackageSource> = match &global.test_registry {
Some(test_registry) => Box::new(TestRegistry::new(test_registry)),
None => Box::new(Registry::from_registry_spec(&manifest.package.registry)?),
true => Box::new(TestRegistry::new(&manifest.package.registry)),
false => Box::new(Registry::from_registry_spec(&manifest.package.registry)?),
};

let package_sources = PackageSourceMap::new(default_registry);
let mut package_sources = PackageSourceMap::new(default_registry);
package_sources.add_fallbacks()?;

let mut try_to_use = BTreeSet::new();
for package in lockfile.packages {
Expand All @@ -47,7 +50,12 @@ impl InstallSubcommand {
}
}

let resolved = resolve(&manifest, &try_to_use, &package_sources)?;
let resolved = resolve(
&manifest,
&try_to_use,
&package_sources,
&PackageSourceId::DefaultRegistry,
)?;

let lockfile = Lockfile::from_resolve(&resolved);
lockfile.save(&self.project_path)?;
Expand Down
10 changes: 4 additions & 6 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ pub use package::PackageSubcommand;
pub use publish::PublishSubcommand;
pub use update::UpdateSubcommand;

use std::path::PathBuf;

use structopt::StructOpt;

#[derive(Debug, StructOpt)]
Expand Down Expand Up @@ -53,11 +51,11 @@ pub struct GlobalOptions {
#[structopt(global = true, parse(from_occurrences), long = "verbose", short)]
pub verbosity: u8,

/// Overrides the registry with a local registry. Usable only by tests.
/// Flag to indidate if we will be using a test registry. Usable only by tests.
#[structopt(skip)]
pub test_registry: Option<PathBuf>,
pub test_registry: bool,

/// Allows tests to specify if the package index should be temporary (to prevent multiple use conflicts). Usable only by tests.
/// Specify if the package index should be temporary (to prevent multiple use conflicts). Usable only by tests.
#[structopt(skip)]
pub use_temp_index: bool,
}
Expand All @@ -66,7 +64,7 @@ impl Default for GlobalOptions {
fn default() -> Self {
Self {
verbosity: 0,
test_registry: None,
test_registry: false,
use_temp_index: false,
}
}
Expand Down
20 changes: 16 additions & 4 deletions src/commands/publish.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
use std::path::PathBuf;
use std::path::{Path, PathBuf};

use anyhow::Context;
use structopt::StructOpt;
use url::Url;

use crate::{
auth::AuthStore, manifest::Manifest, package_contents::PackageContents,
Expand All @@ -19,12 +20,23 @@ pub struct PublishSubcommand {
impl PublishSubcommand {
pub fn run(self, global: GlobalOptions) -> anyhow::Result<()> {
let manifest = Manifest::load(&self.project_path)?;
let registry = url::Url::parse(&manifest.package.registry)?;
let auth_store = AuthStore::load()?;

let index_url = match global.test_registry {
true => {
let index_path = Path::new(&manifest.package.registry)
.join("index")
.canonicalize()?;
Url::from_directory_path(index_path).unwrap()
}
false => Url::parse(&manifest.package.registry)?,
};

let package_index = match global.use_temp_index {
true => PackageIndex::new_temp(&registry, None)?,
false => PackageIndex::new(&registry, None)?,
true => PackageIndex::new_temp(&index_url, None)?,
false => PackageIndex::new(&index_url, None)?,
};

let api = package_index.config()?.api;
let contents = PackageContents::pack_from_path(&self.project_path)?;

Expand Down
46 changes: 45 additions & 1 deletion src/git_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,11 @@ use std::path::Path;

use anyhow::{bail, format_err, Context};
use git2::build::RepoBuilder;
use git2::{Cred, CredentialType, FetchOptions, RemoteCallbacks, Repository};
use git2::{
Cred, CredentialType, FetchOptions, RemoteCallbacks, Repository, RepositoryInitOptions,
};
use url::Url;
use walkdir::WalkDir;

/// Based roughly on Cargo's approach to handling authentication, but very pared
/// down.
Expand Down Expand Up @@ -46,6 +49,47 @@ fn make_credentials_callback(
}
}

/// We want to use a mock repo in tests but we don't want have to manually initialise it
/// or manage commiting new files. This method will ensure the test repo is a valid git repo
/// with all files commited to the main branch. Typically test-registries/primary-registry/index.
pub fn init_test_repo(path: &Path) -> anyhow::Result<()> {
// If tests previous ran then this may already be a repo however we want to start fresh
if path.join(".git").exists() {
fs_err::remove_dir_all(path.join(".git"))?;
}

let repository = Repository::init_opts(
path,
RepositoryInitOptions::new().initial_head("refs/heads/main"),
)?;
let mut git_index = repository.index()?;

for entry in WalkDir::new(path).min_depth(1) {
let entry = entry?;
let relative_path = entry.path().strip_prefix(path)?;

if !relative_path.starts_with(".git") && entry.file_type().is_file() {
// git add $file
git_index.add_path(relative_path)?;
}
}

// git commit -m "..."
let sig = git2::Signature::now("PackageUser", "PackageUser@localhost")?;
let tree = repository.find_tree(git_index.write_tree()?)?;

repository.commit(
Some("HEAD"),
&sig,
&sig,
"Commit all files in test repo",
&tree,
&[],
)?;

Ok(())
}

pub fn open_or_clone(
access_token: Option<String>,
url: &Url,
Expand Down
14 changes: 6 additions & 8 deletions src/installation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,8 @@ use fs_err as fs;
use indoc::formatdoc;

use crate::{
manifest::Realm,
package_contents::PackageContents,
package_id::PackageId,
package_source::{PackageSourceId, PackageSourceMap},
resolution::Resolve,
manifest::Realm, package_contents::PackageContents, package_id::PackageId,
package_source::PackageSourceMap, resolution::Resolve,
};

pub struct InstallationContext {
Expand Down Expand Up @@ -69,8 +66,6 @@ impl InstallationContext {
root_package_id: PackageId,
resolved: &Resolve,
) -> anyhow::Result<()> {
let default_registry = sources.get(&PackageSourceId::DefaultRegistry).unwrap();

for package_id in &resolved.activated {
log::debug!("Installing {}...", package_id);

Expand Down Expand Up @@ -104,7 +99,10 @@ impl InstallationContext {
self.write_package_links(package_id, package_realm, deps, Realm::Server)?;
}

let contents = default_registry.download_package(package_id)?;
let source_registry = &resolved.metadata[package_id].source_registry;
let package_source = sources.get(&source_registry).unwrap();
let contents = package_source.download_package(package_id)?;

self.write_contents(package_id, &contents, package_realm)?;
}
}
Expand Down
6 changes: 5 additions & 1 deletion src/package_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::package_name::PackageName;
pub struct PackageIndexConfig {
pub api: Url,
pub github_oauth_id: Option<String>,
pub fallback_registries: Option<Vec<String>>,
magnalite marked this conversation as resolved.
Show resolved Hide resolved
}

pub struct PackageIndex {
Expand Down Expand Up @@ -90,7 +91,10 @@ impl PackageIndex {
pub fn update(&self) -> anyhow::Result<()> {
let repository = self.repository.lock().unwrap();

log::info!("Updating package index...");
log::info!(
"Updating package index {}...",
repository.find_remote("origin")?.url().unwrap()
);
git_util::update_index(self.access_token.clone(), &repository)
.with_context(|| format!("could not update package index"))?;

Expand Down
46 changes: 42 additions & 4 deletions src/package_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,35 +9,70 @@ pub use self::test_registry::TestRegistry;
use std::collections::HashMap;
use std::path::PathBuf;

use url::Url;
use serde::Serialize;

use crate::manifest::Manifest;
use crate::package_contents::PackageContents;
use crate::package_id::PackageId;
use crate::package_req::PackageReq;

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
pub enum PackageSourceId {
DefaultRegistry,
Git(Url),
Git(String),
Path(PathBuf),
}

pub struct PackageSourceMap {
sources: HashMap<PackageSourceId, Box<dyn PackageSource>>,
pub source_order: Vec<PackageSourceId>,
magnalite marked this conversation as resolved.
Show resolved Hide resolved
}

impl PackageSourceMap {
pub fn new(default_registry: Box<dyn PackageSource>) -> Self {
let mut sources = HashMap::new();
sources.insert(PackageSourceId::DefaultRegistry, default_registry);

Self { sources }
Self {
sources,
source_order: vec![PackageSourceId::DefaultRegistry],
}
}

pub fn get(&self, id: &PackageSourceId) -> Option<&dyn PackageSource> {
self.sources.get(id).map(|source| source.as_ref())
}

/// Searches the current list of sources for fallbacks and adds any not yet in the list, producing
/// a complete tree of reachable sources for packages.
/// Sources are searched breadth-first to ensure correct fallback priority.
pub fn add_fallbacks(&mut self) -> anyhow::Result<()> {
let mut source_index = 0;

while source_index < self.source_order.len() {
let registry = self.sources.get(&self.source_order[source_index]).unwrap();

for fallback in registry.fallback_sources()? {
// Prevent circular references by only adding new sources
if !self.source_order.contains(&fallback) {
let source: Box<dyn PackageSource> = match &fallback {
PackageSourceId::Git(url) => Box::new(Registry::from_registry_spec(url)?),
PackageSourceId::Path(path) => Box::new(TestRegistry::new(path.clone())),
PackageSourceId::DefaultRegistry => {
panic!("Default registry should never be added as a fallback source!")
}
};

self.sources.insert(fallback.clone(), source);
self.source_order.push(fallback);
}
}

source_index += 1;
}

Ok(())
}
}

pub trait PackageSource {
Expand All @@ -51,4 +86,7 @@ pub trait PackageSource {
/// Downloads the contents of a package given its fully-qualified
/// `PackageId`.
fn download_package(&self, package_id: &PackageId) -> anyhow::Result<PackageContents>;

/// Provide a list of fallback sources to search if this source can't provide a package
fn fallback_sources(&self) -> anyhow::Result<Vec<PackageSourceId>>;
}
6 changes: 5 additions & 1 deletion src/package_source/in_memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use crate::{
package_source::PackageSource, test_package::PackageBuilder,
};

use super::PackageContents;
use super::{PackageContents, PackageSourceId};

/// An in-memory registry that can have packages published to it.
///
Expand Down Expand Up @@ -108,6 +108,10 @@ impl PackageSource for InMemoryRegistrySource {

Ok(entry.contents.clone())
}

fn fallback_sources(&self) -> anyhow::Result<Vec<PackageSourceId>> {
todo!("Implement in-memory fallback sources");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use anyhow::bail here? There's no reason to panic since this is already a fallible function

Copy link
Member Author

@magnalite magnalite Nov 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there something wrong with a panic here? I'm not sure I understand why a bail would be preferable to a panic since this is code that shouldn't be running anyway. I like how the todo! here implies that this is something that needs to be implemented rather than some condition that hasn't been met.

}
}

struct PackageEntry {
Expand Down
15 changes: 15 additions & 0 deletions src/package_source/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ use crate::package_index::PackageIndex;
use crate::package_req::PackageReq;
use crate::package_source::{PackageContents, PackageSource};

use super::PackageSourceId;

pub struct Registry {
index_url: Url,
auth_token: OnceCell<Option<Arc<str>>>,
Expand Down Expand Up @@ -109,4 +111,17 @@ impl PackageSource for Registry {

Ok(PackageContents::from_buffer(data))
}

fn fallback_sources(&self) -> anyhow::Result<Vec<PackageSourceId>> {
let fallback_registries = self.index()?.config()?.fallback_registries;
let mut sources = Vec::new();

if let Some(fallbacks) = fallback_registries {
for fallback in fallbacks {
sources.push(PackageSourceId::Git(fallback));
}
}
magnalite marked this conversation as resolved.
Show resolved Hide resolved

Ok(sources)
}
}
21 changes: 21 additions & 0 deletions src/package_source/test_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ use fs_err::File;

use crate::manifest::Manifest;
use crate::package_id::PackageId;
use crate::package_index::PackageIndexConfig;
use crate::package_req::PackageReq;
use crate::package_source::{PackageContents, PackageSource};

use super::PackageSourceId;

pub struct TestRegistry {
path: PathBuf,
}
Expand Down Expand Up @@ -76,4 +79,22 @@ impl PackageSource for TestRegistry {
let data = fs_err::read(&package_path)?;
Ok(PackageContents::from_buffer(data))
}

fn fallback_sources(&self) -> anyhow::Result<Vec<PackageSourceId>> {
let config_path = self.path.join("index/config.json");
let contents = fs_err::read_to_string(config_path)?;
let config: PackageIndexConfig = serde_json::from_str(&contents)?;

let mut sources = Vec::new();

if let Some(fallbacks) = config.fallback_registries {
for fallback in fallbacks {
sources.push(PackageSourceId::Path(
self.path.join(fallback).canonicalize()?,
));
}
}

Ok(sources)
}
}
Loading