From 68492c20d8286a5243fd6004891d6c56c0d233e8 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 12 Aug 2024 12:42:00 +0800 Subject: [PATCH 01/42] refactor cliInputs --- check_diff/src/lib.rs | 2 ++ check_diff/src/main.rs | 17 +---------------- check_diff/src/structs.rs | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 16 deletions(-) create mode 100644 check_diff/src/structs.rs diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index b83d67c8b6e..a96fd719af2 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -4,6 +4,8 @@ use std::path::Path; use std::process::Command; use tracing::info; +pub mod structs; + pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, IO(std::io::Error), diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 01c5926c490..d814e466f6a 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,21 +1,6 @@ +use check_diff::structs::CliInputs; use clap::Parser; -/// Inputs for the check_diff script -#[derive(Parser)] -struct CliInputs { - /// Git url of a rustfmt fork to compare against the latest master rustfmt - remote_repo_url: String, - /// Name of the feature branch on the forked repo - feature_branch: String, - /// Optional commit hash from the feature branch - #[arg(short, long)] - commit_hash: Option, - /// Optional comma separated list of rustfmt config options to - /// pass when running the feature branch - #[arg(value_delimiter = ',', short, long, num_args = 1..)] - rustfmt_config: Option>, -} - fn main() { let _args = CliInputs::parse(); } diff --git a/check_diff/src/structs.rs b/check_diff/src/structs.rs new file mode 100644 index 00000000000..ec313855929 --- /dev/null +++ b/check_diff/src/structs.rs @@ -0,0 +1,17 @@ +use clap::Parser; + +/// Inputs for the check_diff script +#[derive(Parser)] +pub struct CliInputs { + /// Git url of a rustfmt fork to compare against the latest master rustfmt + remote_repo_url: String, + /// Name of the feature branch on the forked repo + feature_branch: String, + /// Optional commit hash from the feature branch + #[arg(short, long)] + commit_hash: Option, + /// Optional comma separated list of rustfmt config options to + /// pass when running the feature branch + #[arg(value_delimiter = ',', short, long, num_args = 1..)] + rustfmt_config: Option>, +} From 7a0e5dcf64280381a934a8e0aebd67c2e5902dea Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 12 Aug 2024 12:51:53 +0800 Subject: [PATCH 02/42] add compile_rustfmt skeleton --- check_diff/Cargo.toml | 1 - check_diff/src/lib.rs | 5 +++++ check_diff/src/main.rs | 9 +++++++-- 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/check_diff/Cargo.toml b/check_diff/Cargo.toml index 4ae8a5f1f3a..6c277f4af64 100644 --- a/check_diff/Cargo.toml +++ b/check_diff/Cargo.toml @@ -9,5 +9,4 @@ edition = "2021" clap = { version = "4.4.2", features = ["derive"] } tracing = "0.1.37" tracing-subscriber = { version = "0.3.17", features = ["env-filter"] } -[dev-dependencies] tempfile = "3" diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index a96fd719af2..3b4f8d3575c 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -2,6 +2,7 @@ use std::env; use std::io; use std::path::Path; use std::process::Command; +use structs::CliInputs; use tracing::info; pub mod structs; @@ -58,3 +59,7 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { ); return Ok(()); } + +pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> Result<(), GitError> { + return Ok(()); +} diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index d814e466f6a..43525209022 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,6 +1,11 @@ -use check_diff::structs::CliInputs; +use check_diff::{compile_rustfmt, structs::CliInputs}; use clap::Parser; +use tempfile::Builder; +use tracing::info; fn main() { - let _args = CliInputs::parse(); + let args = CliInputs::parse(); + let tmp_dir = Builder::new().tempdir_in("").unwrap(); + info!("Created tmp_dir {:?}", tmp_dir); + let _ = compile_rustfmt(tmp_dir.path(), args); } From e80001f611183ffeb35f1ea083674edb8d599666 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 12 Aug 2024 13:38:26 +0800 Subject: [PATCH 03/42] refactor git commands --- check_diff/src/git.rs | 83 +++++++++++++++++++++++++++++++++++++++ check_diff/src/lib.rs | 72 ++++++++++++++------------------- check_diff/src/structs.rs | 8 ++-- check_diff/tests/git.rs | 2 +- 4 files changed, 117 insertions(+), 48 deletions(-) create mode 100644 check_diff/src/git.rs diff --git a/check_diff/src/git.rs b/check_diff/src/git.rs new file mode 100644 index 00000000000..b3fa260ad9e --- /dev/null +++ b/check_diff/src/git.rs @@ -0,0 +1,83 @@ +use std::io; +use std::path::Path; +use std::process::Command; +use tracing::info; + +pub enum GitError { + FailedClone { stdout: Vec, stderr: Vec }, + FailedRemoteAdd { stdout: Vec, stderr: Vec }, + FailedFetch { stdout: Vec, stderr: Vec }, + IO(std::io::Error), +} + +impl From for GitError { + fn from(error: io::Error) -> Self { + GitError::IO(error) + } +} + +/// Clone a git repository +/// +/// Parameters: +/// url: git clone url +/// dest: directory where the repo should be cloned +pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .env("GIT_TERMINAL_PROMPT", "0") + .args([ + "clone", + "--quiet", + url, + "--depth", + "1", + dest.to_str().unwrap(), + ]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedClone { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully clone repository."); + return Ok(()); +} + +pub fn git_remote_add(url: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["remote", "add", "feature", url]) + .output()?; + + if !git_cmd.status.success() { + let error = GitError::FailedRemoteAdd { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully added remote."); + return Ok(()); +} + +pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["fetch", "feature", branch_name]) + .output()?; + + if !git_cmd.status.success() { + let error = GitError::FailedFetch { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully fetched."); + return Ok(()); +} diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 3b4f8d3575c..c6981e731fb 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,54 +1,17 @@ +use git::clone_git_repo; +use git::git_fetch; +use git::git_remote_add; +use git::GitError; use std::env; use std::io; use std::path::Path; -use std::process::Command; use structs::CliInputs; use tracing::info; +pub mod git; pub mod structs; -pub enum GitError { - FailedClone { stdout: Vec, stderr: Vec }, - IO(std::io::Error), -} - -impl From for GitError { - fn from(error: io::Error) -> Self { - GitError::IO(error) - } -} - -/// Clone a git repository -/// -/// Parameters: -/// url: git clone url -/// dest: directory where the repo should be cloned -pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { - let git_cmd = Command::new("git") - .env("GIT_TERMINAL_PROMPT", "0") - .args([ - "clone", - "--quiet", - url, - "--depth", - "1", - dest.to_str().unwrap(), - ]) - .output()?; - - // if the git command does not return successfully, - // any command on the repo will fail. So fail fast. - if !git_cmd.status.success() { - let error = GitError::FailedClone { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }; - return Err(error); - } - - info!("Successfully clone repository."); - return Ok(()); -} +const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); @@ -60,6 +23,29 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { return Ok(()); } +// Compiles and produces two rustfmt binaries. +// One for the current master, and another for the feature branch +// Parameters: +// dest: Directory where rustfmt will be cloned pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> Result<(), GitError> { + let clone_repo_result = clone_git_repo(RUSTFMT_REPO, dest); + + if clone_repo_result.is_err() { + return clone_repo_result; + } + + let remote_add_result = git_remote_add(inputs.remote_repo_url.as_str()); + if remote_add_result.is_err() { + return remote_add_result; + } + + let fetch_result = git_fetch(inputs.feature_branch.as_str()); + if fetch_result.is_err() { + return fetch_result; + } + + let cargo_version = env!("CARGO_PKG_VERSION"); + info!("Compiling with {}", cargo_version); + return Ok(()); } diff --git a/check_diff/src/structs.rs b/check_diff/src/structs.rs index ec313855929..eabf273e739 100644 --- a/check_diff/src/structs.rs +++ b/check_diff/src/structs.rs @@ -4,14 +4,14 @@ use clap::Parser; #[derive(Parser)] pub struct CliInputs { /// Git url of a rustfmt fork to compare against the latest master rustfmt - remote_repo_url: String, + pub remote_repo_url: String, /// Name of the feature branch on the forked repo - feature_branch: String, + pub feature_branch: String, /// Optional commit hash from the feature branch #[arg(short, long)] - commit_hash: Option, + pub commit_hash: Option, /// Optional comma separated list of rustfmt config options to /// pass when running the feature branch #[arg(value_delimiter = ',', short, long, num_args = 1..)] - rustfmt_config: Option>, + pub rustfmt_config: Option>, } diff --git a/check_diff/tests/git.rs b/check_diff/tests/git.rs index 677c3840e1e..4b2c7877710 100644 --- a/check_diff/tests/git.rs +++ b/check_diff/tests/git.rs @@ -1,4 +1,4 @@ -use check_diff::clone_git_repo; +use check_diff::git::clone_git_repo; use tempfile::Builder; From 4fbb6de175bf0e70245ecb52981f70861c550e3e Mon Sep 17 00:00:00 2001 From: benluiwj Date: Tue, 13 Aug 2024 10:30:56 +0800 Subject: [PATCH 04/42] refactor git commands --- check_diff/src/git.rs | 92 +++++++++++++++++++------------------------ 1 file changed, 40 insertions(+), 52 deletions(-) diff --git a/check_diff/src/git.rs b/check_diff/src/git.rs index b3fa260ad9e..412cf7cd3ea 100644 --- a/check_diff/src/git.rs +++ b/check_diff/src/git.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::io; use std::path::Path; use std::process::Command; @@ -15,69 +16,56 @@ impl From for GitError { GitError::IO(error) } } - -/// Clone a git repository -/// -/// Parameters: -/// url: git clone url -/// dest: directory where the repo should be cloned -pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { - let git_cmd = Command::new("git") - .env("GIT_TERMINAL_PROMPT", "0") - .args([ - "clone", - "--quiet", - url, - "--depth", - "1", - dest.to_str().unwrap(), - ]) - .output()?; - +/// Runs the git command with specified args, env vars, +/// error and log messages +fn git_runner( + args: Vec<&str>, + envs: HashMap<&str, &str>, + error: GitError, + log_message: &str, +) -> Result<(), GitError> { + let git_cmd = Command::new("git").envs(&envs).args(args).output()?; // if the git command does not return successfully, - // any command on the repo will fail. So fail fast. + // any command on the repo will fail. So fail fast. if !git_cmd.status.success() { - let error = GitError::FailedClone { + let git_error = error { stdout: git_cmd.stdout, stderr: git_cmd.stderr, }; - return Err(error); + return Err(git_error); } - - info!("Successfully clone repository."); + info!("{}", log_message); return Ok(()); } -pub fn git_remote_add(url: &str) -> Result<(), GitError> { - let git_cmd = Command::new("git") - .args(["remote", "add", "feature", url]) - .output()?; - - if !git_cmd.status.success() { - let error = GitError::FailedRemoteAdd { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }; - return Err(error); - } +/// Clone a git repository +/// +/// Parameters: +/// url: git clone url +/// dest: directory where the repo should be cloned +pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { + let args = [ + "clone", + "--quiet", + url, + "--depth", + "1", + dest.to_str().unwrap(), + ] + .to_vec(); + let env_vars = HashMap::from([("GIT_TERMINAL_PROMPT", "0")]); + let log_message = "Successfully clone repository."; + git_runner(args, env_vars, GitError::FailedClone, log_message) +} - info!("Successfully added remote."); - return Ok(()); +pub fn git_remote_add(url: &str) -> Result<(), GitError> { + let args = ["remote", "add", "feature", url].to_vec(); + let log_message = "Successfully added remote."; + git_runner(args, HashMap::new(), GitError::FailedRemoteAdd, log_message) } pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { - let git_cmd = Command::new("git") - .args(["fetch", "feature", branch_name]) - .output()?; - - if !git_cmd.status.success() { - let error = GitError::FailedFetch { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }; - return Err(error); - } - - info!("Successfully fetched."); - return Ok(()); + let args = ["fetch", "feature", branch_name].to_vec(); + let log_message = "Successfully fetched."; + git_runner(args, HashMap::new(), GitError::FailedFetch, log_message) } From 273eb9bec68f6b2ba15533f1e311b1b3c0476d41 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Tue, 13 Aug 2024 10:46:06 +0800 Subject: [PATCH 05/42] improve git error handling --- check_diff/src/git.rs | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/check_diff/src/git.rs b/check_diff/src/git.rs index 412cf7cd3ea..6381eda48a0 100644 --- a/check_diff/src/git.rs +++ b/check_diff/src/git.rs @@ -4,6 +4,12 @@ use std::path::Path; use std::process::Command; use tracing::info; +pub enum GitCommand { + Clone, + RemoteAdd, + Fetch, +} + pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, @@ -21,17 +27,28 @@ impl From for GitError { fn git_runner( args: Vec<&str>, envs: HashMap<&str, &str>, - error: GitError, + command: GitCommand, log_message: &str, ) -> Result<(), GitError> { let git_cmd = Command::new("git").envs(&envs).args(args).output()?; // if the git command does not return successfully, // any command on the repo will fail. So fail fast. if !git_cmd.status.success() { - let git_error = error { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, + let git_error = match command { + GitCommand::Clone => GitError::FailedClone { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }, + GitCommand::Fetch => GitError::FailedFetch { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }, + GitCommand::RemoteAdd => GitError::FailedRemoteAdd { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }, }; + return Err(git_error); } info!("{}", log_message); @@ -55,17 +72,17 @@ pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { .to_vec(); let env_vars = HashMap::from([("GIT_TERMINAL_PROMPT", "0")]); let log_message = "Successfully clone repository."; - git_runner(args, env_vars, GitError::FailedClone, log_message) + git_runner(args, env_vars, GitCommand::Clone, log_message) } pub fn git_remote_add(url: &str) -> Result<(), GitError> { let args = ["remote", "add", "feature", url].to_vec(); let log_message = "Successfully added remote."; - git_runner(args, HashMap::new(), GitError::FailedRemoteAdd, log_message) + git_runner(args, HashMap::new(), GitCommand::RemoteAdd, log_message) } pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { let args = ["fetch", "feature", branch_name].to_vec(); let log_message = "Successfully fetched."; - git_runner(args, HashMap::new(), GitError::FailedFetch, log_message) + git_runner(args, HashMap::new(), GitCommand::Fetch, log_message) } From bf82485a617e92f0711bb206d25b040b1a4cdedf Mon Sep 17 00:00:00 2001 From: benluiwj Date: Tue, 13 Aug 2024 10:48:20 +0800 Subject: [PATCH 06/42] change access modifiers --- check_diff/src/git.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_diff/src/git.rs b/check_diff/src/git.rs index 6381eda48a0..65abbc55183 100644 --- a/check_diff/src/git.rs +++ b/check_diff/src/git.rs @@ -4,7 +4,7 @@ use std::path::Path; use std::process::Command; use tracing::info; -pub enum GitCommand { +enum GitCommand { Clone, RemoteAdd, Fetch, From 51a743e75982835055acc0f2c756f24f1164b621 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Tue, 13 Aug 2024 15:16:41 +0800 Subject: [PATCH 07/42] add ld lib path functionality --- check_diff/src/lib.rs | 54 ++++++++++++++++++++++++++++++------------- 1 file changed, 38 insertions(+), 16 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index c6981e731fb..fe9ece3e811 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,10 +1,11 @@ use git::clone_git_repo; use git::git_fetch; use git::git_remote_add; -use git::GitError; use std::env; use std::io; +use std::io::Error; use std::path::Path; +use std::process::Command; use structs::CliInputs; use tracing::info; @@ -27,25 +28,46 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { // One for the current master, and another for the feature branch // Parameters: // dest: Directory where rustfmt will be cloned -pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> Result<(), GitError> { - let clone_repo_result = clone_git_repo(RUSTFMT_REPO, dest); +pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> io::Result { + let Ok(_) = clone_git_repo(RUSTFMT_REPO, dest) else { + return Err(Error::other("Error cloning repo while compiling rustfmt")); + }; - if clone_repo_result.is_err() { - return clone_repo_result; - } + let Ok(_) = git_remote_add(inputs.remote_repo_url.as_str()) else { + return Err(Error::other(format!( + "Error adding remote from {} while compiling rustfmt", + inputs.remote_repo_url + ))); + }; - let remote_add_result = git_remote_add(inputs.remote_repo_url.as_str()); - if remote_add_result.is_err() { - return remote_add_result; - } - - let fetch_result = git_fetch(inputs.feature_branch.as_str()); - if fetch_result.is_err() { - return fetch_result; - } + let Ok(_) = git_fetch(inputs.feature_branch.as_str()) else { + return Err(Error::other(format!( + "Error fetching from {} while compiling rustfmt", + inputs.feature_branch + ))); + }; let cargo_version = env!("CARGO_PKG_VERSION"); info!("Compiling with {}", cargo_version); - return Ok(()); + //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each + // binary can find it's runtime dependencies. See https://github.com/rust-lang/rustfmt/issues/5675 + // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary + + let Ok(command) = std::process::Command::new("rustc") + .args(["--print", "sysroot"]) + .output() + else { + return Err(Error::other("Error getting sysroot")); + }; + + let Ok(sysroot) = String::from_utf8(command.stdout) else { + return Err(Error::other("Error converting sysroot to string")); + }; + + let ld_lib_path = format!("{}/lib", sysroot.trim_end()); + env::set_var("LD_LIBRARY_PATH", ld_lib_path); + let result = Command::new("ls"); + + return Ok(result); } From f7775dfbd9d0882351226d865edbec6c1570ce8f Mon Sep 17 00:00:00 2001 From: benluiwj Date: Tue, 13 Aug 2024 16:02:50 +0800 Subject: [PATCH 08/42] fix ci --- check_diff/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index fe9ece3e811..223ade22d29 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -51,7 +51,8 @@ pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> io::Result { info!("Compiling with {}", cargo_version); //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each - // binary can find it's runtime dependencies. See https://github.com/rust-lang/rustfmt/issues/5675 + // binary can find it's runtime dependencies. + // See https://github.com/rust-lang/rustfmt/issues/5675 // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary let Ok(command) = std::process::Command::new("rustc") @@ -67,6 +68,8 @@ pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> io::Result { let ld_lib_path = format!("{}/lib", sysroot.trim_end()); env::set_var("LD_LIBRARY_PATH", ld_lib_path); + info!("Building rustfmt from scratch"); + let result = Command::new("ls"); return Ok(result); From c2a04475d620b08e3634a7e9c76200c771cf5055 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Wed, 14 Aug 2024 18:34:14 +0800 Subject: [PATCH 09/42] revert refactors --- check_diff/src/git.rs | 88 -------------------------------- check_diff/src/lib.rs | 103 ++++++++++++++++++++++++++++++++++---- check_diff/src/main.rs | 25 ++++++++- check_diff/src/structs.rs | 17 ------- 4 files changed, 115 insertions(+), 118 deletions(-) delete mode 100644 check_diff/src/git.rs delete mode 100644 check_diff/src/structs.rs diff --git a/check_diff/src/git.rs b/check_diff/src/git.rs deleted file mode 100644 index 65abbc55183..00000000000 --- a/check_diff/src/git.rs +++ /dev/null @@ -1,88 +0,0 @@ -use std::collections::HashMap; -use std::io; -use std::path::Path; -use std::process::Command; -use tracing::info; - -enum GitCommand { - Clone, - RemoteAdd, - Fetch, -} - -pub enum GitError { - FailedClone { stdout: Vec, stderr: Vec }, - FailedRemoteAdd { stdout: Vec, stderr: Vec }, - FailedFetch { stdout: Vec, stderr: Vec }, - IO(std::io::Error), -} - -impl From for GitError { - fn from(error: io::Error) -> Self { - GitError::IO(error) - } -} -/// Runs the git command with specified args, env vars, -/// error and log messages -fn git_runner( - args: Vec<&str>, - envs: HashMap<&str, &str>, - command: GitCommand, - log_message: &str, -) -> Result<(), GitError> { - let git_cmd = Command::new("git").envs(&envs).args(args).output()?; - // if the git command does not return successfully, - // any command on the repo will fail. So fail fast. - if !git_cmd.status.success() { - let git_error = match command { - GitCommand::Clone => GitError::FailedClone { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }, - GitCommand::Fetch => GitError::FailedFetch { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }, - GitCommand::RemoteAdd => GitError::FailedRemoteAdd { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, - }, - }; - - return Err(git_error); - } - info!("{}", log_message); - return Ok(()); -} - -/// Clone a git repository -/// -/// Parameters: -/// url: git clone url -/// dest: directory where the repo should be cloned -pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { - let args = [ - "clone", - "--quiet", - url, - "--depth", - "1", - dest.to_str().unwrap(), - ] - .to_vec(); - let env_vars = HashMap::from([("GIT_TERMINAL_PROMPT", "0")]); - let log_message = "Successfully clone repository."; - git_runner(args, env_vars, GitCommand::Clone, log_message) -} - -pub fn git_remote_add(url: &str) -> Result<(), GitError> { - let args = ["remote", "add", "feature", url].to_vec(); - let log_message = "Successfully added remote."; - git_runner(args, HashMap::new(), GitCommand::RemoteAdd, log_message) -} - -pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { - let args = ["fetch", "feature", branch_name].to_vec(); - let log_message = "Successfully fetched."; - git_runner(args, HashMap::new(), GitCommand::Fetch, log_message) -} diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 223ade22d29..00a4d32925f 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,19 +1,95 @@ -use git::clone_git_repo; -use git::git_fetch; -use git::git_remote_add; use std::env; use std::io; use std::io::Error; use std::path::Path; use std::process::Command; -use structs::CliInputs; use tracing::info; -pub mod git; -pub mod structs; +pub enum GitError { + FailedClone { stdout: Vec, stderr: Vec }, + FailedRemoteAdd { stdout: Vec, stderr: Vec }, + FailedFetch { stdout: Vec, stderr: Vec }, + IO(std::io::Error), +} const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; +impl From for GitError { + fn from(error: io::Error) -> Self { + GitError::IO(error) + } +} + +/// Clone a git repository +/// +/// Parameters: +/// url: git clone url +/// dest: directory where the repo should be cloned +pub fn clone_git_repo(url: &str, dest: &Path) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .env("GIT_TERMINAL_PROMPT", "0") + .args([ + "clone", + "--quiet", + url, + "--depth", + "1", + dest.to_str().unwrap(), + ]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedClone { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully clone repository."); + return Ok(()); +} + +pub fn git_remote_add(url: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["remote", "add", "feature", url]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedRemoteAdd { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully added remote."); + return Ok(()); +} + +pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { + let git_cmd = Command::new("git") + .args(["fetch", "feature", branch_name]) + .output()?; + + // if the git command does not return successfully, + // any command on the repo will fail. So fail fast. + if !git_cmd.status.success() { + let error = GitError::FailedFetch { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully fetched."); + return Ok(()); +} + pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); env::set_current_dir(&dest_path)?; @@ -28,22 +104,27 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { // One for the current master, and another for the feature branch // Parameters: // dest: Directory where rustfmt will be cloned -pub fn compile_rustfmt(dest: &Path, inputs: CliInputs) -> io::Result { +pub fn compile_rustfmt( + dest: &Path, + remote_repo_url: String, + feature_branch: String, + rustfmt_config: Option>, +) -> io::Result { let Ok(_) = clone_git_repo(RUSTFMT_REPO, dest) else { return Err(Error::other("Error cloning repo while compiling rustfmt")); }; - let Ok(_) = git_remote_add(inputs.remote_repo_url.as_str()) else { + let Ok(_) = git_remote_add(remote_repo_url.as_str()) else { return Err(Error::other(format!( "Error adding remote from {} while compiling rustfmt", - inputs.remote_repo_url + remote_repo_url ))); }; - let Ok(_) = git_fetch(inputs.feature_branch.as_str()) else { + let Ok(_) = git_fetch(feature_branch.as_str()) else { return Err(Error::other(format!( "Error fetching from {} while compiling rustfmt", - inputs.feature_branch + feature_branch ))); }; diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 43525209022..a07f48d6aa2 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -1,11 +1,32 @@ -use check_diff::{compile_rustfmt, structs::CliInputs}; +use check_diff::compile_rustfmt; use clap::Parser; use tempfile::Builder; use tracing::info; +/// Inputs for the check_diff script +#[derive(Parser)] +struct CliInputs { + /// Git url of a rustfmt fork to compare against the latest master rustfmt + remote_repo_url: String, + /// Name of the feature branch on the forked repo + feature_branch: String, + /// Optional commit hash from the feature branch + #[arg(short, long)] + commit_hash: Option, + /// Optional comma separated list of rustfmt config options to + /// pass when running the feature branch + #[arg(value_delimiter = ',', short, long, num_args = 1..)] + rustfmt_config: Option>, +} + fn main() { let args = CliInputs::parse(); let tmp_dir = Builder::new().tempdir_in("").unwrap(); info!("Created tmp_dir {:?}", tmp_dir); - let _ = compile_rustfmt(tmp_dir.path(), args); + let _ = compile_rustfmt( + tmp_dir.path(), + args.remote_repo_url, + args.feature_branch, + args.rustfmt_config, + ); } diff --git a/check_diff/src/structs.rs b/check_diff/src/structs.rs deleted file mode 100644 index eabf273e739..00000000000 --- a/check_diff/src/structs.rs +++ /dev/null @@ -1,17 +0,0 @@ -use clap::Parser; - -/// Inputs for the check_diff script -#[derive(Parser)] -pub struct CliInputs { - /// Git url of a rustfmt fork to compare against the latest master rustfmt - pub remote_repo_url: String, - /// Name of the feature branch on the forked repo - pub feature_branch: String, - /// Optional commit hash from the feature branch - #[arg(short, long)] - pub commit_hash: Option, - /// Optional comma separated list of rustfmt config options to - /// pass when running the feature branch - #[arg(value_delimiter = ',', short, long, num_args = 1..)] - pub rustfmt_config: Option>, -} From 28775aad98a74a13e65c3d066c8b074711db3a27 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Wed, 14 Aug 2024 18:35:11 +0800 Subject: [PATCH 10/42] revert test refactor --- check_diff/tests/git.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_diff/tests/git.rs b/check_diff/tests/git.rs index 4b2c7877710..677c3840e1e 100644 --- a/check_diff/tests/git.rs +++ b/check_diff/tests/git.rs @@ -1,4 +1,4 @@ -use check_diff::git::clone_git_repo; +use check_diff::clone_git_repo; use tempfile::Builder; From 8295dd7942b44c8b28304b77073b443e50846cfe Mon Sep 17 00:00:00 2001 From: benluiwj Date: Wed, 14 Aug 2024 18:39:54 +0800 Subject: [PATCH 11/42] fix ci --- check_diff/src/lib.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 00a4d32925f..8a2678dd78f 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -12,8 +12,6 @@ pub enum GitError { IO(std::io::Error), } -const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; - impl From for GitError { fn from(error: io::Error) -> Self { GitError::IO(error) @@ -108,8 +106,10 @@ pub fn compile_rustfmt( dest: &Path, remote_repo_url: String, feature_branch: String, - rustfmt_config: Option>, + _rustfmt_config: Option>, ) -> io::Result { + const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; + let Ok(_) = clone_git_repo(RUSTFMT_REPO, dest) else { return Err(Error::other("Error cloning repo while compiling rustfmt")); }; @@ -135,7 +135,6 @@ pub fn compile_rustfmt( // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary - let Ok(command) = std::process::Command::new("rustc") .args(["--print", "sysroot"]) .output() @@ -147,8 +146,7 @@ pub fn compile_rustfmt( return Err(Error::other("Error converting sysroot to string")); }; - let ld_lib_path = format!("{}/lib", sysroot.trim_end()); - env::set_var("LD_LIBRARY_PATH", ld_lib_path); + let _ld_lib_path = format!("{}/lib", sysroot.trim_end()); info!("Building rustfmt from scratch"); let result = Command::new("ls"); From 5b93dc73b0e936825ac4a88a8ba1b72c3530ccdd Mon Sep 17 00:00:00 2001 From: benluiwj Date: Wed, 14 Aug 2024 19:12:12 +0800 Subject: [PATCH 12/42] implement checkdiff error --- check_diff/src/lib.rs | 44 +++++++++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 16 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 8a2678dd78f..1b3c3f63efc 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -5,6 +5,19 @@ use std::path::Path; use std::process::Command; use tracing::info; +pub enum CheckDiffError { + FailedGit(GitError), + FailedCommand, + FailedUtf8, + IO(std::io::Error), +} + +impl From for CheckDiffError { + fn from(error: io::Error) -> Self { + CheckDiffError::IO(error) + } +} + pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, @@ -107,26 +120,25 @@ pub fn compile_rustfmt( remote_repo_url: String, feature_branch: String, _rustfmt_config: Option>, -) -> io::Result { +) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; - let Ok(_) = clone_git_repo(RUSTFMT_REPO, dest) else { - return Err(Error::other("Error cloning repo while compiling rustfmt")); - }; + let clone_git_result = clone_git_repo(RUSTFMT_REPO, dest); + if clone_git_result.is_err() { + return Err(CheckDiffError::FailedGit(x.err().unwrap())); + } - let Ok(_) = git_remote_add(remote_repo_url.as_str()) else { - return Err(Error::other(format!( - "Error adding remote from {} while compiling rustfmt", - remote_repo_url - ))); - }; + let git_remote_add_result = git_remote_add(remote_repo_url.as_str()); + if git_remote_add_result.is_err() { + return Err(CheckDiffError::FailedGit( + git_remote_add_result.err().unwrap(), + )); + } - let Ok(_) = git_fetch(feature_branch.as_str()) else { - return Err(Error::other(format!( - "Error fetching from {} while compiling rustfmt", - feature_branch - ))); - }; + let git_fetch_result = git_fetch(feature_branch.as_str()); + if git_fetch_result.is_err() { + return Err(CheckDiffError::FailedGit(git_fetch_result.err().unwrap())); + } let cargo_version = env!("CARGO_PKG_VERSION"); info!("Compiling with {}", cargo_version); From e7aec13cf8d526e86fedd25798abc17c582dcebf Mon Sep 17 00:00:00 2001 From: benluiwj Date: Wed, 14 Aug 2024 19:16:21 +0800 Subject: [PATCH 13/42] add check diff error --- check_diff/src/lib.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 1b3c3f63efc..f68ff736990 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,14 +1,13 @@ use std::env; use std::io; -use std::io::Error; use std::path::Path; use std::process::Command; use tracing::info; pub enum CheckDiffError { FailedGit(GitError), - FailedCommand, - FailedUtf8, + FailedCommand(String), + FailedUtf8(String), IO(std::io::Error), } @@ -125,7 +124,7 @@ pub fn compile_rustfmt( let clone_git_result = clone_git_repo(RUSTFMT_REPO, dest); if clone_git_result.is_err() { - return Err(CheckDiffError::FailedGit(x.err().unwrap())); + return Err(CheckDiffError::FailedGit(clone_git_result.err().unwrap())); } let git_remote_add_result = git_remote_add(remote_repo_url.as_str()); @@ -151,11 +150,15 @@ pub fn compile_rustfmt( .args(["--print", "sysroot"]) .output() else { - return Err(Error::other("Error getting sysroot")); + return Err(CheckDiffError::FailedCommand( + "Error getting sysroot".to_string(), + )); }; let Ok(sysroot) = String::from_utf8(command.stdout) else { - return Err(Error::other("Error converting sysroot to string")); + return Err(CheckDiffError::FailedUtf8( + "Error converting sysroot to string".to_string(), + )); }; let _ld_lib_path = format!("{}/lib", sysroot.trim_end()); From f8c8953cc16abc5985791ac2556577293df4d6a7 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 17 Aug 2024 21:13:16 +0800 Subject: [PATCH 14/42] add more functionality --- check_diff/src/lib.rs | 52 +++++++++++++++++++++++++++++++++++++----- check_diff/src/main.rs | 2 +- 2 files changed, 47 insertions(+), 7 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index f68ff736990..5ef77e1f718 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -8,6 +8,8 @@ pub enum CheckDiffError { FailedGit(GitError), FailedCommand(String), FailedUtf8(String), + FailedSourceBuild(String), + FailedCopy(String), IO(std::io::Error), } @@ -21,6 +23,7 @@ pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, FailedFetch { stdout: Vec, stderr: Vec }, + FailedSwitch { stdout: Vec, stderr: Vec }, IO(std::io::Error), } @@ -100,6 +103,22 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { return Ok(()); } +pub fn git_switch(arg: &str, should_detach: bool) -> Result<(), GitError> { + let detach_arg = if should_detach { "--detach" } else { "" }; + let git_cmd = Command::new("git").args([arg, detach_arg]).output()?; + + if !git_cmd.status.success() { + let error = GitError::FailedSwitch { + stdout: git_cmd.stdout, + stderr: git_cmd.stderr, + }; + return Err(error); + } + + info!("Successfully switched to {}", arg); + return Ok(()); +} + pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { let dest_path = Path::new(&dest); env::set_current_dir(&dest_path)?; @@ -118,7 +137,7 @@ pub fn compile_rustfmt( dest: &Path, remote_repo_url: String, feature_branch: String, - _rustfmt_config: Option>, + commit_hash: Option, ) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; @@ -146,10 +165,7 @@ pub fn compile_rustfmt( // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary - let Ok(command) = std::process::Command::new("rustc") - .args(["--print", "sysroot"]) - .output() - else { + let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { return Err(CheckDiffError::FailedCommand( "Error getting sysroot".to_string(), )); @@ -162,7 +178,31 @@ pub fn compile_rustfmt( }; let _ld_lib_path = format!("{}/lib", sysroot.trim_end()); - info!("Building rustfmt from scratch"); + info!("Building rustfmt from source"); + let Ok(_) = Command::new("cargo") + .args(["build", "-q", "--release", "--bin", "rustfmt"]) + .output() + else { + return Err(CheckDiffError::FailedSourceBuild( + "Error building rustfmt from source".to_string(), + )); + }; + + let dest_rustfmt = format!("{}/rustfmt", dest.display()); + let Ok(_) = Command::new("cp") + .args(["target/release/rustfmt", dest_rustfmt.as_str()]) + .output() + else { + return Err(CheckDiffError::FailedCopy( + "Error copying rustfmt release to destination".to_string(), + )); + }; + + if (commit_hash.is_none() || feature_branch == commit_hash.unwrap()) { + git_switch(feature_branch.as_str(), false); + } else { + git_switch(commit_hash.unwrap().as_str(), true); + } let result = Command::new("ls"); diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index a07f48d6aa2..8aac6647db9 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -27,6 +27,6 @@ fn main() { tmp_dir.path(), args.remote_repo_url, args.feature_branch, - args.rustfmt_config, + args.commit_hash, ); } From 94cc2e099f3d761b627901beb422479003d4cd7d Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sun, 18 Aug 2024 21:38:26 +0800 Subject: [PATCH 15/42] fix build errors --- check_diff/src/lib.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 5ef77e1f718..1514039a816 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -198,11 +198,11 @@ pub fn compile_rustfmt( )); }; - if (commit_hash.is_none() || feature_branch == commit_hash.unwrap()) { - git_switch(feature_branch.as_str(), false); - } else { - git_switch(commit_hash.unwrap().as_str(), true); - } + let should_detach = commit_hash.is_some(); + let _ = git_switch( + commit_hash.unwrap_or(feature_branch).as_str(), + should_detach, + ); let result = Command::new("ls"); From 02908e470f3a335aad391525fd0a2299233f3367 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 19 Aug 2024 10:46:55 +0800 Subject: [PATCH 16/42] cleanup git errors --- check_diff/src/lib.rs | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 1514039a816..878131eeced 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -19,6 +19,12 @@ impl From for CheckDiffError { } } +impl From for CheckDiffError { + fn from(error: GitError) -> Self { + CheckDiffError::FailedGit(error) + } +} + pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, @@ -141,22 +147,11 @@ pub fn compile_rustfmt( ) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; - let clone_git_result = clone_git_repo(RUSTFMT_REPO, dest); - if clone_git_result.is_err() { - return Err(CheckDiffError::FailedGit(clone_git_result.err().unwrap())); - } + let _clone_git_result = clone_git_repo(RUSTFMT_REPO, dest)?; - let git_remote_add_result = git_remote_add(remote_repo_url.as_str()); - if git_remote_add_result.is_err() { - return Err(CheckDiffError::FailedGit( - git_remote_add_result.err().unwrap(), - )); - } + let _git_remote_add_result = git_remote_add(remote_repo_url.as_str())?; - let git_fetch_result = git_fetch(feature_branch.as_str()); - if git_fetch_result.is_err() { - return Err(CheckDiffError::FailedGit(git_fetch_result.err().unwrap())); - } + let _git_fetch_result = git_fetch(feature_branch.as_str())?; let cargo_version = env!("CARGO_PKG_VERSION"); info!("Compiling with {}", cargo_version); From 6af17e44fb69afe1b8a70130122150390665cb4b Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 19 Aug 2024 11:11:30 +0800 Subject: [PATCH 17/42] minor refactor --- check_diff/src/lib.rs | 91 ++++++++++++++++++++++++++++--------------- 1 file changed, 60 insertions(+), 31 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 878131eeced..e01ca76e75b 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -10,6 +10,7 @@ pub enum CheckDiffError { FailedUtf8(String), FailedSourceBuild(String), FailedCopy(String), + FailedCargoVersion(String), IO(std::io::Error), } @@ -135,27 +136,7 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { return Ok(()); } -// Compiles and produces two rustfmt binaries. -// One for the current master, and another for the feature branch -// Parameters: -// dest: Directory where rustfmt will be cloned -pub fn compile_rustfmt( - dest: &Path, - remote_repo_url: String, - feature_branch: String, - commit_hash: Option, -) -> Result { - const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; - - let _clone_git_result = clone_git_repo(RUSTFMT_REPO, dest)?; - - let _git_remote_add_result = git_remote_add(remote_repo_url.as_str())?; - - let _git_fetch_result = git_fetch(feature_branch.as_str())?; - - let cargo_version = env!("CARGO_PKG_VERSION"); - info!("Compiling with {}", cargo_version); - +pub fn get_ld_lib_path() -> Result { //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 @@ -172,9 +153,38 @@ pub fn compile_rustfmt( )); }; - let _ld_lib_path = format!("{}/lib", sysroot.trim_end()); - info!("Building rustfmt from source"); + let ld_lib_path = format!("{}/lib", sysroot.trim_end()); + return Ok(ld_lib_path); +} + +pub fn get_cargo_version() -> Result { + let Ok(command) = Command::new("cargo").args(["--version"]).output() else { + return Err(CheckDiffError::FailedCargoVersion( + "Failed to obtain cargo version".to_string(), + )); + }; + + let Ok(cargo_version) = String::from_utf8(command.stdout) else { + return Err(CheckDiffError::FailedUtf8( + "Error converting cargo version to string".to_string(), + )); + }; + + return Ok(cargo_version); +} + +pub fn copy_src_to_dst(src: &str, dst: &str) -> Result<(), CheckDiffError> { + let Ok(_) = Command::new("cp").args([src, dst]).output() else { + return Err(CheckDiffError::FailedCopy( + "Error copying rustfmt release to destination".to_string(), + )); + }; + return Ok(()); +} + +pub fn build_rsfmt_from_src(ld_lib_path: String) -> Result<(), CheckDiffError> { let Ok(_) = Command::new("cargo") + .env("LD_LIB_PATH", ld_lib_path) .args(["build", "-q", "--release", "--bin", "rustfmt"]) .output() else { @@ -182,16 +192,35 @@ pub fn compile_rustfmt( "Error building rustfmt from source".to_string(), )); }; + return Ok(()); +} + +// Compiles and produces two rustfmt binaries. +// One for the current master, and another for the feature branch +// Parameters: +// dest: Directory where rustfmt will be cloned +pub fn compile_rustfmt( + dest: &Path, + remote_repo_url: String, + feature_branch: String, + commit_hash: Option, +) -> Result { + const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; + + let _clone_git_result = clone_git_repo(RUSTFMT_REPO, dest)?; + let _git_remote_add_result = git_remote_add(remote_repo_url.as_str())?; + let _git_fetch_result = git_fetch(feature_branch.as_str())?; + + let cargo_version = get_cargo_version()?; + info!("Compiling with {}", cargo_version); + + let ld_lib_path = get_ld_lib_path()?; + + info!("Building rustfmt from source"); + let _build_from_src = build_rsfmt_from_src(ld_lib_path)?; let dest_rustfmt = format!("{}/rustfmt", dest.display()); - let Ok(_) = Command::new("cp") - .args(["target/release/rustfmt", dest_rustfmt.as_str()]) - .output() - else { - return Err(CheckDiffError::FailedCopy( - "Error copying rustfmt release to destination".to_string(), - )); - }; + let _cp = copy_src_to_dst("target/release/rustfmt", dest_rustfmt.as_str())?; let should_detach = commit_hash.is_some(); let _ = git_switch( From abe74b7a619fb1ba0c75b60ac4a1f99efc70d0c0 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 19 Aug 2024 11:22:37 +0800 Subject: [PATCH 18/42] implement initial rewrite --- check_diff/src/lib.rs | 39 ++++++++++++++++++++++++++++++++------- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index e01ca76e75b..d943f2b629c 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -11,6 +11,7 @@ pub enum CheckDiffError { FailedSourceBuild(String), FailedCopy(String), FailedCargoVersion(String), + FailedVersioning(String), IO(std::io::Error), } @@ -137,10 +138,6 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { } pub fn get_ld_lib_path() -> Result { - //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each - // binary can find it's runtime dependencies. - // See https://github.com/rust-lang/rustfmt/issues/5675 - // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { return Err(CheckDiffError::FailedCommand( "Error getting sysroot".to_string(), @@ -195,6 +192,13 @@ pub fn build_rsfmt_from_src(ld_lib_path: String) -> Result<(), CheckDiffError> { return Ok(()); } +pub fn get_binary_version(binary: String) -> Result<(), CheckDiffError> { + let Ok(_) = Command::new(binary.as_str()).args(["--version"]).output() else { + return Err(CheckDiffError::FailedVersioning(format!("Failed to get version for {}", binary))); + }; + return Ok(()); +} + // Compiles and produces two rustfmt binaries. // One for the current master, and another for the feature branch // Parameters: @@ -214,13 +218,17 @@ pub fn compile_rustfmt( let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); + //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each + // binary can find it's runtime dependencies. + // See https://github.com/rust-lang/rustfmt/issues/5675 + // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary let ld_lib_path = get_ld_lib_path()?; info!("Building rustfmt from source"); let _build_from_src = build_rsfmt_from_src(ld_lib_path)?; - let dest_rustfmt = format!("{}/rustfmt", dest.display()); - let _cp = copy_src_to_dst("target/release/rustfmt", dest_rustfmt.as_str())?; + let rustfmt_binary = format!("{}/rustfmt", dest.display()); + let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; let should_detach = commit_hash.is_some(); let _ = git_switch( @@ -228,7 +236,24 @@ pub fn compile_rustfmt( should_detach, ); - let result = Command::new("ls"); + // This will prepend the `LD_LIBRARY_PATH` for the feature branch rustfmt binary. + // In most cases the `LD_LIBRARY_PATH` should be the same for both rustfmt binaries that we build + // in `compile_rustfmt`, however, there are scenarios where each binary has different runtime + // dependencies. For example, during subtree syncs we bump the nightly toolchain required to build + // rustfmt, and therefore the feature branch relies on a newer set of runtime dependencies. + let ld_lib_path = get_ld_lib_path()?; + info!("Building rustfmt from source"); + let _build_from_src = build_rsfmt_from_src(ld_lib_path)?; + let feature_binary = format!("{}/feature_rustfmt", dest.display()); + + let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; + info!("\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", ld_lib_path); + + let rustfmt_version = get_binary_version(rustfmt_binary)?; + info!("\nRUSFMT_BIN {}\n", rustfmt_version); + + let feature_binary_version = get_binary_version(feature_binary)?; + info!("FEATURE_BIN {}\n", feature_binary_version); return Ok(result); } From 31943e28fe9280d7b395691c4e9ea176779567ee Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 19 Aug 2024 11:33:03 +0800 Subject: [PATCH 19/42] fix build errors --- check_diff/src/lib.rs | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index d943f2b629c..0b10cd1728c 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -179,7 +179,7 @@ pub fn copy_src_to_dst(src: &str, dst: &str) -> Result<(), CheckDiffError> { return Ok(()); } -pub fn build_rsfmt_from_src(ld_lib_path: String) -> Result<(), CheckDiffError> { +pub fn build_rsfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> { let Ok(_) = Command::new("cargo") .env("LD_LIB_PATH", ld_lib_path) .args(["build", "-q", "--release", "--bin", "rustfmt"]) @@ -192,11 +192,17 @@ pub fn build_rsfmt_from_src(ld_lib_path: String) -> Result<(), CheckDiffError> { return Ok(()); } -pub fn get_binary_version(binary: String) -> Result<(), CheckDiffError> { - let Ok(_) = Command::new(binary.as_str()).args(["--version"]).output() else { +pub fn get_binary_version(binary: String) -> Result { + let Ok(command) = Command::new(binary.as_str()).args(["--version"]).output() else { return Err(CheckDiffError::FailedVersioning(format!("Failed to get version for {}", binary))); }; - return Ok(()); + + let Ok(binary_version) = String::from_utf8(command.stdout) else { + return Err(CheckDiffError::FailedUtf8( + "Error converting binary version to string".to_string(), + )); + }; + return Ok(binary_version); } // Compiles and produces two rustfmt binaries. @@ -225,7 +231,7 @@ pub fn compile_rustfmt( let ld_lib_path = get_ld_lib_path()?; info!("Building rustfmt from source"); - let _build_from_src = build_rsfmt_from_src(ld_lib_path)?; + let _build_from_src = build_rsfmt_from_src(&ld_lib_path)?; let rustfmt_binary = format!("{}/rustfmt", dest.display()); let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; @@ -243,11 +249,11 @@ pub fn compile_rustfmt( // rustfmt, and therefore the feature branch relies on a newer set of runtime dependencies. let ld_lib_path = get_ld_lib_path()?; info!("Building rustfmt from source"); - let _build_from_src = build_rsfmt_from_src(ld_lib_path)?; + let _build_from_src = build_rsfmt_from_src(&ld_lib_path)?; let feature_binary = format!("{}/feature_rustfmt", dest.display()); let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; - info!("\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", ld_lib_path); + info!("\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", &ld_lib_path); let rustfmt_version = get_binary_version(rustfmt_binary)?; info!("\nRUSFMT_BIN {}\n", rustfmt_version); @@ -255,5 +261,7 @@ pub fn compile_rustfmt( let feature_binary_version = get_binary_version(feature_binary)?; info!("FEATURE_BIN {}\n", feature_binary_version); + let result = Command::new("ls"); + return Ok(result); } From 9f76de2905c0a1383532365d3a223bec45a1cb68 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Mon, 19 Aug 2024 11:37:07 +0800 Subject: [PATCH 20/42] fix formatting issues --- check_diff/src/lib.rs | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 0b10cd1728c..5910fa2de73 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -194,7 +194,10 @@ pub fn build_rsfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> pub fn get_binary_version(binary: String) -> Result { let Ok(command) = Command::new(binary.as_str()).args(["--version"]).output() else { - return Err(CheckDiffError::FailedVersioning(format!("Failed to get version for {}", binary))); + return Err(CheckDiffError::FailedVersioning(format!( + "Failed to get version for {}", + binary + ))); }; let Ok(binary_version) = String::from_utf8(command.stdout) else { @@ -243,17 +246,21 @@ pub fn compile_rustfmt( ); // This will prepend the `LD_LIBRARY_PATH` for the feature branch rustfmt binary. - // In most cases the `LD_LIBRARY_PATH` should be the same for both rustfmt binaries that we build - // in `compile_rustfmt`, however, there are scenarios where each binary has different runtime - // dependencies. For example, during subtree syncs we bump the nightly toolchain required to build - // rustfmt, and therefore the feature branch relies on a newer set of runtime dependencies. + // In most cases the `LD_LIBRARY_PATH` should be the same for both rustfmt binaries + // that we build in `compile_rustfmt`, however, there are scenarios where each binary + // has different runtime dependencies. For example, during subtree syncs we + // bump the nightly toolchain required to build rustfmt, and therefore the feature branch + // relies on a newer set of runtime dependencies. let ld_lib_path = get_ld_lib_path()?; info!("Building rustfmt from source"); let _build_from_src = build_rsfmt_from_src(&ld_lib_path)?; let feature_binary = format!("{}/feature_rustfmt", dest.display()); let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; - info!("\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", &ld_lib_path); + info!( + "\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", + &ld_lib_path + ); let rustfmt_version = get_binary_version(rustfmt_binary)?; info!("\nRUSFMT_BIN {}\n", rustfmt_version); From 769285f199c111e323b57ca9cff8f80c9a0c24f8 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 23 Aug 2024 16:54:51 +0800 Subject: [PATCH 21/42] update paths --- check_diff/src/lib.rs | 85 ++++++++++++++++++++++++++----------------- 1 file changed, 51 insertions(+), 34 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 5910fa2de73..d71c94d8b21 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,6 +1,7 @@ use std::env; use std::io; use std::path::Path; +use std::path::PathBuf; use std::process::Command; use tracing::info; @@ -12,6 +13,7 @@ pub enum CheckDiffError { FailedCopy(String), FailedCargoVersion(String), FailedVersioning(String), + FailedPathBuf(String), IO(std::io::Error), } @@ -170,8 +172,9 @@ pub fn get_cargo_version() -> Result { return Ok(cargo_version); } -pub fn copy_src_to_dst(src: &str, dst: &str) -> Result<(), CheckDiffError> { - let Ok(_) = Command::new("cp").args([src, dst]).output() else { +pub fn copy_src_to_dst(src: &str, dst: &PathBuf) -> Result<(), CheckDiffError> { + let dst_str = pathbuf_to_str(dst)?; + let Ok(_) = Command::new("cp").args([src, dst_str]).output() else { return Err(CheckDiffError::FailedCopy( "Error copying rustfmt release to destination".to_string(), )); @@ -179,7 +182,7 @@ pub fn copy_src_to_dst(src: &str, dst: &str) -> Result<(), CheckDiffError> { return Ok(()); } -pub fn build_rsfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> { +pub fn build_rustfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> { let Ok(_) = Command::new("cargo") .env("LD_LIB_PATH", ld_lib_path) .args(["build", "-q", "--release", "--bin", "rustfmt"]) @@ -192,10 +195,18 @@ pub fn build_rsfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> return Ok(()); } -pub fn get_binary_version(binary: String) -> Result { - let Ok(command) = Command::new(binary.as_str()).args(["--version"]).output() else { +pub fn get_binary_version( + binary: &PathBuf, + ld_lib_path: &String, +) -> Result { + let binary_str = pathbuf_to_str(binary)?; + let Ok(command) = Command::new(binary_str) + .env("LD_LIB_PATH", ld_lib_path) + .args(["--version"]) + .output() + else { return Err(CheckDiffError::FailedVersioning(format!( - "Failed to get version for {}", + "Failed to get version for {:?}", binary ))); }; @@ -208,6 +219,27 @@ pub fn get_binary_version(binary: String) -> Result { return Ok(binary_version); } +pub fn build_rustfmt_with_lib_path() -> Result { + //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each + // binary can find it's runtime dependencies. + // See https://github.com/rust-lang/rustfmt/issues/5675 + // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary + let ld_lib_path = get_ld_lib_path()?; + info!("Building rustfmt from source"); + build_rustfmt_from_src(&ld_lib_path)?; + return Ok(ld_lib_path); +} + +fn pathbuf_to_str(pathbuf: &PathBuf) -> Result<&str, CheckDiffError> { + let Some(result) = pathbuf.to_str() else { + return Err(CheckDiffError::FailedPathBuf(format!( + "Unable to convert {:?} to str", + pathbuf + ))); + }; + return Ok(result); +} + // Compiles and produces two rustfmt binaries. // One for the current master, and another for the feature branch // Parameters: @@ -220,52 +252,37 @@ pub fn compile_rustfmt( ) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; - let _clone_git_result = clone_git_repo(RUSTFMT_REPO, dest)?; - let _git_remote_add_result = git_remote_add(remote_repo_url.as_str())?; - let _git_fetch_result = git_fetch(feature_branch.as_str())?; + clone_git_repo(RUSTFMT_REPO, dest)?; + git_remote_add(remote_repo_url.as_str())?; + git_fetch(feature_branch.as_str())?; let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); - //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each - // binary can find it's runtime dependencies. - // See https://github.com/rust-lang/rustfmt/issues/5675 - // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary - let ld_lib_path = get_ld_lib_path()?; - - info!("Building rustfmt from source"); - let _build_from_src = build_rsfmt_from_src(&ld_lib_path)?; + build_rustfmt_with_lib_path()?; - let rustfmt_binary = format!("{}/rustfmt", dest.display()); - let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; + let rustfmt_binary = dest.join("/rustfmt"); + copy_src_to_dst("target/release/rustfmt", &rustfmt_binary)?; let should_detach = commit_hash.is_some(); - let _ = git_switch( + git_switch( commit_hash.unwrap_or(feature_branch).as_str(), should_detach, - ); + )?; - // This will prepend the `LD_LIBRARY_PATH` for the feature branch rustfmt binary. - // In most cases the `LD_LIBRARY_PATH` should be the same for both rustfmt binaries - // that we build in `compile_rustfmt`, however, there are scenarios where each binary - // has different runtime dependencies. For example, during subtree syncs we - // bump the nightly toolchain required to build rustfmt, and therefore the feature branch - // relies on a newer set of runtime dependencies. - let ld_lib_path = get_ld_lib_path()?; - info!("Building rustfmt from source"); - let _build_from_src = build_rsfmt_from_src(&ld_lib_path)?; - let feature_binary = format!("{}/feature_rustfmt", dest.display()); + let ld_lib_path = build_rustfmt_with_lib_path()?; + let feature_binary = dest.join("/feature_rustfmt"); - let _cp = copy_src_to_dst("target/release/rustfmt", rustfmt_binary.as_str())?; + copy_src_to_dst("target/release/rustfmt", &rustfmt_binary)?; info!( "\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", &ld_lib_path ); - let rustfmt_version = get_binary_version(rustfmt_binary)?; + let rustfmt_version = get_binary_version(&rustfmt_binary, &ld_lib_path)?; info!("\nRUSFMT_BIN {}\n", rustfmt_version); - let feature_binary_version = get_binary_version(feature_binary)?; + let feature_binary_version = get_binary_version(&feature_binary, &ld_lib_path)?; info!("FEATURE_BIN {}\n", feature_binary_version); let result = Command::new("ls"); From 0b38c471697b7215aed7d361587cd186d127a474 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 23 Aug 2024 17:39:03 +0800 Subject: [PATCH 22/42] implement rustfmt runner --- check_diff/src/lib.rs | 78 +++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index d71c94d8b21..cc51c12dc7f 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -3,6 +3,7 @@ use std::io; use std::path::Path; use std::path::PathBuf; use std::process::Command; +use std::process::Output; use tracing::info; pub enum CheckDiffError { @@ -17,6 +18,20 @@ pub enum CheckDiffError { IO(std::io::Error), } +pub struct RustfmtRunner { + pub ld_library_path: String, + pub binary_path: PathBuf, +} + +impl RustfmtRunner { + fn run(&self, args: &[&str]) -> io::Result { + Command::new(&self.binary_path) + .env("LD_LIBRARY_PATH", &self.ld_library_path) + .args(args) + .output() + } +} + impl From for CheckDiffError { fn from(error: io::Error) -> Self { CheckDiffError::IO(error) @@ -172,29 +187,6 @@ pub fn get_cargo_version() -> Result { return Ok(cargo_version); } -pub fn copy_src_to_dst(src: &str, dst: &PathBuf) -> Result<(), CheckDiffError> { - let dst_str = pathbuf_to_str(dst)?; - let Ok(_) = Command::new("cp").args([src, dst_str]).output() else { - return Err(CheckDiffError::FailedCopy( - "Error copying rustfmt release to destination".to_string(), - )); - }; - return Ok(()); -} - -pub fn build_rustfmt_from_src(ld_lib_path: &String) -> Result<(), CheckDiffError> { - let Ok(_) = Command::new("cargo") - .env("LD_LIB_PATH", ld_lib_path) - .args(["build", "-q", "--release", "--bin", "rustfmt"]) - .output() - else { - return Err(CheckDiffError::FailedSourceBuild( - "Error building rustfmt from source".to_string(), - )); - }; - return Ok(()); -} - pub fn get_binary_version( binary: &PathBuf, ld_lib_path: &String, @@ -219,15 +211,32 @@ pub fn get_binary_version( return Ok(binary_version); } -pub fn build_rustfmt_with_lib_path() -> Result { +/// Obtains the ld_lib path and then builds rustfmt from source +/// If that operation succeeds, the source is then copied to the output path specified +pub fn build_rustfmt_from_src(output_path: &PathBuf) -> Result { //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 // This will prepend the `LD_LIBRARY_PATH` for the master rustfmt binary let ld_lib_path = get_ld_lib_path()?; + info!("Building rustfmt from source"); - build_rustfmt_from_src(&ld_lib_path)?; - return Ok(ld_lib_path); + let Ok(_) = Command::new("cargo") + .env("LD_LIB_PATH", &ld_lib_path.as_str()) + .args(["build", "-q", "--release", "--bin", "rustfmt"]) + .output() + else { + return Err(CheckDiffError::FailedSourceBuild( + "Error building rustfmt from source".to_string(), + )); + }; + + std::fs::copy("target/release/rustfmt", &output_path)?; + + return Ok(RustfmtRunner { + ld_library_path: ld_lib_path, + binary_path: output_path.to_path_buf(), + }); } fn pathbuf_to_str(pathbuf: &PathBuf) -> Result<&str, CheckDiffError> { @@ -258,31 +267,28 @@ pub fn compile_rustfmt( let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); - - build_rustfmt_with_lib_path()?; - let rustfmt_binary = dest.join("/rustfmt"); - copy_src_to_dst("target/release/rustfmt", &rustfmt_binary)?; + build_rustfmt_from_src(&rustfmt_binary)?; let should_detach = commit_hash.is_some(); git_switch( commit_hash.unwrap_or(feature_branch).as_str(), should_detach, )?; - - let ld_lib_path = build_rustfmt_with_lib_path()?; let feature_binary = dest.join("/feature_rustfmt"); - copy_src_to_dst("target/release/rustfmt", &rustfmt_binary)?; + let feature_runner = build_rustfmt_from_src(&feature_binary)?; + info!( "\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", - &ld_lib_path + &feature_runner.ld_library_path ); - let rustfmt_version = get_binary_version(&rustfmt_binary, &ld_lib_path)?; + let rustfmt_version = get_binary_version(&rustfmt_binary, &feature_runner.ld_library_path)?; info!("\nRUSFMT_BIN {}\n", rustfmt_version); - let feature_binary_version = get_binary_version(&feature_binary, &ld_lib_path)?; + let feature_binary_version = + get_binary_version(&feature_binary, &(feature_runner.ld_library_path))?; info!("FEATURE_BIN {}\n", feature_binary_version); let result = Command::new("ls"); From 5fb2424f5dbca7595067952336439294cb290819 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 23 Aug 2024 17:45:38 +0800 Subject: [PATCH 23/42] update return types --- check_diff/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index cc51c12dc7f..639a7ea0e12 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -258,7 +258,7 @@ pub fn compile_rustfmt( remote_repo_url: String, feature_branch: String, commit_hash: Option, -) -> Result { +) -> Result<[RustfmtRunner; 2], CheckDiffError> { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; clone_git_repo(RUSTFMT_REPO, dest)?; @@ -268,7 +268,7 @@ pub fn compile_rustfmt( let cargo_version = get_cargo_version()?; info!("Compiling with {}", cargo_version); let rustfmt_binary = dest.join("/rustfmt"); - build_rustfmt_from_src(&rustfmt_binary)?; + let src_runner = build_rustfmt_from_src(&rustfmt_binary)?; let should_detach = commit_hash.is_some(); git_switch( @@ -293,5 +293,5 @@ pub fn compile_rustfmt( let result = Command::new("ls"); - return Ok(result); + return Ok([src_runner, feature_runner]); } From 66a2bbf9e6b08fef99d27a747dfb89b83170d30d Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 23 Aug 2024 17:45:54 +0800 Subject: [PATCH 24/42] remove unused lines --- check_diff/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 639a7ea0e12..6b441ca0522 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -291,7 +291,5 @@ pub fn compile_rustfmt( get_binary_version(&feature_binary, &(feature_runner.ld_library_path))?; info!("FEATURE_BIN {}\n", feature_binary_version); - let result = Command::new("ls"); - return Ok([src_runner, feature_runner]); } From 08281e1758a84b922a2d3ac5af509f31f83a1e47 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 23 Aug 2024 17:50:02 +0800 Subject: [PATCH 25/42] update return type --- check_diff/src/lib.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 6b441ca0522..bbe9086c355 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -18,6 +18,11 @@ pub enum CheckDiffError { IO(std::io::Error), } +pub struct GlobalRunners { + pub feature_runner: RustfmtRunner, + pub src_runner: RustfmtRunner, +} + pub struct RustfmtRunner { pub ld_library_path: String, pub binary_path: PathBuf, @@ -258,7 +263,7 @@ pub fn compile_rustfmt( remote_repo_url: String, feature_branch: String, commit_hash: Option, -) -> Result<[RustfmtRunner; 2], CheckDiffError> { +) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; clone_git_repo(RUSTFMT_REPO, dest)?; @@ -291,5 +296,8 @@ pub fn compile_rustfmt( get_binary_version(&feature_binary, &(feature_runner.ld_library_path))?; info!("FEATURE_BIN {}\n", feature_binary_version); - return Ok([src_runner, feature_runner]); + return Ok(GlobalRunners { + src_runner, + feature_runner, + }); } From d639b808c19a1d9df2e9081a97b6fe5b1c52305d Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 13 Sep 2024 12:13:51 +0800 Subject: [PATCH 26/42] update access modifiers --- check_diff/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index bbe9086c355..e8a8e45742a 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -19,13 +19,14 @@ pub enum CheckDiffError { } pub struct GlobalRunners { - pub feature_runner: RustfmtRunner, - pub src_runner: RustfmtRunner, + feature_runner: RustfmtRunner, + src_runner: RustfmtRunner, } +#[allow(dead_code)] pub struct RustfmtRunner { - pub ld_library_path: String, - pub binary_path: PathBuf, + ld_library_path: String, + binary_path: PathBuf, } impl RustfmtRunner { From 611aa07e6b3392d6d009a5c048f9e64b69862796 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Fri, 13 Sep 2024 12:18:02 +0800 Subject: [PATCH 27/42] update struct names --- check_diff/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index e8a8e45742a..2bde9571121 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -18,7 +18,7 @@ pub enum CheckDiffError { IO(std::io::Error), } -pub struct GlobalRunners { +pub struct CheckDiffRunners { feature_runner: RustfmtRunner, src_runner: RustfmtRunner, } From b366ad73e428fd68ce43b47c9bfc684a8b8532ef Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 14 Sep 2024 10:24:33 +0800 Subject: [PATCH 28/42] fix build errors --- check_diff/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 2bde9571121..3fac09bdc87 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -264,7 +264,7 @@ pub fn compile_rustfmt( remote_repo_url: String, feature_branch: String, commit_hash: Option, -) -> Result { +) -> Result { const RUSTFMT_REPO: &str = "https://github.com/rust-lang/rustfmt.git"; clone_git_repo(RUSTFMT_REPO, dest)?; @@ -297,7 +297,7 @@ pub fn compile_rustfmt( get_binary_version(&feature_binary, &(feature_runner.ld_library_path))?; info!("FEATURE_BIN {}\n", feature_binary_version); - return Ok(GlobalRunners { + return Ok(CheckDiffRunners { src_runner, feature_runner, }); From 7c9961081a97f2d9a83da04866d10fd63fd8f2a1 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 14 Sep 2024 10:28:41 +0800 Subject: [PATCH 29/42] add allow of dead code --- check_diff/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 3fac09bdc87..68cb2fb88ba 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -18,18 +18,19 @@ pub enum CheckDiffError { IO(std::io::Error), } +#[allow(dead_code)] pub struct CheckDiffRunners { feature_runner: RustfmtRunner, src_runner: RustfmtRunner, } -#[allow(dead_code)] pub struct RustfmtRunner { ld_library_path: String, binary_path: PathBuf, } impl RustfmtRunner { + #[allow(dead_code)] fn run(&self, args: &[&str]) -> io::Result { Command::new(&self.binary_path) .env("LD_LIBRARY_PATH", &self.ld_library_path) From 4c9069750cc5773f995ea1ad31ce4ba262657e47 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 14 Sep 2024 10:49:52 +0800 Subject: [PATCH 30/42] add comments --- check_diff/src/lib.rs | 2 ++ check_diff/src/main.rs | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 68cb2fb88ba..7df383afc87 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -18,6 +18,7 @@ pub enum CheckDiffError { IO(std::io::Error), } +// will be used in future PRs, just added to make the compiler happy #[allow(dead_code)] pub struct CheckDiffRunners { feature_runner: RustfmtRunner, @@ -30,6 +31,7 @@ pub struct RustfmtRunner { } impl RustfmtRunner { + // will be used in future PRs, just added to make the compiler happy #[allow(dead_code)] fn run(&self, args: &[&str]) -> io::Result { Command::new(&self.binary_path) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 8aac6647db9..58b5330a994 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -23,7 +23,7 @@ fn main() { let args = CliInputs::parse(); let tmp_dir = Builder::new().tempdir_in("").unwrap(); info!("Created tmp_dir {:?}", tmp_dir); - let _ = compile_rustfmt( + let runner = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, From 1cda209a017c982231b5edde78fe0f47df45789c Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 14 Sep 2024 10:56:23 +0800 Subject: [PATCH 31/42] remove unused var --- check_diff/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/check_diff/src/main.rs b/check_diff/src/main.rs index 58b5330a994..8aac6647db9 100644 --- a/check_diff/src/main.rs +++ b/check_diff/src/main.rs @@ -23,7 +23,7 @@ fn main() { let args = CliInputs::parse(); let tmp_dir = Builder::new().tempdir_in("").unwrap(); info!("Created tmp_dir {:?}", tmp_dir); - let runner = compile_rustfmt( + let _ = compile_rustfmt( tmp_dir.path(), args.remote_repo_url, args.feature_branch, From 7094a00e97439c13e6ddf69d858cf4ac916e37b0 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 21 Sep 2024 11:32:32 +0800 Subject: [PATCH 32/42] minor refactor --- check_diff/src/lib.rs | 30 +++++++----------------------- 1 file changed, 7 insertions(+), 23 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 7df383afc87..33cfcf84795 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,9 +1,7 @@ use std::env; use std::io; -use std::path::Path; -use std::path::PathBuf; -use std::process::Command; -use std::process::Output; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; use tracing::info; pub enum CheckDiffError { @@ -196,12 +194,8 @@ pub fn get_cargo_version() -> Result { return Ok(cargo_version); } -pub fn get_binary_version( - binary: &PathBuf, - ld_lib_path: &String, -) -> Result { - let binary_str = pathbuf_to_str(binary)?; - let Ok(command) = Command::new(binary_str) +pub fn get_binary_version(binary: &Path, ld_lib_path: &String) -> Result { + let Ok(command) = Command::new(binary) .env("LD_LIB_PATH", ld_lib_path) .args(["--version"]) .output() @@ -222,7 +216,7 @@ pub fn get_binary_version( /// Obtains the ld_lib path and then builds rustfmt from source /// If that operation succeeds, the source is then copied to the output path specified -pub fn build_rustfmt_from_src(output_path: &PathBuf) -> Result { +pub fn build_rustfmt_from_src(binary_path: &Path) -> Result { //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 @@ -240,24 +234,14 @@ pub fn build_rustfmt_from_src(output_path: &PathBuf) -> Result Result<&str, CheckDiffError> { - let Some(result) = pathbuf.to_str() else { - return Err(CheckDiffError::FailedPathBuf(format!( - "Unable to convert {:?} to str", - pathbuf - ))); - }; - return Ok(result); -} - // Compiles and produces two rustfmt binaries. // One for the current master, and another for the feature branch // Parameters: From 25aa30d7b089c8717761a118963da5fdf6ca66b6 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 21 Sep 2024 11:54:45 +0800 Subject: [PATCH 33/42] improve error handling --- check_diff/src/lib.rs | 37 ++++++++++++++----------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 33cfcf84795..6abf591138f 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -2,17 +2,18 @@ use std::env; use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; +use std::str::Utf8Error; +use std::string::FromUtf8Error; use tracing::info; pub enum CheckDiffError { FailedGit(GitError), FailedCommand(String), - FailedUtf8(String), + FailedUtf8(Utf8Error), FailedSourceBuild(String), FailedCopy(String), FailedCargoVersion(String), FailedVersioning(String), - FailedPathBuf(String), IO(std::io::Error), } @@ -51,6 +52,12 @@ impl From for CheckDiffError { } } +impl From for CheckDiffError { + fn from(error: FromUtf8Error) -> Self { + CheckDiffError::FailedUtf8(error.utf8_error()) + } +} + pub enum GitError { FailedClone { stdout: Vec, stderr: Vec }, FailedRemoteAdd { stdout: Vec, stderr: Vec }, @@ -168,28 +175,15 @@ pub fn get_ld_lib_path() -> Result { )); }; - let Ok(sysroot) = String::from_utf8(command.stdout) else { - return Err(CheckDiffError::FailedUtf8( - "Error converting sysroot to string".to_string(), - )); - }; + let sysroot = String::from_utf8(command.stdout)?; let ld_lib_path = format!("{}/lib", sysroot.trim_end()); return Ok(ld_lib_path); } pub fn get_cargo_version() -> Result { - let Ok(command) = Command::new("cargo").args(["--version"]).output() else { - return Err(CheckDiffError::FailedCargoVersion( - "Failed to obtain cargo version".to_string(), - )); - }; - - let Ok(cargo_version) = String::from_utf8(command.stdout) else { - return Err(CheckDiffError::FailedUtf8( - "Error converting cargo version to string".to_string(), - )); - }; + let command = Command::new("cargo").args(["--version"]).output()?; + let cargo_version = String::from_utf8(command.stdout)?; return Ok(cargo_version); } @@ -206,11 +200,8 @@ pub fn get_binary_version(binary: &Path, ld_lib_path: &String) -> Result Date: Sat, 21 Sep 2024 12:30:20 +0800 Subject: [PATCH 34/42] improve errors --- check_diff/src/lib.rs | 127 ++++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 48 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 6abf591138f..82f0bf26436 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -6,40 +6,31 @@ use std::str::Utf8Error; use std::string::FromUtf8Error; use tracing::info; +// ========= Start of error enums =========== + pub enum CheckDiffError { + /// Git related errors FailedGit(GitError), - FailedCommand(String), + /// Error for `rustc --print sysroot` command + FailedToGetSysroot { + stdout: Vec, + stderr: Vec, + }, + /// UTF8 related errors FailedUtf8(Utf8Error), - FailedSourceBuild(String), - FailedCopy(String), - FailedCargoVersion(String), - FailedVersioning(String), + /// Error for building rustfmt from source + FailedSourceBuild { + stdout: Vec, + stderr: Vec, + }, + /// Error when `--version` flag is used in a command + FailedVersioning { + stdout: Vec, + stderr: Vec, + }, IO(std::io::Error), } -// will be used in future PRs, just added to make the compiler happy -#[allow(dead_code)] -pub struct CheckDiffRunners { - feature_runner: RustfmtRunner, - src_runner: RustfmtRunner, -} - -pub struct RustfmtRunner { - ld_library_path: String, - binary_path: PathBuf, -} - -impl RustfmtRunner { - // will be used in future PRs, just added to make the compiler happy - #[allow(dead_code)] - fn run(&self, args: &[&str]) -> io::Result { - Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) - .args(args) - .output() - } -} - impl From for CheckDiffError { fn from(error: io::Error) -> Self { CheckDiffError::IO(error) @@ -72,6 +63,31 @@ impl From for GitError { } } +// ========= End of error enums =========== + +// will be used in future PRs, just added to make the compiler happy +#[allow(dead_code)] +pub struct CheckDiffRunners { + feature_runner: RustfmtRunner, + src_runner: RustfmtRunner, +} + +pub struct RustfmtRunner { + ld_library_path: String, + binary_path: PathBuf, +} + +impl RustfmtRunner { + // will be used in future PRs, just added to make the compiler happy + #[allow(dead_code)] + fn run(&self, args: &[&str]) -> io::Result { + Command::new(&self.binary_path) + .env("LD_LIBRARY_PATH", &self.ld_library_path) + .args(args) + .output() + } +} + /// Clone a git repository /// /// Parameters: @@ -169,11 +185,16 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { } pub fn get_ld_lib_path() -> Result { - let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { - return Err(CheckDiffError::FailedCommand( - "Error getting sysroot".to_string(), - )); - }; + let command = Command::new("rustc") + .args(["--print", "sysroot"]) + .output()?; + + if !command.status.success() { + return Err(CheckDiffError::FailedToGetSysroot { + stdout: command.stdout, + stderr: command.stderr, + }); + } let sysroot = String::from_utf8(command.stdout)?; @@ -183,22 +204,30 @@ pub fn get_ld_lib_path() -> Result { pub fn get_cargo_version() -> Result { let command = Command::new("cargo").args(["--version"]).output()?; + + if !command.status.success() { + return Err(CheckDiffError::FailedVersioning { + stdout: command.stdout, + stderr: command.stderr, + }); + } let cargo_version = String::from_utf8(command.stdout)?; return Ok(cargo_version); } pub fn get_binary_version(binary: &Path, ld_lib_path: &String) -> Result { - let Ok(command) = Command::new(binary) + let command = Command::new(binary) .env("LD_LIB_PATH", ld_lib_path) .args(["--version"]) - .output() - else { - return Err(CheckDiffError::FailedVersioning(format!( - "Failed to get version for {:?}", - binary - ))); - }; + .output()?; + + if !command.status.success() { + return Err(CheckDiffError::FailedVersioning { + stdout: command.stdout, + stderr: command.stderr, + }); + } let binary_version = String::from_utf8(command.stdout)?; @@ -215,15 +244,17 @@ pub fn build_rustfmt_from_src(binary_path: &Path) -> Result Date: Sun, 22 Sep 2024 10:01:03 +0800 Subject: [PATCH 35/42] update error handling --- check_diff/src/lib.rs | 76 +++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 49 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 82f0bf26436..487f5c751e4 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -11,23 +11,16 @@ use tracing::info; pub enum CheckDiffError { /// Git related errors FailedGit(GitError), - /// Error for `rustc --print sysroot` command - FailedToGetSysroot { - stdout: Vec, - stderr: Vec, - }, + /// Error for generic commands + FailedCommand(&'static str), /// UTF8 related errors FailedUtf8(Utf8Error), /// Error for building rustfmt from source - FailedSourceBuild { - stdout: Vec, - stderr: Vec, - }, - /// Error when `--version` flag is used in a command - FailedVersioning { - stdout: Vec, - stderr: Vec, - }, + FailedSourceBuild(&'static str), + /// Error when obtaining binary version + FailedBinaryVersioning(PathBuf), + /// Error when obtaining cargo version + FailedCargoVersion(&'static str), IO(std::io::Error), } @@ -185,16 +178,9 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { } pub fn get_ld_lib_path() -> Result { - let command = Command::new("rustc") - .args(["--print", "sysroot"]) - .output()?; - - if !command.status.success() { - return Err(CheckDiffError::FailedToGetSysroot { - stdout: command.stdout, - stderr: command.stderr, - }); - } + let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { + return Err(CheckDiffError::FailedCommand("Error getting sysroot")); + }; let sysroot = String::from_utf8(command.stdout)?; @@ -203,31 +189,25 @@ pub fn get_ld_lib_path() -> Result { } pub fn get_cargo_version() -> Result { - let command = Command::new("cargo").args(["--version"]).output()?; + let Ok(command) = Command::new("cargo").args(["--version"]).output() else { + return Err(CheckDiffError::FailedCargoVersion( + "Failed to obtain cargo version", + )); + }; - if !command.status.success() { - return Err(CheckDiffError::FailedVersioning { - stdout: command.stdout, - stderr: command.stderr, - }); - } let cargo_version = String::from_utf8(command.stdout)?; return Ok(cargo_version); } pub fn get_binary_version(binary: &Path, ld_lib_path: &String) -> Result { - let command = Command::new(binary) + let Ok(command) = Command::new(binary) .env("LD_LIB_PATH", ld_lib_path) .args(["--version"]) - .output()?; - - if !command.status.success() { - return Err(CheckDiffError::FailedVersioning { - stdout: command.stdout, - stderr: command.stderr, - }); - } + .output() + else { + return Err(CheckDiffError::FailedBinaryVersioning(binary.to_path_buf())); + }; let binary_version = String::from_utf8(command.stdout)?; @@ -244,17 +224,15 @@ pub fn build_rustfmt_from_src(binary_path: &Path) -> Result Date: Sun, 29 Sep 2024 11:42:58 +0800 Subject: [PATCH 36/42] fix git switch function --- check_diff/src/lib.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 487f5c751e4..79f0d91ccf3 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -6,8 +6,6 @@ use std::str::Utf8Error; use std::string::FromUtf8Error; use tracing::info; -// ========= Start of error enums =========== - pub enum CheckDiffError { /// Git related errors FailedGit(GitError), @@ -56,8 +54,6 @@ impl From for GitError { } } -// ========= End of error enums =========== - // will be used in future PRs, just added to make the compiler happy #[allow(dead_code)] pub struct CheckDiffRunners { @@ -153,7 +149,9 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { pub fn git_switch(arg: &str, should_detach: bool) -> Result<(), GitError> { let detach_arg = if should_detach { "--detach" } else { "" }; - let git_cmd = Command::new("git").args([arg, detach_arg]).output()?; + let git_cmd = Command::new("git") + .args(["switch", arg, detach_arg]) + .output()?; if !git_cmd.status.success() { let error = GitError::FailedSwitch { From 4597a07503f9b396bf6588c955983d67fea21b3a Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sun, 29 Sep 2024 11:44:46 +0800 Subject: [PATCH 37/42] update logs --- check_diff/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 79f0d91ccf3..6ac82768316 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -124,7 +124,7 @@ pub fn git_remote_add(url: &str) -> Result<(), GitError> { return Err(error); } - info!("Successfully added remote."); + info!("Successfully added remote: {url}"); return Ok(()); } @@ -143,7 +143,7 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { return Err(error); } - info!("Successfully fetched."); + info!("Successfully fetched: {branch_name}"); return Ok(()); } From 8dff7beb756cc87a6ca47a7e8999c8a6da13d478 Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sun, 29 Sep 2024 11:46:58 +0800 Subject: [PATCH 38/42] remove env when compiling binaries --- check_diff/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 6ac82768316..292d48b2d15 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -223,7 +223,6 @@ pub fn build_rustfmt_from_src(binary_path: &Path) -> Result Date: Sun, 29 Sep 2024 12:04:46 +0800 Subject: [PATCH 39/42] refactor get_binary_version --- check_diff/src/lib.rs | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 292d48b2d15..3782d393f60 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -75,6 +75,22 @@ impl RustfmtRunner { .args(args) .output() } + + fn get_binary_version(&self) -> Result { + let Ok(command) = Command::new(&self.binary_path) + .env("LD_LIB_PATH", &self.ld_library_path) + .args(["--version"]) + .output() + else { + return Err(CheckDiffError::FailedBinaryVersioning( + self.binary_path.to_path_buf(), + )); + }; + + let binding = String::from_utf8(command.stdout)?; + let binary_version = binding.trim(); + return Ok(binary_version.to_string()); + } } /// Clone a git repository @@ -198,23 +214,9 @@ pub fn get_cargo_version() -> Result { return Ok(cargo_version); } -pub fn get_binary_version(binary: &Path, ld_lib_path: &String) -> Result { - let Ok(command) = Command::new(binary) - .env("LD_LIB_PATH", ld_lib_path) - .args(["--version"]) - .output() - else { - return Err(CheckDiffError::FailedBinaryVersioning(binary.to_path_buf())); - }; - - let binary_version = String::from_utf8(command.stdout)?; - - return Ok(binary_version); -} - /// Obtains the ld_lib path and then builds rustfmt from source /// If that operation succeeds, the source is then copied to the output path specified -pub fn build_rustfmt_from_src(binary_path: &Path) -> Result { +pub fn build_rustfmt_from_src(binary_path: PathBuf) -> Result { //Because we're building standalone binaries we need to set `LD_LIBRARY_PATH` so each // binary can find it's runtime dependencies. // See https://github.com/rust-lang/rustfmt/issues/5675 @@ -235,7 +237,7 @@ pub fn build_rustfmt_from_src(binary_path: &Path) -> Result Date: Sun, 29 Sep 2024 12:12:16 +0800 Subject: [PATCH 40/42] update errors --- check_diff/src/lib.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index 3782d393f60..d33481a5574 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -3,7 +3,6 @@ use std::io; use std::path::{Path, PathBuf}; use std::process::{Command, Output}; use std::str::Utf8Error; -use std::string::FromUtf8Error; use tracing::info; pub enum CheckDiffError { @@ -34,9 +33,9 @@ impl From for CheckDiffError { } } -impl From for CheckDiffError { - fn from(error: FromUtf8Error) -> Self { - CheckDiffError::FailedUtf8(error.utf8_error()) +impl From for CheckDiffError { + fn from(error: Utf8Error) -> Self { + CheckDiffError::FailedUtf8(error) } } @@ -87,8 +86,7 @@ impl RustfmtRunner { )); }; - let binding = String::from_utf8(command.stdout)?; - let binary_version = binding.trim(); + let binary_version = std::str::from_utf8(&command.stdout)?.trim_end(); return Ok(binary_version.to_string()); } } @@ -196,7 +194,7 @@ pub fn get_ld_lib_path() -> Result { return Err(CheckDiffError::FailedCommand("Error getting sysroot")); }; - let sysroot = String::from_utf8(command.stdout)?; + let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); let ld_lib_path = format!("{}/lib", sysroot.trim_end()); return Ok(ld_lib_path); @@ -209,9 +207,8 @@ pub fn get_cargo_version() -> Result { )); }; - let cargo_version = String::from_utf8(command.stdout)?; - - return Ok(cargo_version); + let cargo_version = std::str::from_utf8(&command.stdout)?.trim_end(); + return Ok(cargo_version.to_string()); } /// Obtains the ld_lib path and then builds rustfmt from source From 1b3f05387385e5814b15e8b1729cd18b238749bb Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sun, 29 Sep 2024 12:15:43 +0800 Subject: [PATCH 41/42] update logs --- check_diff/src/lib.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index d33481a5574..b2b011a7c33 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -267,16 +267,16 @@ pub fn compile_rustfmt( let feature_runner = build_rustfmt_from_src(dest.join("/feature_rustfmt"))?; + info!("RUSFMT_BIN {}", src_runner.get_binary_version()?); info!( - "\nRuntime dependencies for rustfmt -- LD_LIBRARY_PATH: {}", - &feature_runner.ld_library_path + "Runtime dependencies for (src) rustfmt -- LD_LIBRARY_PATH: {}", + src_runner.ld_library_path + ); + info!("FEATURE_BIN {}", feature_runner.get_binary_version()?); + info!( + "Runtime dependencies for (feature) rustfmt -- LD_LIBRARY_PATH: {}", + feature_runner.ld_library_path ); - - let rustfmt_version = src_runner.get_binary_version()?; - info!("\nRUSFMT_BIN {}\n", rustfmt_version); - - let feature_binary_version = feature_runner.get_binary_version()?; - info!("FEATURE_BIN {}\n", feature_binary_version); return Ok(CheckDiffRunners { src_runner, From 4c1328ce1a18aea21fb7bcf1da6310cba326a43d Mon Sep 17 00:00:00 2001 From: benluiwj Date: Sat, 5 Oct 2024 15:58:57 +0800 Subject: [PATCH 42/42] fix git switch --- check_diff/src/lib.rs | 49 ++++++++++++++++-------------------------- check_diff/src/main.rs | 3 +++ 2 files changed, 21 insertions(+), 31 deletions(-) diff --git a/check_diff/src/lib.rs b/check_diff/src/lib.rs index b2b011a7c33..072b2f5d5c1 100644 --- a/check_diff/src/lib.rs +++ b/check_diff/src/lib.rs @@ -1,7 +1,7 @@ use std::env; use std::io; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::Command; use std::str::Utf8Error; use tracing::info; @@ -66,27 +66,18 @@ pub struct RustfmtRunner { } impl RustfmtRunner { - // will be used in future PRs, just added to make the compiler happy - #[allow(dead_code)] - fn run(&self, args: &[&str]) -> io::Result { - Command::new(&self.binary_path) - .env("LD_LIBRARY_PATH", &self.ld_library_path) - .args(args) - .output() - } - fn get_binary_version(&self) -> Result { let Ok(command) = Command::new(&self.binary_path) - .env("LD_LIB_PATH", &self.ld_library_path) + .env("LD_LIBRARY_PATH", &self.ld_library_path) .args(["--version"]) .output() else { return Err(CheckDiffError::FailedBinaryVersioning( - self.binary_path.to_path_buf(), + self.binary_path.clone(), )); }; - let binary_version = std::str::from_utf8(&command.stdout)?.trim_end(); + let binary_version = std::str::from_utf8(&command.stdout)?.trim(); return Ok(binary_version.to_string()); } } @@ -161,21 +152,21 @@ pub fn git_fetch(branch_name: &str) -> Result<(), GitError> { return Ok(()); } -pub fn git_switch(arg: &str, should_detach: bool) -> Result<(), GitError> { +pub fn git_switch(git_ref: &str, should_detach: bool) -> Result<(), GitError> { let detach_arg = if should_detach { "--detach" } else { "" }; - let git_cmd = Command::new("git") - .args(["switch", arg, detach_arg]) + let args = ["switch", git_ref, detach_arg]; + let output = Command::new("git") + .args(args.iter().filter(|arg| !arg.is_empty())) .output()?; - - if !git_cmd.status.success() { + if !output.status.success() { + tracing::error!("Git switch failed: {output:?}"); let error = GitError::FailedSwitch { - stdout: git_cmd.stdout, - stderr: git_cmd.stderr, + stdout: output.stdout, + stderr: output.stderr, }; return Err(error); } - - info!("Successfully switched to {}", arg); + info!("Successfully switched to {git_ref}"); return Ok(()); } @@ -189,14 +180,12 @@ pub fn change_directory_to_path(dest: &Path) -> io::Result<()> { return Ok(()); } -pub fn get_ld_lib_path() -> Result { +pub fn get_ld_library_path() -> Result { let Ok(command) = Command::new("rustc").args(["--print", "sysroot"]).output() else { return Err(CheckDiffError::FailedCommand("Error getting sysroot")); }; - let sysroot = std::str::from_utf8(&command.stdout)?.trim_end(); - - let ld_lib_path = format!("{}/lib", sysroot.trim_end()); + let ld_lib_path = format!("{}/lib", sysroot); return Ok(ld_lib_path); } @@ -218,7 +207,7 @@ pub fn build_rustfmt_from_src(binary_path: PathBuf) -> Result