Patches 1.70 #5422

fritterhoff · 2022-08-07T05:01:15Z

Hello :)

I started playing around with the new version 1.70. Some patches must be adjusted...

The telemetry will need a larger fix - I simply made it a nop client for my first tests
The storage service maybe must be checked ...
I assume the language fix is still required but I am not sure if the fix still works. The previously new file languagePacks.ts now contains the following fragment(?):

export class WebLanguagePacksService extends LanguagePackBaseService {
	// Web doesn't have a concept of language packs, so we just return an empty array
	getInstalledLanguages(): Promise<ILanguagePackItem[]> {
		return Promise.resolve([]);
	}
}

Feel free to close the PR if my changes don't help at all 😉

jsjoeio · 2022-08-08T17:11:34Z

@fritterhoff first, thank you so much for doing this! This is a huge help 🎉

@code-asher since you have more context on some of these patches, do you mind reviewing?

fritterhoff · 2022-08-08T17:47:02Z

For sure, some further work will be required... Esp. fixing the telemetry seems to be a bit cumberstone

code-asher · 2022-08-09T17:07:35Z

Thanks for getting us started! I will hopefully have time later this week to finish it out. 🤞

fritterhoff · 2022-08-10T07:47:24Z

FYI: I just rebased my branch on the code-server main branch. Sadly the CI needs an approval again 😉

codecov · 2022-08-10T17:25:56Z

Codecov Report

Merging #5422 (92a6ca2) into main (bef78e6) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #5422   +/-   ##
=======================================
  Coverage   72.44%   72.44%           
=======================================
  Files          30       30           
  Lines        1673     1673           
  Branches      366      366           
=======================================
  Hits         1212     1212           
  Misses        398      398           
  Partials       63       63

Continue to review full report at Codecov.

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

jsjoeio · 2022-08-10T17:51:40Z

I'm going to try adding a label to this PR which should auto-rebase (I think) and even deploy your PR to npm. Let's give it a try.

UPDATE: looks like I still have to Approve each time :/ I'll be out Thurs/Fri so @code-asher can help if you need an approval

fritterhoff · 2022-08-10T17:57:34Z

Alright. Maybe it would help if a dummy PR created by me gets merged... According to GitHub only first time contributors require approval 🤣

fritterhoff · 2022-08-10T18:48:08Z

From my point of view the e2e tests did fail due to an internal error and not due to my changes?

jsjoeio · 2022-08-10T18:56:44Z

Yes, looks like an internal GitHub Actions error:

Also remove it from script-src since it is invalid anyway.

Also no need to set the key since it will be in the migration (and even if not it will be set by the next function call).

Just to keep it consistent with the other imports. We initially added the patch like this so it was not part of the upgrade but might as well fix it now.

code-asher · 2022-08-12T19:46:29Z

Awesome work @fritterhoff, I implemented the telemetry and everything else looks great (I just added back some comments and made two other small tweaks not really related to the update).

Going to do a bit more testing then open this as ready for review.

code-asher · 2022-08-12T20:03:05Z

Display language is not working for me, maybe there is some conflict between our patch and the stuff upstream has been adding. Ideally we would use upstream's but they mentioned their implementation is not quite ready so we might need to look into how to cut it out for now.

fritterhoff · 2022-08-13T04:36:00Z

Awesome work @fritterhoff, I implemented the telemetry and everything else looks great (I just added back some comments and made two other small tweaks not really related to the update).

Going to do a bit more testing then open this as ready for review.

Great thanks for the feedback and sorry about the comments... in a first step I was happy that I had a working version 1.70 😉

Display language is not working for me, maybe there is some conflict between our patch and the stuff upstream has been adding. Ideally we would use upstream's but they mentioned their implementation is not quite ready so we might need to look into how to cut it out for now.

I'm not sure what the expected behavior is or what isn't working here? I only tried to install a language pack and that was working 😁 So feel free to give me a hint what got broken in the new version 😉

