From 6a7e64d04a8b0ad932169df43e2d93efe8ceedf8 Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 18 Aug 2026 07:18:07 +0900 Subject: fix: propagate ported exceptions instead of flattening them `ProcessExecutor::execute_args` existed only to turn `execute`'s `anyhow::Result` into an exit code of 1, so every one of its ~87 call sites silently took the "command failed" branch on an error PHP would have thrown. It is gone; callers use `execute` and propagate with `?`. Where the enclosing function had no `Result` to propagate into, its signature grew one, up to and including `Git::get_version`, `Svn::binary_version`, `GitHub`/`GitLab`/`Bitbucket::authorize_oauth`, `InitCommand::get_git_config` and `DiagnoseCommand::check_git`. The VCS drivers had the same problem in the other direction: their `get_contents` returned `Result>`, a type too narrow for the PHP method, which lets any Throwable out of the `catch (TransportException $e)` block. Every non-transport error was therefore rewritten into a `TransportException` with code 0, which the callers switch on. They now return `anyhow::Result>>`: the outer `Result` carries what PHP does not catch, the inner one the exception the drivers handle. That signature also restores `GitLabDriver::getContents`: the 400/401 `TransportException`s it raises to force authentication are thrown inside its own `try` block and handled by its own `catch`, but the port returned them straight to the caller, so the authentication flow behind them never ran. `impl_php_exception!` gains `From> for anyhow::Error` so a caught exception can be re-propagated with `?`. Co-Authored-By: Claude Opus 5 (1M context) --- crates/shirabe-php-shim/src/exception.rs | 6 + crates/shirabe/src/command/diagnose_command.rs | 23 ++- crates/shirabe/src/command/init_command.rs | 50 ++--- crates/shirabe/src/downloader/fossil_downloader.rs | 4 +- crates/shirabe/src/downloader/git_downloader.rs | 94 ++++----- crates/shirabe/src/downloader/hg_downloader.rs | 14 +- crates/shirabe/src/downloader/svn_downloader.rs | 14 +- crates/shirabe/src/package/locker.rs | 8 +- .../shirabe/src/package/version/version_guesser.rs | 40 ++-- crates/shirabe/src/repository/path_repository.rs | 6 +- .../shirabe/src/repository/vcs/forgejo_driver.rs | 114 +++++------ crates/shirabe/src/repository/vcs/fossil_driver.rs | 40 ++-- .../src/repository/vcs/git_bitbucket_driver.rs | 69 ++++--- crates/shirabe/src/repository/vcs/git_driver.rs | 26 +-- crates/shirabe/src/repository/vcs/github_driver.rs | 64 +++---- crates/shirabe/src/repository/vcs/gitlab_driver.rs | 212 ++++++++++----------- crates/shirabe/src/repository/vcs/hg_driver.rs | 38 ++-- .../shirabe/src/repository/vcs/perforce_driver.rs | 5 +- crates/shirabe/src/repository/vcs/svn_driver.rs | 6 +- crates/shirabe/src/repository/vcs/vcs_driver.rs | 39 ++-- crates/shirabe/src/util/auth_helper.rs | 6 +- crates/shirabe/src/util/bitbucket.rs | 23 +-- crates/shirabe/src/util/filesystem.rs | 10 +- crates/shirabe/src/util/git.rs | 98 +++++----- crates/shirabe/src/util/github.rs | 14 +- crates/shirabe/src/util/gitlab.rs | 24 +-- crates/shirabe/src/util/hg.rs | 49 ++--- crates/shirabe/src/util/perforce.rs | 9 +- crates/shirabe/src/util/platform.rs | 2 +- crates/shirabe/src/util/process_executor.rs | 32 +--- crates/shirabe/src/util/svn.rs | 8 +- crates/shirabe/tests/command/init_command_test.rs | 2 +- crates/shirabe/tests/util/bitbucket_test.rs | 10 +- crates/shirabe/tests/util/git_test.rs | 4 - crates/shirabe/tests/util/perforce_test.rs | 6 +- 35 files changed, 568 insertions(+), 601 deletions(-) diff --git a/crates/shirabe-php-shim/src/exception.rs b/crates/shirabe-php-shim/src/exception.rs index f87ce7c8..281058de 100644 --- a/crates/shirabe-php-shim/src/exception.rs +++ b/crates/shirabe-php-shim/src/exception.rs @@ -269,6 +269,12 @@ macro_rules! impl_php_exception { ::anyhow::Error::new($crate::AnyThrowable::new(exception)) } } + + impl From> for ::anyhow::Error { + fn from(exception: Box<$ty>) -> Self { + ::anyhow::Error::new($crate::AnyThrowable::new(*exception)) + } + } }; // For an exception the port cannot let travel as a Rust error, because its state is not // `Send + Sync`. It gets the accessors but no [`Throwable`], so asking for it in a `catch` diff --git a/crates/shirabe/src/command/diagnose_command.rs b/crates/shirabe/src/command/diagnose_command.rs index 860c058b..9c1fb2f8 100644 --- a/crates/shirabe/src/command/diagnose_command.rs +++ b/crates/shirabe/src/command/diagnose_command.rs @@ -119,9 +119,11 @@ impl DiagnoseCommand { Ok(PhpMixed::Bool(true)) } - fn check_git(&self) -> String { + fn check_git(&self) -> anyhow::Result { if !shirabe_php_rpc::get_diagnostics().function_exists("proc_open") { - return "proc_open is not available, git cannot be used".to_string(); + return Ok( + "proc_open is not available, git cannot be used".to_string(), + ); } let mut output = String::new(); @@ -141,24 +143,27 @@ impl DiagnoseCommand { None, ); if strtolower(&trim(&output, Some(" \t\n\r\0\u{0B}"))) == "always" { - return "Your git color.ui setting is set to always, this is known to create issues. Use \"git config --global color.ui true\" to set it correctly.".to_string(); + return Ok("Your git color.ui setting is set to always, this is known to create issues. Use \"git config --global color.ui true\" to set it correctly.".to_string()); } let process = self.process.borrow(); - let git_version = Git::get_version(process.as_ref().unwrap()); + let git_version = Git::get_version(process.as_ref().unwrap())?; let git_version = match git_version { Some(v) => v, - None => return "No git process found".to_string(), + None => return Ok("No git process found".to_string()), }; if version_compare("2.24.0", &git_version, CmpOp::Gt) { - return format!( + return Ok(format!( "Your git version ({}) is too old and possibly will cause issues. Please upgrade to git 2.24 or above", git_version - ); + )); } - format!("OK git version {}", git_version) + Ok(format!( + "OK git version {}", + git_version + )) } fn check_http( @@ -1325,7 +1330,7 @@ impl Command for DiagnoseCommand { self.output_result(r); io.write_no_newline("Checking git settings: "); - let r = self.check_git(); + let r = self.check_git()?; self.output_result(PhpMixed::String(r)); io.write_no_newline("Checking http connectivity to packagist: "); diff --git a/crates/shirabe/src/command/init_command.rs b/crates/shirabe/src/command/init_command.rs index add7c237..2205b852 100644 --- a/crates/shirabe/src/command/init_command.rs +++ b/crates/shirabe/src/command/init_command.rs @@ -152,19 +152,19 @@ impl InitCommand { Some(implode("\\", &namespace)) } - fn get_git_config(&self) -> IndexMap { + fn get_git_config(&self) -> anyhow::Result> { if self.git_config.borrow().is_some() { - return self.git_config.borrow().clone().unwrap_or_default(); + return Ok(self.git_config.borrow().clone().unwrap_or_default()); } let mut process = ProcessExecutor::new(Some(self.get_io().clone())); let mut output = String::new(); - if process.execute_args( + if process.execute( &["git".to_string(), "config".to_string(), "-l".to_string()], &mut output, None, - ) == 0 + )? == 0 { *self.git_config.borrow_mut() = Some(IndexMap::new()); for m in preg_match_all(php_regex!(r"{^([^=]+)=(.*)$}m"), &output) { @@ -178,11 +178,11 @@ impl InitCommand { ); } - return self.git_config.borrow().clone().unwrap_or_default(); + return Ok(self.git_config.borrow().clone().unwrap_or_default()); } *self.git_config.borrow_mut() = Some(IndexMap::new()); - IndexMap::new() + Ok(IndexMap::new()) } /// Checks the local .gitignore file for the Composer vendor directory. @@ -241,7 +241,7 @@ impl InitCommand { } /// For testing only: invoke the crate-private `get_git_config`. - pub fn __get_git_config(&self) -> IndexMap { + pub fn __get_git_config(&self) -> anyhow::Result> { self.get_git_config() } @@ -333,8 +333,8 @@ impl InitCommand { preg_replace(php_regex!(r"{([_.-]){2,}}u"), "$1", &name) } - fn get_default_package_name(&self) -> String { - let git = self.get_git_config(); + fn get_default_package_name(&self) -> anyhow::Result { + let git = self.get_git_config()?; let cwd = realpath(".").unwrap_or_default(); let name = basename(&cwd); let name = self.sanitize_package_name_component(&name); @@ -384,11 +384,11 @@ impl InitCommand { let vendor = self.sanitize_package_name_component(&vendor); - format!("{}/{}", vendor, name) + Ok(format!("{}/{}", vendor, name)) } - fn get_default_author(&self) -> Option { - let git = self.get_git_config(); + fn get_default_author(&self) -> anyhow::Result> { + let git = self.get_git_config()?; let mut author_name: Option = None; let composer_default_author = PHP_SERVER @@ -425,10 +425,10 @@ impl InitCommand { } if let (Some(name), Some(email)) = (author_name, author_email) { - return Some(format!("{} <{}>", name, email)); + return Ok(Some(format!("{} <{}>", name, email))); } - None + Ok(None) } } @@ -753,7 +753,7 @@ impl Command for InitCommand { if !input.borrow().is_interactive() { if input.borrow().get_option("name")?.is_null() { - let name = self.get_default_package_name(); + let name = self.get_default_package_name()?; input .borrow_mut() .set_option("name", PhpMixed::from(name)) @@ -761,7 +761,7 @@ impl Command for InitCommand { } if input.borrow().get_option("author")?.is_null() { - let author = self.get_default_author(); + let author = self.get_default_author()?; input .borrow_mut() .set_option("author", PhpMixed::from(author)) @@ -881,12 +881,15 @@ impl Command for InitCommand { io_interface::NORMAL, ); - let mut name = input + let name_option = input .borrow() .get_option("name")? .as_string() - .map(|s| s.to_string()) - .unwrap_or_else(|| self.get_default_package_name()); + .map(|s| s.to_string()); + let mut name = match name_option { + Some(name) => name, + None => self.get_default_package_name()?, + }; let name_default = name.clone(); let name_for_validate = name.clone(); @@ -943,12 +946,15 @@ impl Command for InitCommand { )?; input.borrow_mut().set_option("description", description); - let author = input + let author_option = input .borrow() .get_option("author")? .as_string() - .map(|s| s.to_string()) - .unwrap_or_else(|| self.get_default_author().unwrap_or_default()); + .map(|s| s.to_string()); + let author = match author_option { + Some(author) => author, + None => self.get_default_author()?.unwrap_or_default(), + }; let author_for_validate = author.clone(); let author_default = author.clone(); diff --git a/crates/shirabe/src/downloader/fossil_downloader.rs b/crates/shirabe/src/downloader/fossil_downloader.rs index 96886ca0..d04529b3 100644 --- a/crates/shirabe/src/downloader/fossil_downloader.rs +++ b/crates/shirabe/src/downloader/fossil_downloader.rs @@ -254,11 +254,11 @@ impl ChangeReportInterface for FossilDownloader { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &["fossil".to_string(), "changes".to_string()], &mut output, shirabe_php_shim::realpath(path).as_deref(), - ); + )?; let output = output.trim().to_string(); diff --git a/crates/shirabe/src/downloader/git_downloader.rs b/crates/shirabe/src/downloader/git_downloader.rs index 2d506fdf..cf1bfd82 100644 --- a/crates/shirabe/src/downloader/git_downloader.rs +++ b/crates/shirabe/src/downloader/git_downloader.rs @@ -82,7 +82,7 @@ impl GitDownloader { .inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(&path)) + .execute(&command, &mut output, Some(&path))? != 0 { return Err(RuntimeException::new(format!( @@ -173,11 +173,11 @@ impl GitDownloader { "--".to_string(), ]; let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &command, &mut output, Some(&path), - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "Failed to execute {}\n\n{}", @@ -201,11 +201,11 @@ impl GitDownloader { // remotes and then try again as outdated remotes can sometimes cause false-positives if unpushed_changes.is_some() && i == 0 { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &["git".to_string(), "fetch".to_string(), "--all".to_string()], &mut output, Some(&path), - ); + )?; // update list of refs after fetching let command = vec![ @@ -219,7 +219,7 @@ impl GitDownloader { .inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(&path)) + .execute(&command, &mut output, Some(&path))? != 0 { return Err(RuntimeException::new(format!( @@ -288,11 +288,11 @@ impl GitDownloader { let mut branches: Option = None; { let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "branch".to_string(), "-r".to_string()], &mut output, Some(path), - ) == 0 + )? == 0 { branches = Some(output); } @@ -328,14 +328,14 @@ impl GitDownloader { self.inner .process .borrow_mut() - .execute_args(&command1, &mut output, Some(path)) + .execute(&command1, &mut output, Some(path))? == 0; let ok2 = if ok1 { let mut output = String::new(); self.inner .process .borrow_mut() - .execute_args(&command2, &mut output, Some(path)) + .execute(&command2, &mut output, Some(path))? == 0 } else { false @@ -388,25 +388,25 @@ impl GitDownloader { self.inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(path)) + .execute(&command, &mut output, Some(path))? == 0; let ok_fallback = if !ok_command { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &fallback_command, &mut output, Some(path), - ) == 0 + )? == 0 } else { false }; let ok_reset = if ok_command || ok_fallback { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( - &reset_command, - &mut output, - Some(path), - ) == 0 + self.inner + .process + .borrow_mut() + .execute(&reset_command, &mut output, Some(path))? + == 0 } else { false }; @@ -431,14 +431,14 @@ impl GitDownloader { self.inner .process .borrow_mut() - .execute_args(&command1, &mut output, Some(path)) + .execute(&command1, &mut output, Some(path))? == 0; let ok2 = if ok1 { let mut output = String::new(); self.inner .process .borrow_mut() - .execute_args(&command2, &mut output, Some(path)) + .execute(&command2, &mut output, Some(path))? == 0 } else { false @@ -482,9 +482,9 @@ impl GitDownloader { .into()) } - fn update_origin_url(&self, path: &str, url: &str) { + fn update_origin_url(&self, path: &str, url: &str) -> anyhow::Result<()> { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "remote".to_string(), @@ -495,11 +495,11 @@ impl GitDownloader { ], &mut output, Some(path), - ); - self.set_push_url(path, url); + )?; + self.set_push_url(path, url) } - fn set_push_url(&self, path: &str, url: &str) { + fn set_push_url(&self, path: &str, url: &str) -> anyhow::Result<()> { // set push url for github projects if let Some(match_) = preg_match( format!( @@ -529,30 +529,32 @@ impl GitDownloader { self.inner .process .borrow_mut() - .execute_args(&cmd, &mut ignored_output, Some(path)); + .execute(&cmd, &mut ignored_output, Some(path))?; } + + Ok(()) } /// @throws \RuntimeException async fn discard_changes(&self, path: &str) -> anyhow::Result> { let path = self.normalize_path(path); let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "clean".to_string(), "-df".to_string()], &mut output, Some(&path), - ) != 0 + )? != 0 { return Err( RuntimeException::new(format!("Could not reset changes\n\n:{}", output)).into(), ); } let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "reset".to_string(), "--hard".to_string()], &mut output, Some(&path), - ) != 0 + )? != 0 { return Err( RuntimeException::new(format!("Could not reset changes\n\n:{}", output)).into(), @@ -568,7 +570,7 @@ impl GitDownloader { async fn stash_changes(&self, path: &str) -> anyhow::Result> { let path = self.normalize_path(path); let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &[ "git".to_string(), "stash".to_string(), @@ -576,7 +578,7 @@ impl GitDownloader { ], &mut output, Some(&path), - ) != 0 + )? != 0 { return Err( RuntimeException::new(format!("Could not stash changes\n\n:{}", output)).into(), @@ -592,11 +594,11 @@ impl GitDownloader { fn view_diff(&self, path: &str) -> anyhow::Result<()> { let path = self.normalize_path(path); let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "diff".to_string(), "HEAD".to_string()], &mut output, Some(&path), - ) != 0 + )? != 0 { return Err( RuntimeException::new(format!("Could not view diff\n\n:{}", output)).into(), @@ -700,7 +702,7 @@ impl ChangeReportInterface for GitDownloader { .inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(path)) + .execute(&command, &mut output, Some(path))? != 0 { return Err(RuntimeException::new(format!( @@ -772,7 +774,7 @@ impl VcsDownloader for GitDownloader { .unwrap_or(""), preg_replace(r"{[^a-z0-9.]}i", "-", &Url::sanitize(url.to_string())), ); - let git_version = GitUtil::get_version(&self.inner.process); + let git_version = GitUtil::get_version(&self.inner.process)?; // --dissociate option is only available since git 2.3.0-rc0 if git_version.is_some() @@ -1026,7 +1028,7 @@ impl VcsDownloader for GitDownloader { self.inner.io.write_error3(&msg, true, io_interface::NORMAL); let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &[ "git".to_string(), "rev-parse".to_string(), @@ -1036,7 +1038,7 @@ impl VcsDownloader for GitDownloader { ], &mut output, Some(&path), - ) != 0 + )? != 0 { let commands = vec![ vec![ @@ -1089,11 +1091,11 @@ impl VcsDownloader for GitDownloader { let mut update_origin_url = false; let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "remote".to_string(), "-v".to_string()], &mut output, Some(&path), - ) == 0 + )? == 0 && let Some(origin_match) = preg_match(php_regex!(r"{^origin\s+(?P\S+)}m"), &output) && let Some(composer_match) = @@ -1287,11 +1289,11 @@ impl VcsDownloader for GitDownloader { io_interface::NORMAL, ); let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &["git".to_string(), "stash".to_string(), "pop".to_string()], &mut output, Some(&path), - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "Failed to apply stashed changes:\n\n{}", @@ -1316,15 +1318,15 @@ impl VcsDownloader for GitDownloader { "--format=%h - %an: %s".to_string(), format!("{}..{}", from_reference, to_reference), ]; - args.extend(GitUtil::get_no_show_signature_flags(&self.inner.process)); - let command = GitUtil::build_rev_list_command(&self.inner.process, args); + args.extend(GitUtil::get_no_show_signature_flags(&self.inner.process)?); + let command = GitUtil::build_rev_list_command(&self.inner.process, args)?; let mut output = String::new(); if self .inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(&path)) + .execute(&command, &mut output, Some(&path))? != 0 { return Err(RuntimeException::new(format!( @@ -1335,7 +1337,7 @@ impl VcsDownloader for GitDownloader { .into()); } - Ok(GitUtil::parse_rev_list_output(&output, &self.inner.process)) + GitUtil::parse_rev_list_output(&output, &self.inner.process) } fn has_metadata_repository(&self, path: &str) -> bool { diff --git a/crates/shirabe/src/downloader/hg_downloader.rs b/crates/shirabe/src/downloader/hg_downloader.rs index dfd25618..a36b4c57 100644 --- a/crates/shirabe/src/downloader/hg_downloader.rs +++ b/crates/shirabe/src/downloader/hg_downloader.rs @@ -63,7 +63,7 @@ impl VcsDownloader for HgDownloader { _url: &str, _prev_package: Option, ) -> anyhow::Result> { - if HgUtils::get_version(&self.inner.process).is_none() { + if HgUtils::get_version(&self.inner.process)?.is_none() { return Err(RuntimeException::new( "hg was not found in your PATH, skipping source download".to_string(), ) @@ -104,11 +104,11 @@ impl VcsDownloader for HgDownloader { package.get_source_reference().unwrap_or_default(), ]; let mut ignored_output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &command, &mut ignored_output, shirabe_php_shim::realpath(path).as_deref(), - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "Failed to execute {}\n\n{}", @@ -182,11 +182,11 @@ impl VcsDownloader for HgDownloader { ]; let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( &command, &mut output, shirabe_php_shim::realpath(path).as_deref(), - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "Failed to execute {}\n\n{}", @@ -215,11 +215,11 @@ impl ChangeReportInterface for HgDownloader { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &["hg".to_string(), "st".to_string()], &mut output, shirabe_php_shim::realpath(path).as_deref(), - ); + )?; let output = output.trim().to_string(); diff --git a/crates/shirabe/src/downloader/svn_downloader.rs b/crates/shirabe/src/downloader/svn_downloader.rs index 93b860f0..8c062320 100644 --- a/crates/shirabe/src/downloader/svn_downloader.rs +++ b/crates/shirabe/src/downloader/svn_downloader.rs @@ -69,11 +69,11 @@ impl SvnDownloader { async fn discard_changes(&self, path: &str) -> anyhow::Result> { let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["svn", "revert", "-R", "."].map(|s| s.to_string()).as_ref(), &mut output, Some(path), - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "Could not reset changes\n\n:{}", @@ -139,7 +139,7 @@ impl VcsDownloader for SvnDownloader { self.inner.config.clone(), Some(self.inner.process.clone()), ); - if util.binary_version().is_none() { + if util.binary_version()?.is_none() { return Err(RuntimeException::new( "svn was not found in your PATH, skipping source download".to_string(), ) @@ -217,7 +217,7 @@ impl VcsDownloader for SvnDownloader { ); let mut flags: Vec = vec![]; if version_compare( - &util.binary_version().unwrap_or_default(), + &util.binary_version()?.unwrap_or_default(), "1.7.0", CmpOp::Ge, ) { @@ -370,7 +370,7 @@ impl VcsDownloader for SvnDownloader { .inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(path)) + .execute(&command, &mut output, Some(path))? != 0 { return Err(RuntimeException::new(format!( @@ -444,13 +444,13 @@ impl ChangeReportInterface for SvnDownloader { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["svn", "status", "--ignore-externals"] .map(|s| s.to_string()) .as_ref(), &mut output, Some(path), - ); + )?; Ok(if preg_is_match(php_regex!("{^ *[^X ] +}m"), &output) { Some(output) diff --git a/crates/shirabe/src/package/locker.rs b/crates/shirabe/src/package/locker.rs index 36edd29f..d6db1ab7 100644 --- a/crates/shirabe/src/package/locker.rs +++ b/crates/shirabe/src/package/locker.rs @@ -798,14 +798,14 @@ impl Locker { .unwrap_or_default(); match source_type.as_deref().unwrap_or("") { "git" => { - GitUtil::clean_env(&self.process); + GitUtil::clean_env(&self.process)?; let no_show_signature_flags = - GitUtil::get_no_show_signature_flags(&self.process); + GitUtil::get_no_show_signature_flags(&self.process)?; let mut args: Vec = vec!["-n1".to_string(), "--format=%ct".to_string(), source_ref]; args.extend(no_show_signature_flags); - let command = GitUtil::build_rev_list_command(&self.process, args); + let command = GitUtil::build_rev_list_command(&self.process, args)?; let mut output = PhpMixed::Null; if 0 == self.process.borrow_mut().execute( command, @@ -816,7 +816,7 @@ impl Locker { &GitUtil::parse_rev_list_output( output.as_string().unwrap_or(""), &self.process, - ), + )?, None, ); if preg_is_match(php_regex!(r"{^\s*\d+\s*$}"), &output_str) { diff --git a/crates/shirabe/src/package/version/version_guesser.rs b/crates/shirabe/src/package/version/version_guesser.rs index 38c4f0a3..ebd7ade8 100644 --- a/crates/shirabe/src/package/version/version_guesser.rs +++ b/crates/shirabe/src/package/version/version_guesser.rs @@ -206,7 +206,7 @@ impl VersionGuesser { // try to fetch current version from git branch let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &[ "git".to_string(), "branch".to_string(), @@ -217,7 +217,7 @@ impl VersionGuesser { ], &mut output, Some(path), - ) { + )? { let mut branches: Vec = vec![]; let mut is_feature_branch = false; @@ -309,7 +309,7 @@ impl VersionGuesser { PhpMixed::String("HEAD".to_string()), ]), PhpMixed::List( - GitUtil::get_no_show_signature_flags(&self.process) + GitUtil::get_no_show_signature_flags(&self.process)? .into_iter() .map(PhpMixed::String) .collect(), @@ -322,15 +322,15 @@ impl VersionGuesser { .collect() }) .unwrap_or_default(), - ); + )?; let mut command_output = String::new(); - if 0 == self.process.borrow_mut().execute_args( - &command, - &mut command_output, - Some(path), - ) { + if 0 == self + .process + .borrow_mut() + .execute(&command, &mut command_output, Some(path))? + { let parsed = trim( - &GitUtil::parse_rev_list_output(&command_output, &self.process), + &GitUtil::parse_rev_list_output(&command_output, &self.process)?, None, ); commit = if parsed.is_empty() { @@ -353,7 +353,7 @@ impl VersionGuesser { fn version_from_git_tags(&mut self, path: &str) -> anyhow::Result> { // try to fetch current version from git tags let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &[ "git".to_string(), "describe".to_string(), @@ -362,7 +362,7 @@ impl VersionGuesser { ], &mut output, Some(path), - ) { + )? { match self.version_parser.normalize(&trim(&output, None), None) { Ok(version) => return Ok(Some((version, trim(&output, None)))), Err(_e) => {} @@ -379,11 +379,11 @@ impl VersionGuesser { ) -> anyhow::Result> { // try to fetch current version from hg branch let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &["hg".to_string(), "branch".to_string()], &mut output, Some(path), - ) { + )? { let branch = trim(&output, None); let version = self.version_parser.normalize_branch(&branch)?; let is_feature_branch = strpos(&version, "dev-") == Some(0); @@ -617,7 +617,7 @@ impl VersionGuesser { // try to fetch current version from fossil let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &[ "fossil".to_string(), "branch".to_string(), @@ -625,7 +625,7 @@ impl VersionGuesser { ], &mut output, Some(path), - ) { + )? { let branch = trim(&output, None); version = Some(self.version_parser.normalize_branch(&branch)?); pretty_version = Some(format!("dev-{}", branch)); @@ -633,11 +633,11 @@ impl VersionGuesser { // try to fetch current version from fossil tags let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &["fossil".to_string(), "tag".to_string(), "list".to_string()], &mut output, Some(path), - ) { + )? { match self.version_parser.normalize(&trim(&output, None), None) { Ok(v) => { version = Some(v); @@ -665,11 +665,11 @@ impl VersionGuesser { // try to fetch current version from svn let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &["svn".to_string(), "info".to_string(), "--xml".to_string()], &mut output, Some(path), - ) { + )? { let trunk_path = package_config .get("trunk-path") .and_then(|v| v.as_string()) diff --git a/crates/shirabe/src/repository/path_repository.rs b/crates/shirabe/src/repository/path_repository.rs index d619865b..17f22d40 100644 --- a/crates/shirabe/src/repository/path_repository.rs +++ b/crates/shirabe/src/repository/path_repository.rs @@ -284,9 +284,9 @@ impl PathRepository { "--format=%H".to_string(), "HEAD".to_string(), ]; - args.extend(GitUtil::get_no_show_signature_flags(&self.process)); + args.extend(GitUtil::get_no_show_signature_flags(&self.process)?); args - }); + })?; if reference == "auto" && shirabe_php_shim::is_dir(format!("{}/.git", path.trim_end_matches('/'))) && self @@ -297,7 +297,7 @@ impl PathRepository { == 0 { let output_str = output.as_string().unwrap_or("").to_string(); - let ref_val = GitUtil::parse_rev_list_output(&output_str, &self.process) + let ref_val = GitUtil::parse_rev_list_output(&output_str, &self.process)? .trim() .to_string(); if let Some(PhpMixed::Array(dist)) = package.get_mut("dist") { diff --git a/crates/shirabe/src/repository/vcs/forgejo_driver.rs b/crates/shirabe/src/repository/vcs/forgejo_driver.rs index 4238be8f..8c5cb345 100644 --- a/crates/shirabe/src/repository/vcs/forgejo_driver.rs +++ b/crates/shirabe/src/repository/vcs/forgejo_driver.rs @@ -106,9 +106,7 @@ impl ForgejoDriver { file, urlencode(identifier) ); - let response = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&resource_url, false)??; let mut resource = response.decode_json()?; // The Forgejo contents API only returns files up to 1MB as base64 encoded files; @@ -133,10 +131,7 @@ impl ForgejoDriver { None }; if let Some(git_url) = git_url { - resource = self - .get_contents(&git_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + resource = self.get_contents(&git_url, false)??.decode_json()?; } } @@ -192,10 +187,7 @@ impl ForgejoDriver { api_url, urlencode(identifier) ); - let commit = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let commit = self.get_contents(&resource_url, false)??.decode_json()?; let date_str = if let PhpMixed::Array(ref arr) = commit { arr.get("commit") @@ -241,9 +233,7 @@ impl ForgejoDriver { let mut resource: Option = Some(format!("{}/branches?per_page=100", api_url)); while let Some(url) = resource { - let response = self - .get_contents(&url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&url, false)??; let branch_data = response.decode_json()?; if let PhpMixed::List(ref list) = branch_data { for branch in list { @@ -284,9 +274,7 @@ impl ForgejoDriver { let mut resource: Option = Some(format!("{}/tags?per_page=100", api_url)); while let Some(url) = resource { - let response = self - .get_contents(&url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(&url, false)??; let tags_data = response.decode_json()?; if let PhpMixed::List(ref list) = tags_data { for tag in list { @@ -558,7 +546,7 @@ impl ForgejoDriver { } let api_url = self.forgejo_url.as_ref().unwrap().api_url.clone(); - match self.get_contents(&api_url, true) { + match self.get_contents(&api_url, true)? { Err(_) => { if self.git_driver.is_some() { return Ok(()); @@ -598,56 +586,56 @@ impl ForgejoDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - match self.inner.get_contents(url) { - Ok(response) => Ok(response), - Err(e) => match e.get_code() { - 401 | 403 | 404 | 429 => { - if !fetching_repo_data { - return Err(e); - } + ) -> anyhow::Result>> { + let e = match self.inner.get_contents(url)? { + Ok(response) => return Ok(Ok(response)), + Err(e) => e, + }; - if !self.inner.io.is_interactive() { - self.attempt_clone_fallback() - .map_err(|inner_e| TransportException::new(inner_e.to_string(), 0))?; + match e.get_code() { + 401 | 403 | 404 | 429 => { + if !fetching_repo_data { + return Ok(Err(e)); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } + if !self.inner.io.is_interactive() { + self.attempt_clone_fallback()?; - if !self.inner.io.has_authentication(&self.inner.origin_url) { - let origin_url = self.forgejo_url.as_ref().unwrap().origin_url.clone(); - let message = if e.get_code() == 429 { - Some(format!( - "API limit exhausted. Enter your Forgejo credentials to get a larger API limit ({})", - self.inner.url - )) - } else { - None - }; - - let mut forgejo = Forgejo::new( - self.inner.io.clone(), - self.inner.config.clone(), - self.inner.http_downloader.clone(), - ); - let auth_result = forgejo - .authorize_o_auth_interactively(&origin_url, message.as_deref()) - .map_err(|inner_e| TransportException::new(inner_e.to_string(), 0))?; - - if let Ok(true) = auth_result { - return self.inner.get_contents(url); - } - } + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); + } - Err(e) + if !self.inner.io.has_authentication(&self.inner.origin_url) { + let origin_url = self.forgejo_url.as_ref().unwrap().origin_url.clone(); + let message = if e.get_code() == 429 { + Some(format!( + "API limit exhausted. Enter your Forgejo credentials to get a larger API limit ({})", + self.inner.url + )) + } else { + None + }; + + let mut forgejo = Forgejo::new( + self.inner.io.clone(), + self.inner.config.clone(), + self.inner.http_downloader.clone(), + ); + let auth_result = + forgejo.authorize_o_auth_interactively(&origin_url, message.as_deref())?; + + if let Ok(true) = auth_result { + return self.inner.get_contents(url); + } } - _ => Err(e), - }, + + Ok(Err(e)) + } + _ => Ok(Err(e)), } } diff --git a/crates/shirabe/src/repository/vcs/fossil_driver.rs b/crates/shirabe/src/repository/vcs/fossil_driver.rs index 9a678284..1824ad5f 100644 --- a/crates/shirabe/src/repository/vcs/fossil_driver.rs +++ b/crates/shirabe/src/repository/vcs/fossil_driver.rs @@ -97,11 +97,11 @@ impl FossilDriver { fn check_fossil(&self) -> anyhow::Result<()> { let mut ignored_output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "version"].map(|s| s.to_string()).as_ref(), &mut ignored_output, None, - ) != 0 + )? != 0 { return Err(RuntimeException::new(format!( "fossil was not found, check that it is installed and in your PATH env.\n\n{}", @@ -131,17 +131,17 @@ impl FossilDriver { // update the repo if it is a valid fossil repository if is_file(&repo_file) && is_dir(&self.checkout_dir) - && self.inner.process.borrow_mut().execute_args( + && self.inner.process.borrow_mut().execute( ["fossil", "info"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.checkout_dir), - ) == 0 + )? == 0 { - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "pull"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.checkout_dir), - ) != 0 + )? != 0 { self.inner.io.write_error3(&format!( "Failed to update {}, package information from this repository may be outdated ({})", @@ -156,13 +156,13 @@ impl FossilDriver { fs.ensure_directory_exists(&self.checkout_dir)?; let mut output = String::new(); - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "clone", "--", &self.inner.url, &repo_file] .map(|s| s.to_string()) .as_ref(), &mut output, None, - ) != 0 + )? != 0 { let output = self.inner.process.borrow().get_error_output().to_string(); return Err(RuntimeException::new(format!( @@ -172,13 +172,13 @@ impl FossilDriver { .into()); } - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["fossil", "open", "--nested", "--", &repo_file] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.checkout_dir), - ) != 0 + )? != 0 { let output = self.inner.process.borrow().get_error_output().to_string(); return Err(RuntimeException::new(format!( @@ -225,13 +225,13 @@ impl FossilDriver { } let mut content = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "cat", "-r", identifier, "--", file] .map(|s| s.to_string()) .as_ref(), &mut content, Some(&self.checkout_dir), - ); + )?; if content.trim().is_empty() { return Ok(None); @@ -245,13 +245,13 @@ impl FossilDriver { _identifier: &str, ) -> anyhow::Result>> { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "finfo", "-b", "-n", "1", "composer.json"] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; let parts: Vec<&str> = output.trim().splitn(3, ' ').collect(); let date = parts.get(1).copied().unwrap_or(""); @@ -263,11 +263,11 @@ impl FossilDriver { if self.tags.is_none() { let mut tags: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "tag", "list"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { tags.insert(tag.clone(), tag); } @@ -280,11 +280,11 @@ impl FossilDriver { if self.branches.is_none() { let mut branches: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["fossil", "branch", "list"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.checkout_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { let branch = preg_replace(php_regex!(r"/^\*/"), "", branch.trim()); let branch = branch.trim().to_string(); @@ -321,11 +321,11 @@ impl FossilDriver { let mut process = ProcessExecutor::new(Some(io)); let mut output = String::new(); - if process.execute_args( + if process.execute( ["fossil", "info"].map(|s| s.to_string()).as_ref(), &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } diff --git a/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs b/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs index b1960974..3761144f 100644 --- a/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs +++ b/crates/shirabe/src/repository/vcs/git_bitbucket_driver.rs @@ -641,48 +641,43 @@ impl GitBitbucketDriver { url: &str, fetching_repo_data: bool, ) -> anyhow::Result { - match self.inner.get_contents(url) { - Ok(r) => Ok(r), - Err(e) => { - let mut bitbucket_util = Bitbucket::new( - self.inner.io.clone(), - self.inner.config.clone(), - Some(self.inner.process.clone()), - Some(self.inner.http_downloader.clone()), - None, - )?; - - { - let te = &e; - let code = te.get_code(); - let in_set = matches!(code, 403 | 404); - if in_set - || (401 == code - && strpos(te.get_message(), "Could not authenticate against") - == Some(0)) - { - if !self.inner.io.has_authentication(&self.inner.origin_url) - && bitbucket_util.authorize_oauth(&self.inner.origin_url) - { - return self.inner.get_contents(url).map_err(|e| (*e).into()); - } + let e = match self.inner.get_contents(url)? { + Ok(r) => return Ok(r), + Err(e) => e, + }; - if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback()?; + let mut bitbucket_util = Bitbucket::new( + self.inner.io.clone(), + self.inner.config.clone(), + Some(self.inner.process.clone()), + Some(self.inner.http_downloader.clone()), + None, + )?; + + let code = e.get_code(); + let in_set = matches!(code, 403 | 404); + if in_set + || (401 == code && strpos(e.get_message(), "Could not authenticate against") == Some(0)) + { + if !self.inner.io.has_authentication(&self.inner.origin_url) + && bitbucket_util.authorize_oauth(&self.inner.origin_url)? + { + return Ok(self.inner.get_contents(url)??); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } - } - } + if !self.inner.io.is_interactive() && fetching_repo_data { + self.attempt_clone_fallback()?; - Err((*e).into()) + return Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + )); } } + + Err((*e).into()) } /// Generate an SSH URL diff --git a/crates/shirabe/src/repository/vcs/git_driver.rs b/crates/shirabe/src/repository/vcs/git_driver.rs index f45dbcaa..776a18e5 100644 --- a/crates/shirabe/src/repository/vcs/git_driver.rs +++ b/crates/shirabe/src/repository/vcs/git_driver.rs @@ -185,7 +185,7 @@ impl GitDriver { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "branch".to_string(), @@ -193,7 +193,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; let branches = self.inner.process.borrow().split_lines(&output); if !branches.contains(&"* master".to_string()) { for branch in &branches { @@ -241,7 +241,7 @@ impl GitDriver { } let mut content = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "show".to_string(), @@ -249,7 +249,7 @@ impl GitDriver { ], &mut content, Some(&self.repo_dir), - ); + )?; if content.trim().is_empty() { return Ok(None); @@ -277,14 +277,14 @@ impl GitDriver { "--format=%at".to_string(), identifier.to_string(), ], - ); + )?; let mut output = String::new(); self.inner .process .borrow_mut() - .execute_args(&command, &mut output, Some(&self.repo_dir)); + .execute(&command, &mut output, Some(&self.repo_dir))?; - let timestamp_str = GitUtil::parse_rev_list_output(&output, &self.inner.process); + let timestamp_str = GitUtil::parse_rev_list_output(&output, &self.inner.process)?; let timestamp: i64 = timestamp_str.trim().parse().unwrap_or(0); Ok(Some( Utc.timestamp_opt(timestamp, 0).unwrap().fixed_offset(), @@ -296,7 +296,7 @@ impl GitDriver { self.tags = Some(IndexMap::new()); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "show-ref".to_string(), @@ -305,7 +305,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { if !tag.is_empty() && let Some(caps) = preg_match( @@ -330,7 +330,7 @@ impl GitDriver { let mut branches = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( &[ "git".to_string(), "branch".to_string(), @@ -340,7 +340,7 @@ impl GitDriver { ], &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && !preg_is_match(php_regex!(r"{^ *[^/]+/HEAD }"), &branch) @@ -384,11 +384,11 @@ impl GitDriver { io.clone(), )))); let mut output = String::new(); - if process.borrow_mut().execute_args( + if process.borrow_mut().execute( &["git".to_string(), "tag".to_string()], &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } diff --git a/crates/shirabe/src/repository/vcs/github_driver.rs b/crates/shirabe/src/repository/vcs/github_driver.rs index c41cc2d0..f2b2d458 100644 --- a/crates/shirabe/src/repository/vcs/github_driver.rs +++ b/crates/shirabe/src/repository/vcs/github_driver.rs @@ -696,10 +696,7 @@ impl GitHubDriver { file, urlencode(identifier) ); - let mut resource = self - .get_contents(&resource_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let mut resource = self.get_contents(&resource_url, false)??.decode_json()?; // The GitHub contents API only returns files up to 1MB as base64 encoded files // larger files either need be fetched with a raw accept header or by using the git blob endpoint @@ -724,10 +721,7 @@ impl GitHubDriver { .and_then(|v| v.as_string()) .unwrap_or("") .to_string(); - resource = self - .get_contents(&git_url, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + resource = self.get_contents(&git_url, false)??.decode_json()?; } let resource_map = match resource { @@ -776,10 +770,7 @@ impl GitHubDriver { self.repository, urlencode(identifier) ); - let commit = self - .get_contents(&resource, false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let commit = self.get_contents(&resource, false)??.decode_json()?; let date_str = match commit { PhpMixed::Array(m) => m @@ -812,9 +803,7 @@ impl GitHubDriver { )); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let tags_data = response.decode_json()?; if let PhpMixed::List(ref list) = tags_data { for tag in list { @@ -862,9 +851,7 @@ impl GitHubDriver { )); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let branch_data = response.decode_json()?; if let PhpMixed::List(ref list) = branch_data { for branch in list { @@ -969,40 +956,38 @@ impl GitHubDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - let response_result = self.inner.get_contents(url); + ) -> anyhow::Result>> { + let response_result = self.inner.get_contents(url)?; match response_result { - Ok(r) => Ok(r), + Ok(r) => Ok(Ok(r)), Err(e) => { let mut git_hub_util = GitHub::new( self.inner.io.clone(), self.inner.config.clone(), Some(self.inner.process.clone()), Some(self.inner.http_downloader.clone()), - ) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + )?; match e.get_code() { 401 | 404 => { // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 if !fetching_repo_data { - return Err(e); + return Ok(Err(e)); } - if git_hub_util.authorize_oauth(&self.inner.origin_url) { + if git_hub_util.authorize_oauth(&self.inner.origin_url)? { return self.inner.get_contents(url); } if !self.inner.io.is_interactive() { - self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into()))) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into())))?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } let mut scopes_issued: Vec = vec![]; @@ -1033,28 +1018,27 @@ impl GitHubDriver { "Your GitHub credentials are required to fetch private repository metadata ({})", self.inner.url )), - ); + )?; } self.inner.get_contents(url) } 403 => { if !self.inner.io.has_authentication(&self.inner.origin_url) - && git_hub_util.authorize_oauth(&self.inner.origin_url) + && git_hub_util.authorize_oauth(&self.inner.origin_url)? { return self.inner.get_contents(url); } if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into()))) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.attempt_clone_fallback(Some(std::sync::Arc::new((*e).into())))?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } let rate_limited = git_hub_util @@ -1070,7 +1054,7 @@ impl GitHubDriver { true, io_interface::NORMAL, ); - return Err(e); + return Ok(Err(e)); } git_hub_util.authorize_oauth_interactively( @@ -1079,7 +1063,7 @@ impl GitHubDriver { "API limit exhausted. Enter your GitHub credentials to get a larger API limit ({})", self.inner.url )), - ); + )?; return self.inner.get_contents(url); } @@ -1099,9 +1083,9 @@ impl GitHubDriver { ); } - Err(e) + Ok(Err(e)) } - _ => Err(e), + _ => Ok(Err(e)), } } } @@ -1122,7 +1106,7 @@ impl GitHubDriver { self.repository ); - let repo_data_result = self.get_contents(&repo_data_url, true); + let repo_data_result = self.get_contents(&repo_data_url, true)?; match repo_data_result { Ok(response) => { let data = response.decode_json()?; diff --git a/crates/shirabe/src/repository/vcs/gitlab_driver.rs b/crates/shirabe/src/repository/vcs/gitlab_driver.rs index b40297da..d21a9ca1 100644 --- a/crates/shirabe/src/repository/vcs/gitlab_driver.rs +++ b/crates/shirabe/src/repository/vcs/gitlab_driver.rs @@ -396,7 +396,7 @@ impl GitLabDriver { identifier, ); - let content = match self.get_contents(&resource, false) { + let content = match self.get_contents(&resource, false)? { Ok(response) => response.get_body().map(|s| s.to_string()), Err(e) => { if e.get_code() != 404 { @@ -587,9 +587,7 @@ impl GitLabDriver { let mut references: IndexMap = IndexMap::new(); loop { - let response = self - .get_contents(resource.as_deref().unwrap_or(""), false) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))?; + let response = self.get_contents(resource.as_deref().unwrap_or(""), false)??; let data = response.decode_json()?; if let PhpMixed::List(ref list) = data { @@ -646,10 +644,7 @@ impl GitLabDriver { // we need to fetch the default branch from the api let resource = self.get_api_url(); - let project = self - .get_contents(&resource, true) - .map_err(|e| anyhow::anyhow!("{}", e.get_message()))? - .decode_json()?; + let project = self.get_contents(&resource, true)??.decode_json()?; self.project = match project { PhpMixed::Array(m) => Some(m), _ => None, @@ -741,14 +736,18 @@ impl GitLabDriver { &mut self, url: &str, fetching_repo_data: bool, - ) -> anyhow::Result> { - let response_result = self.inner.get_contents(url); - match response_result { - Ok(response) => { + ) -> anyhow::Result>> { + // PHP body of the `try` block: a TransportException raised anywhere in here, including the + // ones thrown below to force authentication, is handled by the `catch` that follows. + let response_result = + (|this: &mut Self| -> anyhow::Result>> { + let response = match this.inner.get_contents(url)? { + Ok(response) => response, + Err(e) => return Ok(Err(e)), + }; + if fetching_repo_data { - let json = response - .decode_json() - .map_err(|e| TransportException::new(e.to_string(), 0))?; + let json = response.decode_json()?; let json_map = match json { PhpMixed::Array(ref m) => m.clone(), _ => IndexMap::new(), @@ -760,7 +759,7 @@ impl GitLabDriver { if !json_map.contains_key("default_branch") && json_map.contains_key("permissions") { - self.is_private = json_map + this.is_private = json_map .get("visibility") .and_then(|v| v.as_string()) .map(|s| s != "public") @@ -785,21 +784,20 @@ impl GitLabDriver { } if !more_than_guest_access { - self.inner.io.write_error3( - "GitLab token with Guest or Planner only access detected", - true, - io_interface::NORMAL, - ); + this.inner.io.write_error3( + "GitLab token with Guest or Planner only access detected", + true, + io_interface::NORMAL, + ); - self.attempt_clone_fallback() - .map_err(|e| TransportException::new(e.to_string(), 0))?; + this.attempt_clone_fallback()?; - return Ok(Response::new( + return Ok(Ok(Response::new( "dummy".to_string(), Some(200), vec![], Some("null".to_string()), - )); + ))); } } @@ -811,110 +809,108 @@ impl GitLabDriver { .and_then(|v| v.as_string()) == Some("disabled") { - return Err(Box::new(TransportException::new( + return Ok(Err(Box::new(TransportException::new( "The GitLab repository is disabled in the project".to_string(), 400, - ))); + )))); } if !empty(&json_map.get("id").cloned().unwrap_or(PhpMixed::Null)) { - self.is_private = false; + this.is_private = false; } - return Err(Box::new(TransportException::new( - "GitLab API seems to not be authenticated as it did not return a default_branch" - .to_string(), - 401, - ))); + return Ok(Err(Box::new(TransportException::new( + "GitLab API seems to not be authenticated as it did not return a default_branch" + .to_string(), + 401, + )))); } } - Ok(response) - } - Err(e) => { - let mut git_lab_util = GitLab::new( - self.inner.io.clone(), - self.inner.config.clone(), - Some(self.inner.process.clone()), - Some(self.inner.http_downloader.clone()), - ) - .map_err(|err| TransportException::new(err.to_string(), 0))?; + Ok(Ok(response)) + })(self); - match e.get_code() { - 401 | 404 => { - // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 - if !fetching_repo_data { - return Err(e); - } + let e = match response_result? { + Ok(response) => return Ok(Ok(response)), + Err(e) => e, + }; - if git_lab_util.authorize_oauth(&self.inner.origin_url) { - return self.inner.get_contents(url); - } + let mut git_lab_util = GitLab::new( + self.inner.io.clone(), + self.inner.config.clone(), + Some(self.inner.process.clone()), + Some(self.inner.http_downloader.clone()), + )?; + + match e.get_code() { + 401 | 404 => { + // try to authorize only if we are fetching the main /repos/foo/bar data, otherwise it must be a real 404 + if !fetching_repo_data { + return Ok(Err(e)); + } - if git_lab_util.is_oauth_expired(&self.inner.origin_url) - && git_lab_util - .authorize_oauth_refresh(&self.scheme, &self.inner.origin_url) - .map_err(|err| TransportException::new(err.to_string(), 0))? - { - return self.inner.get_contents(url); - } + if git_lab_util.authorize_oauth(&self.inner.origin_url)? { + return self.inner.get_contents(url); + } - if !self.inner.io.is_interactive() { - self.attempt_clone_fallback() - .map_err(|err| TransportException::new(err.to_string(), 0))?; + if git_lab_util.is_oauth_expired(&self.inner.origin_url) + && git_lab_util.authorize_oauth_refresh(&self.scheme, &self.inner.origin_url)? + { + return self.inner.get_contents(url); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } - self.inner.io.write_error3( - &format!( - "Failed to download {}/{}:{}", - self.namespace, - self.repository, - e.get_message() - ), - true, - io_interface::NORMAL, - ); - git_lab_util.authorize_oauth_interactively( - &self.scheme, - &self.inner.origin_url, - Some(&format!( - "Your credentials are required to fetch private repository metadata ({})", - self.inner.url - )), - ); + if !self.inner.io.is_interactive() { + self.attempt_clone_fallback()?; - self.inner.get_contents(url) - } - 403 => { - if !self.inner.io.has_authentication(&self.inner.origin_url) - && git_lab_util.authorize_oauth(&self.inner.origin_url) - { - return self.inner.get_contents(url); - } + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); + } + self.inner.io.write_error3( + &format!( + "Failed to download {}/{}:{}", + self.namespace, + self.repository, + e.get_message() + ), + true, + io_interface::NORMAL, + ); + git_lab_util.authorize_oauth_interactively( + &self.scheme, + &self.inner.origin_url, + Some(&format!( + "Your credentials are required to fetch private repository metadata ({})", + self.inner.url + )), + )?; - if !self.inner.io.is_interactive() && fetching_repo_data { - self.attempt_clone_fallback() - .map_err(|err| TransportException::new(err.to_string(), 0))?; + self.inner.get_contents(url) + } + 403 => { + if !self.inner.io.has_authentication(&self.inner.origin_url) + && git_lab_util.authorize_oauth(&self.inner.origin_url)? + { + return self.inner.get_contents(url); + } - return Ok(Response::new( - "dummy".to_string(), - Some(200), - vec![], - Some("null".to_string()), - )); - } + if !self.inner.io.is_interactive() && fetching_repo_data { + self.attempt_clone_fallback()?; - Err(e) - } - _ => Err(e), + return Ok(Ok(Response::new( + "dummy".to_string(), + Some(200), + vec![], + Some("null".to_string()), + ))); } + + Ok(Err(e)) } + _ => Ok(Err(e)), } } diff --git a/crates/shirabe/src/repository/vcs/hg_driver.rs b/crates/shirabe/src/repository/vcs/hg_driver.rs index 54341070..c952eb0f 100644 --- a/crates/shirabe/src/repository/vcs/hg_driver.rs +++ b/crates/shirabe/src/repository/vcs/hg_driver.rs @@ -89,17 +89,17 @@ impl HgDriver { ); if is_dir(&self.repo_dir) - && self.inner.process.borrow_mut().execute_args( + && self.inner.process.borrow_mut().execute( ["hg", "summary"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.repo_dir), - ) == 0 + )? == 0 { - if self.inner.process.borrow_mut().execute_args( + if self.inner.process.borrow_mut().execute( ["hg", "pull"].map(|s| s.to_string()).as_ref(), &mut String::new(), Some(&self.repo_dir), - ) != 0 + )? != 0 { self.inner.io.write_error3(&format!("Failed to update {}, package information from this repository may be outdated ({})", self.inner.url, self.inner.process.borrow().get_error_output()), true, crate::io::NORMAL); } @@ -132,13 +132,13 @@ impl HgDriver { pub fn get_root_identifier(&mut self) -> anyhow::Result { if self.root_identifier.is_none() { let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "tip", "--template", "{node}"] .map(|s| s.to_string()) .as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; let lines = self.inner.process.borrow().split_lines(&output); self.root_identifier = lines.into_iter().next(); } @@ -183,7 +183,7 @@ impl HgDriver { self.inner .process .borrow_mut() - .execute_args(&resource, &mut content, Some(&self.repo_dir)); + .execute(&resource, &mut content, Some(&self.repo_dir))?; if content.trim().is_empty() { return Ok(None); @@ -205,7 +205,7 @@ impl HgDriver { } let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( [ "hg", "log", @@ -218,7 +218,7 @@ impl HgDriver { .as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; let date: DateTime = shirabe_php_shim::date_create(output.trim())?; Ok(Some(date.fixed_offset())) @@ -228,11 +228,11 @@ impl HgDriver { if self.tags.is_none() { let mut tags: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "tags"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for tag in self.inner.process.borrow().split_lines(&output) { if !tag.is_empty() && let Some(m) = preg_match(php_regex!(r"(^([^\s]+)\s+\d+:(.*)$)"), &tag) @@ -257,11 +257,11 @@ impl HgDriver { let mut bookmarks: IndexMap = IndexMap::new(); let mut output = String::new(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "branches"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && let Some(m) = @@ -275,11 +275,11 @@ impl HgDriver { } output.clear(); - self.inner.process.borrow_mut().execute_args( + self.inner.process.borrow_mut().execute( ["hg", "bookmarks"].map(|s| s.to_string()).as_ref(), &mut output, Some(&self.repo_dir), - ); + )?; for branch in self.inner.process.borrow().split_lines(&output) { if !branch.is_empty() && let Some(m) = @@ -323,11 +323,11 @@ impl HgDriver { let mut process = crate::util::ProcessExecutor::new(Some(io.clone())); let mut output = String::new(); - if process.execute_args( + if process.execute( ["hg", "summary"].map(|s| s.to_string()).as_ref(), &mut output, Some(&url), - ) == 0 + )? == 0 { return Ok(true); } @@ -339,13 +339,13 @@ impl HgDriver { let mut process = crate::util::ProcessExecutor::new(Some(io)); let mut ignored = String::new(); - let exit = process.execute_args( + let exit = process.execute( ["hg", "identify", "--", url] .map(|s| s.to_string()) .as_ref(), &mut ignored, None, - ); + )?; Ok(exit == 0) } diff --git a/crates/shirabe/src/repository/vcs/perforce_driver.rs b/crates/shirabe/src/repository/vcs/perforce_driver.rs index d55d361c..6a1c1d70 100644 --- a/crates/shirabe/src/repository/vcs/perforce_driver.rs +++ b/crates/shirabe/src/repository/vcs/perforce_driver.rs @@ -191,10 +191,7 @@ impl PerforceDriver { deep: bool, ) -> anyhow::Result { if deep || preg_is_match(php_regex!(r"#\b(perforce|p4)\b#i"), url) { - return Ok(Perforce::check_server_exists( - url, - &mut ProcessExecutor::new(Some(io)), - )); + return Perforce::check_server_exists(url, &mut ProcessExecutor::new(Some(io))); } Ok(false) } diff --git a/crates/shirabe/src/repository/vcs/svn_driver.rs b/crates/shirabe/src/repository/vcs/svn_driver.rs index 0431c33b..c92c9c9f 100644 --- a/crates/shirabe/src/repository/vcs/svn_driver.rs +++ b/crates/shirabe/src/repository/vcs/svn_driver.rs @@ -451,7 +451,7 @@ impl SvnDriver { let mut process = ProcessExecutor::new(Some(io)); let mut ignored_output = String::new(); - let exit = process.execute_args( + let exit = process.execute( &[ "svn".to_string(), "info".to_string(), @@ -461,7 +461,7 @@ impl SvnDriver { ], &mut ignored_output, None, - ); + )?; if exit == 0 { // This is definitely a Subversion repository. @@ -523,7 +523,7 @@ impl SvnDriver { { Ok(o) => Ok(o), Err(e) => { - if self.util.as_mut().unwrap().binary_version().is_none() { + if self.util.as_mut().unwrap().binary_version()?.is_none() { return Err(RuntimeException::new(format!( "Failed to load {}, svn was not found, check that it is installed and in your PATH env.\n\n{}", self.inner.url, diff --git a/crates/shirabe/src/repository/vcs/vcs_driver.rs b/crates/shirabe/src/repository/vcs/vcs_driver.rs index 17616c36..90cfd2db 100644 --- a/crates/shirabe/src/repository/vcs/vcs_driver.rs +++ b/crates/shirabe/src/repository/vcs/vcs_driver.rs @@ -66,7 +66,10 @@ impl VcsDriverBase { "http" } - pub fn get_contents(&self, url: &str) -> anyhow::Result> { + pub fn get_contents( + &self, + url: &str, + ) -> anyhow::Result>> { let options_mixed = self .repo_config .get("options") @@ -76,15 +79,13 @@ impl VcsDriverBase { PhpMixed::Array(a) => a, _ => IndexMap::new(), }; - self.http_downloader - .borrow_mut() - .get(url, options) - .map_err(|e| { - Box::new(match e.catch::() { - Some(te) => te.clone(), - None => TransportException::new(e.to_string(), 0), - }) - }) + match self.http_downloader.borrow_mut().get(url, options) { + Ok(response) => Ok(Ok(response)), + Err(e) => match e.catch::() { + Some(te) => Ok(Err(Box::new(te.clone()))), + None => Err(e), + }, + } } // Helper for concrete drivers: produces the same value as the trait default @@ -301,7 +302,7 @@ pub trait VcsDriver: VcsDriverInterface { "http" } - fn get_contents(&self, url: &str) -> anyhow::Result> { + fn get_contents(&self, url: &str) -> anyhow::Result>> { let options_mixed = self .repo_config() .get("options") @@ -311,15 +312,13 @@ pub trait VcsDriver: VcsDriverInterface { PhpMixed::Array(a) => a, _ => IndexMap::new(), }; - self.http_downloader() - .borrow_mut() - .get(url, options) - .map_err(|e| { - Box::new(match e.catch::() { - Some(te) => te.clone(), - None => TransportException::new(e.to_string(), 0), - }) - }) + match self.http_downloader().borrow_mut().get(url, options) { + Ok(response) => Ok(Ok(response)), + Err(e) => match e.catch::() { + Some(te) => Ok(Err(Box::new(te.clone()))), + None => Err(e), + }, + } } fn cleanup(&self) {} diff --git a/crates/shirabe/src/util/auth_helper.rs b/crates/shirabe/src/util/auth_helper.rs index 72f50aa3..5e6ea16e 100644 --- a/crates/shirabe/src/util/auth_helper.rs +++ b/crates/shirabe/src/util/auth_helper.rs @@ -213,7 +213,7 @@ impl AuthHelper { } } - if !git_hub_util.authorize_oauth(origin) + if !git_hub_util.authorize_oauth(origin)? && (!self.io.is_interactive() || !git_hub_util.authorize_oauth_interactively(origin, Some(&message))?) { @@ -257,7 +257,7 @@ impl AuthHelper { } let scheme = parse_url(url).and_then(|parsed| parsed.scheme); - if !git_lab_util.authorize_oauth(origin) + if !git_lab_util.authorize_oauth(origin)? && (!self.io.is_interactive() || !git_lab_util.authorize_oauth_interactively( scheme.as_deref().unwrap_or(""), @@ -337,7 +337,7 @@ impl AuthHelper { ); let mut bit_bucket_util = Bitbucket::new(self.io.clone(), self.config.clone(), None, None, None)?; - if !bit_bucket_util.authorize_oauth(&origin) + if !bit_bucket_util.authorize_oauth(&origin)? && (!self.io.is_interactive() || !bit_bucket_util .authorize_oauth_interactively(&origin, Some(&message))?) diff --git a/crates/shirabe/src/util/bitbucket.rs b/crates/shirabe/src/util/bitbucket.rs index 9a16c9a6..f50dc878 100644 --- a/crates/shirabe/src/util/bitbucket.rs +++ b/crates/shirabe/src/util/bitbucket.rs @@ -71,22 +71,17 @@ impl Bitbucket { } } - pub fn authorize_oauth(&mut self, origin_url: &str) -> bool { + pub fn authorize_oauth(&mut self, origin_url: &str) -> anyhow::Result { if origin_url != "bitbucket.org" { - return false; + return Ok(false); } let mut output = PhpMixed::Null; - if self - .process - .borrow_mut() - .execute( - &["git", "config", "bitbucket.accesstoken"], - &mut output, - None, - ) - .unwrap_or(1) - == 0 + if self.process.borrow_mut().execute( + &["git", "config", "bitbucket.accesstoken"], + &mut output, + None, + )? == 0 { let output_str = output.as_string().unwrap_or("").trim().to_string(); self.io.borrow_mut().set_authentication( @@ -94,10 +89,10 @@ impl Bitbucket { "x-token-auth".to_string(), Some(output_str), ); - return true; + return Ok(true); } - false + Ok(false) } fn request_access_token(&mut self) -> anyhow::Result { diff --git a/crates/shirabe/src/util/filesystem.rs b/crates/shirabe/src/util/filesystem.rs index 8726df63..bca18771 100644 --- a/crates/shirabe/src/util/filesystem.rs +++ b/crates/shirabe/src/util/filesystem.rs @@ -475,7 +475,7 @@ impl Filesystem { if Platform::is_windows() { // Try to copy & delete - this is a workaround for random "Access denied" errors. let mut output = String::new(); - let result = self.get_process().execute_args( + let result = self.get_process().execute( &[ "xcopy".to_string(), source.to_string(), @@ -487,7 +487,7 @@ impl Filesystem { ], &mut output, None, - ); + )?; // clear stat cache because external processes aren't tracked by the php stat cache clearstatcache2(false, ""); @@ -501,11 +501,11 @@ impl Filesystem { // We do not use PHP's "rename" function here since it does not support // the case where $source, and $target are located on different partitions. let mut output = String::new(); - let result = self.get_process().execute_args( + let result = self.get_process().execute( &["mv".to_string(), source.to_string(), target.to_string()], &mut output, None, - ); + )?; // clear stat cache because external processes aren't tracked by the php stat cache clearstatcache2(false, ""); @@ -932,7 +932,7 @@ impl Filesystem { Platform::realpath(target), ]; let mut output = String::new(); - if self.get_process().execute_args(&cmd, &mut output, None) != 0 { + if self.get_process().execute(&cmd, &mut output, None)? != 0 { return Err(IOException::new( format!( "Failed to create junction to \"{}\" at \"{}\".", diff --git a/crates/shirabe/src/util/git.rs b/crates/shirabe/src/util/git.rs index 5577250f..aea8b7b8 100644 --- a/crates/shirabe/src/util/git.rs +++ b/crates/shirabe/src/util/git.rs @@ -362,8 +362,8 @@ impl Git { )?; let message = "Cloning failed using an ssh key for authentication, enter your GitHub credentials to access private repos"; - if !git_hub_util.authorize_oauth(&m1) && self.io.is_interactive() { - git_hub_util.authorize_oauth_interactively(&m1, Some(message)); + if !git_hub_util.authorize_oauth(&m1)? && self.io.is_interactive() { + git_hub_util.authorize_oauth_interactively(&m1, Some(message))?; } } @@ -423,8 +423,8 @@ impl Git { if !self.io.has_authentication(&domain) { let message = "Enter your Bitbucket credentials to access private repos"; - if !bitbucket_util.authorize_oauth(&domain) && self.io.is_interactive() { - bitbucket_util.authorize_oauth_interactively(&domain, Some(message)); + if !bitbucket_util.authorize_oauth(&domain)? && self.io.is_interactive() { + bitbucket_util.authorize_oauth_interactively(&domain, Some(message))?; let access_token = bitbucket_util.get_token(); self.io.borrow_mut().set_authentication( domain.clone(), @@ -575,8 +575,8 @@ impl Git { let message = "Cloning failed, enter your GitLab credentials to access private repos"; - if !git_lab_util.authorize_oauth(&m2) && self.io.is_interactive() { - git_lab_util.authorize_oauth_interactively(&m1, &m2, Some(message)); + if !git_lab_util.authorize_oauth(&m2)? && self.io.is_interactive() { + git_lab_util.authorize_oauth_interactively(&m1, &m2, Some(message))?; } } @@ -783,7 +783,7 @@ impl Git { // update the repo if it is a valid git repository let mut output = String::new(); if is_dir(dir) - && self.process.borrow_mut().execute_args( + && self.process.borrow_mut().execute( &[ "git".to_string(), "rev-parse".to_string(), @@ -791,7 +791,7 @@ impl Git { ], &mut output, Some(dir), - ) == 0 + )? == 0 && trim(&output, None) == "." { // PHP try/finally @@ -915,20 +915,20 @@ impl Git { let mut branches: Option = None; let mut tags: Option = None; let mut output = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &["git".to_string(), "branch".to_string()], &mut output, Some(dir), - ) == 0 + )? == 0 { branches = Some(output); } let mut output = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &["git".to_string(), "tag".to_string()], &mut output, Some(dir), - ) == 0 + )? == 0 { tags = Some(output); } @@ -964,26 +964,26 @@ impl Git { pub fn get_no_show_signature_flag( process: &std::rc::Rc>, - ) -> String { - let git_version = Self::get_version(process); + ) -> anyhow::Result { + let git_version = Self::get_version(process)?; if let Some(v) = git_version && version_compare(&v, "2.10.0-rc0", CmpOp::Ge) { - return " --no-show-signature".to_string(); + return Ok(" --no-show-signature".to_string()); } - String::new() + Ok(String::new()) } pub fn get_no_show_signature_flags( process: &std::rc::Rc>, - ) -> Vec { - let flags = Self::get_no_show_signature_flag(process); + ) -> anyhow::Result> { + let flags = Self::get_no_show_signature_flag(process)?; if flags.is_empty() { - return vec![]; + return Ok(vec![]); } - explode(" ", &substr(&flags, 1, None)) + Ok(explode(" ", &substr(&flags, 1, None))) } /// Checks if git version supports --no-commit-header flag (git 2.33+) @@ -991,12 +991,12 @@ impl Git { /// @internal pub fn supports_no_commit_header_flag( process: &std::rc::Rc>, - ) -> bool { - let git_version = Self::get_version(process); + ) -> anyhow::Result { + let git_version = Self::get_version(process)?; - git_version + Ok(git_version .map(|v| version_compare(&v, "2.33.0-rc0", CmpOp::Ge)) - .unwrap_or(false) + .unwrap_or(false)) } /// Builds a git rev-list command with --no-commit-header flag when supported (git 2.33+) @@ -1006,14 +1006,14 @@ impl Git { pub fn build_rev_list_command( process: &std::rc::Rc>, arguments: Vec, - ) -> Vec { + ) -> anyhow::Result> { let mut command = vec!["git".to_string(), "rev-list".to_string()]; - if Self::supports_no_commit_header_flag(process) { + if Self::supports_no_commit_header_flag(process)? { command.push("--no-commit-header".to_string()); } command.extend(arguments); - command + Ok(command) } /// Parses git rev-list output, removing 'commit ' header lines for git < 2.33. @@ -1025,20 +1025,24 @@ impl Git { pub fn parse_rev_list_output( output: &str, process: &std::rc::Rc>, - ) -> String { + ) -> anyhow::Result { // If git supports --no-commit-header, output is already clean - if Self::supports_no_commit_header_flag(process) { - return output.to_string(); + if Self::supports_no_commit_header_flag(process)? { + return Ok(output.to_string()); } // Filter out "commit " lines for older git versions - preg_replace(php_regex!(r"{^commit [a-f0-9]{40}\n?}m"), "", output) + Ok(preg_replace( + php_regex!(r"{^commit [a-f0-9]{40}\n?}m"), + "", + output, + )) } fn check_ref_is_in_mirror(&mut self, dir: &str, r#ref: &str) -> anyhow::Result { let mut output = String::new(); if is_dir(dir) - && self.process.borrow_mut().execute_args( + && self.process.borrow_mut().execute( &[ "git".to_string(), "rev-parse".to_string(), @@ -1046,11 +1050,11 @@ impl Git { ], &mut output, Some(dir), - ) == 0 + )? == 0 && trim(&output, None) == "." { let mut ignored_output = String::new(); - let exit_code = self.process.borrow_mut().execute_args( + let exit_code = self.process.borrow_mut().execute( &[ "git".to_string(), "rev-parse".to_string(), @@ -1060,7 +1064,7 @@ impl Git { ], &mut ignored_output, Some(dir), - ); + )?; if exit_code == 0 { return Ok(true); } @@ -1108,7 +1112,7 @@ impl Git { let mut output_mixed = PhpMixed::Null; if is_local_path_repository { let mut output = String::new(); - self.process.borrow_mut().execute_args( + self.process.borrow_mut().execute( &[ "git".to_string(), "remote".to_string(), @@ -1117,7 +1121,7 @@ impl Git { ], &mut output, Some(dir), - ); + )?; output_mixed = PhpMixed::String(output); } else { let commands = vec![ @@ -1178,9 +1182,11 @@ impl Git { } } - pub fn clean_env(process: &std::rc::Rc>) { + pub fn clean_env( + process: &std::rc::Rc>, + ) -> anyhow::Result<()> { // PHP: $process ?? new ProcessExecutor() - let git_version = Self::get_version(process); + let git_version = Self::get_version(process)?; if let Some(v) = git_version { if version_compare(&v, "2.3.0", CmpOp::Ge) { // added in git 2.3.0, prevents prompting the user for username/password @@ -1210,6 +1216,8 @@ impl Git { // clean up env for OSX, see https://github.com/composer/composer/issues/2146#issuecomment-35478940 Platform::clear_env("DYLD_LIBRARY_PATH"); + + Ok(()) } pub fn get_github_domains_regex(config: &Config) -> String { @@ -1241,11 +1249,11 @@ impl Git { clearstatcache(); let mut ignored_output = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &["git".to_string(), "--version".to_string()], &mut ignored_output, Option::<&str>::None, - ) != 0 + )? != 0 { return Err(RuntimeException::new(Url::sanitize(format!( "Failed to clone {}, git was not found, check that it is installed and in your PATH env.\n\n{}", @@ -1263,16 +1271,16 @@ impl Git { /// @return string|null The git version number, if present. pub fn get_version( process: &std::rc::Rc>, - ) -> Option { + ) -> anyhow::Result> { let mut version = VERSION.lock().unwrap(); if version.is_none() { *version = Some(None); let mut output = String::new(); - let exit_code: i64 = process.borrow_mut().execute_args( + let exit_code: i64 = process.borrow_mut().execute( &["git".to_string(), "--version".to_string()], &mut output, Option::<&str>::None, - ); + )?; if exit_code == 0 && let Some(matches) = preg_match(php_regex!(r"/^git version (\d+(?:\.\d+)+)/m"), &output) @@ -1280,7 +1288,7 @@ impl Git { *version = Some(matches.get(1).map(str::to_string)); } } - version.clone().unwrap_or(None) + Ok(version.clone().unwrap_or(None)) } /// For testing only. Resets the cached git `version` static so the next diff --git a/crates/shirabe/src/util/github.rs b/crates/shirabe/src/util/github.rs index ccfedcf8..d4fd3283 100644 --- a/crates/shirabe/src/util/github.rs +++ b/crates/shirabe/src/util/github.rs @@ -52,14 +52,14 @@ impl GitHub { }) } - pub fn authorize_oauth(&mut self, origin_url: &str) -> bool { + pub fn authorize_oauth(&mut self, origin_url: &str) -> anyhow::Result { let github_domains = self.config.borrow_mut().get("github-domains"); if !in_array_loose(origin_url.to_string(), github_domains.values()) { - return false; + return Ok(false); } let mut output = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &[ "git".to_string(), "config".to_string(), @@ -67,17 +67,17 @@ impl GitHub { ], &mut output, None, - ) == 0 + )? == 0 { self.io.borrow_mut().set_authentication( origin_url.to_string(), output.trim().to_string(), Some("x-oauth-basic".to_string()), ); - return true; + return Ok(true); } - false + Ok(false) } pub fn authorize_oauth_interactively( @@ -101,7 +101,7 @@ impl GitHub { if self .process .borrow_mut() - .execute_args(&["hostname".to_string()], &mut output, None) + .execute(&["hostname".to_string()], &mut output, None)? == 0 { note += &format!(" on {}", output.trim()); diff --git a/crates/shirabe/src/util/gitlab.rs b/crates/shirabe/src/util/gitlab.rs index c101b1f9..fa9bef69 100644 --- a/crates/shirabe/src/util/gitlab.rs +++ b/crates/shirabe/src/util/gitlab.rs @@ -51,7 +51,7 @@ impl GitLab { }) } - pub fn authorize_oauth(&mut self, origin_url: &str) -> bool { + pub fn authorize_oauth(&mut self, origin_url: &str) -> anyhow::Result { // before composer 1.9, origin URLs had no port number in them let bc_origin_url = preg_replace(php_regex!("{:\\d+}"), "", origin_url); @@ -59,12 +59,12 @@ impl GitLab { if !in_array_strict(origin_url.to_string(), gitlab_domains.values()) && !in_array_strict(bc_origin_url.clone(), gitlab_domains.values()) { - return false; + return Ok(false); } // if available use token from git config let mut output = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &[ "git".to_string(), "config".to_string(), @@ -72,20 +72,20 @@ impl GitLab { ], &mut output, None, - ) == 0 + )? == 0 { self.io.borrow_mut().set_authentication( origin_url.to_string(), output.trim().to_string(), Some("oauth2".to_string()), ); - return true; + return Ok(true); } // if available use deploy token from git config let mut token_user = String::new(); let mut token_password = String::new(); - if self.process.borrow_mut().execute_args( + if self.process.borrow_mut().execute( &[ "git".to_string(), "config".to_string(), @@ -93,8 +93,8 @@ impl GitLab { ], &mut token_user, None, - ) == 0 - && self.process.borrow_mut().execute_args( + )? == 0 + && self.process.borrow_mut().execute( &[ "git".to_string(), "config".to_string(), @@ -102,14 +102,14 @@ impl GitLab { ], &mut token_password, None, - ) == 0 + )? == 0 { self.io.borrow_mut().set_authentication( origin_url.to_string(), token_user.trim().to_string(), Some(token_password.trim().to_string()), ); - return true; + return Ok(true); } // if available use token from composer config @@ -165,10 +165,10 @@ impl GitLab { ); } - return true; + return Ok(true); } - false + Ok(false) } pub fn authorize_oauth_interactively( diff --git a/crates/shirabe/src/util/hg.rs b/crates/shirabe/src/util/hg.rs index 3180f4f9..575c2c1f 100644 --- a/crates/shirabe/src/util/hg.rs +++ b/crates/shirabe/src/util/hg.rs @@ -6,9 +6,9 @@ use crate::io::IOInterfaceImmutable; use crate::util::ProcessExecutor; use crate::util::Url; use shirabe_php_shim::{php_regex, preg_match, rawurlencode}; -use std::sync::OnceLock; +use std::sync::Mutex; -static VERSION: OnceLock> = OnceLock::new(); +static VERSION: Mutex>> = Mutex::new(None); #[derive(Debug)] pub struct Hg { @@ -48,7 +48,7 @@ impl Hg { if self .process .borrow_mut() - .execute_args(&command, &mut ignored_output, cwd.as_deref()) + .execute(&command, &mut ignored_output, cwd.as_deref())? == 0 { return Ok(()); @@ -107,7 +107,7 @@ impl Hg { if self .process .borrow_mut() - .execute_args(&command, &mut ignored_output, cwd.as_deref()) + .execute(&command, &mut ignored_output, cwd.as_deref())? == 0 { return Ok(()); @@ -125,7 +125,7 @@ impl Hg { } fn throw_exception(&self, message: &str, url: &str) -> anyhow::Result<()> { - if Self::get_version(&self.process).is_none() { + if Self::get_version(&self.process)?.is_none() { anyhow::bail!( "{}", Url::sanitize(format!( @@ -141,24 +141,25 @@ impl Hg { pub fn get_version( process: &std::rc::Rc>, - ) -> Option<&'static str> { - VERSION - .get_or_init(|| { - let mut output = String::new(); - if process.borrow_mut().execute_args( - &["hg".to_string(), "--version".to_string()], - &mut output, - None, - ) == 0 - && let Some(matches) = preg_match( - php_regex!(r"/^.+? (\d+(?:\.\d+)+)(?:\+.*?)?\)?\r?\n/"), - &output, - ) - { - return matches.get(1).map(str::to_string); - } - None - }) - .as_deref() + ) -> anyhow::Result> { + let mut version = VERSION.lock().unwrap(); + if version.is_none() { + *version = Some(None); + let mut output = String::new(); + if process.borrow_mut().execute( + &["hg".to_string(), "--version".to_string()], + &mut output, + None, + )? == 0 + && let Some(matches) = preg_match( + php_regex!(r"/^.+? (\d+(?:\.\d+)+)(?:\+.*?)?\)?\r?\n/"), + &output, + ) + { + *version = Some(matches.get(1).map(str::to_string)); + } + } + + Ok(version.clone().unwrap_or(None)) } } diff --git a/crates/shirabe/src/util/perforce.rs b/crates/shirabe/src/util/perforce.rs index 95cff0ca..295851c6 100644 --- a/crates/shirabe/src/util/perforce.rs +++ b/crates/shirabe/src/util/perforce.rs @@ -78,9 +78,12 @@ impl Perforce { Self::new(repo_config, port, path, process, Platform::is_windows(), io) } - pub fn check_server_exists(url: &str, process_executor: &mut ProcessExecutor) -> bool { + pub fn check_server_exists( + url: &str, + process_executor: &mut ProcessExecutor, + ) -> anyhow::Result { let mut ignored_output = String::new(); - process_executor.execute_args( + Ok(process_executor.execute( &[ "p4".to_string(), "-p".to_string(), @@ -90,7 +93,7 @@ impl Perforce { ], &mut ignored_output, Option::<&str>::None, - ) == 0 + )? == 0) } pub fn initialize(&mut self, repo_config: &IndexMap) { diff --git a/crates/shirabe/src/util/platform.rs b/crates/shirabe/src/util/platform.rs index c40f3ec5..ad530db3 100644 --- a/crates/shirabe/src/util/platform.rs +++ b/crates/shirabe/src/util/platform.rs @@ -310,7 +310,7 @@ impl Platform { let mut process = ProcessExecutor::new(None); let mut output = String::new(); let result: anyhow::Result<()> = (|| { - if process.execute_args(&["lsmod".to_string()], &mut output, None) == 0 + if process.execute(&["lsmod".to_string()], &mut output, None)? == 0 && output.contains("vboxguest") { *cached = Some(true); diff --git a/crates/shirabe/src/util/process_executor.rs b/crates/shirabe/src/util/process_executor.rs index c1be0e18..eae6692b 100644 --- a/crates/shirabe/src/util/process_executor.rs +++ b/crates/shirabe/src/util/process_executor.rs @@ -160,32 +160,6 @@ impl ProcessExecutor { self.do_execute(command, cwd, false, output) } - /// Convenience wrapper used by phase-A code that calls - /// `process.execute(&[String], &mut String, Option<&str>) == 0`. - /// Forwards to `execute`, returning the status code (1 on Err for compatibility) — this - /// mirrors PHP call sites that check the `int` return of `execute()` without a surrounding - /// `try`/`catch`, where an uncaught mock-mismatch exception would otherwise propagate. - // TODO(mock): under a strict `ProcessExecutorMock`, an incomplete expectation list now - // surfaces here as a swallowed "exit code 1" instead of the old `panic!`, so a future test - // ported through this call site could silently take a wrong branch instead of failing loudly. - // `ProcessExecutorMockGuard::__assert_complete` still catches unconsumed expectations at - // scope exit, but not a mismatch that happened to consume nothing. Distinguishing "expectation - // mismatch" from "real process failure" here would need a marker type incompatible with - // `RuntimeException` (see `mock_match`'s doc comment) — deferred until a concrete test needs it. - pub fn execute_args( - &mut self, - command: &[String], - output: &mut String, - cwd: Option<&str>, - ) -> i64 { - let mut buf = String::new(); - let rc = self - .execute(CommandLine::Args(command.to_vec()), &mut buf, cwd) - .unwrap_or(1); - *output = buf; - rc - } - /// runs a process on the commandline in TTY mode pub fn execute_tty(&mut self, command: C, cwd: Option<&str>) -> anyhow::Result where @@ -984,6 +958,12 @@ impl IntoExecCommand for &[String] { } } +impl IntoExecCommand for &[String; N] { + fn into_exec_command(self) -> CommandLine { + CommandLine::Args(self.to_vec()) + } +} + /// Models the `mixed &$output` parameter of `ProcessExecutor::execute` (cf. /// `composer/src/Composer/Util/ProcessExecutor.php`). In PHP the behaviour is selected by /// `func_num_args()` and `is_callable($output)`; here each behaviour is a distinct implementing type: diff --git a/crates/shirabe/src/util/svn.rs b/crates/shirabe/src/util/svn.rs index b4f53470..aab90133 100644 --- a/crates/shirabe/src/util/svn.rs +++ b/crates/shirabe/src/util/svn.rs @@ -396,20 +396,20 @@ impl Svn { } /// Returns the version of the svn binary contained in PATH - pub fn binary_version(&mut self) -> Option { + pub fn binary_version(&mut self) -> anyhow::Result> { let mut cached = VERSION.lock().unwrap(); if cached.is_none() { let mut output = String::new(); - if 0 == self.process.borrow_mut().execute_args( + if 0 == self.process.borrow_mut().execute( &["svn".to_string(), "--version".to_string()], &mut output, None, - ) && let Some(matches) = preg_match(php_regex!(r"{(\d+(?:\.\d+)+)}"), &output) + )? && let Some(matches) = preg_match(php_regex!(r"{(\d+(?:\.\d+)+)}"), &output) { *cached = Some(matches.get(1).unwrap_or_default().to_string()); } } - cached.clone() + Ok(cached.clone()) } } diff --git a/crates/shirabe/tests/command/init_command_test.rs b/crates/shirabe/tests/command/init_command_test.rs index e82f2bd7..ffb98fbb 100644 --- a/crates/shirabe/tests/command/init_command_test.rs +++ b/crates/shirabe/tests/command/init_command_test.rs @@ -617,7 +617,7 @@ fn test_get_git_config() { let _restore_home = RestoreEnv::new("HOME", original_home); let command = InitCommand::new(); - let git_config = command.__get_git_config(); + let git_config = command.__get_git_config().unwrap(); assert!(git_config.contains_key("user.name")); assert!(git_config.contains_key("user.email")); } diff --git a/crates/shirabe/tests/util/bitbucket_test.rs b/crates/shirabe/tests/util/bitbucket_test.rs index e0fb9ef2..19641fed 100644 --- a/crates/shirabe/tests/util/bitbucket_test.rs +++ b/crates/shirabe/tests/util/bitbucket_test.rs @@ -555,7 +555,11 @@ fn test_get_token_without_access_token() { fn test_authorize_oauth_with_wrong_origin_url() { let config = ConfigStubBuilder::new().build_shared(); let mut f = set_up_with_config_and_http(config, vec![]); - assert!(!f.bitbucket.authorize_oauth(&format!("non-{}", ORIGIN))); + assert!( + !f.bitbucket + .authorize_oauth(&format!("non-{}", ORIGIN)) + .unwrap() + ); } #[test] @@ -578,7 +582,7 @@ fn test_authorize_oauth_without_available_git_config_token() { let mut bitbucket = Bitbucket::new(io, config, Some(process), Some(http_downloader), Some(time)).unwrap(); - assert!(!bitbucket.authorize_oauth(ORIGIN)); + assert!(!bitbucket.authorize_oauth(ORIGIN).unwrap()); } #[test] @@ -595,5 +599,5 @@ fn test_authorize_oauth_with_available_git_config_token() { let mut bitbucket = Bitbucket::new(io, config, Some(process), Some(http_downloader), Some(time)).unwrap(); - assert!(bitbucket.authorize_oauth(ORIGIN)); + assert!(bitbucket.authorize_oauth(ORIGIN).unwrap()); } diff --git a/crates/shirabe/tests/util/git_test.rs b/crates/shirabe/tests/util/git_test.rs index 3d447553..448935ec 100644 --- a/crates/shirabe/tests/util/git_test.rs +++ b/crates/shirabe/tests/util/git_test.rs @@ -68,10 +68,6 @@ impl ConfigSourceInterface for NullConfigSource { } } -// PHP's `commandCallable` returns a bare string (`'git command'`); Rust's `run_command` -// flattens each callable to a `Vec` and hands it to `execute_args`, which always -// builds a `PhpMixed::List`. So the single-token string command becomes a one-element list, -// and the corresponding process expectation is a one-element list as well. fn build_git( io: IOStub, config: Config, diff --git a/crates/shirabe/tests/util/perforce_test.rs b/crates/shirabe/tests/util/perforce_test.rs index dfeae457..76ea3f8e 100644 --- a/crates/shirabe/tests/util/perforce_test.rs +++ b/crates/shirabe/tests/util/perforce_test.rs @@ -823,7 +823,8 @@ fn test_check_server_exists() { ); let result = - Perforce::check_server_exists("perforce.does.exist:port", &mut process.borrow_mut()); + Perforce::check_server_exists("perforce.does.exist:port", &mut process.borrow_mut()) + .unwrap(); assert!(result); } @@ -843,7 +844,8 @@ fn test_check_server_client_error() { ); let result = - Perforce::check_server_exists("perforce.does.exist:port", &mut process.borrow_mut()); + Perforce::check_server_exists("perforce.does.exist:port", &mut process.borrow_mut()) + .unwrap(); assert!(!result); } -- cgit v1.3.1-4-g156e