Skip to content

Increase project search depth, allow it to be cancelled, cache/reuse results #3974

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

Closed
guidezpl opened this issue May 19, 2022 · 10 comments
Closed

Comments

@guidezpl
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Use case: I clone a repo containing plenty of puspecs, some nested. Tons of analysis errors are surfaced. e.g. flutter/samples

Describe the solution you'd like
Clicking the button in the "Some packages are outdated. Run Flutter pub get" notification does pub get recursively, rather than at the top level and for example directories.

Screenshot 2022-05-19 at 19 03 50

Describe alternatives you've considered
The alternative are various, but it'd be nice if the extension did it without me thinking.

@DanTup
Copy link
Member

DanTup commented May 23, 2022

Describe the solution you'd like
Clicking the button in the "Some packages are outdated. Run Flutter pub get" notification does pub get recursively, rather than at the top level and for example directories.

This is what the current behaviour should be. Is it not what you see? Although the notification is bad and doesn't include the project name (I'll fix that), if I run Dart: Capture Logs and then accept the prompt to fetch, the logs include this:

[1:59:07 PM] [CommandProcesses] [Info] (PROC 7866) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/isolate_example
[1:59:09 PM] [CommandProcesses] [Info] (PROC 7906) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/jsonexample
[1:59:12 PM] [CommandProcesses] [Info] (PROC 7948) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/material_3_demo
[1:59:13 PM] [CommandProcesses] [Info] (PROC 7981) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/navigation_and_routing
[1:59:17 PM] [CommandProcesses] [Info] (PROC 8024) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/place_tracker
[1:59:20 PM] [CommandProcesses] [Info] (PROC 8064) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/platform_channels
[1:59:22 PM] [CommandProcesses] [Info] (PROC 8096) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/platform_design
[1:59:24 PM] [CommandProcesses] [Info] (PROC 8130) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/platform_view_swift
[1:59:25 PM] [CommandProcesses] [Info] (PROC 8162) Spawned /Users/danny/Dev/Google/flutter/bin/flutter pub get in /Users/danny/Dev/Google/flutter-samples/provider_counter
...

Is this different to what you're seeing? (note: it runs them sequentially, so it may take a while to get through all of flutter/samples, but it will only run for those that need it running).

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label May 23, 2022
@DanTup
Copy link
Member

DanTup commented May 23, 2022

Oh, after letting things finish I still had some that hadn't been done. The reason for this is that we only search to a certain depth for projects, and these were more deeply nested. The depth can be configured with this setting:

"dart.projectSearchDepth": 5 // default is 3

Setting it to 5 and then restarted VS Code and accepting the prompt caused it to fetch all.

Improving the default value has performance implications when opening large workspaces. There are some planned improvements at #3856 but it'll only help for projects where you've opened a file.

I don't know of any signals we could use to figure out there may be more projects without having to walk the filesystem though.

@guidezpl
Copy link
Contributor Author

Oh, after letting things finish I still had some that hadn't been done. The reason for this is that we only search to a certain depth for projects, and these were more deeply nested.

Ah I see. I would collect all projects with find . -name "pubspec.yaml". Because the analyzer is going to go and analyze the whole project regardless of depth, and surface hundreds if not thousands of errors due to unfetched packages.

I wouldn't have an issue if it did all the fetching if there was a single notification that used something like VSCode's ProgressLocation.

@DanTup
Copy link
Member

DanTup commented May 23, 2022

I would collect all projects with find . -name "pubspec.yaml"

We'd need to use something that works across all OSes. VS Code does have a find API, but when I tested it it was even slower than our custom walking. Sometimes people open huge folders (like their home directory) and have huge node_modules folders, so it can take some time.

I wouldn't have an issue if it did all the fetching if there was a single notification that used something like VSCode's ProgressLocation.

I like this idea. If we showed a progress bar if the search took more than x ms with the ability to cancel, that might make it more reasonable to increase the default. We do search for projects more frequently than just at startup, so we might need to decide whether to increase it everywhere (and potentially trigger this), or cache deeper projects found at startup and use them in addition to things we find in a shallow search.

I'll have a play around and see how it feels. Thanks!

@DanTup DanTup removed the awaiting info Requires more information from the customer to progress label May 23, 2022
@DanTup DanTup added this to the v3.44.0 milestone May 23, 2022
@DanTup DanTup closed this as completed in fb400a1 Jun 20, 2022
@DanTup
Copy link
Member

DanTup commented Jun 20, 2022

I've made some changes:

  • Workspace search results are cached for a few minutes
  • If a search is in-progress when something else tries to start one, it will wait for the existing results instead
  • After 2s a progress indicator will be shown saying "Searching for projects". Clicking Cancel will terminate the search and use the projects found so far as the results
  • Increased the default from 3 levels to 5

@DanTup DanTup changed the title Extend package detection to subdirectories Increase project search depth, allow it to be cancelled, cache/reuse results Jun 20, 2022
@DanTup
Copy link
Member

DanTup commented Jun 21, 2022

If you want to try this out before it ships, you can switch to the pre-release versions of the Dart+Flutter extension in VS Code shown here:

Dart/Flutter pre-release versions

The versions with the fix are v3.43.20220621 (and the stable version they'll ship in is v3.44).

Please let me know if you hit any issues. Thanks!

@guidezpl
Copy link
Contributor Author

Thanks, it worked as expected!

@DanTup
Copy link
Member

DanTup commented Jun 21, 2022

Great! Out of interest, did you see "Searching for projects" or was it fast enough to not show up? If you use Dart: Open Extension Log it should log the timings for the project search that might be interesting for me to keep an eye on.

@guidezpl
Copy link
Contributor Author

Yeah, it didn't show up

[3:17:20 PM] [General] [Info] Took 409ms to search for projects (5 levels)
[3:17:20 PM] [General] [Info] Checking 44 projects for supported platforms
[3:17:20 PM] [General] [Info] Supported platforms for the workspace are android, linux, macos, windows, ios, web
[3:24:22 PM] [General] [Info] Took 123ms to search for projects (5 levels)

@DanTup
Copy link
Member

DanTup commented Jun 21, 2022

Cool, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants