feat(cli): --filter flag

[gvilums]

What does this PR do?

Adds support for the --filter <PATTERN> argument for bun run. See the pnpm docs for details.

Note that currently only --filter <name_pattern> and --filter ./<dir_pattern> are supported. Selecting dependencies or dependents of packages is not yet possible.

Resolves #2232 Resolves #5207 Fixes #8167

How did you verify your code works?

Tests in test/cli/run/filter-workspace.test.ts.

[github-actions[bot]]

❌ @Jarred-Sumner 4 files with test failures on bun-darwin-aarch64:

• test/js/bun/http/serve.test.ts 2 failing
• test/js/bun/shell/leak.test.ts SIGTERM
• test/js/bun/test/test-test.test.ts 1 failing
• test/js/node/http2/node-http2.test.js 2 failing

View test output

^{#4abf4f379cac12843d60daf03d7f770024888d1c}

[github-actions[bot]]

❌ @Jarred-Sumner 3 files with test failures on linux-x64:

• test/integration/next-pages/test/dev-server.test.ts 1 failing
• test/integration/next-pages/test/next-build.test.ts 1 failing
• test/js/node/watch/fs.watchFile.test.ts 1 failing

View test output

^{#4abf4f379cac12843d60daf03d7f770024888d1c}

[github-actions[bot]]

❌ @Jarred-Sumner 3 files with test failures on linux-x64-baseline:

• test/integration/next-pages/test/dev-server.test.ts 1 failing
• test/integration/next-pages/test/next-build.test.ts 1 failing
• test/js/node/http2/node-http2.test.js 2 failing

View test output

^{#4abf4f379cac12843d60daf03d7f770024888d1c}

[github-actions[bot]]

❌ @gvilums 3 files with test failures on bun-darwin-x64:

• test/js/bun/spawn/spawn.test.ts 1 failing
• test/js/node/http2/node-http2.test.js 1 failing
• test/js/third_party/jsonwebtoken/claim-private.test.js 1 failing

View test output

^{#7496867be0d3e09feb132710190a237de5cf8cb8}

[github-actions[bot]]

❌ @Jarred-Sumner 8 files with test failures on bun-windows-x86_64-haswell

• test\cli\install\bunx.test.ts 1 failing
• test\cli\install\registry\bun-install-registry.test.ts 7 failing
• test\integration\next-pages\test\dev-server-ssr-100.test.ts 1 failing
• test\integration\next-pages\test\dev-server.test.ts 1 failing
• test\integration\next-pages\test\next-build.test.ts 1 failing
• test\js\bun\shell\bunshell.test.ts 2 failing
• test\js\node\dns\node-dns.test.js 2 failing
• test\js\third_party\esbuild\esbuild-child_process.test.ts 1 failing

View test output

^{#4abf4f379cac12843d60daf03d7f770024888d1c}

[gvilums]

Just pushed a commit that includes the root package in script resolution

[Jarred-Sumner]

Running bun-debug run --filter="*" echo hi crashes

[gvilums]

4 issues before this can be shipped.

1. This approach is not fast enough. It needs to be an iterator so that the developer gets immediate feedback instead of waiting for potentially their entire directory tree to parse every package.json file.

This is now implemented in the PackageFilterIterator in filter_args.zig

2. There is a resource leak. After enough scripts are run, spawning any process fails with SystemResources which usually means a file descriptor leak.

I'm able to reproduce this, but running lsof -p <pid> to list the currently open file descriptors shows only a handful at any given time. Not sure what's causing this issue.

Interestingly, when running a large number of js files, one fd seems to be leaked per file that's executed. Not sure where those are coming (EDIT: there was a preexisting fd leak in run_command.zig that is now fixed. Error when just running echo hi persists). (EDIT 2: Error when running echo hi is fixed)

3. it scans node_modules. It should not scan node_modules, .git, or any . directories.

Implemented by passing an ignore function to the GlobWalker

4. We need tests for what happens when you run multiple JavaScript/TypeScript files in different projects same process (bun run --filter="*" index.js). Currently, Bun assumes only one JavaScript file every executes for a given thread. That assumption is not true when using these code paths.

We now fork before executing any JS. This of course doesn't work on windows, and more generally we should decide how to handle this. Options include:

• Figure out the correct command line arguments and spawn a new copy of the bun process. Slowest, but would provide complete isolation.
• Continue using fork and use something like RtlCloneUserProcess on windows. Caveat is that these are not "official" windows APIs and their stability is unknown.
• Work on isolating VM instances and run everything in the same process. Fastest, but also potentially risky?

5. It would be best to use the module resolution cache so that when running a JavaScript file later, the subsequent calls to getdents64() get cached. This will involve a small change to the GlobWalker.

Implemented by abstracting how the GlobWalker does filesystem access.

[gvilums]

After investigating 2. more, it seems like the error lies somewhere in child_process.spawnAndWait. Specifically, the error that's happening is that the child fails between being forked and calling execv, and signals this error to the parent.

EDIT: After switching the implementation to posix_spawn, the specific error that comes up seems to be E2BIG, which has a description of "The number of bytes in the new process's argument list is larger than the system-imposed limit. This limit is specified by the sysctl(3) MIB variable KERN_ARGMAX." On my machine, this error starts occurring after spawning roughly 3000 processes. I'm not aware of any resource leaks that could cause this (after all, each process is spawned with an argument list that's basically identical, and each process has its own argument list), but I propose that we leave this issue be for now, as it is an extreme edge case

EDIT 2: So I was able to solve the mystery: The culprit was an ever-growing path variable, which, each time an attempt at executing a package was made, was extended with new items. This lead to it growing unmanageably large after a few thousand executions. The fix was to reset PATH after every execution.

[MickL]

Does this also work with bun add? It is very annoying that I always need to go in the directory. pnpn has pnp add --filter workspace

[dylan-conway]

this needs more tests:

• what happens when a package.json is malformed?

Missing name and empty name are two tests we should have. Pnpm seems to not allow filtering with --filter "" so we can exit early when that pattern is given

[Jarred-Sumner]

Missing name and empty name are two tests we should have.

It's reading package.jsons to check the name/version in this code, so we need to test that here too

[beorn]

Perhaps also support the '-F' shorthand?