From 73cb236535573258cd2f9a89d1b86e9224e15a2f Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 27 Nov 2023 17:03:22 -0900 Subject: [PATCH] Add back local storage patch And fix the workspace bug. It is caused by an issue with how some global variables are being used asynchronously and is exacerbated by the delay reading settings from the remote introduces. 1. The workspace is created and is marked as not initialized. 2. The configuration's change handler is triggered, and now initialization is complete. 3. The handler tries to set the global workspace variable to initialized but the workspace has not been set yet so we get an undefined error. 4. The workspace global is now set, but it is set to the old value with initialized still set to false. 5. Workspace is never marked as initialized until something else triggers the on change handler again. Fixes #3061, and closes #6546. My guess is this logic changed in one of the VS Code updates, introducing this async bug but never getting caught probably because for them the settings are always local thus minimal delay. --- patches/disable-downloads.diff | 16 +++--- patches/display-language.diff | 4 +- patches/getting-started.diff | 10 ++-- patches/local-storage.diff | 90 ++++++++++++++++++++++++++++++++++ patches/series | 1 + 5 files changed, 106 insertions(+), 15 deletions(-) create mode 100644 patches/local-storage.diff diff --git a/patches/disable-downloads.diff b/patches/disable-downloads.diff index e95d1243d..e60802b77 100644 --- a/patches/disable-downloads.diff +++ b/patches/disable-downloads.diff @@ -12,9 +12,9 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts -@@ -276,6 +276,11 @@ export interface IWorkbenchConstructionO +@@ -281,6 +281,11 @@ export interface IWorkbenchConstructionO */ - readonly configurationDefaults?: Record; + readonly userDataPath?: string + /** + * Whether the "Download..." option is enabled for files. @@ -40,9 +40,9 @@ Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/envi * Gets whether a resolver extension is expected for the environment. */ readonly expectsResolverExtension: boolean; -@@ -110,6 +115,13 @@ export class BrowserWorkbenchEnvironment - @memoize - get cacheHome(): URI { return joinPath(this.userRoamingDataHome, 'caches'); } +@@ -111,6 +116,13 @@ export class BrowserWorkbenchEnvironment + return this.options.userDataPath; + } + get isEnabledFileDownloads(): boolean { + if (typeof this.options.isEnabledFileDownloads === "undefined") { @@ -52,7 +52,7 @@ Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/envi + } + @memoize - get workspaceStorageHome(): URI { return joinPath(this.userRoamingDataHome, 'workspaceStorage'); } + get argvResource(): URI { return joinPath(this.userRoamingDataHome, 'argv.json'); } Index: code-server/lib/vscode/src/vs/server/node/serverEnvironmentService.ts =================================================================== @@ -78,10 +78,10 @@ Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/server/node/webClientServer.ts +++ code-server/lib/vscode/src/vs/server/node/webClientServer.ts -@@ -331,6 +331,7 @@ export class WebClientServer { - const workbenchWebConfiguration = { +@@ -332,6 +332,7 @@ export class WebClientServer { remoteAuthority, webviewEndpoint: vscodeBase + this._staticRoute + '/out/vs/workbench/contrib/webview/browser/pre', + userDataPath: this._environmentService.userDataPath, + isEnabledFileDownloads: !this._environmentService.args['disable-file-downloads'], _wrapWebWorkerExtHostInIframe, developmentOptions: { enableSmokeTestDriver: this._environmentService.args['enable-smoke-test-driver'] ? true : undefined, logLevel: this._logService.getLevel() }, diff --git a/patches/display-language.diff b/patches/display-language.diff index a0c381f34..5bade0167 100644 --- a/patches/display-language.diff +++ b/patches/display-language.diff @@ -220,7 +220,7 @@ Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts import { CharCode } from 'vs/base/common/charCode'; import { getRemoteServerRootPath } from 'vs/platform/remote/common/remoteHosts'; import { IExtensionManifest } from 'vs/platform/extensions/common/extensions'; -@@ -343,6 +344,8 @@ export class WebClientServer { +@@ -344,6 +345,8 @@ export class WebClientServer { callbackRoute: this._callbackRoute }; @@ -229,7 +229,7 @@ Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts const nlsBaseUrl = this._productService.extensionsGallery?.nlsBaseUrl; const values: { [key: string]: string } = { WORKBENCH_WEB_CONFIGURATION: asJSON(workbenchWebConfiguration), -@@ -351,6 +354,7 @@ export class WebClientServer { +@@ -352,6 +355,7 @@ export class WebClientServer { WORKBENCH_NLS_BASE_URL: vscodeBase + (nlsBaseUrl ? `${nlsBaseUrl}${!nlsBaseUrl.endsWith('/') ? '/' : ''}${this._productService.commit}/${this._productService.version}/` : ''), BASE: base, VS_BASE: vscodeBase, diff --git a/patches/getting-started.diff b/patches/getting-started.diff index 170df73e4..31a91d38d 100644 --- a/patches/getting-started.diff +++ b/patches/getting-started.diff @@ -135,7 +135,7 @@ Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts +++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts -@@ -281,6 +281,11 @@ export interface IWorkbenchConstructionO +@@ -286,6 +286,11 @@ export interface IWorkbenchConstructionO */ readonly isEnabledFileDownloads?: boolean @@ -163,7 +163,7 @@ Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/envi * Gets whether a resolver extension is expected for the environment. */ readonly expectsResolverExtension: boolean; -@@ -122,6 +127,13 @@ export class BrowserWorkbenchEnvironment +@@ -123,6 +128,13 @@ export class BrowserWorkbenchEnvironment return this.options.isEnabledFileDownloads; } @@ -175,7 +175,7 @@ Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/envi + } + @memoize - get workspaceStorageHome(): URI { return joinPath(this.userRoamingDataHome, 'workspaceStorage'); } + get argvResource(): URI { return joinPath(this.userRoamingDataHome, 'argv.json'); } Index: code-server/lib/vscode/src/vs/server/node/serverEnvironmentService.ts =================================================================== @@ -201,9 +201,9 @@ Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts =================================================================== --- code-server.orig/lib/vscode/src/vs/server/node/webClientServer.ts +++ code-server/lib/vscode/src/vs/server/node/webClientServer.ts -@@ -334,6 +334,7 @@ export class WebClientServer { - remoteAuthority, +@@ -335,6 +335,7 @@ export class WebClientServer { webviewEndpoint: vscodeBase + this._staticRoute + '/out/vs/workbench/contrib/webview/browser/pre', + userDataPath: this._environmentService.userDataPath, isEnabledFileDownloads: !this._environmentService.args['disable-file-downloads'], + isEnabledCoderGettingStarted: !this._environmentService.args['disable-getting-started-override'], _wrapWebWorkerExtHostInIframe, diff --git a/patches/local-storage.diff b/patches/local-storage.diff new file mode 100644 index 000000000..45827be98 --- /dev/null +++ b/patches/local-storage.diff @@ -0,0 +1,90 @@ +Make storage local to the remote server + +This makes user settings will be stored in the file system instead of in browser +storage. Using browser storage makes sharing or seeding settings between +browsers difficult and remote settings is not a sufficient replacement because +some settings are only allowed to be set on the user level. + +Unfortunately this does not affect state which uses a separate method with +IndexedDB and does not appear nearly as easy to redirect to disk. + +To test change settings from the UI and on disk while making sure they appear on +the other side. + +This patch also resolves a bug where a global value for workspace initialization +is used in a non async-safe way. + +Index: code-server/lib/vscode/src/vs/server/node/webClientServer.ts +=================================================================== +--- code-server.orig/lib/vscode/src/vs/server/node/webClientServer.ts ++++ code-server/lib/vscode/src/vs/server/node/webClientServer.ts +@@ -327,6 +327,7 @@ export class WebClientServer { + const workbenchWebConfiguration = { + remoteAuthority, + webviewEndpoint: vscodeBase + this._staticRoute + '/out/vs/workbench/contrib/webview/browser/pre', ++ userDataPath: this._environmentService.userDataPath, + _wrapWebWorkerExtHostInIframe, + developmentOptions: { enableSmokeTestDriver: this._environmentService.args['enable-smoke-test-driver'] ? true : undefined, logLevel: this._logService.getLevel() }, + settingsSyncOptions: !this._environmentService.isBuilt && this._environmentService.args['enable-sync'] ? { enabled: true } : undefined, +Index: code-server/lib/vscode/src/vs/workbench/browser/web.api.ts +=================================================================== +--- code-server.orig/lib/vscode/src/vs/workbench/browser/web.api.ts ++++ code-server/lib/vscode/src/vs/workbench/browser/web.api.ts +@@ -276,6 +276,11 @@ export interface IWorkbenchConstructionO + */ + readonly configurationDefaults?: Record; + ++ /** ++ * Path to the user data directory. ++ */ ++ readonly userDataPath?: string ++ + //#endregion + + //#region Profile options +Index: code-server/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts +=================================================================== +--- code-server.orig/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts ++++ code-server/lib/vscode/src/vs/workbench/services/environment/browser/environmentService.ts +@@ -102,7 +102,14 @@ export class BrowserWorkbenchEnvironment + get logFile(): URI { return joinPath(this.windowLogsPath, 'window.log'); } + + @memoize +- get userRoamingDataHome(): URI { return URI.file('/User').with({ scheme: Schemas.vscodeUserData }); } ++ get userRoamingDataHome(): URI { return joinPath(URI.file(this.userDataPath).with({ scheme: Schemas.vscodeRemote }), 'User'); } ++ ++ get userDataPath(): string { ++ if (!this.options.userDataPath) { ++ throw new Error('userDataPath was not provided to the browser'); ++ } ++ return this.options.userDataPath; ++ } + + @memoize + get argvResource(): URI { return joinPath(this.userRoamingDataHome, 'argv.json'); } +Index: code-server/lib/vscode/src/vs/workbench/services/configuration/browser/configurationService.ts +=================================================================== +--- code-server.orig/lib/vscode/src/vs/workbench/services/configuration/browser/configurationService.ts ++++ code-server/lib/vscode/src/vs/workbench/services/configuration/browser/configurationService.ts +@@ -143,8 +143,10 @@ export class WorkspaceService extends Di + this.workspaceConfiguration = this._register(new WorkspaceConfiguration(configurationCache, fileService, uriIdentityService, logService)); + this._register(this.workspaceConfiguration.onDidUpdateConfiguration(fromCache => { + this.onWorkspaceConfigurationChanged(fromCache).then(() => { +- this.workspace.initialized = this.workspaceConfiguration.initialized; +- this.checkAndMarkWorkspaceComplete(fromCache); ++ if (this.workspace) { // The workspace may not have been created yet. ++ this.workspace.initialized = this.workspaceConfiguration.initialized; ++ this.checkAndMarkWorkspaceComplete(fromCache); ++ } + }); + })); + +@@ -550,6 +552,8 @@ export class WorkspaceService extends Di + previousFolders = this.workspace.folders; + this.workspace.update(workspace); + } else { ++ // The configuration could have updated before the promise resolved. ++ workspace.initialized = this.workspaceConfiguration.initialized; + this.workspace = workspace; + } + diff --git a/patches/series b/patches/series index b5b1f13ad..898d2974a 100644 --- a/patches/series +++ b/patches/series @@ -10,6 +10,7 @@ logout.diff store-socket.diff proxy-uri.diff unique-db.diff +local-storage.diff service-worker.diff sourcemaps.diff disable-downloads.diff