fix: add missing package.json and source maps #5040
Conversation
1adcedf to
ecdbc78
Compare
Instead of copying and then deleting them. This will also catch some node_modules directories that were missed.
Code packages all the dependencies using webpack for each extension so there are no dependencies to install.
I also moved this to its own patch because it feels sufficiently standalone. Fixes coder#5026.
The base is slightly different so it needed to be refreshed.
This was caused by switching to Code's package step which does not include the package.json. Fixes coder#5019.
It seems this actually is used now.
ecdbc78 to
4ae5e26
Compare
Codecov Report
@@ Coverage Diff @@
## main #5040 +/- ##
=======================================
Coverage 71.30% 71.30%
=======================================
Files 30 30
Lines 1683 1683
Branches 373 373
=======================================
Hits 1200 1200
Misses 413 413
Partials 70 70 Continue to review full report at Codecov.
|
| rsync -avh --exclude .gitignore --exclude /node ./lib/vscode-reh-web-*/ "$VSCODE_OUT_PATH" | ||
| local rsync_opts=() | ||
| if [[ ${DEBUG-} = 1 ]]; then | ||
| rsync_opts+=(-vh) |
There was a problem hiding this comment.
What does -vh stand for here?
There was a problem hiding this comment.
Ah:
-h: -h, --human-readable output numbers in a human-readable format-v: -v, --verbose increase verbosity
| fi | ||
|
|
||
| # Some extensions have a .gitignore which excludes their built source from the | ||
| # npm package so exclude any .gitignore files. | ||
| rsync_opts+=(--exclude .gitignore) | ||
|
|
||
| # Exclude Node as we will add it ourselves for the standalone and will not | ||
| # need it for the npm package. | ||
| rsync_opts+=(--exclude /node) | ||
|
|
||
| # Exclude Node modules. | ||
| if [[ $KEEP_MODULES = 0 ]]; then | ||
| rsync_opts+=(--exclude node_modules) | ||
| fi | ||
|
|
||
| rsync "${rsync_opts[@]}" ./lib/vscode-reh-web-*/ "$VSCODE_OUT_PATH" |
There was a problem hiding this comment.
This approach looks a lot cleaner 👏🏼
|
|
||
| for ext in */; do | ||
| ext="${ext%/}" | ||
| echo "extensions/$ext: installing dependencies" | ||
| cd "$ext" | ||
| yarn --production --frozen-lockfile | ||
| cd "$OLDPWD" | ||
| done |
There was a problem hiding this comment.
Hopefully I am not wrong about this removal hahaha
There was a problem hiding this comment.
Before the next release we might want to run through all the builtin extensions to see if we have any more issues with them
There was a problem hiding this comment.
Good idea. I'll add a note to the release issue
| @@ -79,7 +79,7 @@ Index: code-server/lib/vscode/src/vs/code/browser/workbench/workbench.html | |||
| + } catch (error) { /* Probably fine. */ } | |||
| Object.keys(self.webPackagePaths).map(function (key, index) { | |||
| self.webPackagePaths[key] = new URL( | |||
| `{{VS_BASE}}/static/remote/web/node_modules/${key}/${self.webPackagePaths[key]}`, | |||
| `{{VS_BASE}}/static/node_modules/${key}/${self.webPackagePaths[key]}`, | |||
There was a problem hiding this comment.
Does this need to be changed in my upstream PR?
There was a problem hiding this comment.
Nope, this is not part of the changes this patch applies, it is just part of the surrounding context. So this change is from an earlier patch.
| # Include global extension dependencies as well. | ||
| rsync "$VSCODE_SRC_PATH/extensions/package.json" "$VSCODE_OUT_PATH/extensions/package.json" | ||
| rsync "$VSCODE_SRC_PATH/extensions/yarn.lock" "$VSCODE_OUT_PATH/extensions/yarn.lock" | ||
| rsync "$VSCODE_SRC_PATH/extensions/postinstall.js" "$VSCODE_OUT_PATH/extensions/postinstall.js" | ||
|
|
| # version though so pull that from the main package.json. | ||
| jq --slurp '.[0] * {version: .[1].version}' \ |
There was a problem hiding this comment.
Nice catch! How'd you figure this out? That log message when code-server starts up? IIRC
There was a problem hiding this comment.
Yup hahaha, no idea how I did not see it before
There was a problem hiding this comment.
Fortunately we still have libsecret as a dependency (which keytar needs) in our docs (as far as I could tell, hopefully I did not miss a spot) so nothing else changes
* Use --exclude to skip node_modules Instead of copying and then deleting them. This will also catch some node_modules directories that were missed. * Remove per-extension dependency install Code packages all the dependencies using webpack for each extension so there are no dependencies to install. * Include source maps I also moved this to its own patch because it feels sufficiently standalone. Fixes coder#5026. * Refresh language patch The base is slightly different so it needed to be refreshed. * Add missing package.json This was caused by switching to Code's package step which does not include the package.json. Fixes coder#5019. * Include keytar It seems this actually is used now.
These issues were caused when I switched Code's packaged build instead of putting it together ourselves.
See individual commits for more information.