From 8693657cb0dd5e82690f3f6eac67b7970fa8e475 Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Sun, 25 Sep 2022 21:07:25 +0200 Subject: [PATCH 1/7] saving work, there is still an issue with unpacking the tarball after the asest it downloaded --- src/config/schema.rs | 5 ++++ src/config/toml.rs | 19 +++++++++++---- src/infra/client.rs | 55 ++++++++++++++++++++++++++++++++++++-------- src/model/tool.rs | 1 + src/sync/install.rs | 5 +++- src/sync/prefetch.rs | 11 ++++++++- 6 files changed, 80 insertions(+), 16 deletions(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index b0d0b88..6b48ef2 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -15,6 +15,7 @@ pub struct Config { /// Directory to store all locally downloaded tools pub store_directory: String, + pub proxy: Option, /// Info about each individual tool pub tools: BTreeMap, } @@ -38,6 +39,9 @@ pub struct ConfigAsset { /// Name of the specific asset to download pub asset_name: AssetName, + + /// Proxy which will get used for all communication except hardcoded tools + pub proxy: Option, } impl From for ConfigAsset { @@ -53,6 +57,7 @@ impl From for ConfigAsset { exe_name: Some(tool_info.exe_name), tag, asset_name: tool_info.asset_name, + proxy: None, } } } diff --git a/src/config/toml.rs b/src/config/toml.rs index 807b6c2..718073b 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -58,36 +58,47 @@ fn parse_string(contents: &str) -> Result { fn decode_config(toml: Value) -> Option { let str_store_directory = toml.get("store_directory")?.as_str()?; + let proxy: Option = match toml.get("proxy") { + Some(p) => Some(p.as_str()?.into()), + None => None, + }; let store_directory = String::from(str_store_directory); let mut tools = BTreeMap::new(); for (key, val) in toml.as_table()?.iter() { if let Value::Table(table) = val { - tools.insert(key.clone(), decode_config_asset(table)); + tools.insert(key.clone(), decode_config_asset(table, &proxy)); } } Some(Config { store_directory, tools, + proxy, }) } -fn decode_config_asset(table: &Map) -> ConfigAsset { +fn decode_config_asset(table: &Map, proxy: &Option) -> ConfigAsset { let owner = str_by_key(table, "owner"); let repo = str_by_key(table, "repo"); let exe_name = str_by_key(table, "exe_name"); let asset_name = decode_asset_name(table); let tag = str_by_key(table, "tag"); - ConfigAsset { + let mut config_asset = ConfigAsset { owner, repo, exe_name, asset_name, tag, - } + proxy: None, + }; + if let Some(p) = proxy { + config_asset.proxy = Some(ureq::Proxy::new(p.clone()).unwrap_or_else(|_| panic!("Could not parse proxy address, please check the syntax: {}", + p))); + }; + config_asset } fn decode_asset_name(table: &Map) -> AssetName { diff --git a/src/infra/client.rs b/src/infra/client.rs index a7eb78e..05bb850 100644 --- a/src/infra/client.rs +++ b/src/infra/client.rs @@ -5,10 +5,13 @@ use std::io::Read; use crate::model::release::{Asset, Release}; /// GitHub API client to handle all API requests +#[derive(Debug)] pub struct Client { pub owner: String, pub repo: String, pub version: String, + + pub proxy: Option, } impl Client { @@ -33,11 +36,23 @@ impl Client { pub fn fetch_release_info(&self) -> Result> { let release_url = self.release_url(); - let req = add_auth_header( - ureq::get(&release_url) - .set("Accept", "application/vnd.github+json") - .set("User-Agent", "chshersh/tool-sync-0.2.0"), - ); + let req = match &self.proxy { + Some(proxy) => { + let agent = ureq::AgentBuilder::new().proxy(proxy.clone()).build(); + + add_auth_header( + agent + .get(&release_url) + .set("Accept", "application/vnd.github+json") + .set("User-Agent", "chshersh/tool-sync-0.2.0"), + ) + } + None => add_auth_header( + ureq::get(&release_url) + .set("Accept", "application/vnd.github+json") + .set("User-Agent", "chshersh/tool-sync-0.2.0"), + ), + }; let release: Release = req.call()?.into_json()?; @@ -49,12 +64,30 @@ impl Client { asset: &Asset, ) -> Result, ureq::Error> { let asset_url = self.asset_url(asset.id); + let req = match &self.proxy { + Some(proxy) => { + let agent = ureq::AgentBuilder::new().proxy(proxy.clone()).build(); + + add_auth_header( + agent + .get(&asset_url) + .set("Accept", "application/vnd.github+json") + .set("User-Agent", "chshersh/tool-sync-0.2.0"), + ) + } + None => add_auth_header( + ureq::get(&asset_url) + .set("Accept", "application/vnd.github+json") + .set("User-Agent", "chshersh/tool-sync-0.2.0"), + ), + }; - let req = add_auth_header( - ureq::get(&asset_url) - .set("Accept", "application/octet-stream") - .set("User-Agent", "chshersh/tool-sync-0.2.0"), - ); + //println!("{:?}", &self.proxy); + //let req = add_auth_header( + // ureq::get(&asset_url) + // .set("Accept", "application/octet-stream") + // .set("User-Agent", "chshersh/tool-sync-0.2.0"), + //); Ok(req.call()?.into_reader()) } @@ -79,6 +112,7 @@ mod tests { owner: String::from("OWNER"), repo: String::from("REPO"), version: ToolInfoTag::Latest.to_str_version(), + proxy: None, }; assert_eq!( @@ -93,6 +127,7 @@ mod tests { owner: String::from("OWNER"), repo: String::from("REPO"), version: ToolInfoTag::Specific(String::from("SPECIFIC_TAG")).to_str_version(), + proxy: None, }; assert_eq!( diff --git a/src/model/tool.rs b/src/model/tool.rs index f218f4b..5e8bd4b 100644 --- a/src/model/tool.rs +++ b/src/model/tool.rs @@ -100,6 +100,7 @@ impl ToolInfo { /// All information about the tool, needed to download its asset after fetching /// the release and asset info. Values of this type are created in /// `src/sync/prefetch.rs` from `ToolInfo`. +#[derive(Debug)] pub struct ToolAsset { /// Name of the tool (e.g. "ripgrep" or "exa") pub tool_name: String, diff --git a/src/sync/install.rs b/src/sync/install.rs index 7868e66..cf15f45 100644 --- a/src/sync/install.rs +++ b/src/sync/install.rs @@ -82,7 +82,10 @@ impl<'a> Installer<'a> { match archive { None => Err(format!("Unsupported asset type: {}", tool_asset.asset.name).into()), Some(archive) => match archive.unpack() { - Err(unpack_err) => Err(unpack_err.to_string().into()), + Err(unpack_err) => { + println!("{:?}", &tool_asset); + Err(unpack_err.to_string().into()) + } Ok(tool_path) => { copy_file(tool_path, self.store_directory, &tool_asset.exe_name)?; Ok(()) diff --git a/src/sync/prefetch.rs b/src/sync/prefetch.rs index cd66507..78d6793 100644 --- a/src/sync/prefetch.rs +++ b/src/sync/prefetch.rs @@ -83,7 +83,14 @@ pub fn prefetch(tools: BTreeMap) -> Vec { .iter() .enumerate() .filter_map(|(index, (tool_name, config_asset))| { - prefetch_tool(tool_name, config_asset, &prefetch_progress, index) + //println!("{:?}", config_asset.proxy.clone()); + prefetch_tool( + tool_name, + config_asset, + &prefetch_progress, + index, + config_asset.proxy.clone(), + ) }) .collect(); @@ -105,6 +112,7 @@ fn prefetch_tool( config_asset: &ConfigAsset, prefetch_progress: &PrefetchProgress, current_index: usize, + proxy: Option, ) -> Option { // indexes start with 0 so we add 1 to calculate already fetched tools let already_completed = current_index + 1; @@ -120,6 +128,7 @@ fn prefetch_tool( owner: tool_info.owner.clone(), repo: tool_info.repo.clone(), version: tool_info.tag.to_str_version(), + proxy, }; match client.fetch_release_info() { From db0bbc65551d5788e036c7ef4a0c48a50c4bf86f Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Mon, 26 Sep 2022 09:58:33 +0200 Subject: [PATCH 2/7] corrected Accept header in asset download method --- src/config/toml.rs | 8 ++++++-- src/infra/client.rs | 11 ++--------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/config/toml.rs b/src/config/toml.rs index 718073b..cbb543e 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -95,8 +95,12 @@ fn decode_config_asset(table: &Map, proxy: &Option) -> Co proxy: None, }; if let Some(p) = proxy { - config_asset.proxy = Some(ureq::Proxy::new(p.clone()).unwrap_or_else(|_| panic!("Could not parse proxy address, please check the syntax: {}", - p))); + config_asset.proxy = Some(ureq::Proxy::new(p.clone()).unwrap_or_else(|_| { + panic!( + "Could not parse proxy address, please check the syntax: {}", + p + ) + })); }; config_asset } diff --git a/src/infra/client.rs b/src/infra/client.rs index 05bb850..fa76d65 100644 --- a/src/infra/client.rs +++ b/src/infra/client.rs @@ -71,24 +71,17 @@ impl Client { add_auth_header( agent .get(&asset_url) - .set("Accept", "application/vnd.github+json") + .set("Accept", "application/octet-stream") .set("User-Agent", "chshersh/tool-sync-0.2.0"), ) } None => add_auth_header( ureq::get(&asset_url) - .set("Accept", "application/vnd.github+json") + .set("Accept", "application/octet-stream") .set("User-Agent", "chshersh/tool-sync-0.2.0"), ), }; - //println!("{:?}", &self.proxy); - //let req = add_auth_header( - // ureq::get(&asset_url) - // .set("Accept", "application/octet-stream") - // .set("User-Agent", "chshersh/tool-sync-0.2.0"), - //); - Ok(req.call()?.into_reader()) } } From 4bd024f5de2e0069f07b586518549abfa1496820 Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Mon, 26 Sep 2022 11:15:48 +0200 Subject: [PATCH 3/7] added proxy flag that overwrites config proxy if passed --- src/config/cli.rs | 3 +++ src/config/toml.rs | 28 ++++++++++++++++++---------- src/infra/client.rs | 2 ++ src/install.rs | 4 ++-- src/lib.rs | 4 ++-- src/sync.rs | 6 +++--- 6 files changed, 30 insertions(+), 17 deletions(-) diff --git a/src/config/cli.rs b/src/config/cli.rs index 0a82a07..ff035a6 100644 --- a/src/config/cli.rs +++ b/src/config/cli.rs @@ -9,6 +9,9 @@ pub struct Cli { #[clap(short, long, value_name = "FILE")] pub config: Option, + #[clap(short, long, value_name = "uri")] + pub proxy: Option, + #[clap(subcommand)] pub command: Command, } diff --git a/src/config/toml.rs b/src/config/toml.rs index cbb543e..8863d3a 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -25,8 +25,12 @@ impl Display for TomlError { } } -pub fn with_parsed_file(config_path: PathBuf, on_success: F) { - match parse_file(&config_path) { +pub fn with_parsed_file( + config_path: PathBuf, + proxy: Option, + on_success: F, +) { + match parse_file(&config_path, proxy) { Ok(config) => { on_success(config); } @@ -40,28 +44,32 @@ pub fn with_parsed_file(config_path: PathBuf, on_success: F) } } -fn parse_file(config_path: &PathBuf) -> Result { +fn parse_file(config_path: &PathBuf, proxy: Option) -> Result { let contents = fs::read_to_string(config_path).map_err(|e| TomlError::IO(format!("{}", e)))?; - parse_string(&contents) + parse_string(&contents, proxy) } -fn parse_string(contents: &str) -> Result { +fn parse_string(contents: &str, proxy: Option) -> Result { contents .parse::() .map_err(TomlError::Parse) - .and_then(|toml| match decode_config(toml) { + .and_then(|toml| match decode_config(toml, proxy) { None => Err(TomlError::Decode), Some(config) => Ok(config), }) } -fn decode_config(toml: Value) -> Option { +fn decode_config(toml: Value, proxy: Option) -> Option { let str_store_directory = toml.get("store_directory")?.as_str()?; - let proxy: Option = match toml.get("proxy") { - Some(p) => Some(p.as_str()?.into()), - None => None, + let proxy = match proxy { + Some(p) => Some(p), + None => match toml.get("proxy") { + Some(p) => Some(p.as_str()?.into()), + None => None, + }, }; + let store_directory = String::from(str_store_directory); let mut tools = BTreeMap::new(); diff --git a/src/infra/client.rs b/src/infra/client.rs index fa76d65..f0107c6 100644 --- a/src/infra/client.rs +++ b/src/infra/client.rs @@ -25,6 +25,7 @@ impl Client { } fn asset_url(&self, asset_id: u32) -> String { + println!("{:?}", &self.proxy); format!( "https://api.github.com/repos/{owner}/{repo}/releases/assets/{asset_id}", owner = self.owner, @@ -34,6 +35,7 @@ impl Client { } pub fn fetch_release_info(&self) -> Result> { + println!("{:?}", &self.proxy); let release_url = self.release_url(); let req = match &self.proxy { diff --git a/src/install.rs b/src/install.rs index b612512..d52c072 100644 --- a/src/install.rs +++ b/src/install.rs @@ -7,8 +7,8 @@ use crate::sync; use crate::sync::db::{fmt_tool_names, lookup_tool}; /// Install a single tool -pub fn install(config_path: PathBuf, name: String) { - toml::with_parsed_file(config_path, |config| install_tool(config, name)) +pub fn install(config_path: PathBuf, name: String, proxy: Option) { + toml::with_parsed_file(config_path, proxy, |config| install_tool(config, name)) } /// Find if the tool is already mentioned in the config diff --git a/src/lib.rs b/src/lib.rs index f9d45cb..95924cf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,8 +19,8 @@ pub fn run() { match cli.command { Command::DefaultConfig => config::template::generate_default_config(), - Command::Sync { tool } => sync::sync_from_path(config_path, tool), - Command::Install { name } => install::install(config_path, name), + Command::Sync { tool } => sync::sync_from_path(config_path, tool, cli.proxy), + Command::Install { name } => install::install(config_path, name, cli.proxy), } } diff --git a/src/sync.rs b/src/sync.rs index 6641b2a..5eb1c62 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -18,8 +18,8 @@ use self::prefetch::prefetch; use self::progress::SyncProgress; use self::progress::ToolPair; -pub fn sync_from_path(config_path: PathBuf, tool: Option) { - toml::with_parsed_file(config_path.clone(), |config| { +pub fn sync_from_path(config_path: PathBuf, tool: Option, proxy: Option) { + toml::with_parsed_file(config_path.clone(), proxy, |config| { sync_from_config(config, config_path, tool) }); } @@ -67,7 +67,7 @@ fn tool_not_in_config_message(tool: &str, path: &Path) { eprintln!( r#"The '{}' tool is not listed in the configuration file: {} -Add the tool to the configuration file or use the 'tool install' command for +Add the tool to the configuration file or use the 'tool install' command for installing one of the tools natively supported by 'tool-sync'."#, tool, path.display(), From 58f89c94b0eb08b39ec1d232d5e37255962c5c52 Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Mon, 26 Sep 2022 11:47:46 +0200 Subject: [PATCH 4/7] updated tests, these tests don't account for any proxy so this feature remains untested --- src/config/toml.rs | 32 +++++++++++++++++++++----------- src/sync/configure.rs | 8 ++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/src/config/toml.rs b/src/config/toml.rs index 8863d3a..de40c6e 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -156,7 +156,7 @@ mod tests { #[test] fn test_toml_error_display_parse() { let broken_toml_str: String = "broken toml".into(); - match parse_string(&broken_toml_str) { + match parse_string(&broken_toml_str, None) { Err(error) => { assert_eq!( String::from( @@ -179,7 +179,7 @@ mod tests { fn test_parse_file_correct_output() { let result = std::panic::catch_unwind(|| { let test_config_path = PathBuf::from("tests/sync-full.toml"); - parse_file(&test_config_path).expect("This should not fail") + parse_file(&test_config_path, None).expect("This should not fail") }); if let Ok(config) = result { @@ -190,7 +190,7 @@ mod tests { #[test] fn test_parse_file_error() { let test_config_path = PathBuf::from("src/main.rs"); - match parse_file(&test_config_path) { + match parse_file(&test_config_path, None) { Ok(_) => { assert!(false, "Unexpected succces") } @@ -203,7 +203,7 @@ mod tests { #[test] fn empty_file() { let toml = ""; - let res = parse_string(toml); + let res = parse_string(toml, None); assert_eq!(res, Err(TomlError::Decode)); } @@ -211,7 +211,7 @@ mod tests { #[test] fn store_directory_is_dotted() { let toml = "store.directory = \"pancake\""; - let res = parse_string(toml); + let res = parse_string(toml, None); assert_eq!(res, Err(TomlError::Decode)); } @@ -219,7 +219,7 @@ mod tests { #[test] fn store_directory_is_a_number() { let toml = "store_directory = 42"; - let res = parse_string(toml); + let res = parse_string(toml, None); assert_eq!(res, Err(TomlError::Decode)); } @@ -227,11 +227,12 @@ mod tests { #[test] fn only_store_directory() { let toml = "store_directory = \"pancake\""; - let res = parse_string(toml); + let res = parse_string(toml, None); let cfg = Config { store_directory: String::from("pancake"), tools: BTreeMap::new(), + proxy: None, }; assert_eq!(res, Ok(cfg)); @@ -245,7 +246,7 @@ mod tests { [ripgrep] "#; - let res = parse_string(toml); + let res = parse_string(toml, None); let cfg = Config { store_directory: String::from("pancake"), @@ -261,8 +262,10 @@ mod tests { windows: None, }, tag: None, + proxy: None, }, )]), + proxy: None, }; assert_eq!(res, Ok(cfg)); @@ -277,7 +280,7 @@ mod tests { [bat] "#; - let res = parse_string(toml); + let res = parse_string(toml, None); let cfg = Config { store_directory: String::from("pancake"), @@ -294,6 +297,7 @@ mod tests { windows: None, }, tag: None, + proxy: None, }, ), ( @@ -308,9 +312,11 @@ mod tests { windows: None, }, tag: None, + proxy: None, }, ), ]), + proxy: None, }; assert_eq!(res, Ok(cfg)); @@ -326,7 +332,7 @@ mod tests { asset_name.linux = "R2D2" "#; - let res = parse_string(toml); + let res = parse_string(toml, None); let cfg = Config { store_directory: String::from("pancake"), @@ -342,8 +348,10 @@ mod tests { windows: None, }, tag: None, + proxy: None, }, )]), + proxy: None, }; assert_eq!(res, Ok(cfg)); @@ -364,7 +372,7 @@ mod tests { tag = "4.2.0" "#; - let res = parse_string(toml); + let res = parse_string(toml, None); let cfg = Config { store_directory: String::from("pancake"), @@ -380,8 +388,10 @@ mod tests { windows: Some("IG-88".to_owned()), }, tag: Some("4.2.0".to_owned()), + proxy: None, }, )]), + proxy: None, }; assert_eq!(res, Ok(cfg)); diff --git a/src/sync/configure.rs b/src/sync/configure.rs index 4bdcc50..fcf1780 100644 --- a/src/sync/configure.rs +++ b/src/sync/configure.rs @@ -114,6 +114,7 @@ mod tests { windows: None, }, tag: None, + proxy: None, }; assert_eq!( @@ -136,6 +137,7 @@ mod tests { windows: None, }, tag: None, + proxy: None, }; assert_eq!( @@ -158,6 +160,7 @@ mod tests { windows: None, }, tag: None, + proxy: None, }; assert_eq!( @@ -182,6 +185,7 @@ mod tests { windows: None, }, tag: Some(String::from("1.2.3")), + proxy: None, }; assert_eq!( @@ -204,6 +208,7 @@ mod tests { windows: Some(String::from("yours-windows")), }, tag: Some(String::from("1.2.3")), + proxy: None, }; assert_eq!( @@ -236,6 +241,7 @@ mod tests { windows: Some(String::from("yours-windows")), }, tag: Some(String::from("1.0.0")), + proxy: None, }; assert_eq!( @@ -268,6 +274,7 @@ mod tests { windows: None, }, tag: None, + proxy: None, }; assert_eq!( @@ -300,6 +307,7 @@ mod tests { windows: Some(String::from("yours-windows")), }, tag: Some(String::from("3.2.1")), + proxy: None, }; assert_eq!( From 32f4b0e2f460d5f5908c04813cf5df935ff2b5d5 Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Tue, 27 Sep 2022 12:24:55 +0200 Subject: [PATCH 5/7] updated some comments: --- src/config/schema.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/config/schema.rs b/src/config/schema.rs index 6b48ef2..0825c07 100644 --- a/src/config/schema.rs +++ b/src/config/schema.rs @@ -40,7 +40,7 @@ pub struct ConfigAsset { /// Name of the specific asset to download pub asset_name: AssetName, - /// Proxy which will get used for all communication except hardcoded tools + /// Proxy which will get used for all communication pub proxy: Option, } @@ -57,6 +57,8 @@ impl From for ConfigAsset { exe_name: Some(tool_info.exe_name), tag, asset_name: tool_info.asset_name, + + /// Hardcoded tools don't supply their own proxy automatically proxy: None, } } From cdd8c8f778fecea380106c78b9939ceb77378c7e Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Tue, 27 Sep 2022 12:28:53 +0200 Subject: [PATCH 6/7] added changelog change --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b638e0d..49d04c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,9 @@ available [on GitHub][2]. Adds the `tool sync ` command to install only one tool from the configuration file (by [@zixuanzhang-x][zixuanzhang-x]) +* [#82](https://github.com/chshersh/tool-sync/issues/82): + Adds proxy as a flag and config option + (by [@MitchellBerend][MitchellBerend]) ### Fixed From 32f80cda691e5a27c67daf94986efa30edd88486 Mon Sep 17 00:00:00 2001 From: Mitchell Berendhuysen Date: Sat, 1 Oct 2022 11:15:35 +0200 Subject: [PATCH 7/7] incorporated feedback --- src/config/toml.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/config/toml.rs b/src/config/toml.rs index de40c6e..84f8123 100644 --- a/src/config/toml.rs +++ b/src/config/toml.rs @@ -62,13 +62,10 @@ fn parse_string(contents: &str, proxy: Option) -> Result) -> Option { let str_store_directory = toml.get("store_directory")?.as_str()?; - let proxy = match proxy { - Some(p) => Some(p), - None => match toml.get("proxy") { - Some(p) => Some(p.as_str()?.into()), - None => None, - }, - }; + let proxy: Option = proxy.or_else(|| match toml.get("proxy") { + Some(p) => Some(p.as_str().unwrap_or("").into()), + None => None, + }); let store_directory = String::from(str_store_directory);