Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add --affected to turbo ls #8970

Merged
merged 5 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions crates/turborepo-lib/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,6 +494,10 @@ pub enum Command {
Config,
/// EXPERIMENTAL: List packages in your monorepo.
Ls {
/// Show only packages that are affected by changes between
/// the current branch and `main`
#[clap(long, group = "scope-filter-group")]
affected: bool,
/// Use the given selector to specify package(s) to act as
/// entry points. The syntax mirrors pnpm's syntax, and
/// additional documentation and examples can be found in
Expand Down Expand Up @@ -1157,15 +1161,20 @@ pub async fn run(
config::run(base).await?;
Ok(0)
}
Command::Ls { filter, packages } => {
Command::Ls {
affected,
filter,
packages,
} => {
warn!("ls command is experimental and may change in the future");
let event = CommandEventBuilder::new("info").with_parent(&root_telemetry);

event.track_call();
let affected = *affected;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ls currently suffers from the same "double run arg" problem. Doesn't need to happen in this PR, but there should be checks to make sure cli_args.execution_args.affected and cli_args.execution_args.filter aren't defined.

let filter = filter.clone();
let packages = packages.clone();
let base = CommandBase::new(cli_args, repo_root, version, color_config);
ls::run(base, packages, event, filter).await?;
ls::run(base, packages, event, filter, affected).await?;

Ok(0)
}
Expand Down
8 changes: 5 additions & 3 deletions crates/turborepo-lib/src/commands/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub async fn run(
packages: Vec<String>,
telemetry: CommandEventBuilder,
filter: Vec<String>,
affected: bool,
) -> Result<(), cli::Error> {
let signal = get_signal()?;
let handler = SignalHandler::new(signal);
Expand All @@ -51,6 +52,7 @@ pub async fn run(
run_args: Box::default(),
execution_args: Box::new(ExecutionArgs {
filter,
affected,
..Default::default()
}),
});
Expand Down Expand Up @@ -82,6 +84,9 @@ impl<'a> RepositoryDetails<'a> {
if !filtered_pkgs.contains(package_name) {
return None;
}
if matches!(package_name, PackageName::Root) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not strictly speaking part of --affected, but in adding these tests, I noticed we had a bug

return None;
}

Some((package_name, package_info.package_path()))
})
Expand All @@ -107,9 +112,6 @@ impl<'a> RepositoryDetails<'a> {
}

for (package_name, entry) in &self.packages {
if matches!(package_name, PackageName::Root) {
continue;
}
println!(" {} {}", package_name, GREY.apply_to(entry));
}

Expand Down
45 changes: 44 additions & 1 deletion turborepo-tests/integration/tests/affected.t
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ Validate that we only run `my-app#build` with change not committed
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

my-app apps[\/\\]my-app (re)

Commit the change
$ git add .
Expand All @@ -47,6 +53,12 @@ Validate that we only run `my-app#build` with change committed
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
1 package

my-app apps[\/\\]my-app (re)

Override the SCM base to be HEAD, so nothing runs
$ TURBO_SCM_BASE="HEAD" ${TURBO} run build --affected --log-order grouped
Expand All @@ -61,6 +73,13 @@ Override the SCM base to be HEAD, so nothing runs
Time:\s*[\.0-9]+m?s (re)


Do the same thing with the `ls` command
$ TURBO_SCM_BASE="HEAD" ${TURBO} ls --affected
WARNING ls command is experimental and may change in the future
0 packages



Override the SCM head to be main, so nothing runs
$ TURBO_SCM_HEAD="main" ${TURBO} run build --affected --log-order grouped
\xe2\x80\xa2 Packages in scope: (esc)
Expand All @@ -74,6 +93,13 @@ Override the SCM head to be main, so nothing runs
Time:\s*[\.0-9]+m?s (re)


Do the same thing with the `ls` command
$ TURBO_SCM_HEAD="main" ${TURBO} ls --affected
WARNING ls command is experimental and may change in the future
0 packages



Now add a commit to `main` so the merge base is different from `main`
$ git checkout main --quiet
$ echo "foo" >> packages/util/index.js
Expand All @@ -98,7 +124,12 @@ 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
1 package

my-app apps[\/\\]my-app (re)

Now do some magic to change the repo to be shallow
$ SHALLOW=$(git rev-parse --show-toplevel)/.git/shallow
Expand Down Expand Up @@ -131,3 +162,15 @@ Now try running `--affected` again, we should run all tasks
Cached: 1 cached, 2 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
WARNING unable to detect git range, assuming all files have changed: git error: fatal: main...HEAD: no merge base

3 packages

another packages[\/\\]another (re)
my-app apps[\/\\]my-app (re)
util packages[\/\\]util (re)


Loading