Skip to content

Commit

Permalink
Copy extension binaries during main vscode task (#6392)
Browse files Browse the repository at this point in the history
This change addresses an issue in which the `tree-sitter.wasm` file does
not load in release builds, which causes R testing to fail.

This problem is a regression from the 1.96 merge. The problem was that
we were copying the binaries as part of the build task
`compileExtensionsBuildTask`.


https://github.com/posit-dev/positron/blob/4ea5c56a3d2b23ddf08bee7a237ff48c6e2e0f9c/build/gulpfile.vscode.js#L617-L623

After 1.96, this build task is no longer invoked as part of the main
vscode task; it was replaced with `compileNonNativeExtensionsBuildTask`.


https://github.com/posit-dev/positron/blob/2d4492c35638ea01e611dec599e9d6bbd60e9df8/build/gulpfile.vscode.js#L603-L610

This change did not generate a merge conflict since there are no
Positron edits to this list.

The fix is to add the `copyExtensionBinariesTask` to the main vscode
task in a Positron code fence, to reduce the odds of this situation
happening again.

Addresses #6345.

### Reusing VS Code's tree-sitter-wasm

As a side note, the first thing I tried was using VS Code's
`tree-sitter-wasm` instead of continuing to bundle a redundant copy in
`positron-r`. This turned out to be more of a mess than it was worth.
Specifically:

- it is possible to cause VS Code's `tree-sitter.wasm` file to load
using the [locateFile option in
Parser.init](https://github.com/tree-sitter/tree-sitter/blob/b26b7f8d62d8508a675ab6f0fa34b628a3b96b31/lib/binding_web/README.md?plain=1#L224),
but it is not compatible with the JavaScript side of the module
- it is also not possible (or at least not easy) to wholly switch from
`web-tree-sitter` => `@vscode/tree-sitter-wasm` due to API differences

### Release Notes

<!--
Optionally, replace `N/A` with text to be included in the next release
notes.
The `N/A` bullets are ignored. If you refer to one or more Positron
issues,
these issues are used to collect information about the feature or
bugfix, such
as the relevant language pack as determined by Github labels of type
`lang: `.
  The note will automatically be tagged with the language.

These notes are typically filled by the Positron team. If you are an
external
  contributor, you may ignore this section.
-->

#### New Features

- N/A

#### Bug Fixes

- Restore R package testing functionality
(#6345)


### QA Notes

I'm surprised this problem is not causing more failures! This fix isn't
really specific to R or testing UI. If you see any issues post 1.96 that
appear to be due to a missing binary in an extension, this is a likely
cause.
  • Loading branch information
jmcphers authored Feb 19, 2025
1 parent 92dec46 commit 44ee79e
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 2 deletions.
5 changes: 4 additions & 1 deletion build/gulpfile.reh.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const File = require('vinyl');
const fs = require('fs');
const glob = require('glob');
const { compileBuildTask } = require('./gulpfile.compile');
const { cleanExtensionsBuildTask, compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileExtensionMediaBuildTask } = require('./gulpfile.extensions');
const { cleanExtensionsBuildTask, compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileExtensionMediaBuildTask, copyExtensionBinariesTask } = require('./gulpfile.extensions');
// --- Start Positron ---
const { vscodeWebEntryPoints, vscodeWebResourceIncludes, createVSCodeWebFileContentMapper } = require('./gulpfile.vscode.web');
const { positronBuildNumber } = require('./utils');
Expand Down Expand Up @@ -550,6 +550,9 @@ function tweakProductForServerWeb(product) {
cleanExtensionsBuildTask,
compileNonNativeExtensionsBuildTask,
compileExtensionMediaBuildTask,
// --- Start Positron ---
copyExtensionBinariesTask,
// --- End Positron ---
minified ? minifyTask : bundleTask,
serverTaskCI
));
Expand Down
5 changes: 4 additions & 1 deletion build/gulpfile.vscode.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const { config } = require('./lib/electron');
const createAsar = require('./lib/asar').createAsar;
const minimist = require('minimist');
const { compileBuildTask } = require('./gulpfile.compile');
const { compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileAllExtensionsBuildTask, compileExtensionMediaBuildTask, cleanExtensionsBuildTask } = require('./gulpfile.extensions');
const { compileNonNativeExtensionsBuildTask, compileNativeExtensionsBuildTask, compileAllExtensionsBuildTask, compileExtensionMediaBuildTask, cleanExtensionsBuildTask, copyExtensionBinariesTask } = require('./gulpfile.extensions');
const { promisify } = require('util');
const glob = promisify(require('glob'));
const rcedit = promisify(require('rcedit'));
Expand Down Expand Up @@ -605,6 +605,9 @@ BUILD_TARGETS.forEach(buildTarget => {
cleanExtensionsBuildTask,
compileNonNativeExtensionsBuildTask,
compileExtensionMediaBuildTask,
// --- Start Positron ---
copyExtensionBinariesTask,
// --- End Positron ---
minified ? minifyVSCodeTask : bundleVSCodeTask,
vscodeTaskCI
));
Expand Down

0 comments on commit 44ee79e

Please sign in to comment.