Switch to a simpler module for reading from registry #2410
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The @vscode/windows-registry module is maintained by Microsoft and is described like so: This module only has what is needed to support VS Code and is intended to be a lightweight module. Seems like a great fit for our purposes.
jmcphers
added a commit
that referenced
this pull request
Mar 11, 2024
Revert "Switch to a simpler module for reading from registry (#2410)"
|
Storing more backstory from slack re: why we reverted this. It caused failures in CI: @jmcphers was able to reproduce the webpack-stream error locally, but only with very specific moves:
More of the convo, in case we ever want to revisit this, i.e. because we really do want to switch to the I said:
@jmcphers said:
I noted that this PR in vscode might be relevant: microsoft/vscode#184741 |
jennybc
added a commit
that referenced
this pull request
Sep 26, 2024
Addresses #3787. See the issue for why I want to use `@vscode/windows-registry`. This PR reinstates my original, eventually-reverted attempt at this #2410 and includes the changes necessary to successfully create a release build on all platforms. The big challenge is that [`@vscode/windows-registry`](https://github.com/microsoft/vscode-windows-registry) is a native module, i.e. it contains C++ code. This then requires special care when building the application. VS Code (and, therefore, Positron) itself handles this with specific entries in `.moduleignore` files: * In [`build/.moduleignore`](https://github.com/posit-dev/positron/blob/89a59c04d7dfdc36f1e38119db19e1731729caa5/build/.moduleignore#L53-L56). This default ignore spec excludes most of the module files, but explicitly includes the `*.node` file. This is the effective ignore spec for `@vscode/windows-registry` on Windows. * In [`build/.moduleignore.darwin`](https://github.com/posit-dev/positron/blob/main/build/.moduleignore.darwin#L12-L17). The macOS ignores exclude almost all files, but explicitly include the types. * In [`build/.moduleignore.linux`](https://github.com/posit-dev/positron/blob/main/build/.moduleignore.linux#L12-L17). Same deal as macOS. * *[`build/.moduleignore.win32`](https://github.com/posit-dev/positron/blob/main/build/.moduleignore.win32) does exist, but it is empty, as there are no additional ignores needed/wanted for native modules on Windows. The default ignores are what you want.* These ignores are then put into force in various build scripts, such as (but not limited to): [`build/gulpfile.vscode.js`](https://github.com/posit-dev/positron/blob/cf127d6c38010e753122f96d9a871eeeb189f838/build/gulpfile.vscode.js#L332-L333). At first I tried to emulate these ignores in positron-r's webpack config ([`extensions/positron-r/extension.webpack.config.js`](https://github.com/posit-dev/positron/blob/cf127d6c38010e753122f96d9a871eeeb189f838/extensions/positron-r/extension.webpack.config.js#L1)), because the immediate error you are confronted with is from `webpack-stream`. It looks like this (yeah, you get NO INFORMATION on which extension or which file is the problem, lovely): ``` [03:12:24] 'vscode-win32-x64' errored after 49 min [03:12:24] Error in plugin "webpack-stream" Message: Module parse failed: Unexpected character '�' (1:2) You may need an appropriate loader to handle this file type, currently no loaders are configured to process this file. See https://webpack.js.org/concepts#loaders (Source code omitted for this binary file) Details: domainEmitter: [object Object] domainThrown: false error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. Error: Process completed with exit code 1. ``` There are various active ways to intervene in webpack's processing, via loaders and plugins, but I never managed to achieve my goal, although presumably that approach is workable in the right hands. Instead I decided to do as little as possible and rely on the fact that that Positron itself is already handling the fussy packaging of this module and the positron-r extension can assume `@vscode/windows-registry` is already provided by the environment in which it is running. In the end two small changes are needed in extension config: * Declare `@vscode/windows-registry` as an external dependency, which keeps webpack from bundling it. * List `@vscode/windows-registry` in `peerDependencies` in `package.json` instead of `dependencies`, which keeps it from being installed into positron-r's `node_modules/` directory. ### QA Notes This PR is successful if we can create usable release builds on all OSes and continue to consult with Windows registry re: the current version of R. On Windows, assuming the current R version has been recorded in the registry (which is default behaviour), the Positron R Extension output channel should show something like this (along with lots of other keys that don't pan out, because they don't exist, which is to be expected): ``` 2024-09-25 16:35:17.653 [info] Checking for 'InstallPath' in registry key HKEY_LOCAL_MACHINE\SOFTWARE\R-core\R64 2024-09-25 16:35:17.653 [info] Identified the current version of R from the registry: C:\Program Files\R\R-4.4.1\bin\x64\R.exe ``` I have just kicked off a daily release build: https://github.com/posit-dev/positron-builds/actions/runs/11041496695.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The @vscode/windows-registry module is maintained by Microsoft and is described like so:
Seems like a great fit for our purposes. I see it used elsewhere in VS Code.
Given our simple needs at this point, this is more pleasant to work with than winreg. Not sure why I didn't find/choose it the first time.