Skip to content

Commit

Permalink
Remove deprecated CLI options (#111)
Browse files Browse the repository at this point in the history
  • Loading branch information
rtimush authored Oct 30, 2023
1 parent 72bb0b6 commit a03b024
Show file tree
Hide file tree
Showing 6 changed files with 11 additions and 95 deletions.
34 changes: 2 additions & 32 deletions src/api/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{env, error::Error, path::PathBuf};

use home::home_dir;

use crate::{cache::ProtofetchGitCache, cli::HttpGitAuth, Protofetch};
use crate::{cache::ProtofetchGitCache, Protofetch};

#[derive(Default)]
pub struct ProtofetchBuilder {
Expand All @@ -12,10 +12,6 @@ pub struct ProtofetchBuilder {
lock_file_name: Option<PathBuf>,
cache_directory_path: Option<PathBuf>,
output_directory_name: Option<PathBuf>,

// These fields are deprecated
http_credentials: Option<HttpGitAuth>,
cache_dependencies_directory_name: Option<PathBuf>,
}

impl ProtofetchBuilder {
Expand Down Expand Up @@ -58,35 +54,13 @@ impl ProtofetchBuilder {
self
}

#[deprecated(
since = "0.0.23",
note = "configure credentials using standard git configuration instead"
)]
#[doc(hidden)]
pub fn http_credentials(mut self, username: String, password: String) -> Self {
self.http_credentials = Some(HttpGitAuth::new(username, password));
self
}

#[deprecated(
since = "0.0.23",
note = "this is an implementation detail and should not be overridden"
)]
#[doc(hidden)]
pub fn cache_dependencies_directory_name(mut self, path: impl Into<PathBuf>) -> Self {
self.cache_dependencies_directory_name = Some(path.into());
self
}

