bug: --filter=/apps/* option doesn't work

[rxliuli]

What version of Turborepo are you using?

v1.2.0-canary.0

What package manager are you using / does the bug impact?

pnpm

What operating system are you using?

Windows

Describe the Bug

Added support for the --filter option in PR #887, but it doesn't seem to work correctly. When I use the --filter=/apps/* option to filter the specified directory, it executes the task in all modules

repo：https://github.com/rxliuli/liuli-tools/tree/feat_turbo
command

turbo run setup --filter=/apps/* --dry=json > log.json

output
log.zip

Expected Behavior

Can correctly filter the required modules according to the directory

To Reproduce

https://github.com/rxliuli/liuli-tools/tree/feat_turbo

[weyert]

Does it work when you use --filter=./apps/* or --filter={/apps/*}?

[rxliuli]

Does it work when you use --filter=./apps/* or --filter={/apps/*}?

It works, but it's weird, pnpm doesn't seem to do that, and also doesn't seem to support filtering by directory directly at the moment

[rxliuli]

Sadly, just using --dry the output is correct, but still has issues when running

$ pnpx turbo run setup --filter=./apps/*
• Packages in scope: @liuli-util/cli, @liuli-util/cmd-shim
• Running setup in 2 packages
@liuli-util/async:setup: cache miss, executing ce98eb3c00c3005d
@liuli-util/cli:setup: cache hit, replaying output dbb29952651ff7e4
@liuli-util/cli:setup:
@liuli-util/cli:setup: > @liuli-util/cli@3.19.2 setup C:\Users\rxliuli\Code\Web\liuli-tools\apps\liuli-cli
@liuli-util/cli:setup: > pnpm build
@liuli-util/cli:setup:
@liuli-util/cli:setup:
@liuli-util/cli:setup: > @liuli-util/cli@3.19.2 build C:\Users\rxliuli\Code\Web\liuli-tools\apps\liuli-cli
@liuli-util/cli:setup: > esno src/bin.ts build cli
@liuli-util/cli:setup:
@liuli-util/cli:setup: - 构建 cli
@liuli-util/cli:setup: - 构建 cli
@liuli-util/cli:setup: - 构建 esm
@liuli-util/cli:setup: - 构建 cli
@liuli-util/cli:setup: - 构建 esm
@liuli-util/cli:setup: - 构建 cjs
@liuli-util/cli:setup: - 构建 cli
@liuli-util/cli:setup: - 构建 esm
@liuli-util/cli:setup: - 构建 cjs
@liuli-util/cli:setup: - 生成类型定义
@liuli-util/cli:setup: - 构建 cli
@liuli-util/cli:setup: - 构建 esm: 59ms
@liuli-util/cli:setup: - 构建 cjs
@liuli-util/cli:setup: - 生成类型定义
@liuli-util/cli:setup: - 构建 cli: 7973ms
@liuli-util/cli:setup: - 构建 esm: 59ms
@liuli-util/cli:setup: - 构建 cjs
@liuli-util/cli:setup: - 生成类型定义
@liuli-util/cli:setup: - 构建 cli: 7973ms
@liuli-util/cli:setup: - 构建 esm: 59ms
@liuli-util/cli:setup: - 构建 cjs: 7957ms
@liuli-util/cli:setup: - 生成类型定义
@liuli-util/cli:setup: - 构建 cli: 7973ms
@liuli-util/cli:setup: - 构建 esm: 59ms
@liuli-util/cli:setup: - 构建 cjs: 7957ms
@liuli-util/cli:setup: - 生成类型定义: 7980ms
@liuli-util/cli:setup: 构建完成: 7997ms
@liuli-util/async:setup: 
@liuli-util/async:setup: > @liuli-util/async@3.3.0 setup C:\Users\rxliuli\Code\Web\liuli-tools\libs\async
@liuli-util/async:setup: > pnpm build
@liuli-util/async:setup:
@liuli-util/async:setup: 
@liuli-util/async:setup: > @liuli-util/async@3.3.0 build C:\Users\rxliuli\Code\Web\liuli-tools\libs\async
@liuli-util/async:setup: > liuli-cli build lib
@liuli-util/async:setup:
@liuli-util/async:setup: - 构建 esm
@liuli-util/async:setup: - 构建 esm
@liuli-util/async:setup: - 构建 cjs
@liuli-util/async:setup: - 构建 esm
@liuli-util/async:setup: - 构建 cjs
@liuli-util/async:setup: - 生成类型定义
@liuli-util/async:setup: - 构建 esm: 52ms
@liuli-util/async:setup: - 构建 cjs
@liuli-util/async:setup: - 生成类型定义
@liuli-util/async:setup: - 构建 esm: 52ms
@liuli-util/async:setup: - 构建 cjs: 45ms
@liuli-util/async:setup: - 生成类型定义
@liuli-util/async:setup: - 构建 esm: 52ms
@liuli-util/async:setup: - 构建 cjs: 45ms
@liuli-util/async:setup: - 生成类型定义: 2219ms
@liuli-util/async:setup: 构建完成: 2233ms

 Tasks:    2 successful, 2 total
Cached:    1 cached, 2 total
  Time:    4.614s

As can be observed, there is a module not in ./apps that also executes the setup script

commit link: rxliuli/liuli-tools@460578c#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R8

[gsoltis]

@rxliuli As best I can tell, this is working as expected.

1. To select directories, you need either the leading . or {} and to use a glob to match recursively. ./apps/* and {/apps/*} are both selecting the correct entrypoints, namely @liuli-util/cli and @lioli-util/cmd-shim.
2. You asked turbo to run the setup command. @lioli-util/cmd-shim does not have a setup, so there is no work to be done there.
3. @lioli-util/cli does have a setup, and via turbo.json, you have told turbo that setup depends on running setup in each package's dependencies, "dependsOn": ["^setup"]. Therefore, turbo will run setup in each of @lioli-util/cli's dependencies, before running it in @lioli-util/cli itself. In this case, that means running @lioli-util/async#setup, as shown in your log.

As a general note, --filter and other commandline flags select entrypoints. turbo will then use your configured pipeline from turbo.json to figure out what tasks need to run. This is different from pnpm, since pnpm does not have a notion of a full pipeline (depending on tasks being run in dependencies first, dependencies between tasks, etc.) so in that case you are generally selecting the full complement of packages that you are interested in.

It's possible in your case that you don't want the pipeline dependency from setup on ^setup, if you don't want @lioli-util/async#setup task to run in this particular invocation.

I'd also like to say thanks for the easy repo. It's very helpful for getting to the bottom of these issues quickly!

I'm going to close this issue because I believe turbo is behaving as intended, but I'm happy to reopen if it turns out there is a bug here.

[rxliuli]

3. @lioli-util/cli does have a setup, and via turbo.json, you have told turbo that setup depends on running setup in each package's dependencies, "dependsOn": ["^setup"]. Therefore, turbo will run setup in each of @lioli-util/cli's dependencies, before running it in @lioli-util/cli itself. In this case, that means running @lioli-util/async#setup, as shown in your log.

@liuli-util/cli doesn't depend on @liuli-util/async but the other way around, so I don't think @liuli-util/async should be run

ref: https://github.com/rxliuli/liuli-tools/blob/6cd9c0ae448ef5c4cbe57ee804f0ceed5368b576/apps/liuli-cli/package.json#L30

[gsoltis]

Ah, you're right. However, @liuli-util/cmd-shim does depend on @liuli-util/async. That's how @liuli-util/async is getting picked up. Currently, we calculate the tasks to run before checking if they actually exist, so this dependency gets picked up. Changing this is being tracked in #937, but is unrelated to --filter, which selects the entrypoints. The entrypoints are being correctly selected here, but it appears the downstream behavior is then unexpected in this case.

Please feel free to chime in on #937, it's helpful to get examples of what people expect to happen in these scenarios.

[rxliuli]

Ah, you're right. However, @liuli-util/cmd-shim does depend on @liuli-util/async. That's how @liuli-util/async is getting picked up. Currently, we calculate the tasks to run before checking if they actually exist, so this dependency gets picked up. Changing this is being tracked in #937, but is unrelated to --filter, which selects the entrypoints. The entrypoints are being correctly selected here, but it appears the downstream behavior is then unexpected in this case.

Please feel free to chime in on #937, it's helpful to get examples of what people expect to happen in these scenarios.

It seems strange, why does a module cmd-shim not running let other modules async it depend on run?

[gsoltis]

I agree that it is perhaps unexpected behavior, but the existing logic goes like this:

1. select packages as entrypoints. In your case this is @liuli-util/cmd-shim and @liuli-util/cli.
2. select all of the dependencies for the specified task. setup in your case depends on the setup being run in package dependencies
3. run all of the tasks we've now selected in the correct order. It is only at this point that we determine that a particular task might not exist for a particular package.

#937 tracks the bug for changing the check in step 3 above to occur earlier, possibly in step 1.