From 92e9148f36ea0b441377447a6fb49f6e54be91c1 Mon Sep 17 00:00:00 2001 From: Dimitri Mitropoulos Date: Thu, 22 Aug 2024 18:23:08 -0400 Subject: [PATCH] allow multiple fallbacks for --affected base branch (#9045) ### Description We use `main` as our default for base branch, but a lot of repos are likely using `master`, we should fall back to that if `main` isn't a valid ref. ### Testing Instructions see unit tests https://github.com/vercel/turborepo/pull/9045/files#diff-db1296fb6f1a06e197e7cd6cdf5979b738c6b56b7700b048ab3b00fd767f8aecR734-R746 for situations to test manually. --------- Co-authored-by: Chris Olszewski --- crates/turborepo-lib/src/commands/config.rs | 2 +- crates/turborepo-lib/src/config.rs | 4 +- crates/turborepo-lib/src/opts.rs | 6 +- crates/turborepo-lib/src/query.rs | 2 +- .../src/run/scope/change_detector.rs | 8 +- crates/turborepo-lib/src/run/scope/filter.rs | 28 +-- .../src/run/scope/target_selector.rs | 28 +-- crates/turborepo-scm/src/git.rs | 166 +++++++++++++----- crates/turborepo-scm/src/lib.rs | 2 + turborepo-tests/integration/tests/config.t | 2 +- 10 files changed, 163 insertions(+), 85 deletions(-) diff --git a/crates/turborepo-lib/src/commands/config.rs b/crates/turborepo-lib/src/commands/config.rs index 79318c201e61a..a5fc906554929 100644 --- a/crates/turborepo-lib/src/commands/config.rs +++ b/crates/turborepo-lib/src/commands/config.rs @@ -23,7 +23,7 @@ struct ConfigOutput<'a> { package_manager: PackageManager, daemon: Option, env_mode: EnvMode, - scm_base: &'a str, + scm_base: Option<&'a str>, scm_head: &'a str, cache_dir: &'a Utf8Path, } diff --git a/crates/turborepo-lib/src/config.rs b/crates/turborepo-lib/src/config.rs index c58480844ff64..3aaea1ca81d71 100644 --- a/crates/turborepo-lib/src/config.rs +++ b/crates/turborepo-lib/src/config.rs @@ -298,8 +298,8 @@ impl ConfigurationOptions { self.ui.unwrap_or(UIMode::Stream) } - pub fn scm_base(&self) -> &str { - self.scm_base.as_deref().unwrap_or("main") + pub fn scm_base(&self) -> Option<&str> { + self.scm_base.as_deref() } pub fn scm_head(&self) -> &str { diff --git a/crates/turborepo-lib/src/opts.rs b/crates/turborepo-lib/src/opts.rs index 05a8643290fcf..8ad7b9fde38ee 100644 --- a/crates/turborepo-lib/src/opts.rs +++ b/crates/turborepo-lib/src/opts.rs @@ -307,7 +307,7 @@ pub struct ScopeOpts { pub pkg_inference_root: Option, pub global_deps: Vec, pub filter_patterns: Vec, - pub affected_range: Option<(String, String)>, + pub affected_range: Option<(Option, String)>, } impl<'a> TryFrom> for ScopeOpts { @@ -324,7 +324,7 @@ impl<'a> TryFrom> for ScopeOpts { let affected_range = inputs.execution_args.affected.then(|| { let scm_base = inputs.config.scm_base(); let scm_head = inputs.config.scm_head(); - (scm_base.to_string(), scm_head.to_string()) + (scm_base.map(|b| b.to_owned()), scm_head.to_string()) }); Ok(Self { @@ -513,7 +513,7 @@ mod test { pkg_inference_root: None, global_deps: vec![], filter_patterns: opts_input.filter_patterns, - affected_range: opts_input.affected, + affected_range: opts_input.affected.map(|(base, head)| (Some(base), head)), }; let opts = Opts { run_opts, diff --git a/crates/turborepo-lib/src/query.rs b/crates/turborepo-lib/src/query.rs index 6c04f49ef2eca..64faef1a059f2 100644 --- a/crates/turborepo-lib/src/query.rs +++ b/crates/turborepo-lib/src/query.rs @@ -242,7 +242,7 @@ impl Query { base: Option, head: Option, ) -> Result, Error> { - let base = base.unwrap_or_else(|| "main".to_string()); + let base = base.map(|s| s.to_owned()); let head = head.unwrap_or_else(|| "HEAD".to_string()); let mut opts = self.run.opts().clone(); opts.scope_opts.affected_range = Some((base, head)); diff --git a/crates/turborepo-lib/src/run/scope/change_detector.rs b/crates/turborepo-lib/src/run/scope/change_detector.rs index 342090abb4bd2..7f05be41edab9 100644 --- a/crates/turborepo-lib/src/run/scope/change_detector.rs +++ b/crates/turborepo-lib/src/run/scope/change_detector.rs @@ -17,7 +17,7 @@ use crate::{ pub trait GitChangeDetector { fn changed_packages( &self, - from_ref: &str, + from_ref: Option<&str>, to_ref: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, @@ -55,7 +55,7 @@ impl<'a> ScopeChangeDetector<'a> { /// returns an empty lockfile change fn get_lockfile_contents( &self, - from_ref: &str, + from_ref: Option<&str>, changed_files: &HashSet, ) -> Option { let lockfile_path = self @@ -88,13 +88,13 @@ impl<'a> ScopeChangeDetector<'a> { impl<'a> GitChangeDetector for ScopeChangeDetector<'a> { fn changed_packages( &self, - from_ref: &str, + from_ref: Option<&str>, to_ref: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, ) -> Result, ResolutionError> { let mut changed_files = HashSet::new(); - if !from_ref.is_empty() { + if !from_ref.map_or(false, |s| s.is_empty()) { changed_files = match self.scm.changed_files( self.turbo_root, from_ref, diff --git a/crates/turborepo-lib/src/run/scope/filter.rs b/crates/turborepo-lib/src/run/scope/filter.rs index 2ab42cab39a0c..47a08d7321479 100644 --- a/crates/turborepo-lib/src/run/scope/filter.rs +++ b/crates/turborepo-lib/src/run/scope/filter.rs @@ -163,7 +163,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { /// It applies the following rules: pub(crate) fn resolve( &self, - affected: &Option<(String, String)>, + affected: &Option<(Option, String)>, patterns: &[String], ) -> Result<(HashSet, bool), ResolutionError> { // inference is None only if we are in the root @@ -185,7 +185,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { fn get_packages_from_patterns( &self, - affected: &Option<(String, String)>, + affected: &Option<(Option, String)>, patterns: &[String], ) -> Result, ResolutionError> { let mut selectors = patterns @@ -196,7 +196,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { if let Some((from_ref, to_ref)) = affected { selectors.push(TargetSelector { git_range: Some(GitRange { - from_ref: from_ref.to_string(), + from_ref: from_ref.clone(), to_ref: Some(to_ref.to_string()), include_uncommitted: true, allow_unknown_objects: true, @@ -549,7 +549,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { git_range: &GitRange, ) -> Result, ResolutionError> { self.change_detector.changed_packages( - &git_range.from_ref, + git_range.from_ref.as_deref(), git_range.to_ref.as_deref(), git_range.include_uncommitted, git_range.allow_unknown_objects, @@ -1123,7 +1123,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), ..Default::default() } ], @@ -1133,7 +1133,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from(".").unwrap()), ..Default::default() } @@ -1144,7 +1144,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("package-2").unwrap()), ..Default::default() } @@ -1155,7 +1155,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), name_pattern: "package-2*".to_string(), ..Default::default() } @@ -1166,7 +1166,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), name_pattern: "package-1".to_string(), match_dependencies: true, ..Default::default() @@ -1178,7 +1178,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~2".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~2".to_string()), to_ref: None, ..Default::default() }), ..Default::default() } ], @@ -1188,7 +1188,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~2".to_string(), to_ref: Some("HEAD~1".to_string()), ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~2".to_string()), to_ref: Some("HEAD~1".to_string()), ..Default::default() }), ..Default::default() } ], @@ -1198,7 +1198,7 @@ mod test { #[test_case( vec![ TargetSelector { - git_range: Some(GitRange { from_ref: "HEAD~1".to_string(), to_ref: None, ..Default::default() }), + git_range: Some(GitRange { from_ref: Some("HEAD~1".to_string()), to_ref: None, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("package-*").unwrap()), match_dependencies: true, ..Default::default() } @@ -1250,14 +1250,14 @@ mod test { impl<'a> GitChangeDetector for TestChangeDetector<'a> { fn changed_packages( &self, - from: &str, + from: Option<&str>, to: Option<&str>, _include_uncommitted: bool, _allow_unknown_objects: bool, ) -> Result, ResolutionError> { Ok(self .0 - .get(&(from, to)) + .get(&(from.expect("expected base branch"), to)) .map(|h| h.to_owned()) .expect("unsupported range")) } diff --git a/crates/turborepo-lib/src/run/scope/target_selector.rs b/crates/turborepo-lib/src/run/scope/target_selector.rs index c40185356bbaa..1ba61f3e548f7 100644 --- a/crates/turborepo-lib/src/run/scope/target_selector.rs +++ b/crates/turborepo-lib/src/run/scope/target_selector.rs @@ -6,7 +6,7 @@ use turbopath::AnchoredSystemPathBuf; #[derive(Debug, Default, PartialEq)] pub struct GitRange { - pub from_ref: String, + pub from_ref: Option, pub to_ref: Option, pub include_uncommitted: bool, // Allow unknown objects to be included in the range, without returning an error. @@ -155,7 +155,7 @@ impl FromStr for TargetSelector { )); } GitRange { - from_ref: a.to_string(), + from_ref: Some(a.to_string()), to_ref: Some(b.to_string()), include_uncommitted: false, allow_unknown_objects: false, @@ -164,7 +164,7 @@ impl FromStr for TargetSelector { // If only the start of the range is specified, we assume that // we want to include uncommitted changes GitRange { - from_ref: commits_str.to_string(), + from_ref: Some(commits_str.to_string()), to_ref: None, include_uncommitted: true, allow_unknown_objects: false, @@ -252,17 +252,17 @@ mod test { #[test_case("...{./foo}", TargetSelector { raw: "...{./foo}".to_string(), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), include_dependents: true, ..Default::default() }; "dot dot dot curly bracket foo")] #[test_case(".", TargetSelector { raw: ".".to_string(), parent_dir: Some(AnchoredSystemPathBuf::try_from(".").unwrap()), ..Default::default() }; "parent dir dot")] #[test_case("..", TargetSelector { raw: "..".to_string(), parent_dir: Some(AnchoredSystemPathBuf::try_from("..").unwrap()), ..Default::default() }; "parent dir dot dot")] - #[test_case("[master]", TargetSelector { raw: "[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), ..Default::default() }; "square brackets master")] - #[test_case("[from...to]", TargetSelector { raw: "[from...to]".to_string(), git_range: Some(GitRange { from_ref: "from".to_string(), to_ref: Some("to".to_string()), ..Default::default() }), ..Default::default() }; "[from...to]")] - #[test_case("{foo}[master]", TargetSelector { raw: "{foo}[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), ..Default::default() }; "{foo}[master]")] - #[test_case("pattern{foo}[master]", TargetSelector { raw: "pattern{foo}[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), name_pattern: "pattern".to_string(), ..Default::default() }; "pattern{foo}[master]")] - #[test_case("[master]...", TargetSelector { raw: "[master]...".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependencies: true, ..Default::default() }; "square brackets master dot dot dot")] - #[test_case("...[master]", TargetSelector { raw: "...[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependents: true, ..Default::default() }; "dot dot dot master square brackets")] - #[test_case("...[master]...", TargetSelector { raw: "...[master]...".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependencies: true, include_dependents: true, ..Default::default() }; "dot dot dot master square brackets dot dot dot")] - #[test_case("...[from...to]...", TargetSelector { raw: "...[from...to]...".to_string(), git_range: Some(GitRange { from_ref: "from".to_string(), to_ref: Some("to".to_string()), ..Default::default() }), include_dependencies: true, include_dependents: true, ..Default::default() }; "dot dot dot [from...to] dot dot dot")] - #[test_case("foo...[master]", TargetSelector { raw: "foo...[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), name_pattern: "foo".to_string(), match_dependencies: true, ..Default::default() }; "foo...[master]")] - #[test_case("foo...[master]...", TargetSelector { raw: "foo...[master]...".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), name_pattern: "foo".to_string(), match_dependencies: true, include_dependencies: true, ..Default::default() }; "foo...[master] dot dot dot")] - #[test_case("{foo}...[master]", TargetSelector { raw: "{foo}...[master]".to_string(), git_range: Some(GitRange { from_ref: "master".to_string(), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), match_dependencies: true, ..Default::default() }; " curly brackets foo...[master]")] + #[test_case("[master]", TargetSelector { raw: "[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), ..Default::default() }; "square brackets master")] + #[test_case("[from...to]", TargetSelector { raw: "[from...to]".to_string(), git_range: Some(GitRange { from_ref: Some("from".to_string()), to_ref: Some("to".to_string()), ..Default::default() }), ..Default::default() }; "[from...to]")] + #[test_case("{foo}[master]", TargetSelector { raw: "{foo}[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), ..Default::default() }; "{foo}[master]")] + #[test_case("pattern{foo}[master]", TargetSelector { raw: "pattern{foo}[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), name_pattern: "pattern".to_string(), ..Default::default() }; "pattern{foo}[master]")] + #[test_case("[master]...", TargetSelector { raw: "[master]...".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependencies: true, ..Default::default() }; "square brackets master dot dot dot")] + #[test_case("...[master]", TargetSelector { raw: "...[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependents: true, ..Default::default() }; "dot dot dot master square brackets")] + #[test_case("...[master]...", TargetSelector { raw: "...[master]...".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), include_dependencies: true, include_dependents: true, ..Default::default() }; "dot dot dot master square brackets dot dot dot")] + #[test_case("...[from...to]...", TargetSelector { raw: "...[from...to]...".to_string(), git_range: Some(GitRange { from_ref: Some("from".to_string()), to_ref: Some("to".to_string()), ..Default::default() }), include_dependencies: true, include_dependents: true, ..Default::default() }; "dot dot dot [from...to] dot dot dot")] + #[test_case("foo...[master]", TargetSelector { raw: "foo...[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), name_pattern: "foo".to_string(), match_dependencies: true, ..Default::default() }; "foo...[master]")] + #[test_case("foo...[master]...", TargetSelector { raw: "foo...[master]...".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), name_pattern: "foo".to_string(), match_dependencies: true, include_dependencies: true, ..Default::default() }; "foo...[master] dot dot dot")] + #[test_case("{foo}...[master]", TargetSelector { raw: "{foo}...[master]".to_string(), git_range: Some(GitRange { from_ref: Some("master".to_string()), to_ref: None, include_uncommitted: true, ..Default::default() }), parent_dir: Some(AnchoredSystemPathBuf::try_from("foo").unwrap()), match_dependencies: true, ..Default::default() }; " curly brackets foo...[master]")] fn parse_target_selector(raw_selector: &str, want: TargetSelector) { let result = TargetSelector::from_str(raw_selector); diff --git a/crates/turborepo-scm/src/git.rs b/crates/turborepo-scm/src/git.rs index 1793132da68ed..d5134064a61bc 100644 --- a/crates/turborepo-scm/src/git.rs +++ b/crates/turborepo-scm/src/git.rs @@ -1,6 +1,6 @@ use std::{backtrace::Backtrace, collections::HashSet, path::PathBuf, process::Command}; -use tracing::log::warn; +use tracing::warn; use turbopath::{ AbsoluteSystemPath, AbsoluteSystemPathBuf, AnchoredSystemPathBuf, RelativeUnixPath, }; @@ -30,7 +30,7 @@ impl SCM { pub fn changed_files( &self, turbo_root: &AbsoluteSystemPath, - from_commit: &str, + from_commit: Option<&str>, to_commit: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, @@ -57,7 +57,7 @@ impl SCM { pub fn previous_content( &self, - from_commit: &str, + from_commit: Option<&str>, file_path: &AbsoluteSystemPath, ) -> Result, Error> { match self { @@ -80,10 +80,27 @@ impl Git { Ok(output.trim().to_owned()) } + fn resolve_base<'a>(&self, base_override: Option<&'a str>) -> Result<&'a str, Error> { + if let Some(valid_from) = base_override { + return Ok(valid_from); + } + + let main_result = self.execute_git_command(&["rev-parse", "main"], ""); + if main_result.is_ok() { + return Ok("main"); + } + + let master_result = self.execute_git_command(&["rev-parse", "master"], ""); + if master_result.is_ok() { + return Ok("master"); + } + Err(Error::UnableToResolveRef) + } + fn changed_files( &self, turbo_root: &AbsoluteSystemPath, - from_commit: &str, + from_commit: Option<&str>, to_commit: Option<&str>, include_uncommitted: bool, ) -> Result, Error> { @@ -92,12 +109,14 @@ impl Git { let mut files = HashSet::new(); + let valid_from = self.resolve_base(from_commit)?; + if let Some(to_commit) = to_commit { let output = self.execute_git_command( &[ "diff", "--name-only", - &format!("{}...{}", from_commit, to_commit), + &format!("{}...{}", valid_from, to_commit), ], pathspec, )?; @@ -105,7 +124,7 @@ impl Git { self.add_files_from_stdout(&mut files, turbo_root, output); } else { let output = - self.execute_git_command(&["diff", "--name-only", from_commit], pathspec)?; + self.execute_git_command(&["diff", "--name-only", valid_from], pathspec)?; self.add_files_from_stdout(&mut files, turbo_root, output); } @@ -170,11 +189,12 @@ impl Git { fn previous_content( &self, - from_commit: &str, + from_commit: Option<&str>, file_path: &AbsoluteSystemPath, ) -> Result, Error> { let anchored_file_path = self.root.anchor(file_path)?; - let arg = format!("{}:{}", from_commit, anchored_file_path.as_str()); + let valid_from = self.resolve_base(from_commit)?; + let arg = format!("{}:{}", valid_from, anchored_file_path.as_str()); self.execute_git_command(&["show", &arg], "") } @@ -192,7 +212,7 @@ impl Git { /// returns: Result pub fn previous_content( git_root: PathBuf, - from_commit: &str, + from_commit: Option<&str>, file_path: String, ) -> Result, Error> { // If git root is not absolute, we error. @@ -218,17 +238,24 @@ mod tests { process::Command, }; - use git2::{Oid, Repository}; + use git2::{Oid, Repository, RepositoryInitOptions}; use tempfile::TempDir; + use test_case::test_case; use turbopath::{AbsoluteSystemPath, AbsoluteSystemPathBuf, PathError}; use which::which; use super::{previous_content, ChangedFiles}; - use crate::{Error, SCM}; + use crate::{Error, Git, SCM}; - fn setup_repository() -> Result<(TempDir, Repository), Error> { + fn setup_repository( + init_opts: Option<&RepositoryInitOptions>, + ) -> Result<(TempDir, Repository), Error> { let repo_root = tempfile::tempdir()?; - let repo = Repository::init(repo_root.path()).unwrap(); + let repo = Repository::init_opts( + repo_root.path(), + init_opts.unwrap_or(&RepositoryInitOptions::new()), + ) + .unwrap(); let mut config = repo.config().unwrap(); config.set_str("user.name", "test").unwrap(); config.set_str("user.email", "test@example.com").unwrap(); @@ -239,7 +266,7 @@ mod tests { fn changed_files( git_root: PathBuf, turbo_root: PathBuf, - from_commit: &str, + from_commit: Option<&str>, to_commit: Option<&str>, include_uncommitted: bool, ) -> Result, Error> { @@ -328,7 +355,7 @@ mod tests { assert!(changed_files( tmp_dir.path().to_owned(), tmp_dir.path().to_owned(), - "HEAD~1", + Some("HEAD~1"), Some("HEAD"), false, ) @@ -337,7 +364,7 @@ mod tests { assert!(changed_files( tmp_dir.path().to_owned(), tmp_dir.path().to_owned(), - "HEAD", + Some("HEAD"), None, true, ) @@ -348,7 +375,7 @@ mod tests { #[test] fn test_deleted_files() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let file = repo_root.path().join("foo.js"); let file_path = Path::new("foo.js"); @@ -365,7 +392,7 @@ mod tests { let files = changed_files( git_root, turborepo_root, - &first_commit_sha, + Some(&first_commit_sha), Some("HEAD"), false, )?; @@ -376,7 +403,7 @@ mod tests { #[test] fn test_merge_base() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let first_file = repo_root.path().join("foo.js"); fs::write(first_file, "let z = 0;")?; // Create a base commit. This will *not* be the merge base @@ -409,7 +436,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - &third_commit_oid.to_string(), + Some(&third_commit_oid.to_string()), Some(&fourth_commit_oid.to_string()), false, )?; @@ -424,7 +451,7 @@ mod tests { #[test] fn test_changed_files() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let mut index = repo.index().unwrap(); let turbo_root = repo_root.path(); let file = repo_root.path().join("foo.js"); @@ -441,7 +468,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), turbo_root.to_path_buf(), - "HEAD", + Some("HEAD"), None, true, )?; @@ -455,7 +482,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), turbo_root.to_path_buf(), - "HEAD", + Some("HEAD"), None, true, )?; @@ -468,7 +495,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), turbo_root.to_path_buf(), - first_commit_oid.to_string().as_str(), + Some(first_commit_oid.to_string().as_str()), Some(second_commit_oid.to_string().as_str()), false, )?; @@ -483,7 +510,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - first_commit_oid.to_string().as_str(), + Some(first_commit_oid.to_string().as_str()), Some(second_commit_oid.to_string().as_str()), false, )?; @@ -493,7 +520,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - second_commit_oid.to_string().as_str(), + Some(second_commit_oid.to_string().as_str()), None, true, )?; @@ -513,7 +540,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().join("subdir"), - first_commit_oid.to_string().as_str(), + Some(first_commit_oid.to_string().as_str()), Some(third_commit_oid.to_string().as_str()), false, )?; @@ -524,7 +551,7 @@ mod tests { #[test] fn test_changed_files_with_root_as_relative() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let file = repo_root.path().join("foo.js"); fs::write(file, "let z = 0;")?; @@ -540,7 +567,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - "HEAD", + Some("HEAD"), None, true, )?; @@ -553,7 +580,7 @@ mod tests { // (occurs when the monorepo is nested inside a subdirectory of git repository) #[test] fn test_changed_files_with_subdir_as_turbo_root() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; fs::create_dir(repo_root.path().join("subdir"))?; // Create additional nested directory to test that we return a system path @@ -570,7 +597,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().join("subdir"), - "HEAD", + Some("HEAD"), None, true, )?; @@ -590,7 +617,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().join("subdir"), - first_commit.to_string().as_str(), + Some(first_commit.to_string().as_str()), Some( repo.head() .unwrap() @@ -618,7 +645,7 @@ mod tests { #[test] fn test_previous_content() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); let file = root.join_component("foo.js"); @@ -630,7 +657,7 @@ mod tests { let content = previous_content( repo_root.path().to_path_buf(), - first_commit_oid.to_string().as_str(), + Some(first_commit_oid.to_string().as_str()), file.to_string(), )?; @@ -638,14 +665,14 @@ mod tests { let content = previous_content( repo_root.path().to_path_buf(), - second_commit_oid.to_string().as_str(), + Some(second_commit_oid.to_string().as_str()), file.to_string(), )?; assert_eq!(content, b"let z = 1;"); let content = previous_content( repo_root.path().to_path_buf(), - second_commit_oid.to_string().as_str(), + Some(second_commit_oid.to_string().as_str()), "foo.js".to_string(), )?; assert_eq!(content, b"let z = 1;"); @@ -655,7 +682,7 @@ mod tests { #[test] fn test_revparse() -> Result<(), Error> { - let (repo_root, repo) = setup_repository()?; + let (repo_root, repo) = setup_repository(None)?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); let file = root.join_component("foo.js"); @@ -673,13 +700,17 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - "HEAD^", + Some("HEAD^"), Some("HEAD"), false, )?; assert_eq!(files, HashSet::from(["foo.js".to_string()])); - let content = previous_content(repo_root.path().to_path_buf(), "HEAD^", file.to_string())?; + let content = previous_content( + repo_root.path().to_path_buf(), + Some("HEAD^"), + file.to_string(), + )?; assert_eq!(content, b"let z = 0;"); let new_file = repo_root.path().join("bar.js"); @@ -691,7 +722,7 @@ mod tests { let files = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - "HEAD~1", + Some("HEAD~1"), Some("release-1"), false, )?; @@ -700,26 +731,71 @@ mod tests { Ok(()) } + #[test_case(vec!["main"], None, Some("main"))] + #[test_case(vec!["master"], None, Some("master"))] + #[test_case(vec!["ziltoid"], None, None)] + #[test_case(vec!["ziltoid", "main"], Some("ziltoid"), Some("ziltoid"))] + #[test_case(vec!["ziltoid", "main"], Some("main"), Some("main"))] + #[test_case(vec!["ziltoid", "main"], None, Some("main"))] + #[test_case(vec!["ziltoid", "master"], Some("ziltoid"), Some("ziltoid"))] + #[test_case(vec!["ziltoid", "master"], Some("master"), Some("master"))] + #[test_case(vec!["ziltoid", "master"], None, Some("master"))] + #[test_case(vec!["ziltoid", "master", "main"], Some("ziltoid"), Some("ziltoid"))] + #[test_case(vec!["ziltoid", "master", "main"], Some("master"), Some("master"))] + #[test_case(vec!["ziltoid", "master", "main"], Some("main"), Some("main"))] + #[test_case(vec!["ziltoid", "master", "main"], None, Some("main"))] + fn test_base_resolution( + branches_to_create: Vec<&str>, + target_branch: Option<&str>, + expected: Option<&str>, + ) -> Result<(), Error> { + let mut repo_opts = RepositoryInitOptions::new(); + + let (first_branch, remaining_branches) = branches_to_create.split_first().unwrap(); + + let repo_init = repo_opts.initial_head(first_branch); + let (repo_root, repo) = setup_repository(Some(repo_init))?; + let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); + + // WARNING: + // if you do not make a commit, git will show you that you have no branches. + let file = root.join_component("todo.txt"); + file.create_with_contents("1. make async Rust good")?; + let first_commit = commit_file(&repo, Path::new("todo.txt"), None); + let commit = repo.find_commit(first_commit).unwrap(); + + remaining_branches.iter().for_each(|branch| { + repo.branch(branch, &commit, true).unwrap(); + }); + + let thing = Git::find(&root).unwrap(); + let actual = thing.resolve_base(target_branch).ok(); + + assert_eq!(actual, expected); + + Ok(()) + } + #[test] fn test_error_cases() -> Result<(), Error> { let repo_dir = tempfile::tempdir()?; let repo_does_not_exist = changed_files( repo_dir.path().to_path_buf(), repo_dir.path().to_path_buf(), - "HEAD", + Some("HEAD"), None, true, ); assert_matches!(repo_does_not_exist, Err(Error::GitRequired(_))); - let (repo_root, _repo) = setup_repository()?; + let (repo_root, _repo) = setup_repository(None)?; let root = AbsoluteSystemPathBuf::try_from(repo_root.path()).unwrap(); let commit_does_not_exist = changed_files( repo_root.path().to_path_buf(), repo_root.path().to_path_buf(), - "does-not-exist", + Some("does-not-exist"), None, true, ); @@ -728,7 +804,7 @@ mod tests { let file_does_not_exist = previous_content( repo_root.path().to_path_buf(), - "HEAD", + Some("HEAD"), root.join_component("does-not-exist").to_string(), ); assert_matches!(file_does_not_exist, Err(Error::Git(_, _))); @@ -737,7 +813,7 @@ mod tests { let turbo_root_is_not_subdir_of_git_root = changed_files( repo_root.path().to_path_buf(), turbo_root.path().to_path_buf(), - "HEAD", + Some("HEAD"), None, true, ); diff --git a/crates/turborepo-scm/src/lib.rs b/crates/turborepo-scm/src/lib.rs index eb3f369f93bca..509b18e877535 100644 --- a/crates/turborepo-scm/src/lib.rs +++ b/crates/turborepo-scm/src/lib.rs @@ -63,6 +63,8 @@ pub enum Error { Globwalk(#[from] globwalk::GlobError), #[error(transparent)] Walk(#[from] globwalk::WalkError), + #[error("unable to resolve base branch, please set with TURBO_SCM_BASE")] + UnableToResolveRef, } impl From for Error { diff --git a/turborepo-tests/integration/tests/config.t b/turborepo-tests/integration/tests/config.t index 80b7752689ba1..3931257c1c9f7 100644 --- a/turborepo-tests/integration/tests/config.t +++ b/turborepo-tests/integration/tests/config.t @@ -18,7 +18,7 @@ Run test run "packageManager": "npm", "daemon": null, "envMode": "strict", - "scmBase": "main", + "scmBase": null, "scmHead": "HEAD", "cacheDir": ".turbo[\\/]+cache" (re) }