pub fn try_build(self) -> Result<Protofetch, Box<dyn Error>> {
let Self {
root,
module_file_name,
lock_file_name,
output_directory_name,
cache_directory_path,
http_credentials,
cache_dependencies_directory_name,
} = self;
let root = match root {
Some(root) => root,
Expand All @@ -102,18 +76,14 @@ impl ProtofetchBuilder {

let git_config = git2::Config::open_default()?;

let cache = ProtofetchGitCache::new(cache_directory, git_config, http_credentials)?;

let cache_dependencies_directory_name =
cache_dependencies_directory_name.unwrap_or_else(|| PathBuf::from("dependencies"));
let cache = ProtofetchGitCache::new(cache_directory, git_config)?;

Ok(Protofetch {
cache,
root,
module_file_name,
lock_file_name,
output_directory_name,
cache_dependencies_directory_name,
})
}
}
Expand Down
2 changes: 0 additions & 2 deletions src/api/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub struct Protofetch {
module_file_name: PathBuf,
lock_file_name: PathBuf,
output_directory_name: Option<PathBuf>,
cache_dependencies_directory_name: PathBuf,
}

#[derive(Debug, PartialEq, Eq, Clone, Copy)]
Expand Down Expand Up @@ -49,7 +48,6 @@ impl Protofetch {
&self.root,
&self.module_file_name,
&self.lock_file_name,
&self.cache_dependencies_directory_name,
self.output_directory_name.as_deref(),
)
}
Expand Down
25 changes: 5 additions & 20 deletions src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ use git2::{build::RepoBuilder, Cred, CredentialType, FetchOptions, RemoteCallbac
use log::trace;
use thiserror::Error;

use crate::{
cli::HttpGitAuth, model::protofetch::Coordinate, proto_repository::ProtoGitRepository,
};
use crate::{model::protofetch::Coordinate, proto_repository::ProtoGitRepository};

use crate::proto_repository::ProtoRepository;

Expand All @@ -20,7 +18,6 @@ pub trait RepositoryCache {
pub struct ProtofetchGitCache {
pub location: PathBuf,
git_config: Config,
git_auth: Option<HttpGitAuth>,
}

#[derive(Error, Debug)]
Expand Down Expand Up @@ -53,23 +50,17 @@ impl RepositoryCache for ProtofetchGitCache {
}

impl ProtofetchGitCache {
pub fn new(
location: PathBuf,
git_config: Config,
git_auth: Option<HttpGitAuth>,
) -> Result<ProtofetchGitCache, CacheError> {
pub fn new(location: PathBuf, git_config: Config) -> Result<ProtofetchGitCache, CacheError> {
if location.exists() && location.is_dir() {
Ok(ProtofetchGitCache {
location,
git_config,
git_auth,
})
} else if !location.exists() {
std::fs::create_dir_all(&location)?;
Ok(ProtofetchGitCache {
location,
git_config,
git_auth,
})
} else {
Err(CacheError::BadLocation {
Expand All @@ -95,7 +86,7 @@ impl ProtofetchGitCache {

fn clone_repo(&self, entry: &Coordinate) -> Result<Repository, CacheError> {
let mut repo_builder = RepoBuilder::new();
let options = ProtofetchGitCache::fetch_options(&self.git_config, &self.git_auth)?;
let options = ProtofetchGitCache::fetch_options(&self.git_config)?;
repo_builder.bare(true).fetch_options(options);

let url = entry.url();
Expand All @@ -111,16 +102,13 @@ impl ProtofetchGitCache {
.refspecs()
.filter_map(|refspec| refspec.str().map(|s| s.to_string()))
.collect();
let options = &mut ProtofetchGitCache::fetch_options(&self.git_config, &self.git_auth)?;
let options = &mut ProtofetchGitCache::fetch_options(&self.git_config)?;
remote.fetch(&refspecs, Some(options), None)?;

Ok(())
}

fn fetch_options<'a>(
config: &'a Config,
auth: &'a Option<HttpGitAuth>,
) -> Result<FetchOptions<'a>, CacheError> {
fn fetch_options(config: &Config) -> Result<FetchOptions<'_>, CacheError> {
let mut callbacks = RemoteCallbacks::new();
// Consider using https://crates.io/crates/git2_credentials that supports
// more authentication options
Expand All @@ -141,9 +129,6 @@ impl ProtofetchGitCache {
}
// HTTP auth
if allowed_types.contains(CredentialType::USER_PASS_PLAINTEXT) {
if let Some(auth) = auth {
return Cred::userpass_plaintext(&auth.username, &auth.password);
}
return Cred::credential_helper(config, url, username);
}
Err(git2::Error::from_str("no valid authentication available"))
Expand Down
4 changes: 2 additions & 2 deletions src/cli/command_handlers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use std::{
};

const DEFAULT_OUTPUT_DIRECTORY_NAME: &str = "proto_src";
const CACHE_WORKSPACES_DIRECTORY_NAME: &str = "dependencies";

/// Handler to fetch command
pub fn do_fetch(
Expand All @@ -25,14 +26,13 @@ pub fn do_fetch(
root: &Path,
module_file_name: &Path,
lock_file_name: &Path,
cache_dependencies_directory_name: &Path,
output_directory_name: Option<&Path>,
) -> Result<(), Box<dyn Error>> {
let module_descriptor = load_module_descriptor(root, module_file_name)?;

let lockfile = do_lock(lock_mode, cache, root, module_file_name, lock_file_name)?;

let cache_dependencies_directory_path = cache.location.join(cache_dependencies_directory_name);
let cache_dependencies_directory_path = cache.location.join(CACHE_WORKSPACES_DIRECTORY_NAME);
let output_directory_name = output_directory_name
.or_else(|| module_descriptor.proto_out_dir.as_ref().map(Path::new))
.unwrap_or(Path::new(DEFAULT_OUTPUT_DIRECTORY_NAME));
Expand Down
8 changes: 0 additions & 8 deletions src/cli/mod.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1 @@
use derive_new::new;

pub mod command_handlers;

#[derive(Clone, Debug, new)]
pub struct HttpGitAuth {
pub username: String,
pub password: String,
}
33 changes: 2 additions & 31 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::{env, error::Error};
use std::error::Error;

use clap::Parser;
use env_logger::Target;
Expand All @@ -25,12 +25,6 @@ pub struct CliArgs {
/// this will override proto_out_dir from the module toml config
#[clap(short, long)]
pub output_proto_directory: Option<String>,
#[clap(short, long, hide(true))]
/// Git username in case https is used in config
pub username: Option<String>,
#[clap(short, long, hide(true))]
/// Git password in case https is used in config
pub password: Option<String>,
}

#[derive(Debug, Parser)]
Expand All @@ -43,10 +37,6 @@ pub enum Command {
/// forces re-creation of lock file
#[clap(short, long, hide(true))]
force_lock: bool,
/// name of the dependencies repo checkout directory
/// this is a relative path within cache folder
#[clap(short, long, hide(true))]
repo_output_directory: Option<String>,
},
/// Creates a lock file based on toml configuration file
Lock,
Expand Down Expand Up @@ -107,19 +97,10 @@ fn main() {
fn run() -> Result<(), Box<dyn Error>> {
let cli_args: CliArgs = CliArgs::parse();

#[allow(deprecated)]
let mut protofetch = Protofetch::builder()
.module_file_name(&cli_args.module_location)
.lock_file_name(&cli_args.lockfile_location);

#[allow(deprecated)]
if let Some(username) = cli_args.username.or_else(|| env::var("GIT_USERNAME").ok()) {
if let Some(password) = cli_args.password.or_else(|| env::var("GIT_PASSWORD").ok()) {
warn!("Specifying git credentials on the command line or with environment variables is deprecated. Please use standard git configuration to specify credentials, and open a GitHub issue describing your use-case if that does not work for you.");
protofetch = protofetch.http_credentials(username, password);
}
}

if let Some(output_directory_name) = &cli_args.output_proto_directory {
protofetch = protofetch.output_directory_name(output_directory_name)
}
Expand All @@ -128,17 +109,7 @@ fn run() -> Result<(), Box<dyn Error>> {
}

match cli_args.cmd {
Command::Fetch {
locked,
force_lock,
repo_output_directory,
} => {
#[allow(deprecated)]
if let Some(repo_output_directory) = repo_output_directory {
warn!("Specifying --repo-output-directory is deprecated, if you need it please open a GitHub issue describing your use-case.");
protofetch = protofetch.cache_dependencies_directory_name(repo_output_directory);
}

Command::Fetch { locked, force_lock } => {
let lock_mode = if force_lock {
warn!("Specifying --force-lock is deprecated, please use \"protofetch update\" instead.");
LockMode::Recreate
Expand Down

0 comments on commit a03b024

Please sign in to comment.