diff --git a/crates/turborepo-lib/src/commands/config.rs b/crates/turborepo-lib/src/commands/config.rs index a5fc906554929..e43b2175aa37f 100644 --- a/crates/turborepo-lib/src/commands/config.rs +++ b/crates/turborepo-lib/src/commands/config.rs @@ -24,7 +24,7 @@ struct ConfigOutput<'a> { daemon: Option, env_mode: EnvMode, scm_base: Option<&'a str>, - scm_head: &'a str, + scm_head: Option<&'a str>, cache_dir: &'a Utf8Path, } diff --git a/crates/turborepo-lib/src/config/env.rs b/crates/turborepo-lib/src/config/env.rs index f70987712b9fd..bc7e08437eda4 100644 --- a/crates/turborepo-lib/src/config/env.rs +++ b/crates/turborepo-lib/src/config/env.rs @@ -380,7 +380,7 @@ mod test { assert_eq!(config.env_mode, None); assert!(!config.preflight()); assert_eq!(config.scm_base(), None); - assert_eq!(config.scm_head(), "HEAD"); + assert_eq!(config.scm_head(), None); assert_eq!(config.root_turbo_json_path, None); assert!(!config.force()); assert_eq!(config.log_order(), LogOrder::Auto); diff --git a/crates/turborepo-lib/src/config/mod.rs b/crates/turborepo-lib/src/config/mod.rs index c059333421446..a693b601466c6 100644 --- a/crates/turborepo-lib/src/config/mod.rs +++ b/crates/turborepo-lib/src/config/mod.rs @@ -316,8 +316,8 @@ impl ConfigurationOptions { non_empty_str(self.scm_base.as_deref()) } - pub fn scm_head(&self) -> &str { - non_empty_str(self.scm_head.as_deref()).unwrap_or("HEAD") + pub fn scm_head(&self) -> Option<&str> { + non_empty_str(self.scm_head.as_deref()) } pub fn allow_no_package_manager(&self) -> bool { diff --git a/crates/turborepo-lib/src/opts.rs b/crates/turborepo-lib/src/opts.rs index fdd20aa088be4..2442ba3c78326 100644 --- a/crates/turborepo-lib/src/opts.rs +++ b/crates/turborepo-lib/src/opts.rs @@ -310,7 +310,7 @@ pub struct ScopeOpts { pub pkg_inference_root: Option, pub global_deps: Vec, pub filter_patterns: Vec, - pub affected_range: Option<(Option, String)>, + pub affected_range: Option<(Option, Option)>, } impl<'a> TryFrom> for ScopeOpts { @@ -327,7 +327,10 @@ 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.map(|b| b.to_owned()), scm_head.to_string()) + ( + scm_base.map(|b| b.to_owned()), + scm_head.map(|h| h.to_string()), + ) }); Ok(Self { @@ -518,7 +521,9 @@ mod test { pkg_inference_root: None, global_deps: vec![], filter_patterns: opts_input.filter_patterns, - affected_range: opts_input.affected.map(|(base, head)| (Some(base), head)), + affected_range: opts_input + .affected + .map(|(base, head)| (Some(base), Some(head))), }; let opts = Opts { run_opts, diff --git a/crates/turborepo-lib/src/query.rs b/crates/turborepo-lib/src/query.rs index 64faef1a059f2..af3fe373827eb 100644 --- a/crates/turborepo-lib/src/query.rs +++ b/crates/turborepo-lib/src/query.rs @@ -242,8 +242,6 @@ impl Query { base: Option, head: Option, ) -> Result, Error> { - 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 f7439c751b715..b2527a74cc40d 100644 --- a/crates/turborepo-lib/src/run/scope/change_detector.rs +++ b/crates/turborepo-lib/src/run/scope/change_detector.rs @@ -21,6 +21,7 @@ pub trait GitChangeDetector { to_ref: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, + merge_base: bool, ) -> Result, ResolutionError>; } @@ -92,6 +93,7 @@ impl<'a> GitChangeDetector for ScopeChangeDetector<'a> { to_ref: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, + merge_base: bool, ) -> Result, ResolutionError> { let changed_files = match self.scm.changed_files( self.turbo_root, @@ -99,6 +101,7 @@ impl<'a> GitChangeDetector for ScopeChangeDetector<'a> { to_ref, include_uncommitted, allow_unknown_objects, + merge_base, )? { ChangedFiles::All => { debug!("all packages changed"); diff --git a/crates/turborepo-lib/src/run/scope/filter.rs b/crates/turborepo-lib/src/run/scope/filter.rs index b16924d430ab1..331d53f0526f5 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<(Option, String)>, + affected: &Option<(Option, Option)>, 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<(Option, String)>, + affected: &Option<(Option, Option)>, patterns: &[String], ) -> Result, ResolutionError> { let mut selectors = patterns @@ -197,9 +197,10 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { selectors.push(TargetSelector { git_range: Some(GitRange { from_ref: from_ref.clone(), - to_ref: Some(to_ref.to_string()), + to_ref: to_ref.clone(), include_uncommitted: true, allow_unknown_objects: true, + merge_base: true, }), include_dependents: true, ..Default::default() @@ -549,6 +550,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> { git_range.to_ref.as_deref(), git_range.include_uncommitted, git_range.allow_unknown_objects, + git_range.merge_base, ) } @@ -1266,6 +1268,7 @@ mod test { to: Option<&str>, _include_uncommitted: bool, _allow_unknown_objects: bool, + _merge_base: bool, ) -> Result, ResolutionError> { Ok(self .0 diff --git a/crates/turborepo-lib/src/run/scope/target_selector.rs b/crates/turborepo-lib/src/run/scope/target_selector.rs index 38845b0698967..cae2e70fa6d57 100644 --- a/crates/turborepo-lib/src/run/scope/target_selector.rs +++ b/crates/turborepo-lib/src/run/scope/target_selector.rs @@ -13,6 +13,9 @@ pub struct GitRange { // this is useful for shallow clones where objects may not exist. // When this happens, we assume that everything has changed. pub allow_unknown_objects: bool, + // Calculate diff between merge base of the two refs and the second ref + // (this is usually what you want for detecting changes) + pub merge_base: bool, } #[derive(Debug, Default, PartialEq)] @@ -159,6 +162,7 @@ impl FromStr for TargetSelector { to_ref: Some(b.to_string()), include_uncommitted: false, allow_unknown_objects: false, + merge_base: true, } } else { // If only the start of the range is specified, we assume that @@ -168,6 +172,7 @@ impl FromStr for TargetSelector { to_ref: None, include_uncommitted: true, allow_unknown_objects: false, + merge_base: false, } }; Some(git_range) @@ -254,13 +259,13 @@ mod test { #[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: 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("[from...to]", TargetSelector { raw: "[from...to]".to_string(), git_range: Some(GitRange { from_ref: Some("from".to_string()), to_ref: Some("to".to_string()), merge_base: true, ..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("...[from...to]...", TargetSelector { raw: "...[from...to]...".to_string(), git_range: Some(GitRange { from_ref: Some("from".to_string()), to_ref: Some("to".to_string()), merge_base: true, ..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]")] diff --git a/crates/turborepo-scm/src/git.rs b/crates/turborepo-scm/src/git.rs index 30e22c42a21d7..ac475c49a2adf 100644 --- a/crates/turborepo-scm/src/git.rs +++ b/crates/turborepo-scm/src/git.rs @@ -35,6 +35,7 @@ impl SCM { to_commit: Option<&str>, include_uncommitted: bool, allow_unknown_objects: bool, + merge_base: bool, ) -> Result { fn unable_to_detect_range(error: impl std::error::Error) -> Result { warn!( @@ -45,7 +46,13 @@ impl SCM { } match self { Self::Git(git) => { - match git.changed_files(turbo_root, from_commit, to_commit, include_uncommitted) { + match git.changed_files( + turbo_root, + from_commit, + to_commit, + include_uncommitted, + merge_base, + ) { Ok(files) => Ok(ChangedFiles::Some(files)), Err(ref error @ Error::Git(ref message, _)) if allow_unknown_objects && message.contains("no merge base") => @@ -110,6 +117,7 @@ impl Git { from_commit: Option<&str>, to_commit: Option<&str>, include_uncommitted: bool, + merge_base: bool, ) -> Result, Error> { let turbo_root_relative_to_git_root = self.root.anchor(turbo_root)?; let pathspec = turbo_root_relative_to_git_root.as_str(); @@ -118,27 +126,23 @@ impl Git { 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!("{}...{}", valid_from, to_commit), - ], - pathspec, - )?; - - self.add_files_from_stdout(&mut files, turbo_root, output); + let mut args = if let Some(to_commit) = to_commit { + vec!["diff", "--name-only", valid_from, to_commit] } else { - let output = - self.execute_git_command(&["diff", "--name-only", valid_from], pathspec)?; + vec!["diff", "--name-only", valid_from] + }; - self.add_files_from_stdout(&mut files, turbo_root, output); + if merge_base { + args.push("--merge-base"); } + let output = self.execute_git_command(&args, pathspec)?; + self.add_files_from_stdout(&mut files, turbo_root, output); + // We only care about non-tracked files if we haven't specified both ends up the - // comparison or if we are using `--affected` + // comparison if include_uncommitted { + // Add untracked files, i.e. files that are not in git at all let output = self .execute_git_command(&["ls-files", "--others", "--exclude-standard"], pathspec)?; self.add_files_from_stdout(&mut files, turbo_root, output); @@ -281,12 +285,16 @@ mod tests { let scm = SCM::new(git_root); let turbo_root = AbsoluteSystemPathBuf::try_from(turbo_root.as_path())?; + // Replicating the `--filter` behavior where we only do a merge base + // if both ends of the git range are specified. + let merge_base = to_commit.is_some(); let ChangedFiles::Some(files) = scm.changed_files( &turbo_root, from_commit, to_commit, include_uncommitted, false, + merge_base, )? else { unreachable!("changed_files should always return Some"); @@ -849,7 +857,7 @@ mod tests { let scm = SCM::new(&root); let actual = scm - .changed_files(&root, None, Some("HEAD"), true, true) + .changed_files(&root, None, Some("HEAD"), true, true, false) .unwrap(); assert_matches!(actual, ChangedFiles::All); diff --git a/turborepo-tests/integration/tests/affected.t b/turborepo-tests/integration/tests/affected.t index cf42e7c012381..8828d73ddbcac 100644 --- a/turborepo-tests/integration/tests/affected.t +++ b/turborepo-tests/integration/tests/affected.t @@ -5,15 +5,69 @@ Create a new branch $ git checkout -b my-branch Switched to a new branch 'my-branch' -Edit a file that affects `my-app` - $ echo "foo" >> apps/my-app/index.js +Ensure that nothing is affected + $ ${TURBO} ls --affected + WARNING ls command is experimental and may change in the future + 0 no packages (npm) + + +Create a new file that affects `my-app` + $ echo "foo" > apps/my-app/new.js + +Validate that we only run `my-app#build` with change not committed + $ ${TURBO} run build --affected --log-order grouped + \xe2\x80\xa2 Packages in scope: my-app (esc) + \xe2\x80\xa2 Running build in 1 packages (esc) + \xe2\x80\xa2 Remote caching disabled (esc) + my-app:build: cache miss, executing 1b83c3b24476ec9c + my-app:build: + my-app:build: > build + my-app:build: > echo building + my-app:build: + my-app:build: building + + Tasks: 1 successful, 1 total + Cached: 0 cached, 1 total + Time:\s*[\.0-9]+m?s (re) + + + +Do the same thing with the `ls` command + $ ${TURBO} ls --affected + WARNING ls command is experimental and may change in the future + 1 package (npm) + + my-app apps[\/\\]my-app (re) + + + +Do the same thing with the `query` command + $ ${TURBO} query "query { affectedPackages { name } }" + WARNING query command is experimental and may change in the future + { + "data": { + "affectedPackages": [ + { + "name": "my-app" + } + ] + } + } + + +Remove the new file + $ rm apps/my-app/new.js + +Add field to `apps/my-app/package.json` + $ jq '. += {"description": "foo"}' apps/my-app/package.json | tr -d '\r' > apps/my-app/package.json.new + $ mv apps/my-app/package.json.new apps/my-app/package.json Validate that we only run `my-app#build` with change not committed $ ${TURBO} run build --affected --log-order grouped \xe2\x80\xa2 Packages in scope: my-app (esc) \xe2\x80\xa2 Running build in 1 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) - my-app:build: cache miss, executing 97b34acb6e848096 + my-app:build: cache miss, executing c1189254892f813f my-app:build: my-app:build: > build my-app:build: > echo building @@ -55,7 +109,7 @@ Validate that we only run `my-app#build` with change committed \xe2\x80\xa2 Packages in scope: my-app (esc) \xe2\x80\xa2 Running build in 1 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) - my-app:build: cache hit, replaying logs 97b34acb6e848096 + my-app:build: cache hit, replaying logs c1189254892f813f my-app:build: my-app:build: > build my-app:build: > echo building @@ -159,7 +213,7 @@ Run the build and expect only `my-app` to be affected, since between \xe2\x80\xa2 Packages in scope: my-app (esc) \xe2\x80\xa2 Running build in 1 packages (esc) \xe2\x80\xa2 Remote caching disabled (esc) - my-app:build: cache hit, replaying logs 97b34acb6e848096 + my-app:build: cache hit, replaying logs c1189254892f813f my-app:build: my-app:build: > build my-app:build: > echo building @@ -170,6 +224,8 @@ Run the build and expect only `my-app` to be affected, since between Cached: 1 cached, 1 total Time:\s*[\.0-9]+m?s >>> FULL TURBO (re) + + Do the same thing with the `ls` command $ ${TURBO} ls --affected WARNING ls command is experimental and may change in the future @@ -199,33 +255,72 @@ Now do some magic to change the repo to be shallow $ git prune-packed Now try running `--affected` again, we should run all tasks - $ ${TURBO} run build --affected --log-order grouped - WARNING unable to detect git range, assuming all files have changed: git error: fatal: main...HEAD: no merge base + $ ${TURBO} run build --affected --dry-run json | jq '.tasks | map(.taskId)| sort' + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found - \xe2\x80\xa2 Packages in scope: //, another, my-app, util (esc) - \xe2\x80\xa2 Running build in 4 packages (esc) - \xe2\x80\xa2 Remote caching disabled (esc) - my-app:build: cache hit, replaying logs 97b34acb6e848096 - my-app:build: - my-app:build: > build - my-app:build: > echo building - my-app:build: - my-app:build: building - util:build: cache miss, executing bf1798d3e46e1b48 - util:build: - util:build: > build - util:build: > echo building - util:build: - util:build: building + [ + "another#build", + "my-app#build", + "util#build" + ] + +Do the same thing with the `ls` command + $ ${TURBO} ls --affected + WARNING ls command is experimental and may change in the future + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found - Tasks: 2 successful, 2 total - Cached: 1 cached, 2 total - Time:\s*[\.0-9]+m?s (re) + 3 packages (npm) + + another packages[\/\\]another (re) + my-app apps[\/\\]my-app (re) + util packages[\/\\]util (re) + + +Do the same thing with the `query` command + $ ${TURBO} query "query { affectedPackages { name } }" + WARNING query command is experimental and may change in the future + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found + + { + "data": { + "affectedPackages": [ + { + "name": "//" + }, + { + "name": "another" + }, + { + "name": "my-app" + }, + { + "name": "util" + } + ] + } + } + +Now do some magic to change the repo to be shallow + $ SHALLOW=$(git rev-parse --show-toplevel)/.git/shallow + $ git rev-parse HEAD > "$SHALLOW" + $ git reflog expire --expire=0 + $ git prune + $ git prune-packed + +Now try running `--affected` again, we should run all tasks + $ ${TURBO} run build --affected --dry-run json | jq '.tasks | map(.taskId)| sort' + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found + [ + "another#build", + "my-app#build", + "util#build" + ] + Do the same thing with the `ls` command $ ${TURBO} ls --affected WARNING ls command is experimental and may change in the future - WARNING unable to detect git range, assuming all files have changed: git error: fatal: main...HEAD: no merge base + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found 3 packages (npm) @@ -237,7 +332,7 @@ Do the same thing with the `ls` command Do the same thing with the `query` command $ ${TURBO} query "query { affectedPackages { name } }" WARNING query command is experimental and may change in the future - WARNING unable to detect git range, assuming all files have changed: git error: fatal: main...HEAD: no merge base + WARNING unable to detect git range, assuming all files have changed: git error: fatal: no merge base found { "data": { diff --git a/turborepo-tests/integration/tests/config.t b/turborepo-tests/integration/tests/config.t index 3931257c1c9f7..a99d14f85db98 100644 --- a/turborepo-tests/integration/tests/config.t +++ b/turborepo-tests/integration/tests/config.t @@ -19,7 +19,7 @@ Run test run "daemon": null, "envMode": "strict", "scmBase": null, - "scmHead": "HEAD", + "scmHead": null, "cacheDir": ".turbo[\\/]+cache" (re) }