Skip to content

Commit

Permalink
allow multiple fallbacks for --affected base branch (#9045)
Browse files Browse the repository at this point in the history
### 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 <[email protected]>
  • Loading branch information
dimitropoulos and chris-olszewski authored Aug 22, 2024
1 parent f736816 commit 92e9148
Show file tree
Hide file tree
Showing 10 changed files with 163 additions and 85 deletions.
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/commands/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct ConfigOutput<'a> {
package_manager: PackageManager,
daemon: Option<bool>,
env_mode: EnvMode,
scm_base: &'a str,
scm_base: Option<&'a str>,
scm_head: &'a str,
cache_dir: &'a Utf8Path,
}
Expand Down
4 changes: 2 additions & 2 deletions crates/turborepo-lib/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
6 changes: 3 additions & 3 deletions crates/turborepo-lib/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ pub struct ScopeOpts {
pub pkg_inference_root: Option<AnchoredSystemPathBuf>,
pub global_deps: Vec<String>,
pub filter_patterns: Vec<String>,
pub affected_range: Option<(String, String)>,
pub affected_range: Option<(Option<String>, String)>,
}

impl<'a> TryFrom<OptsInputs<'a>> for ScopeOpts {
Expand All @@ -324,7 +324,7 @@ impl<'a> TryFrom<OptsInputs<'a>> 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 {
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion crates/turborepo-lib/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ impl Query {
base: Option<String>,
head: Option<String>,
) -> Result<Vec<Package>, 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));
Expand Down
8 changes: 4 additions & 4 deletions crates/turborepo-lib/src/run/scope/change_detector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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<AnchoredSystemPathBuf>,
) -> Option<LockfileChange> {
let lockfile_path = self
Expand Down Expand Up @@ -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<HashSet<PackageName>, 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,
Expand Down
28 changes: 14 additions & 14 deletions crates/turborepo-lib/src/run/scope/filter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>, String)>,
patterns: &[String],
) -> Result<(HashSet<PackageName>, bool), ResolutionError> {
// inference is None only if we are in the root
Expand All @@ -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>, String)>,
patterns: &[String],
) -> Result<HashSet<PackageName>, ResolutionError> {
let mut selectors = patterns
Expand All @@ -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,
Expand Down Expand Up @@ -549,7 +549,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> {
git_range: &GitRange,
) -> Result<HashSet<PackageName>, 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,
Expand Down Expand Up @@ -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()
}
],
Expand All @@ -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()
}
Expand All @@ -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()
}
Expand All @@ -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()
}
Expand All @@ -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()
Expand All @@ -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()
}
],
Expand All @@ -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()
}
],
Expand All @@ -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()
}
Expand Down Expand Up @@ -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<HashSet<PackageName>, ResolutionError> {
Ok(self
.0
.get(&(from, to))
.get(&(from.expect("expected base branch"), to))
.map(|h| h.to_owned())
.expect("unsupported range"))
}
Expand Down
28 changes: 14 additions & 14 deletions crates/turborepo-lib/src/run/scope/target_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use turbopath::AnchoredSystemPathBuf;

#[derive(Debug, Default, PartialEq)]
pub struct GitRange {
pub from_ref: String,
pub from_ref: Option<String>,
pub to_ref: Option<String>,
pub include_uncommitted: bool,
// Allow unknown objects to be included in the range, without returning an error.
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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);

Expand Down
Loading

0 comments on commit 92e9148

Please sign in to comment.