code-asher · 2022-08-15T17:53:54Z

Here is the behavior I see (I ran from the latest linux-amd64.tar.gz artifact from CI): - Launch code-server - Extensions > Search "Japanese" - Click "Set display language" - Click "Restart" - Still in English Interestingly I just tried with Spanish and it worked. I also tried Chinese but it did *not* work. Which language pack did you try? It is also interesting that the button says "set display language" now instead of "install" like it used to. Lastly, when I run "configure display language" from the command palette only English shows for me. It is not showing all the language packs I have installed.

fritterhoff · 2022-08-15T17:59:26Z

Hi, I tried the German language pack. That worked fine. I'll check it again later and update my comment

fritterhoff · 2022-08-15T18:21:36Z

So spanish and German work at least in the Installation fine. Chinese does not work.
Setting via the command menu does not work at all...

jsjoeio · 2022-08-15T18:57:04Z

1.70 might have broken our language patch :( at least that's what we were discussing offline. Thank you for testing those and reporting back!

fritterhoff · 2022-08-15T18:58:40Z

Well ok. I sort of expected something like that 😔

code-asher · 2022-08-15T20:44:44Z

I should have time to look into it this week if no one else gets to it first.

code-asher · 2022-08-16T23:13:52Z

Language issues should be sorted, I will do a final check on the CI artifact once it is ready then merge.

* Update upstream Code to 1.70 * Update CSP hashes * Update comment on remote authority Also remove it from script-src since it is invalid anyway. * Use absolute path for disable download patch Just to keep it consistent with the other imports. We initially added the patch like this so it was not part of the upgrade but might as well fix it now. * Fix inability to change language while code-server is running Co-authored-by: Asher <[email protected]>

jsjoeio · 2022-08-17T17:54:53Z

patches/unique-db.diff

+				// Add a unique ID based on the current path for per-workspace databases.
+				// This prevents workspaces on different machines that share the same domain
+				// and file path from colliding (since it does not appear IndexedDB can be
+				// scoped to a path) as long as they are hosted on different paths.
 +				return this.payload.id + '-' + hash(location.pathname.toString().replace(/\/$/, "")).toString(16);


Will this commit affect the migration/state collisions e2e test we have?

Nope it operates the same way; things just moved around a little bit.

jsjoeio · 2022-08-17T17:55:32Z

patches/disable-downloads.diff

@@ -108,7 +108,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/contextkeys.ts
 import { Schemas } from 'vs/base/common/network';
 import { WebFileSystemAccess } from 'vs/platform/files/browser/webFileSystemAccess';
 import { IProductService } from 'vs/platform/product/common/productService';
-+import { IBrowserWorkbenchEnvironmentService } from '../services/environment/browser/environmentService';
+import { IBrowserWorkbenchEnvironmentService } from 'vs/workbench/services/environment/browser/environmentService';


Nice catch! I don't know why i didn't do that before lol thanks for fixing.

jsjoeio · 2022-08-17T17:58:20Z

patches/display-language.diff

+				// This must not use Node's require otherwise it will be cached forever.
+				// Code can get away with this since the process actually restarts but
+				// that is not currently the case with code-server.
+				return JSON.parse(fs.readFileSync(configFile, "utf8"));


Nice fix! How did you figure this out?

I actually fixed this before but it was lost when we switched to the fork so basically I was stepping through the code until suddenly I remembered this issue.

The first time around it just involved stepping through the code one step at time to figure out why the language object was empty until I arrived at this line and realized it would never update since requires are cached. I am not sure where that knowledge came from, probably read it somewhere or maybe it was from when we implemented tarfs which hijacks require.

jsjoeio assigned code-asher Aug 8, 2022

This was referenced Aug 9, 2022

[Feat]: Upgrade vscode to 1.70 #5417

Closed

[Feat]: Upgrade to vscode 1.69 #5344

Closed

fritterhoff added 6 commits August 10, 2022 09:46

chore: update upstream code

6ee7e63

patches: start patching 1.70

886894e

fix: use correct storage type

f25e0dc

chore: update patches

833606e

fix: use correct path for localeService

a8a606c

fix: use correct csp hashes

a51136b

fritterhoff force-pushed the patches-1.70 branch from d73ed22 to a51136b Compare August 10, 2022 07:46

jsjoeio added the rebase when passing label Aug 10, 2022

Merge branch 'main' into patches-1.70

56ebd89

Merge branch 'main' into patches-1.70

02dc3d7

repo-ranger bot and others added 8 commits August 10, 2022 21:17

Merge branch 'main' into patches-1.70

e48f91c

Merge branch 'main' into patches-1.70

cd2ee28

Merge branch 'main' into patches-1.70

ae1e4ff

Merge branch 'main' into patches-1.70

0d6935d

Update comment on remote authority

3fbe58a

Also remove it from script-src since it is invalid anyway.

Add back comment to unique db patch

424fb8d

Also no need to set the key since it will be in the migration (and even if not it will be set by the next function call).

Use absolute path for disable download patch

045e4a9

Just to keep it consistent with the other imports. We initially added the patch like this so it was not part of the upgrade but might as well fix it now.

Implement telemetry

53fb3d8

Refresh patch offsets

1798d5e

Fix display languages

7592cb1

code-asher force-pushed the patches-1.70 branch from 1ec6a70 to 7592cb1 Compare August 16, 2022 21:02

repo-ranger bot and others added 2 commits August 16, 2022 21:02

Merge branch 'main' into patches-1.70

2b0725a

Fix inability to change language while code-server is running

92a6ca2

code-asher force-pushed the patches-1.70 branch from fdf6899 to 92a6ca2 Compare August 16, 2022 23:08

code-asher marked this pull request as ready for review August 16, 2022 23:13

code-asher requested a review from a team August 16, 2022 23:13

code-asher approved these changes Aug 17, 2022

View reviewed changes

repo-ranger bot merged commit 3231405 into coder:main Aug 17, 2022

jsjoeio reviewed Aug 17, 2022

View reviewed changes

fritterhoff deleted the patches-1.70 branch August 18, 2022 05:12

Patches 1.70 #5422

Patches 1.70 #5422

Uh oh!

Conversation

fritterhoff commented Aug 7, 2022

Uh oh!

jsjoeio commented Aug 8, 2022

Uh oh!

fritterhoff commented Aug 8, 2022

Uh oh!

code-asher commented Aug 9, 2022

Uh oh!

fritterhoff commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jsjoeio commented Aug 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fritterhoff commented Aug 10, 2022

Uh oh!

fritterhoff commented Aug 10, 2022

Uh oh!

jsjoeio commented Aug 10, 2022

Uh oh!

code-asher commented Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

code-asher commented Aug 12, 2022

Uh oh!

fritterhoff commented Aug 13, 2022

Uh oh!

code-asher commented Aug 15, 2022 via email • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fritterhoff commented Aug 15, 2022

Uh oh!

fritterhoff commented Aug 15, 2022

Uh oh!

jsjoeio commented Aug 15, 2022

Uh oh!

fritterhoff commented Aug 15, 2022

Uh oh!

code-asher commented Aug 15, 2022 via email

Uh oh!

code-asher commented Aug 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsjoeio Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

code-asher Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

jsjoeio Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

jsjoeio Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

code-asher Aug 17, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fritterhoff commented Aug 10, 2022 •

edited

Loading

codecov bot commented Aug 10, 2022 •

edited

Loading

jsjoeio commented Aug 10, 2022 •

edited

Loading

code-asher commented Aug 12, 2022 •

edited

Loading

code-asher commented Aug 15, 2022 via email •

edited

Loading

code-asher commented Aug 16, 2022 •

edited

Loading