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

Update Code to 1.94.0 #7026

Merged
merged 22 commits into from
Oct 18, 2024
Merged

Update Code to 1.94.0 #7026

merged 22 commits into from
Oct 18, 2024

Conversation

benz0li
Copy link
Contributor

@benz0li benz0li commented Oct 4, 2024

Fixes #7018


yarn -> npm

  • vscode: Seems like you are using yarn which is not supported in this repo any more, please use npm i instead.

- vscode: Seems like you are using `yarn` which is not supported in this repo any more, please use `npm i` instead.
@benz0li benz0li requested a review from a team as a code owner October 4, 2024 05:07
@benz0li
Copy link
Contributor Author

benz0li commented Oct 4, 2024

Notes:

--- lib/vscode/src/vs/platform/extensionManagement/node/extensionDownloader.ts
+++ lib/vscode/src/vs/platform/extensionManagement/node/extensionDownloader.ts
@@ -114,7 +114,10 @@ export class ExtensionsDownloader extend
 			return false;
 		}
 
+		return false
+		// @ts-expect-error
 		const value = this.configurationService.getValue('extensions.verifySignature');
+		// @ts-expect-error
 		return isBoolean(value) ? value : true;
 	}

👉 Code does not exist any more


--- lib/vscode/src/vs/workbench/contrib/remote/browser/remoteExplorer.ts
+++ lib/vscode/src/vs/workbench/contrib/remote/browser/remoteExplorer.ts
@@ -77,7 +77,7 @@ export class ForwardedPortsView extends
 	private async enableForwardedPortsView() {
 		this.contextKeyListener.clear();
 
-		const viewEnabled: boolean = !!forwardedPortsViewEnabled.getValue(this.contextKeyService);
+		const viewEnabled: boolean = true;
 
 		if (viewEnabled) {
 			const viewContainer = await this.getViewContainer();

ℹ️ Also const featuresEnabled: boolean = true;


--- lib/vscode/src/vs/workbench/workbench.web.main.ts
+++ lib/vscode/src/vs/workbench/workbench.web.main.ts
@@ -52,7 +52,7 @@ import 'vs/workbench/services/dialogs/br
 import 'vs/workbench/services/host/browser/browserHostService';
 import 'vs/workbench/services/lifecycle/browser/lifecycleService';
 import 'vs/workbench/services/clipboard/browser/clipboardService';
-import 'vs/workbench/services/localization/browser/localeService';
+import 'vs/workbench/services/localization/electron-sandbox/localeService';
 import 'vs/workbench/services/path/browser/pathService';
 import 'vs/workbench/services/themes/browser/browserHostColorSchemeService';
 import 'vs/workbench/services/encryption/browser/encryptionService';
@@ -118,8 +118,9 @@ registerSingleton(ILanguagePackService,
 // Logs
 import 'vs/workbench/contrib/logs/browser/logs.contribution';
 
-// Localization
-import 'vs/workbench/contrib/localization/browser/localization.contribution';
+// Localization.  This does not actually import anything specific to Electron so
+// it should be safe.
+import 'vs/workbench/contrib/localization/electron-sandbox/localization.contribution';
 
 // Performance
 import 'vs/workbench/contrib/performance/browser/performance.web.contribution';

👉 Code does not exist any more

@benz0li
Copy link
Contributor Author

benz0li commented Oct 4, 2024

@code-asher Please review and migrate the CI from yarn to npm.

Thank you.

Cross reference:

@benz0li
Copy link
Contributor Author

benz0li commented Oct 4, 2024

@code-asher npm run build:vscode runs into the following error:

[hh:mm:ss] Error: /home/benz0li/projects/coder/code-server/lib/vscode/src/vs/platform/telemetry/test/browser/telemetryService.test.ts(20,31): Argument of type 'sinon.SinonStatic' is not assignable to parameter of type 'import("/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/@types/sinon-test/node_modules/@types/sinon/index").SinonStatic'.
  Type 'SinonStatic' is not assignable to type 'SinonSandbox'.                                                         
    The types of 'usingPromise(...).replace' are incompatible between these types.                                     
      Property 'usingAccessor' is missing in type '<T, TKey extends keyof T, R extends T[TKey] = T[TKey]>(obj: T, prop: TKey, replacement: R) => R' but required in type 'SandboxReplace'.                                                    
[hh:mm:ss] Finished compilation with 1 errors after 3057276 ms
[hh:mm:ss] Error: /home/benz0li/projects/coder/code-server/lib/vscode/src/vs/platform/telemetry/test/browser/telemetryService.test.ts(20,31): Argument of type 'sinon.SinonStatic' is not assignable to parameter of type 'import("/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/@types/sinon-test/node_modules/@types/sinon/index").SinonStatic'.
  Type 'SinonStatic' is not assignable to type 'SinonSandbox'.                                                         
    The types of 'usingPromise(...).replace' are incompatible between these types.                                     
      Property 'usingAccessor' is missing in type '<T, TKey extends keyof T, R extends T[TKey] = T[TKey]>(obj: T, prop: TKey, replacement: R) => R' but required in type 'SandboxReplace'.                                                    
[hh:mm:ss] Finished compilation with 1 errors after 3057277 ms                                                         
[hh:mm:ss] 'vscode-reh-web-linux-x64-min' errored after 51 min
[hh:mm:ss] Error: Found 1 errors                           
    at Stream.<anonymous> (/home/benz0li/projects/coder/code-server/lib/vscode/build/lib/reporter.js:91:29)            
    at _end (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:65:9)                   
    at stream.end (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:74:5)             
    at Stream.onend (node:internal/streams/legacy:48:10)   
    at Stream.emit (node:events:531:35)                    
    at Stream.emit (node:domain:551:15)                    
    at drain (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:34:23)                 
    at stream.queue.stream.push (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:45:5)                            
    at Stream.end (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:15:35)            
    at _end (/home/benz0li/projects/coder/code-server/lib/vscode/node_modules/through/index.js:65:9)                   
error Command failed with exit code 1.

@code-asher
Copy link
Member

VS Code switched to npm?? Great news. I will try to get to this today.

@benz0li
Copy link
Contributor Author

benz0li commented Oct 4, 2024

VS Code switched to npm??

Yes.

This is to match VS Code.  We were already partially using npm for the
releases so this is some nice alignment.
This was complaining on every unit test.
src/node/routes/login.ts Fixed Show fixed Hide fixed
@code-asher code-asher force-pushed the code-1.94.0 branch 2 times, most recently from b9e4e95 to cdc013f Compare October 5, 2024 02:20
code-asher and others added 2 commits October 4, 2024 18:27
I was having a bunch of dependency conflicts and eslint seemed to be the
culprit so I just removed it and set it up again, since it seems things
have changed quite a bit.
I was getting oom when running the unit tests...updating seems to work.
Copy link

codecov bot commented Oct 5, 2024

Codecov Report

Attention: Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@13d4cd6). Learn more about missing BASE report.
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
src/node/routes/vscode.ts 0.00% 2 Missing ⚠️
src/node/socket.ts 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #7026   +/-   ##
=======================================
  Coverage        ?   72.59%           
=======================================
  Files           ?       31           
  Lines           ?     1905           
  Branches        ?      412           
=======================================
  Hits            ?     1383           
  Misses          ?      442           
  Partials        ?       80           
Files with missing lines Coverage Δ
src/browser/serviceWorker.ts 0.00% <ø> (ø)
src/node/vscodeSocket.ts 85.84% <ø> (ø)
src/node/wsRouter.ts 92.00% <ø> (ø)
src/node/socket.ts 89.65% <0.00%> (ø)
src/node/routes/vscode.ts 24.48% <0.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 13d4cd6...4cd9509. Read the comment docs.

@benz0li benz0li mentioned this pull request Oct 8, 2024
@benz0li
Copy link
Contributor Author

benz0li commented Oct 8, 2024

@code-asher Code - OSS v1.94.1 was released today: https://github.com/microsoft/vscode/releases/tag/1.94.1

The new pre-install script was being included, which is dev-only.

This was always the intent; did not realize jq's merge was recursive.
@benz0li
Copy link
Contributor Author

benz0li commented Oct 10, 2024

@code-asher Code - OSS v1.94.2 was released today: https://github.com/microsoft/vscode/releases/tag/1.94.2

@code-asher code-asher self-assigned this Oct 14, 2024
@code-asher
Copy link
Member

Was not able to get back to this last week but am aiming to get it sorted today or maybe tomorrow.

This appears to be conflicting with the root dependencies.
npm does not let you run binaries like yarn does, as far as I know.
@code-asher
Copy link
Member

The entrypoint having been rewritten in ESM is making things really difficult, I started converting us to ESM but it is a nightmare.

Think tomorrow I am going to break out VS Code into a separate process and communicate via IPC rather than importing it directly in the same process, should completely isolate us from any changes VS Code makes like this, and is the way it really should have been done in the first place.

@benz0li
Copy link
Contributor Author

benz0li commented Oct 16, 2024

Think tomorrow I am going to break out VS Code into a separate process and communicate via IPC rather than importing it directly in the same process, should completely isolate us from any changes VS Code makes like this, and is the way it really should have been done in the first place.

Sounds like a major change. I will do more extensive testing with the release candidate, then.

@code-asher
Copy link
Member

code-asher commented Oct 16, 2024

So, I got most of that done last night but, as you guessed it is getting pretty complicated. I still think it is probably the right move long-term but for this release I think I will go with a simpler fix and just rewrite server-main.js as a CJS module (it is pretty small, the part we need).

@code-asher
Copy link
Member

Actually I can just copy the old server-main.js from an older commit, I think, before it was converted to ESM.

@benz0li
Copy link
Contributor Author

benz0li commented Oct 16, 2024

Actually I can just copy the old server-main.js from an older commit, I think, before it was converted to ESM.

Go for it.

@code-asher code-asher force-pushed the code-1.94.0 branch 3 times, most recently from d303968 to 4744383 Compare October 16, 2024 19:16
Fixes the icons and fetching modules from the browser when behind a
reverse proxy.
@code-asher code-asher merged commit 5b2b3f1 into coder:main Oct 18, 2024
15 of 17 checks passed
code-asher pushed a commit that referenced this pull request Oct 18, 2024
* Update Code to 1.94.2

* Convert from yarn to npm

This is to match VS Code.  We were already partially using npm for the
releases so this is some nice alignment.

* Update caniuse-lite

This was complaining on every unit test.

* Update eslint

I was having a bunch of dependency conflicts and eslint seemed to be the
culprit so I just removed it and set it up again, since it seems things
have changed quite a bit.

* Update test dependencies

I was getting oom when running the unit tests...updating seems to work.

* Remove package.json `scripts` property in release

The new pre-install script was being included, which is dev-only.

This was always the intent; did not realize jq's merge was recursive.

* Remove jest and devDependencies in release as well

* Update test extension dependencies

This appears to be conflicting with the root dependencies.

* Fix playwright exec

npm does not let you run binaries like yarn does, as far as I know.

* Fix import of server-main.js

* Fix several tests by waiting for selectors
@benz0li
Copy link
Contributor Author

benz0li commented Oct 19, 2024

Ping @code-asher: code-server-4.94.2-rc.1-linux-amd64.tar.gz with Code 1.94.2 is deployed at https://coder.jupyter.b-data.ch.

Functionality [modified by patches] tested and found to work:

  • base-path
  • cli-window-open
  • local-storage
  • marketplace
  • proxy-url
  • service-worker
  • web view

Workspaces and Jupyter Notebooks also work fine:

  • ms-toolsai.jupyter@2024.9.1
  • ms-python.python@2024.16.1

ℹ️ Because of issue #7042, these are not installed as built-in extensions.

Safari is unable to render the font's (MesloLGS NF) glyphs and symbols properly:

safari-font

ℹ️ No problem with Firefox.

@benz0li
Copy link
Contributor Author

benz0li commented Oct 20, 2024

I was able to reproduce the Terminal font issue in Codespaces and opened an issue:

@code-asher
Copy link
Member

Sorry for the delay, I was out last week.

Thank you for opening that upstream!

Hmm that extension installation issue sounds like a release blocker.

@benz0li
Copy link
Contributor Author

benz0li commented Oct 29, 2024

Sorry for the delay, I was out last week.

No worries.

Thank you for opening that upstream!

👍

Hmm that extension installation issue sounds like a release blocker.

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Code to 1.94
2 participants