Skip to content
New issue

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

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

Already on GitHub? # to your account

Perform download after resolving version #11201

Conversation

colin-grant-work
Copy link
Contributor

What it does

This PR adjusts the order in which operations are performed when downloading plugins. The resolution of version information is now followed by the download step for each plugin. Previously all dependency versions would be resolved before any downloads were executed. The parallel or no-parallel option is respected for all network calls.

How to test

  1. Run yarn download:plugins and yarn download:plugins --no-parallel. Observe that logs are as expected.

Review checklist

Reminder for reviewers

@colin-grant-work colin-grant-work added the plug-in system issues related to the plug-in system label May 25, 2022
@colin-grant-work colin-grant-work force-pushed the cleanup/resolve-and-download branch from b17989c to 4bc52b0 Compare May 25, 2022 17:50
@vince-fugnitto vince-fugnitto added the builtins Issues related to VS Code builtin extensions label May 25, 2022
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

I can confirm that the changes work as expected, both --parallel (default) and no-parallel work well.

I also confirm that:

  • plugins are correctly downloaded
  • packs are correctly resolved
  • dependencies are correctly resolved
  • plugins are correctly skipped if previously downloaded when attempting to resolve packs or dependencies
  • the exclusion list is respected for all steps

@ccorn
Copy link

ccorn commented May 25, 2022

This certainly looks good. I'd like to try this with a slow connection, but at my current place, connectivity is fast. It might take two days before I get back, so I cannot carry out the tough test for now. Please do proceed.

@colin-grant-work colin-grant-work merged commit 9a10b6b into eclipse-theia:master May 25, 2022
@ccorn
Copy link

ccorn commented May 25, 2022

For quick testing, I have merged

and I can confirm that things work as they should, producing identical contents of the plugins folder for --parallel and --no-parallel.

Note that my output includes the message

- ms-vscode.references-view: already downloaded - skipping

which should be thanks to #11113. Otherwise, I suppose that there would have been two downloads of that extension under different plugins subfolder names, with different versions, resulting in 77 sub-folders instead of 76.

yarn download:plugins durations with a 50-Mbps connection:

Option Duration
--no-parallel 241s
--parallel 45s

I will try it with a slow connection in a few days.
Thanks for the awesome work!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
builtins Issues related to VS Code builtin extensions plug-in system issues related to the plug-in system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants