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

Turbo --filter builds all the packages when a new workspace is added #8125

Closed
1 task done
alainrk opened this issue May 10, 2024 · 3 comments · Fixed by #8127
Closed
1 task done

Turbo --filter builds all the packages when a new workspace is added #8125

alainrk opened this issue May 10, 2024 · 3 comments · Fixed by #8127
Assignees
Labels
kind: bug Something isn't working owned-by: turborepo

Comments

@alainrk
Copy link

alainrk commented May 10, 2024

Verify canary release

  • I verified that the issue exists in the latest Turborepo canary release.

Link to code that reproduces this issue

https://github.com/alainrk/turbo-test

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

pnpm

What operating system are you using?

Mac

Which canary version will you have in your reproduction?

1.13.3

Describe the Bug

When we create a new workspace that have node dependencies and build the monorepo using the filter option we have an unexpected behaviour.
What happens is that even though we haven't changed anything in the other workspaces (proper code changes or installing/updating npm packages), when we build with the filter option all the packages are being built regardless the filter used.
In particular this happens locally when we have unstaged changes and we filter using "HEAD" as mentioned in the documentation, or in CI where we use two commits to have the same result (e.g. PR last commit and Base branch commit).
Please note, that if the new workspace has no dependencies, the build behaviour is instead correct.

Expected Behavior

The expected behaviour is that only the new workspace gets built.

To Reproduce

Go to the repository (https://github.com/alainrk/turbo-test), clone it and just run npm run reproduce-issue, all the other details on how it works are in the README of the repo itself.

Additional context

No response

@alainrk alainrk added kind: bug Something isn't working needs: triage New issues get this label. Remove it after triage owned-by: turborepo labels May 10, 2024
@gsoltis
Copy link
Contributor

gsoltis commented May 10, 2024

This appears to be a result of the code here. When comparing the lockfile against the previous version, we attempt to find all packages that have external dependencies. The previous version of the lockfile doesn't have the package at all (because it was just added), and so we default to considering it a global change.

TBD on what the correct behavior here is, but it's likely ignoring missing dependencies from lockfiles in certain situations.

@gsoltis gsoltis removed the needs: triage New issues get this label. Remove it after triage label May 10, 2024
@gsoltis
Copy link
Contributor

gsoltis commented May 10, 2024

@alainrk thanks for the excellent reproduction, it made tracking down what was happening much easier.

@alainrk
Copy link
Author

alainrk commented May 11, 2024

Thank you for addressing that @gsoltis 👍

gsoltis pushed a commit that referenced this issue May 13, 2024
### Description

When comparing lockfiles between revisions to identify what packages
have been affected, we treated missing packages in the previous lockfile
(added in the new lockfile) as errors, and fell back to considering all
packages as affected. This change adds a boolean to determine what the
behavior should be when we can't find a package that we expect. When
building the package graph, it continues to be an error, as well as when
tracing the dependencies of a package we've already found in the
lockfile. However, for the first round of packages that we look for in a
previous lockfile, we ignore missing packages. This allows us to more
gracefully handle comparisons between commits that add new packages.

### Testing Instructions

Added an integration test inspired by the repro from the linked issue.

Fixes #8125

---------

Co-authored-by: Greg Soltis <Greg Soltis>
Neosoulink pushed a commit to Neosoulink/turbo that referenced this issue Jun 14, 2024
)

### Description

When comparing lockfiles between revisions to identify what packages
have been affected, we treated missing packages in the previous lockfile
(added in the new lockfile) as errors, and fell back to considering all
packages as affected. This change adds a boolean to determine what the
behavior should be when we can't find a package that we expect. When
building the package graph, it continues to be an error, as well as when
tracing the dependencies of a package we've already found in the
lockfile. However, for the first round of packages that we look for in a
previous lockfile, we ignore missing packages. This allows us to more
gracefully handle comparisons between commits that add new packages.

### Testing Instructions

Added an integration test inspired by the repro from the linked issue.

Fixes vercel#8125

---------

Co-authored-by: Greg Soltis <Greg Soltis>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working owned-by: turborepo
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants