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

Add a command to list installed plugins. #12818

Merged
merged 1 commit into from
Nov 24, 2023

Conversation

tsmaeder
Copy link
Contributor

@tsmaeder tsmaeder commented Aug 9, 2023

Contributed on behalf of STMicroelectronics

What it does

Allows cli-contributions to add commands to the yargs instance as well as options. The change required a couple of adjustments, for example:

  • the mock vsx server BackendApplicatinContribution relied on the back-end app to be started in order to finish it's configuration
  • The BackendApplicatinContribution init and configure methods are now called in parallel for all contributions instead of sequentially.

The new command is called list-plugins with the alias list-extensions. yargs does not allow for commands that start with --, so we can't do it just the same as in VS Code. There are two options for the new command:

  • --show-builtins shows the built-ins instead of installed plugins
  • --show-versions shows the versions that are installed.

Fixes #12298

How to test

Make sure the --list-plugins works as described above with various other options like --no-cluster and via yarn as well as npx theia, or when debugging the browser and electron back ends.

Review checklist

Reminder for reviewers

@tsmaeder tsmaeder requested a review from msujew August 16, 2023 14:03
Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I'm getting some pretty weird behavior when running list-plugins:

/workspace/theia/examples/browser $ yarn theia start list-plugins
yarn run v1.22.19
$ /workspace/theia/node_modules/.bin/theia start list-plugins
Configuration directory URI: 'file:///home/gitpod/.theia'
Configuring to accept webviews on '^webview-.+$' hostname.
2023-08-17T15:16:54.089Z root WARN WEBVIEW SECURITY WARNING

    Changing the @theia/plugin-ext webview host pattern can lead to security vulnerabilities.
        Current pattern: "webview-{{hostname}}"
    Please read @theia/plugin-ext/README.md for more information.

2023-08-17T15:16:54.089Z root WARN MINI BROWSER SECURITY WARNING

    Changing the @theia/mini-browser host pattern can lead to security vulnerabilities.
        Current pattern: "browser-{{hostname}}"
    Please read @theia/mini-browser/README.md for more information.

2023-08-17T15:16:54.089Z root INFO Backend Object.initialize: 22.7 ms [Finished 0.452 s after backend start]
2023-08-17T15:16:54.090Z root INFO Backend WebviewBackendSecurityWarnings.initialize: 0.9 ms [Finished 0.452 s after backend start]
2023-08-17T15:16:54.090Z root INFO Backend HostedPluginLocalizationService.initialize: 1.0 ms [Finished 0.452 s after backend start]
2023-08-17T15:16:54.090Z root INFO Backend MiniBrowserBackendSecurityWarnings.initialize: 0.9 ms [Finished 0.452 s after backend start]
2023-08-17T15:16:54.090Z root INFO Backend HostedPluginReader.initialize: 0.8 ms [Finished 0.452 s after backend start]
2023-08-17T15:16:54.091Z root INFO Backend PluginLocalizationBackendContribution.initialize: 24.3 ms [Finished 0.454 s after backend start]
2023-08-17T15:16:54.092Z root WARN The local plugin referenced by local-dir:/home/gitpod/.theia/plugins does not exist.
2023-08-17T15:16:54.092Z root WARN The local plugin referenced by local-dir:/home/gitpod/.theia/extensions does not exist.
2023-08-17T15:16:54.092Z root INFO Resolve plugins list: 2.2 ms [Finished 0.457 s after backend start]
2023-08-17T15:16:54.098Z root INFO Deploy plugins list: 2.2 ms [Finished 0.457 s after backend start]
2023-08-17T15:16:54.098Z root INFO Backend PluginDeployerContribution.initialize: 6.9 ms [Finished 0.457 s after backend start]
installed plugins:
2023-08-17T15:16:54.099Z root INFO configured all backend app contributions
2023-08-17T15:16:54.129Z root INFO >>> Stopping backend contributions...
2023-08-17T15:16:54.130Z root INFO <<< All backend contributions have been stopped.
2023-08-17T15:16:54.130Z root ERROR Error: kill ESRCH
    at process.kill (node:internal/process/per_thread:220:13)
    at ProcessUtils.unixTerminateProcessTree (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:2174:29)
    at ProcessUtils.terminateProcessTree (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:2162:18)
    at BackendApplication.onStop (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:1556:27)
    at process.<anonymous> (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:1437:39)
    at process.emit (node:events:520:28)

It seems to use the list-plugins routine, since there's a installed plugins log in there, but everything else seems to be wrong/unexpected.

packages/core/src/node/cli.ts Show resolved Hide resolved
@tsmaeder
Copy link
Contributor Author

I'm getting some pretty weird behavior when running list-plugins:

...

It seems to use the list-plugins routine, since there's a installed plugins log in there, but everything else seems to be wrong/unexpected.

We're logging at INFO level per default to the console. You can remove the noise by adding --log-level=error. That we're logging to the console that (and how much) is out of scope for this PR. Appearently, you have not plugin installed (I suspect the list of built-ins is different).

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

We're logging at INFO level per default to the console. You can remove the noise by adding --log-level=error. That we're logging to the console that (and how much) is out of scope for this PR. Appearently, you have not plugin installed (I suspect the list of built-ins is different).

Yeah, I'm not concerned about the output of the actual command. Rather all the noise. However, even with --log-level=error I'm getting:

2023-09-12T09:53:56.467Z root ERROR Error: kill ESRCH
    at process.kill (node:internal/process/per_thread:220:13)
    at ProcessUtils.unixTerminateProcessTree (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:2174:29)
    at ProcessUtils.terminateProcessTree (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:2162:18)
    at BackendApplication.onStop (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:1556:27)
    at process.<anonymous> (/workspace/theia/examples/browser/lib/backend/node_modules_express_lib_sync_recursive-node_modules_require-main-filename_sync_recursive-pac-c83c5f.js:1437:39)
    at process.emit (node:events:520:28)

Would it make sense to redirect the console.log/warn/error/info using empty method implementations? You're already using process.stdout which isn't affected by such a change. It's very counter intuitive imo that we get all the regular server output when executing such a command.

Additionally, the theia start list-plugins --help output is pretty confusing:

List the installed plugins

Options:
  --version                          Show version number               [boolean]
  --port, -p                         The port the backend server listens on.
                                                        [number] [default: 3000]
  --hostname, -h                     The allowed hostname for connections.
                                                 [string] [default: "localhost"]
  --ssl                              Use SSL (HTTPS), cert and certkey must also
                                     be set           [boolean] [default: false]
  --cert                             Path to SSL certificate.           [string]
  --certkey                          Path to SSL certificate key.       [string]
  --app-project-path                 Sets the application project directory
                                  [default: "/workspace/theia/examples/browser"]
  --proxy-url                        Sets the proxy URL for outgoing requests.
                                                                        [string]
  --proxy-authorization              Sets the proxy authorization header for
                                     outgoing requests.                 [string]
  --strict-ssl                       Determines whether SSL is strictly set for
                                     outgoing requests.                [boolean]
  --log-level                        Sets the default log level
                   [choices: "fatal", "error", "warn", "info", "debug", "trace"]
  --log-config                       Path to the JSON file specifying the
                                     configuration of various loggers   [string]
  --root-dir                         DEPRECATED: Sets the workspace directory.
  --plugins                          Provides further refinement for the
                                     plugins. Example:
                                     --plugins=local-dir:path/to/your/plugins
                                                                        [string]
  --plugin-max-session-logs-folders  The maximum number of plugin logs sessions
                                     folders to retain. Example:
                                     --plugin-max-session-logs-folders=5
                                                          [number] [default: 10]
  --uncompressed-plugins-in-place    Whether user plugins that are stored on
                                     disk as uncompressed directories should be
                                     run in place or copied to temporary folder.
                                                      [boolean] [default: false]
  --extensionTestsPath                                                  [string]
  --pluginHostTerminateTimeout       Timeout in milliseconds to wait for the
                                     plugin host process to terminate before
                                     killing it. Use 0 for no timeout.
                                                       [number] [default: 10000]
  --pluginHostStopTimeout            Timeout in milliseconds to wait for the
                                     plugin host process to stop internal
                                     services. Use 0 for no timeout.
                                                        [number] [default: 4000]
  --vscode-api-version               Overrides the version returned by VSCode
                                     API 'vscode.version'. Example:
                                     --vscode-api-version=<Wanted Version>.
                                     Default [1.79.0]                   [string]
  --ovsx-router-config               JSON configuration file for the OVSX router
                                     client                             [string]
  --help                             Show help                         [boolean]
  ----show-versions                  List the versions of the installed plugins
                                                      [boolean] [default: false]
  ----show-builtins                  List the built-in plugins
                                                      [boolean] [default: false]

Note the ----show-versions and ----show-builtins parameters. Any idea whether you can do something about that?

@tsmaeder
Copy link
Contributor Author

@msujew I don't think silencing log output is a good idea: if something goes wrong, we need the output to diagnose the problem. If we don't want user visible log output, we need to redirect it somewhere else (a file, most likely), but that is a more general concern that is not in scope for this PR, IMO.
In related news, there is #12234

@tsmaeder
Copy link
Contributor Author

@msujew ping?

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@msujew
Copy link
Member

msujew commented Nov 21, 2023

@tsmaeder I'll take another look at this during the week.

Copy link
Member

@msujew msujew left a comment

Choose a reason for hiding this comment

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

I still think the default output (i.e. the whole contribution lifecycle logging) is kind of weird, but I guess it's still a useful feature.

The changes work as intended and the code looks good, so LGTM 👍

@tsmaeder tsmaeder merged commit 6004188 into eclipse-theia:master Nov 24, 2023
12 of 13 checks passed
@vince-fugnitto vince-fugnitto added this to the 1.44.0 milestone Nov 30, 2023
marcdumais-work added a commit to eclipse-cdt-cloud/theia-trace-extension that referenced this pull request Feb 28, 2024
There were some recent Theia changes[1] that required to change the way our
Theia extension starts. It used to be that our TraceServerUrlProviderImpl
backend contribution could wait in the "initialize" Theia app life-cucle
phase, until the application was started and it received the trace server
port from the front-end, from our PreferencesFrontendContribution.

But the above no longer works. Now backend contributions are started in
parallel, and all are expected to finish their initialize phase, before the
"start" phase is initiated.

In practice, after upgrading to Theia v1.45.1, the Theia application, that
includes the trace viewer extension,  would be killed during startup, with
no clear indication why.

To prevent this from happening, the need to receive information from the
front-end has been removed: the backend contribution will now initialize
with the assumption that the default trace server port is used. If a
different port was configured in the preferences, the information will be
synched after the app has started.

[1]: eclipse-theia/theia#12818

specifically:

"The BackendApplicatinContribution init and configure methods are now called
in parallel for all contributions instead of sequentially."

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit to eclipse-cdt-cloud/theia-trace-extension that referenced this pull request Feb 28, 2024
There were some recent Theia changes[1] that required to change the way our
Theia extension starts. It used to be that our TraceServerUrlProviderImpl
backend contribution could wait in the "initialize" Theia app life-cycle
phase, until the application was started and it received the trace server
port from the front-end, from our PreferencesFrontendContribution.

But the above no longer works. Now backend contributions are started in
parallel, and all are expected to finish their initialize phase, before the
"start" phase is initiated.

In practice, after upgrading to Theia v1.45.1, the Theia application, that
includes the trace viewer extension,  would be killed during startup, with
no clear indication why.

To prevent this from happening, the need to receive information from the
front-end, during initialization, has been removed: the backend contribution
now initializes with the assumption that the default trace server port is used.
If a different port is configured in the preferences, the information will be
synched after the app has started.

[1]: eclipse-theia/theia#12818

specifically:

"The BackendApplicatinContribution init and configure methods are now called
in parallel for all contributions instead of sequentially."

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit to eclipse-cdt-cloud/theia-trace-extension that referenced this pull request Feb 28, 2024
There were some recent Theia changes[1] that required to change the way our
Theia extension starts. It used to be that our TraceServerUrlProviderImpl
backend contribution could wait in the "initialize" Theia app life-cycle
phase, until the application was started and it received the trace server
port from the front-end, from our PreferencesFrontendContribution.

But the above no longer works. Now backend contributions are started in
parallel, and all are expected to finish their initialize phase, before the
"start" phase is initiated.

In practice, after upgrading to Theia v1.45.1, the Theia application, that
includes the trace viewer extension,  would be killed during startup, with
no clear indication why.

To prevent this from happening, the need to receive information from the
front-end, during initialization, has been removed: the backend contribution
now initializes with the assumption that the default trace server port is used.
If a different port is configured in the preferences, the information will be
synched after the app has started.

[1]: eclipse-theia/theia#12818

specifically:

"The BackendApplicatinContribution init and configure methods are now called
in parallel for all contributions instead of sequentially."

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
marcdumais-work added a commit to eclipse-cdt-cloud/theia-trace-extension that referenced this pull request Feb 29, 2024
There were some recent Theia changes[1] that required to change the way our
Theia extension starts. It used to be that our TraceServerUrlProviderImpl
backend contribution could wait in the "initialize" Theia app life-cycle
phase, until the application was started and it received the trace server
port from the front-end, from our PreferencesFrontendContribution.

But the above no longer works. Now backend contributions are started in
parallel, and all are expected to finish their initialize phase, before the
"start" phase is initiated.

In practice, after upgrading to Theia v1.45.1, the Theia application, that
includes the trace viewer extension,  would be killed during startup, with
no clear indication why.

To prevent this from happening, the need to receive information from the
front-end, during initialization, has been removed: the backend contribution
now initializes with the assumption that the default trace server port is used.
If a different port is configured in the preferences, the information will be
synched after the app has started.

[1]: eclipse-theia/theia#12818

specifically:

"The BackendApplicatinContribution init and configure methods are now called
in parallel for all contributions instead of sequentially."

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Additional parameters for Theia CLI --list-extensions
3 participants