Skip to content

Commit

Permalink
Package 'positron-proxy' and 'positron-duckdb' dependencies (#5756)
Browse files Browse the repository at this point in the history
This change fixes the Positron Proxy and Positron-DuckDB
(CSV/TSV/Parquet viewer) extension bundling in release builds. An
upstream change caused dependencies of built-in extensions to be
ignored; this is mostly fine but a couple of our built-in extensions
have third-party dependencies that need to be processed.

The fix is to use the NPM dependency detection strategy on these
specific extensions. We can't just use this strategy on _all_ extensions
since it does not work on extensions that have dependencies that are
externally supplied (which is the case for several Microsoft and
Positron extensions).

Additional fixes:
- Fixes loading the `help.html` file in the Help pane in release builds,
and makes the error visible if we can't.
- Serializes Webpack extension building to avoid running out of file
handles on Windows (`EMFILE`).

Addresses #5742
Addresses #5743



e2e: @:data-explorer

### QA Notes

This problem only reproduces in release builds.
  • Loading branch information
jmcphers authored Dec 17, 2024
1 parent ae95182 commit 135ec72
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 20 deletions.
5 changes: 5 additions & 0 deletions build/gulpfile.vscode.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ const vscodeResourceIncludes = [
// Process Explorer
'out-build/vs/code/electron-sandbox/processExplorer/processExplorer.html',

// --- Start Positron ---
// Positron Help
'out-build/vs/workbench/contrib/positronHelp/browser/resources/help.html',
// --- End Positron ---

// Tree Sitter highlights
'out-build/vs/editor/common/languages/highlights/*.scm',

Expand Down
5 changes: 0 additions & 5 deletions build/gulpfile.vscode.web.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,6 @@ const vscodeWebResourceIncludes = [
// Webview
'out-build/vs/workbench/contrib/webview/browser/pre/*.{js,html}',

// --- Start Positron ---
// Positron Help
'out-build/vs/workbench/contrib/positronHelp/browser/resources/help.html',
// --- End Positron ---

// Tree Sitter highlights
'out-build/vs/editor/common/languages/highlights/*.scm',

Expand Down
42 changes: 37 additions & 5 deletions build/lib/extensions.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

56 changes: 47 additions & 9 deletions build/lib/extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,26 @@ function fromLocalWebpack(extensionPath: string, webpackConfigFileName: string,
// as a temporary workaround.

// --- Start Positron ---

// As noted above in the TODO, the upstream strategy is currently to ignore
// external dependencies, and some built-in extensions (e.g. git) do not
// package correctly with the Npm strategy. However, several Positron
// extensions have npm dependencies that need to be packaged. This list is
// used to determine which extensions should be packaged with the Npm
// strategy.
const extensionsWithNpmDeps = [
'positron-proxy',
'positron-duckdb'
];

// If the extension has npm dependencies, use the Npm package manager
// dependency strategy.
const packageManger = extensionsWithNpmDeps.includes(packageJsonConfig.name) ?
vsce.PackageManager.Npm :
vsce.PackageManager.None;

// Replace vsce.listFiles with listExtensionFiles to queue the work
listExtensionFiles({ cwd: extensionPath, packageManager: vsce.PackageManager.None, packagedDependencies }).then(fileNames => {
listExtensionFiles({ cwd: extensionPath, packageManager: packageManger, packagedDependencies }).then(fileNames => {
const files = fileNames
.map(fileName => path.join(extensionPath, fileName))
.map(filePath => new File({
Expand Down Expand Up @@ -409,14 +427,34 @@ export function packageLocalExtensionsStream(forWeb: boolean, disableMangle: boo
.filter(({ name }) => builtInExtensions.every(b => b.name !== name))
.filter(({ manifestPath }) => (forWeb ? isWebExtension(require(manifestPath)) : true))
);
const localExtensionsStream = minifyExtensionResources(
es.merge(
...localExtensionsDescriptions.map(extension => {
return fromLocal(extension.path, forWeb, disableMangle)
.pipe(rename(p => p.dirname = `extensions/${extension.name}/${p.dirname}`));
})
)
);

// --- Start Positron ---

// Process the local extensions serially to avoid running out of file
// descriptors (EMFILE) when building.

const localExtensionsStream = es.through();
const queue = [...localExtensionsDescriptions];

function processNext() {
if (queue.length === 0) {
localExtensionsStream.end();
return;
}

const extension = queue.shift();
if (!extension) {
return;
}
const stream = fromLocal(extension.path, forWeb, disableMangle)
.pipe(rename(p => p.dirname = `extensions/${extension.name}/${p.dirname}`))
.pipe(es.through(undefined, processNext));

stream.pipe(localExtensionsStream, { end: false });
}

processNext();
// --- End Positron ---

let result: Stream;
if (forWeb) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,14 @@ class PositronHelpService extends Disposable implements IPositronHelpService {

// Load the help HTML file.
this._fileService.readFile(FileAccess.asFileUri(HELP_HTML_FILE_PATH))
.then(fileContent => this._helpHTML = fileContent.value.toString());
.then(fileContent => {
// Set the help HTML to the file's contents.
this._helpHTML = fileContent.value.toString();
}).catch(error => {
// Set the help HTML to an error message. This will be
// displayed in the Help pane.
this._helpHTML = `<!DOCTYPE html><html><body><h1>Error Loading Help</h1><p>Cannot read ${HELP_HTML_FILE_PATH}:</p><p>${error}</body></html>`;
});

// Register onDidColorThemeChange handler.
this._register(this._themeService.onDidColorThemeChange(async _colorTheme => {
Expand Down

0 comments on commit 135ec72

Please sign in to comment.