From 3c101634526ba6b2ec0caf7e205d6930893129b4 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 28 Oct 2021 17:10:14 -0400 Subject: [PATCH 01/27] Flesh out fixes to align with upstream. --- src/node/app.ts | 2 +- src/node/cli.ts | 100 +++++++++++++++++++++----------------- src/node/main.ts | 14 +++--- src/node/routes/vscode.ts | 29 +++++------ src/node/util.ts | 8 +++ 5 files changed, 86 insertions(+), 67 deletions(-) diff --git a/src/node/app.ts b/src/node/app.ts index 1387135583d5..238d74cf36de 100644 --- a/src/node/app.ts +++ b/src/node/app.ts @@ -44,7 +44,7 @@ const listen = (server: http.Server, { host, port, socket }: ListenOptions) => { server.listen(socket, onListen) } else { // [] is the correct format when using :: but Node errors with them. - server.listen(port, host.replace(/^\[|\]$/g, ""), onListen) + server.listen(parseInt(port, 10), host.replace(/^\[|\]$/g, ""), onListen) } }) } diff --git a/src/node/cli.ts b/src/node/cli.ts index 3122fb0049e7..295c56495f6f 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -31,27 +31,7 @@ export enum LogLevel { export class OptionalString extends Optional {} -export interface Args - extends Pick< - CodeServerLib.NativeParsedArgs, - | "_" - | "user-data-dir" - | "enable-proposed-api" - | "extensions-dir" - | "builtin-extensions-dir" - | "extra-extensions-dir" - | "extra-builtin-extensions-dir" - | "ignore-last-opened" - | "locale" - | "log" - | "verbose" - | "install-source" - | "list-extensions" - | "install-extension" - | "uninstall-extension" - | "locate-extension" - // | "telemetry" - > { +export interface Args extends CodeServerLib.ServerParsedArgs { config?: string auth?: AuthType password?: string @@ -67,7 +47,6 @@ export interface Args json?: boolean log?: LogLevel open?: boolean - port?: number "bind-addr"?: string socket?: string version?: boolean @@ -76,6 +55,7 @@ export interface Args "proxy-domain"?: string[] "reuse-window"?: boolean "new-window"?: boolean + verbose?: boolean link?: OptionalString } @@ -169,7 +149,7 @@ const options: Options> = { // These two have been deprecated by bindAddr. host: { type: "string", description: "" }, - port: { type: "number", description: "" }, + port: { type: "string", description: "" }, socket: { type: "string", path: true, description: "Path to a socket (bind-addr will be ignored)." }, version: { type: "boolean", short: "v", description: "Display version information." }, @@ -178,11 +158,8 @@ const options: Options> = { "user-data-dir": { type: "string", path: true, description: "Path to the user data directory." }, "extensions-dir": { type: "string", path: true, description: "Path to the extensions directory." }, "builtin-extensions-dir": { type: "string", path: true }, - "extra-extensions-dir": { type: "string[]", path: true }, - "extra-builtin-extensions-dir": { type: "string[]", path: true }, "list-extensions": { type: "boolean", description: "List installed VS Code extensions." }, force: { type: "boolean", description: "Avoid prompts when installing VS Code extensions." }, - "install-source": { type: "string" }, "locate-extension": { type: "string[]" }, "install-extension": { type: "string[]", @@ -190,19 +167,9 @@ const options: Options> = { "Install or update a VS Code extension by id or vsix. The identifier of an extension is `${publisher}.${name}`.\n" + "To install a specific version provide `@${version}`. For example: 'vscode.csharp@1.2.3'.", }, - "enable-proposed-api": { - type: "string[]", - description: - "Enable proposed API features for extensions. Can receive one or more extension IDs to enable individually.", - }, "uninstall-extension": { type: "string[]", description: "Uninstall a VS Code extension by id." }, "show-versions": { type: "boolean", description: "Show VS Code extension versions." }, "proxy-domain": { type: "string[]", description: "Domain used for proxying ports." }, - "ignore-last-opened": { - type: "boolean", - short: "e", - description: "Ignore the last opened directory or workspace in favor of an empty window.", - }, "new-window": { type: "boolean", short: "n", @@ -214,7 +181,6 @@ const options: Options> = { description: "Force to open a file or folder in an already opened window.", }, - locale: { type: "string" }, log: { type: LogLevel }, verbose: { type: "boolean", short: "vvv", description: "Enable verbose logging." }, @@ -227,6 +193,43 @@ const options: Options> = { `, beta: true, }, + + connectionToken: { type: "string" }, + "connection-secret": { + type: "string", + description: + "Path to file that contains the connection token. This will require that all incoming connections know the secret.", + }, + "socket-path": { type: "string" }, + driver: { type: "string" }, + "start-server": { type: "boolean" }, + "print-startup-performance": { type: "boolean" }, + "print-ip-address": { type: "boolean" }, + "disable-websocket-compression": { type: "boolean" }, + + fileWatcherPolling: { type: "string" }, + + "enable-remote-auto-shutdown": { type: "boolean" }, + "remote-auto-shutdown-without-delay": { type: "boolean" }, + + "without-browser-env-var": { type: "boolean" }, + "extensions-download-dir": { type: "string" }, + "install-builtin-extension": { type: "string[]" }, + + category: { + type: "string", + description: "Filters installed extensions by provided category, when using --list-extensions.", + }, + "do-not-sync": { type: "boolean" }, + "force-disable-user-env": { type: "boolean" }, + + folder: { type: "string" }, + workspace: { type: "string" }, + "web-user-data-dir": { type: "string" }, + "use-host-proxy": { type: "string" }, + "enable-sync": { type: "boolean" }, + "github-auth": { type: "string" }, + logsPath: { type: "string" }, } export const optionDescriptions = (): string[] => { @@ -271,6 +274,14 @@ export function splitOnFirstEquals(str: string): string[] { return split } +const createDefaultArgs = (): Args => { + return { + _: [], + workspace: "", + folder: "", + } +} + export const parse = ( argv: string[], opts?: { @@ -285,7 +296,8 @@ export const parse = ( return new Error(msg) } - const args: Args = { _: [] } + // TODO: parse workspace and folder. + const args: Args = createDefaultArgs() let ended = false for (let i = 0; i < argv.length; ++i) { @@ -403,7 +415,7 @@ export interface DefaultedArgs extends ConfigArgs { value: string } host: string - port: number + port: string "proxy-domain": string[] verbose: boolean usingEnvPassword: boolean @@ -472,15 +484,15 @@ export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promi args.auth = AuthType.Password } - const addr = bindAddrFromAllSources(configArgs || { _: [] }, cliArgs) + const addr = bindAddrFromAllSources(configArgs || createDefaultArgs(), cliArgs) args.host = addr.host - args.port = addr.port + args.port = addr.port.toString() // If we're being exposed to the cloud, we listen on a random address and // disable auth. if (args.link) { args.host = "localhost" - args.port = 0 + args.port = "0" args.socket = undefined args.cert = undefined args.auth = AuthType.None @@ -581,7 +593,7 @@ export async function readConfigFile(configPath?: string): Promise { */ export function parseConfigFile(configFile: string, configPath: string): ConfigArgs { if (!configFile) { - return { _: [], config: configPath } + return { ...createDefaultArgs(), config: configPath } } const config = yaml.load(configFile, { @@ -641,7 +653,7 @@ export function bindAddrFromArgs(addr: Addr, args: Args): Addr { addr.port = parseInt(process.env.PORT, 10) } if (args.port !== undefined) { - addr.port = args.port + addr.port = parseInt(args.port, 10) } return addr } diff --git a/src/node/main.ts b/src/node/main.ts index 9235218f37d1..4e97856f0cea 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -12,13 +12,15 @@ import { startLink } from "./link" import { register } from "./routes" import { humanPath, isFile, loadAMDModule, open } from "./util" -export const shouldSpawnCliProcess = async (args: CodeServerLib.NativeParsedArgs): Promise => { - const shouldSpawn = await loadAMDModule<(argv: CodeServerLib.NativeParsedArgs) => boolean>( - "vs/code/node/cli", - "shouldSpawnCliProcess", +export const shouldSpawnCliProcess = async (args: CodeServerLib.ServerParsedArgs): Promise => { + return ( + !args["start-server"] && + (!!args["list-extensions"] || + !!args["install-extension"] || + !!args["install-builtin-extension"] || + !!args["uninstall-extension"] || + !!args["locate-extension"]) ) - - return shouldSpawn(args) } /** diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index a2b02512bba0..dd50d06a2c17 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -1,7 +1,7 @@ import * as express from "express" import path from "path" -import { AuthType, DefaultedArgs } from "../cli" -import { version as codeServerVersion, vsRootPath } from "../constants" +import { DefaultedArgs } from "../cli" +import { vsRootPath } from "../constants" import { ensureAuthenticated, authenticated, redirect } from "../http" import { loadAMDModule } from "../util" import { Router as WsRouter, WebsocketRouter } from "../wsRouter" @@ -10,7 +10,7 @@ import { errorHandler } from "./errors" export interface VSServerResult { router: express.Router wsRouter: WebsocketRouter - codeServerMain: CodeServerLib.IServerProcessMain + codeServerMain: CodeServerLib.IServerAPI } export const createVSServerRouter = async (args: DefaultedArgs): Promise => { @@ -37,19 +37,15 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise("vs/server/entry", "createVSServer") + const createVSServer = await loadAMDModule( + "vs/server/remoteExtensionHostAgentServer", + "createServer", + ) - const serverUrl = new URL(`${args.cert ? "https" : "http"}://${args.host}:${args.port}`) - const codeServerMain = await createVSServer({ - codeServerVersion, - serverUrl, - args, - authed: args.auth !== AuthType.None, - disableUpdateCheck: !!args["disable-update-check"], - }) - - const netServer = await codeServerMain.startup({ listenWhenReady: false }) + const codeServerMain = await createVSServer(null, { ...args, connectionToken: "0000" }, args["user-data-dir"]) const router = express.Router() const wsRouter = WsRouter() @@ -68,11 +64,12 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise { req.on("error", (error) => errorHandler(error, req, res, next)) - netServer.emit("request", req, res) + codeServerMain.handleRequest(req, res) }) wsRouter.ws("/", ensureAuthenticated, (req) => { - netServer.emit("upgrade", req, req.socket, req.head) + codeServerMain.handleUpgrade(req, req.socket) + // netServer.emit("upgrade", req, req.socket, req.head) req.socket.resume() }) diff --git a/src/node/util.ts b/src/node/util.ts index 37369d91c221..a86f2d0fcbde 100644 --- a/src/node/util.ts +++ b/src/node/util.ts @@ -506,6 +506,14 @@ type AMDModule = { [exportName: string]: T } * @param exportName Given name of export in the file */ export const loadAMDModule = async (amdPath: string, exportName: string): Promise => { + // Set default remote native node modules path, if unset + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"] = + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"] || path.join(vsRootPath, "remote", "node_modules") + + require(path.join(vsRootPath, "out/bootstrap-node")).injectNodeModuleLookupPath( + process.env["VSCODE_INJECT_NODE_MODULE_LOOKUP_PATH"], + ) + const module = await new Promise>((resolve, reject) => { require(path.join(vsRootPath, "out/bootstrap-amd")).load(amdPath, resolve, reject) }) From 0e7eb7e04ea0de28409c22ecc743603750aff6eb Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 29 Oct 2021 15:52:34 -0400 Subject: [PATCH 02/27] Update route handlers to better reflect fallback behavior. --- src/node/routes/errors.ts | 2 +- src/node/routes/vscode.ts | 9 +++++++-- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/node/routes/errors.ts b/src/node/routes/errors.ts index 757e9f7449d5..783cf25ce31a 100644 --- a/src/node/routes/errors.ts +++ b/src/node/routes/errors.ts @@ -8,7 +8,7 @@ import { rootPath } from "../constants" import { replaceTemplates } from "../http" import { getMediaMime } from "../util" -const notFoundCodes = ["ENOENT", "EISDIR", "FileNotFound"] +const notFoundCodes = ["ENOENT", "EISDIR"] export const errorHandler: express.ErrorRequestHandler = async (err, req, res, next) => { if (notFoundCodes.includes(err.code)) { err.status = HttpCode.NotFound diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index dd50d06a2c17..078e40bf7021 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -62,14 +62,19 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise { - req.on("error", (error) => errorHandler(error, req, res, next)) + req.on("error", (error: any) => { + if (error.code === "FileNotFound") { + next() + } + + errorHandler(error, req, res, next) + }) codeServerMain.handleRequest(req, res) }) wsRouter.ws("/", ensureAuthenticated, (req) => { codeServerMain.handleUpgrade(req, req.socket) - // netServer.emit("upgrade", req, req.socket, req.head) req.socket.resume() }) diff --git a/vendor/package.json b/vendor/package.json index d7d800f136ad..2f740af2bb45 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#3fc885904886003d88d1f300d6158bee486f644f" + "code-oss-dev": "cdr/vscode#b3a1bf472c2f08f772cb675dcd43014063d03570" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 3050d752d480..0f51d1c39341 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#3fc885904886003d88d1f300d6158bee486f644f: +code-oss-dev@cdr/vscode#b3a1bf472c2f08f772cb675dcd43014063d03570: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/3fc885904886003d88d1f300d6158bee486f644f" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/b3a1bf472c2f08f772cb675dcd43014063d03570" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From ad787bbe87426eb5c8bf0db887dd35e35bc2bad4 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 29 Oct 2021 16:04:18 -0400 Subject: [PATCH 03/27] Bump vendor. --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 2f740af2bb45..8143c084946e 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#b3a1bf472c2f08f772cb675dcd43014063d03570" + "code-oss-dev": "cdr/vscode#39e2813550c8a796f5c3523049f0e385737a96cb" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 0f51d1c39341..f219898af473 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#b3a1bf472c2f08f772cb675dcd43014063d03570: +code-oss-dev@cdr/vscode#39e2813550c8a796f5c3523049f0e385737a96cb: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/b3a1bf472c2f08f772cb675dcd43014063d03570" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/39e2813550c8a796f5c3523049f0e385737a96cb" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 19c9c23ddf06e65d645d151f9a2ff014bbdb36c2 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 29 Oct 2021 16:38:16 -0400 Subject: [PATCH 04/27] Touch up build, tests. --- ci/build/build-vscode.sh | 2 +- src/node/cli.ts | 2 +- test/unit/node/app.test.ts | 18 ++++---- test/unit/node/cli.test.ts | 85 +++++++++++++++++------------------ test/unit/node/plugin.test.ts | 4 +- 5 files changed, 54 insertions(+), 57 deletions(-) diff --git a/ci/build/build-vscode.sh b/ci/build/build-vscode.sh index 59bd6759b38b..d9729fada7f9 100755 --- a/ci/build/build-vscode.sh +++ b/ci/build/build-vscode.sh @@ -13,7 +13,7 @@ main() { # extensions-ci compiles extensions and includes their media. # compile-web compiles web extensions. TODO: Unsure if used. - yarn gulp extensions-ci compile-web "vscode-server${MINIFY:+-min}" + yarn gulp extensions-ci compile-web "vscode-reh-web${MINIFY:+-min}" } main "$@" diff --git a/src/node/cli.ts b/src/node/cli.ts index 295c56495f6f..5ed9911b6b7e 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -274,7 +274,7 @@ export function splitOnFirstEquals(str: string): string[] { return split } -const createDefaultArgs = (): Args => { +export const createDefaultArgs = (): Args => { return { _: [], workspace: "", diff --git a/test/unit/node/app.test.ts b/test/unit/node/app.test.ts index 5f8e04a5ae06..8c8f0558f7d7 100644 --- a/test/unit/node/app.test.ts +++ b/test/unit/node/app.test.ts @@ -4,14 +4,14 @@ import * as http from "http" import * as https from "https" import * as path from "path" import { createApp, ensureAddress, handleArgsSocketCatchError, handleServerError } from "../../../src/node/app" -import { OptionalString, setDefaults } from "../../../src/node/cli" +import { createDefaultArgs, OptionalString, setDefaults } from "../../../src/node/cli" import { generateCertificate } from "../../../src/node/util" import { getAvailablePort, tmpdir } from "../../utils/helpers" describe("createApp", () => { let spy: jest.SpyInstance let unlinkSpy: jest.SpyInstance - let port: number + let port: string let tmpDirPath: string let tmpFilePath: string @@ -29,7 +29,7 @@ describe("createApp", () => { // then you can spy on those modules methods, like unlink. // See: https://github.com/aelbore/esbuild-jest/issues/26#issuecomment-893763840 unlinkSpy = jest.spyOn(promises, "unlink") - port = await getAvailablePort() + port = (await getAvailablePort()).toString() }) afterEach(() => { @@ -44,8 +44,8 @@ describe("createApp", () => { it("should return an Express app, a WebSockets Express app and an http server", async () => { const defaultArgs = await setDefaults({ + ...createDefaultArgs(), port, - _: [], }) const app = await createApp(defaultArgs) @@ -61,8 +61,8 @@ describe("createApp", () => { it("should handle error events on the server", async () => { const defaultArgs = await setDefaults({ + ...createDefaultArgs(), port, - _: [], }) const app = await createApp(defaultArgs) @@ -82,10 +82,10 @@ describe("createApp", () => { it("should reject errors that happen before the server can listen", async () => { // We listen on an invalid port // causing the app to reject the Promise called at startup - const port = 2 + const port = "2" const defaultArgs = await setDefaults({ + ...createDefaultArgs(), port, - _: [], }) async function masterBall() { @@ -105,7 +105,7 @@ describe("createApp", () => { it("should unlink a socket before listening on the socket", async () => { await promises.writeFile(tmpFilePath, "") const defaultArgs = await setDefaults({ - _: [], + ...createDefaultArgs(), socket: tmpFilePath, }) @@ -119,9 +119,9 @@ describe("createApp", () => { const testCertificate = await generateCertificate("localhost") const cert = new OptionalString(testCertificate.cert) const defaultArgs = await setDefaults({ + ...createDefaultArgs(), port, cert, - _: [], ["cert-key"]: testCertificate.certKey, }) const app = await createApp(defaultArgs) diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index 44987eed9993..4606f0945c6e 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -6,6 +6,7 @@ import * as path from "path" import { Args, bindAddrFromArgs, + createDefaultArgs, defaultConfigFile, parse, setDefaults, @@ -32,9 +33,10 @@ describe("parser", () => { // values the user actually set. These are only set after explicitly calling // `setDefaults`. const defaults = { + ...createDefaultArgs(), auth: "password", host: "localhost", - port: 8080, + port: "8080", "proxy-domain": [], usingEnvPassword: false, usingEnvHashedPassword: false, @@ -43,7 +45,7 @@ describe("parser", () => { } it("should parse nothing", () => { - expect(parse([])).toStrictEqual({ _: [] }) + expect(parse([])).toStrictEqual(createDefaultArgs()) }) it("should parse all available options", () => { @@ -60,10 +62,8 @@ describe("parser", () => { "foo", "--builtin-extensions-dir", "foobar", - "--extra-extensions-dir", "nozzle", "1", - "--extra-builtin-extensions-dir", "bazzle", "--verbose", "2", @@ -116,7 +116,7 @@ describe("parser", () => { it("should work with short options", () => { expect(parse(["-vvv", "-v"])).toEqual({ - _: [], + ...createDefaultArgs(), verbose: true, version: true, }) @@ -124,7 +124,7 @@ describe("parser", () => { it("should use log level env var", async () => { const args = parse([]) - expect(args).toEqual({ _: [] }) + expect(args).toEqual(createDefaultArgs()) process.env.LOG_LEVEL = "debug" const defaults = await setDefaults(args) @@ -152,7 +152,7 @@ describe("parser", () => { it("should prefer --log to env var and --verbose to --log", async () => { let args = parse(["--log", "info"]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), log: "info", }) @@ -180,7 +180,7 @@ describe("parser", () => { args = parse(["--log", "info", "--verbose"]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), log: "info", verbose: true, }) @@ -215,7 +215,7 @@ describe("parser", () => { }) it("should error if value is invalid", () => { - expect(() => parse(["--port", "foo"])).toThrowError(/--port must be a number/) + expect(() => parse(["--port", "foo"])).toThrowError(/--port must be a stringly number/) expect(() => parse(["--auth", "invalid"])).toThrowError(/--auth valid values: \[password, none\]/) expect(() => parse(["--log", "invalid"])).toThrowError(/--log valid values: \[trace, debug, info, warn, error\]/) }) @@ -226,7 +226,7 @@ describe("parser", () => { it("should not error if the value is optional", () => { expect(parse(["--cert"])).toEqual({ - _: [], + ...createDefaultArgs(), cert: { value: undefined, }, @@ -237,7 +237,7 @@ describe("parser", () => { expect(() => parse(["--socket", "--socket-path-value"])).toThrowError(/--socket requires a value/) // If you actually had a path like this you would do this instead: expect(parse(["--socket", "./--socket-path-value"])).toEqual({ - _: [], + ...createDefaultArgs(), socket: path.resolve("--socket-path-value"), }) expect(() => parse(["--cert", "--socket-path-value"])).toThrowError(/Unknown option --socket-path-value/) @@ -245,6 +245,7 @@ describe("parser", () => { it("should allow positional arguments before options", () => { expect(parse(["foo", "test", "--auth", "none"])).toEqual({ + ...createDefaultArgs(), _: ["foo", "test"], auth: "none", }) @@ -252,11 +253,11 @@ describe("parser", () => { it("should support repeatable flags", () => { expect(parse(["--proxy-domain", "*.coder.com"])).toEqual({ - _: [], + ...createDefaultArgs(), "proxy-domain": ["*.coder.com"], }) expect(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ - _: [], + ...createDefaultArgs(), "proxy-domain": ["*.coder.com", "test.com"], }) }) @@ -264,7 +265,7 @@ describe("parser", () => { it("should enforce cert-key with cert value or otherwise generate one", async () => { const args = parse(["--cert"]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), cert: { value: undefined, }, @@ -272,7 +273,7 @@ describe("parser", () => { expect(() => parse(["--cert", "test"])).toThrowError(/--cert-key is missing/) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - _: [], + ...createDefaultArgs(), ...defaults, cert: { value: path.join(paths.data, "localhost.crt"), @@ -285,7 +286,7 @@ describe("parser", () => { const args = parse("--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" ")) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - _: [], + ...createDefaultArgs(), ...defaults, auth: "none", host: "localhost", @@ -303,7 +304,7 @@ describe("parser", () => { process.env.PASSWORD = "test" const args = parse([]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), }) const defaultArgs = await setDefaults(args) @@ -320,7 +321,7 @@ describe("parser", () => { "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" // test const args = parse([]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), }) const defaultArgs = await setDefaults(args) @@ -348,7 +349,7 @@ describe("parser", () => { it("should filter proxy domains", async () => { const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), "proxy-domain": ["*.coder.com", "coder.com", "coder.org"], }) @@ -365,7 +366,7 @@ describe("parser", () => { "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ]) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), "enable-proposed-api": [ "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ], @@ -381,7 +382,7 @@ describe("parser", () => { }, ) expect(args).toEqual({ - _: [], + ...createDefaultArgs(), "hashed-password": "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", }) @@ -389,8 +390,8 @@ describe("parser", () => { }) describe("cli", () => { - let args: Mutable = { _: [] } let testDir: string + let args: Mutable = createDefaultArgs() const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") beforeAll(async () => { @@ -401,7 +402,7 @@ describe("cli", () => { beforeEach(async () => { delete process.env.VSCODE_IPC_HOOK_CLI - args = { _: [] } + args = createDefaultArgs() await fs.rmdir(vscodeIpcPath, { recursive: true }) }) @@ -409,7 +410,7 @@ describe("cli", () => { process.env.VSCODE_IPC_HOOK_CLI = "test" expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") - args.port = 8081 + args.port = "8081" args._.push("./file") expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") }) @@ -421,7 +422,7 @@ describe("cli", () => { await fs.writeFile(vscodeIpcPath, "test") await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") - args.port = 8081 + args.port = "8081" await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") }) @@ -432,7 +433,7 @@ describe("cli", () => { await fs.writeFile(vscodeIpcPath, "test") expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") - args.port = 8081 + args.port = "8081" expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") }) @@ -457,7 +458,7 @@ describe("cli", () => { expect(await shouldOpenInExistingInstance(args)).toStrictEqual(socketPath) - args.port = 8081 + args.port = "8081" expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) }) }) @@ -489,9 +490,7 @@ describe("splitOnFirstEquals", () => { describe("shouldSpawnCliProcess", () => { it("should return false if no 'extension' related args passed in", async () => { - const args = { - _: [], - } + const args = createDefaultArgs() const actual = await shouldSpawnCliProcess(args) const expected = false @@ -500,7 +499,7 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'list-extensions' passed in", async () => { const args = { - _: [], + ...createDefaultArgs(), ["list-extensions"]: true, } const actual = await shouldSpawnCliProcess(args) @@ -511,7 +510,7 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'install-extension' passed in", async () => { const args = { - _: [], + ...createDefaultArgs(), ["install-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -522,7 +521,7 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'uninstall-extension' passed in", async () => { const args = { - _: [], + ...createDefaultArgs(), ["uninstall-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -534,9 +533,7 @@ describe("shouldSpawnCliProcess", () => { describe("bindAddrFromArgs", () => { it("should return the bind address", () => { - const args = { - _: [], - } + const args = createDefaultArgs() const addr = { host: "localhost", @@ -551,7 +548,7 @@ describe("bindAddrFromArgs", () => { it("should use the bind-address if set in args", () => { const args = { - _: [], + ...createDefaultArgs(), ["bind-addr"]: "localhost:3000", } @@ -571,7 +568,7 @@ describe("bindAddrFromArgs", () => { it("should use the host if set in args", () => { const args = { - _: [], + ...createDefaultArgs(), ["host"]: "coder", } @@ -593,9 +590,7 @@ describe("bindAddrFromArgs", () => { const [setValue, resetValue] = useEnv("PORT") setValue("8000") - const args = { - _: [], - } + const args = createDefaultArgs() const addr = { host: "localhost", @@ -614,8 +609,8 @@ describe("bindAddrFromArgs", () => { it("should set port if in args", () => { const args = { - _: [], - port: 3000, + ...createDefaultArgs(), + port: "3000", } const addr = { @@ -637,8 +632,8 @@ describe("bindAddrFromArgs", () => { setValue("8000") const args = { - _: [], - port: 3000, + ...createDefaultArgs(), + port: "3000", } const addr = { diff --git a/test/unit/node/plugin.test.ts b/test/unit/node/plugin.test.ts index acd417316acf..4f9e14a3785f 100644 --- a/test/unit/node/plugin.test.ts +++ b/test/unit/node/plugin.test.ts @@ -34,7 +34,7 @@ describe("plugin", () => { _: [], auth: AuthType.None, host: "localhost", - port: 8080, + port: "8080", "proxy-domain": [], config: "~/.config/code-server/config.yaml", verbose: false, @@ -42,6 +42,8 @@ describe("plugin", () => { usingEnvHashedPassword: false, "extensions-dir": "", "user-data-dir": "", + workspace: "", + folder: "", } next() } From 062ce322d884c71054411debcb9f3a21b4c99ce0 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 12:25:29 -0400 Subject: [PATCH 05/27] bump vscode. --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 8143c084946e..79c310a58465 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#39e2813550c8a796f5c3523049f0e385737a96cb" + "code-oss-dev": "cdr/vscode#3610be0c6e7f3def890289c0927a3135ccfdb94d" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index f219898af473..3b3a16106eca 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#39e2813550c8a796f5c3523049f0e385737a96cb: +code-oss-dev@cdr/vscode#3610be0c6e7f3def890289c0927a3135ccfdb94d: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/39e2813550c8a796f5c3523049f0e385737a96cb" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/3610be0c6e7f3def890289c0927a3135ccfdb94d" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 248171175ab5ba1f4031813d256e5e93f130381e Mon Sep 17 00:00:00 2001 From: Asher Date: Thu, 4 Nov 2021 17:21:53 +0000 Subject: [PATCH 06/27] Add platform to vscode-reh-web task Our strategy has been to build once and then recompile native modules for individual platforms. It looks like VS Code builds from scratch for each platform. But we can target any platform, grab the pre-packaged folder, then continue with own packaging. In the future we may want to rework to match upstream. --- ci/build/build-release.sh | 2 +- ci/build/build-vscode.sh | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/ci/build/build-release.sh b/ci/build/build-release.sh index 263b8c3b802f..4348435c8747 100755 --- a/ci/build/build-release.sh +++ b/ci/build/build-release.sh @@ -68,7 +68,7 @@ EOF bundle_vscode() { mkdir -p "$VSCODE_OUT_PATH" rsync "$VSCODE_SRC_PATH/yarn.lock" "$VSCODE_OUT_PATH" - rsync "$VSCODE_SRC_PATH/out-vscode-server${MINIFY:+-min}/" "$VSCODE_OUT_PATH/out" + rsync "$VSCODE_SRC_PATH/out-vscode-reh-web${MINIFY:+-min}/" "$VSCODE_OUT_PATH/out" rsync "$VSCODE_SRC_PATH/.build/extensions/" "$VSCODE_OUT_PATH/extensions" if [ "$KEEP_MODULES" = 0 ]; then diff --git a/ci/build/build-vscode.sh b/ci/build/build-vscode.sh index d9729fada7f9..be996fceef56 100755 --- a/ci/build/build-vscode.sh +++ b/ci/build/build-vscode.sh @@ -11,9 +11,8 @@ main() { cd vendor/modules/code-oss-dev - # extensions-ci compiles extensions and includes their media. - # compile-web compiles web extensions. TODO: Unsure if used. - yarn gulp extensions-ci compile-web "vscode-reh-web${MINIFY:+-min}" + # Any platform works since we have our own packaging step (for now). + yarn gulp "vscode-reh-web-linux-x64${MINIFY:+-min}" } main "$@" From dde9a08c6747a6703da0d255c98e043b72474bc7 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 15:08:01 -0400 Subject: [PATCH 07/27] Fix issue where workspace args are not parsed. --- src/node/cli.ts | 30 ++++++++++++++++++++++++------ src/node/entry.ts | 4 +++- src/node/main.ts | 10 ++++++++-- test/e2e/models/CodeServer.ts | 2 +- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index 5ed9911b6b7e..9c3a0db394a5 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -3,7 +3,15 @@ import { promises as fs } from "fs" import yaml from "js-yaml" import * as os from "os" import * as path from "path" -import { canConnect, generateCertificate, generatePassword, humanPath, paths, isNodeJSErrnoException } from "./util" +import { + canConnect, + generateCertificate, + generatePassword, + humanPath, + paths, + isNodeJSErrnoException, + isFile, +} from "./util" const DEFAULT_SOCKET_PATH = path.join(os.tmpdir(), "vscode-ipc") @@ -282,12 +290,12 @@ export const createDefaultArgs = (): Args => { } } -export const parse = ( +export const parse = async ( argv: string[], opts?: { configFile?: string }, -): Args => { +): Promise => { const error = (msg: string): Error => { if (opts?.configFile) { msg = `error reading ${opts.configFile}: ${msg}` @@ -296,7 +304,6 @@ export const parse = ( return new Error(msg) } - // TODO: parse workspace and folder. const args: Args = createDefaultArgs() let ended = false @@ -399,6 +406,17 @@ export const parse = ( args._.push(arg) } + if (args._.length && !args.folder && !args.workspace) { + const firstEntry = path.resolve(process.cwd(), args._[0]) + + if ((await isFile(firstEntry)) && path.extname(firstEntry) === ".code-workspace") { + args.workspace = firstEntry + args._.shift() + } else { + args.folder = args._.join(" ") + } + } + // If a cert was provided a key must also be provided. if (args.cert && args.cert.value && !args["cert-key"]) { throw new Error("--cert-key is missing") @@ -591,7 +609,7 @@ export async function readConfigFile(configPath?: string): Promise { * parseConfigFile parses configFile into ConfigArgs. * configPath is used as the filename in error messages */ -export function parseConfigFile(configFile: string, configPath: string): ConfigArgs { +export async function parseConfigFile(configFile: string, configPath: string): Promise { if (!configFile) { return { ...createDefaultArgs(), config: configPath } } @@ -611,7 +629,7 @@ export function parseConfigFile(configFile: string, configPath: string): ConfigA } return `--${optName}=${opt}` }) - const args = parse(configFileArgv, { + const args = await parse(configFileArgv, { configFile: configPath, }) return { diff --git a/src/node/entry.ts b/src/node/entry.ts index 06ce4cccfaa6..fc37927343d3 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -22,7 +22,7 @@ async function entry(): Promise { return } - const cliArgs = parse(process.argv.slice(2)) + const cliArgs = await parse(process.argv.slice(2)) const configArgs = await readConfigFile(cliArgs.config) const args = await setDefaults(cliArgs, configArgs) @@ -30,6 +30,8 @@ async function entry(): Promise { console.log("code-server", version, commit) console.log("") console.log(`Usage: code-server [options] [path]`) + console.log(` - Opening a directory: code-server ./path/to/your/project`) + console.log(` - Opening a saved workspace: code-server ./path/to/your/project.code-workspace`) console.log("") console.log("Options") optionDescriptions().forEach((description) => { diff --git a/src/node/main.ts b/src/node/main.ts index 4e97856f0cea..9621736d6891 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -124,11 +124,17 @@ export const runCodeServer = async ( } const app = await createApp(args) - const serverAddress = ensureAddress(app.server, args.cert ? "https" : "http") + const protocol = args.cert ? "https" : "http" + const serverAddress = ensureAddress(app.server, protocol) const disposeRoutes = await register(app, args) logger.info(`Using config file ${humanPath(args.config)}`) - logger.info(`HTTP server listening on ${serverAddress.toString()} ${args.link ? "(randomized by --link)" : ""}`) + logger.info( + `${protocol.toUpperCase()} server listening on ${serverAddress.toString()} ${ + args.link ? "(randomized by --link)" : "" + }`, + ) + if (args.auth === AuthType.Password) { logger.info(" - Authentication is enabled") if (args.usingEnvPassword) { diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 7f51e84bf0aa..695d241a4782 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -129,7 +129,7 @@ export class CodeServer { if (resolved) { return } - const match = line.trim().match(/HTTP server listening on (https?:\/\/[.:\d]+)\/?$/) + const match = line.trim().match(/HTTPS? server listening on (https?:\/\/[.:\d]+)\/?$/) if (match) { // Cookies don't seem to work on IP address so swap to localhost. // TODO: Investigate whether this is a bug with code-server. From 26733fd5b4bc5aac28c53184b2e32735a1c4a365 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 15:12:28 -0400 Subject: [PATCH 08/27] Update CLI test. --- test/unit/node/cli.test.ts | 31 ++++++++++++++++++++----------- test/utils/integration.ts | 4 ++-- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index 4606f0945c6e..a4ddd8e6ab84 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -123,7 +123,7 @@ describe("parser", () => { }) it("should use log level env var", async () => { - const args = parse([]) + const args = await parse([]) expect(args).toEqual(createDefaultArgs()) process.env.LOG_LEVEL = "debug" @@ -150,7 +150,7 @@ describe("parser", () => { }) it("should prefer --log to env var and --verbose to --log", async () => { - let args = parse(["--log", "info"]) + let args = await parse(["--log", "info"]) expect(args).toEqual({ ...createDefaultArgs(), log: "info", @@ -178,7 +178,7 @@ describe("parser", () => { expect(process.env.LOG_LEVEL).toEqual("info") expect(logger.level).toEqual(Level.Info) - args = parse(["--log", "info", "--verbose"]) + args = await parse(["--log", "info", "--verbose"]) expect(args).toEqual({ ...createDefaultArgs(), log: "info", @@ -199,7 +199,7 @@ describe("parser", () => { it("should ignore invalid log level env var", async () => { process.env.LOG_LEVEL = "bogus" - const defaults = await setDefaults(parse([])) + const defaults = await setDefaults(await parse([])) expect(defaults).toEqual({ ...defaults, _: [], @@ -263,7 +263,7 @@ describe("parser", () => { }) it("should enforce cert-key with cert value or otherwise generate one", async () => { - const args = parse(["--cert"]) + const args = await parse(["--cert"]) expect(args).toEqual({ ...createDefaultArgs(), cert: { @@ -283,7 +283,9 @@ describe("parser", () => { }) it("should override with --link", async () => { - const args = parse("--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" ")) + const args = await parse( + "--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" "), + ) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...createDefaultArgs(), @@ -302,7 +304,7 @@ describe("parser", () => { it("should use env var password", async () => { process.env.PASSWORD = "test" - const args = parse([]) + const args = await parse([]) expect(args).toEqual({ ...createDefaultArgs(), }) @@ -319,7 +321,7 @@ describe("parser", () => { it("should use env var hashed password", async () => { process.env.HASHED_PASSWORD = "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" // test - const args = parse([]) + const args = await parse([]) expect(args).toEqual({ ...createDefaultArgs(), }) @@ -347,7 +349,14 @@ describe("parser", () => { }) it("should filter proxy domains", async () => { - const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"]) + const args = await parse([ + "--proxy-domain", + "*.coder.com", + "--proxy-domain", + "coder.com", + "--proxy-domain", + "coder.org", + ]) expect(args).toEqual({ ...createDefaultArgs(), "proxy-domain": ["*.coder.com", "coder.com", "coder.org"], @@ -361,7 +370,7 @@ describe("parser", () => { }) }) it("should allow '=,$/' in strings", async () => { - const args = parse([ + const args = await parse([ "--enable-proposed-api", "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ]) @@ -373,7 +382,7 @@ describe("parser", () => { }) }) it("should parse options with double-dash and multiple equal signs ", async () => { - const args = parse( + const args = await parse( [ "--hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ], diff --git a/test/utils/integration.ts b/test/utils/integration.ts index 5c4f0cc6aa7f..e9a05c9c4a8e 100644 --- a/test/utils/integration.ts +++ b/test/utils/integration.ts @@ -5,8 +5,8 @@ import * as httpserver from "./httpserver" export async function setup(argv: string[], configFile?: string): Promise { argv = ["--bind-addr=localhost:0", "--log=warn", ...argv] - const cliArgs = parse(argv) - const configArgs = parseConfigFile(configFile || "", "test/integration.ts") + const cliArgs = await parse(argv) + const configArgs = await parseConfigFile(configFile || "", "test/integration.ts") const args = await setDefaults(cliArgs, configArgs) const server = await runCodeServer(args) From 059893f5a361fc1d2da78355e425b4c8c002e326 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 17:02:45 -0400 Subject: [PATCH 09/27] Update CLI tests. --- src/node/cli.ts | 20 ++-- test/unit/node/cli.test.ts | 185 ++++++++++++++++++++----------------- 2 files changed, 112 insertions(+), 93 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index 9c3a0db394a5..c199c23c63b5 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -406,14 +406,20 @@ export const parse = async ( args._.push(arg) } - if (args._.length && !args.folder && !args.workspace) { - const firstEntry = path.resolve(process.cwd(), args._[0]) + if (args.port && isNaN(parseInt(args.port, 10))) { + throw new Error("--port must be a number") + } - if ((await isFile(firstEntry)) && path.extname(firstEntry) === ".code-workspace") { - args.workspace = firstEntry - args._.shift() - } else { - args.folder = args._.join(" ") + if (args._.length && !args.folder && !args.workspace) { + const lastEntry = path.resolve(process.cwd(), args._[args._.length - 1]) + const entryIsFile = await isFile(lastEntry) + + if (entryIsFile && path.extname(lastEntry) === ".code-workspace") { + args.workspace = lastEntry + args._.pop() + } else if (!entryIsFile) { + args.folder = lastEntry + args._.pop() } } diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index a4ddd8e6ab84..af4d15b1c636 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -44,78 +44,87 @@ describe("parser", () => { "user-data-dir": paths.data, } - it("should parse nothing", () => { - expect(parse([])).toStrictEqual(createDefaultArgs()) + it("should parse nothing", async () => { + expect(await parse([])).toStrictEqual(createDefaultArgs()) }) - it("should parse all available options", () => { + it("should parse all available options", async () => { expect( - parse([ - "--enable", - "feature1", - "--enable", - "feature2", - "--bind-addr=192.169.0.1:8080", - "--auth", - "none", - "--extensions-dir", - "foo", - "--builtin-extensions-dir", - "foobar", - "nozzle", - "1", - "bazzle", - "--verbose", - "2", - "--log", - "error", - "--help", - "--open", - "--socket=mumble", - "3", - "--user-data-dir", - "bar", - "--cert=baz", - "--cert-key", - "qux", - "--version", - "--json", - "--port=8081", - "--host", - "0.0.0.0", - "4", - "--", - "-5", - "--6", - ]), + await parse( + [ + ["--enable", "feature1"], + ["--enable", "feature2"], + + "--bind-addr=192.169.0.1:8080", + + ["--auth", "none"], + + ["--extensions-dir", "some/ext/dir"], + + ["--builtin-extensions-dir", "some/built/ext/dir"], + + "1", + "--verbose", + "2", + + ["--log", "error"], + + "--help", + + "--open", + + "--socket=mumble", + + "3", + + "--user-data-dir", + "some/user/dir", + + "--cert=Foo", + "--cert-key", + "someCertKeyBar", + + "--version", + + "--json", + + "--port=8081", + + ["--host", "0.0.0.0"], + "4", + "--", + "--5", + "some/project/directory", + ].flat(), + ), ).toEqual({ - _: ["1", "2", "3", "4", "-5", "--6"], + _: ["1", "2", "3", "4", "--5"], auth: "none", - "builtin-extensions-dir": path.resolve("foobar"), - "cert-key": path.resolve("qux"), + "builtin-extensions-dir": path.resolve("some/built/ext/dir"), + "extensions-dir": path.resolve("some/ext/dir"), + "user-data-dir": path.resolve("some/user/dir"), + folder: path.resolve(process.cwd(), "some/project/directory"), + "cert-key": path.resolve("someCertKeyBar"), cert: { - value: path.resolve("baz"), + value: path.resolve("Foo"), }, enable: ["feature1", "feature2"], - "extensions-dir": path.resolve("foo"), - "extra-builtin-extensions-dir": [path.resolve("bazzle")], - "extra-extensions-dir": [path.resolve("nozzle")], help: true, host: "0.0.0.0", json: true, log: "error", open: true, - port: 8081, + port: "8081", socket: path.resolve("mumble"), - "user-data-dir": path.resolve("bar"), verbose: true, version: true, "bind-addr": "192.169.0.1:8080", + workspace: "", }) }) - it("should work with short options", () => { - expect(parse(["-vvv", "-v"])).toEqual({ + it("should work with short options", async () => { + expect(await parse(["-vvv", "-v"])).toEqual({ ...createDefaultArgs(), verbose: true, version: true, @@ -206,26 +215,28 @@ describe("parser", () => { }) }) - it("should error if value isn't provided", () => { - expect(() => parse(["--auth"])).toThrowError(/--auth requires a value/) - expect(() => parse(["--auth=", "--log=debug"])).toThrowError(/--auth requires a value/) - expect(() => parse(["--auth", "--log"])).toThrowError(/--auth requires a value/) - expect(() => parse(["--auth", "--invalid"])).toThrowError(/--auth requires a value/) - expect(() => parse(["--bind-addr"])).toThrowError(/--bind-addr requires a value/) + it("should error if value isn't provided", async () => { + await expect(parse(["--auth"])).rejects.toThrowError(/--auth requires a value/) + await expect(parse(["--auth=", "--log=debug"])).rejects.toThrowError(/--auth requires a value/) + await expect(parse(["--auth", "--log"])).rejects.toThrowError(/--auth requires a value/) + await expect(parse(["--auth", "--invalid"])).rejects.toThrowError(/--auth requires a value/) + await expect(parse(["--bind-addr"])).rejects.toThrowError(/--bind-addr requires a value/) }) - it("should error if value is invalid", () => { - expect(() => parse(["--port", "foo"])).toThrowError(/--port must be a stringly number/) - expect(() => parse(["--auth", "invalid"])).toThrowError(/--auth valid values: \[password, none\]/) - expect(() => parse(["--log", "invalid"])).toThrowError(/--log valid values: \[trace, debug, info, warn, error\]/) + it("should error if value is invalid", async () => { + await expect(parse(["--port", "foo"])).rejects.toThrowError(/--port must be a number/) + await expect(parse(["--auth", "invalid"])).rejects.toThrowError(/--auth valid values: \[password, none\]/) + await expect(parse(["--log", "invalid"])).rejects.toThrowError( + /--log valid values: \[trace, debug, info, warn, error\]/, + ) }) - it("should error if the option doesn't exist", () => { - expect(() => parse(["--foo"])).toThrowError(/Unknown option --foo/) + it("should error if the option doesn't exist", async () => { + await expect(parse(["--foo"])).rejects.toThrowError(/Unknown option --foo/) }) - it("should not error if the value is optional", () => { - expect(parse(["--cert"])).toEqual({ + it("should not error if the value is optional", async () => { + expect(await parse(["--cert"])).toEqual({ ...createDefaultArgs(), cert: { value: undefined, @@ -233,30 +244,31 @@ describe("parser", () => { }) }) - it("should not allow option-like values", () => { - expect(() => parse(["--socket", "--socket-path-value"])).toThrowError(/--socket requires a value/) + it("should not allow option-like values", async () => { + await expect(parse(["--socket", "--socket-path-value"])).rejects.toThrowError(/--socket requires a value/) // If you actually had a path like this you would do this instead: - expect(parse(["--socket", "./--socket-path-value"])).toEqual({ + expect(await parse(["--socket", "./--socket-path-value"])).toEqual({ ...createDefaultArgs(), socket: path.resolve("--socket-path-value"), }) - expect(() => parse(["--cert", "--socket-path-value"])).toThrowError(/Unknown option --socket-path-value/) + await expect(parse(["--cert", "--socket-path-value"])).rejects.toThrowError(/Unknown option --socket-path-value/) }) - it("should allow positional arguments before options", () => { - expect(parse(["foo", "test", "--auth", "none"])).toEqual({ + it("should allow positional arguments before options", async () => { + expect(await parse(["test", "some/project/directory", "--auth", "none"])).toEqual({ ...createDefaultArgs(), - _: ["foo", "test"], + _: ["test"], auth: "none", + folder: path.resolve(process.cwd(), "some/project/directory"), }) }) - it("should support repeatable flags", () => { - expect(parse(["--proxy-domain", "*.coder.com"])).toEqual({ + it("should support repeatable flags", async () => { + expect(await parse(["--proxy-domain", "*.coder.com"])).toEqual({ ...createDefaultArgs(), "proxy-domain": ["*.coder.com"], }) - expect(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ + expect(await parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ ...createDefaultArgs(), "proxy-domain": ["*.coder.com", "test.com"], }) @@ -270,7 +282,7 @@ describe("parser", () => { value: undefined, }, }) - expect(() => parse(["--cert", "test"])).toThrowError(/--cert-key is missing/) + await expect(parse(["--cert", "test"])).rejects.toThrowError(/--cert-key is missing/) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...createDefaultArgs(), @@ -295,7 +307,7 @@ describe("parser", () => { link: { value: "test", }, - port: 0, + port: "0", cert: undefined, "cert-key": path.resolve("test"), socket: undefined, @@ -336,14 +348,14 @@ describe("parser", () => { }) }) - it("should error if password passed in", () => { - expect(() => parse(["--password", "supersecret123"])).toThrowError( + it("should error if password passed in", async () => { + await expect(parse(["--password", "supersecret123"])).rejects.toThrowError( "--password can only be set in the config file or passed in via $PASSWORD", ) }) - it("should error if hashed-password passed in", () => { - expect(() => parse(["--hashed-password", "fdas423fs8a"])).toThrowError( + it("should error if hashed-password passed in", async () => { + await expect(parse(["--hashed-password", "fdas423fs8a"])).rejects.toThrowError( "--hashed-password can only be set in the config file or passed in via $HASHED_PASSWORD", ) }) @@ -371,14 +383,15 @@ describe("parser", () => { }) it("should allow '=,$/' in strings", async () => { const args = await parse([ - "--enable-proposed-api", + "--disable-update-check", "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", + "foobar", ]) expect(args).toEqual({ ...createDefaultArgs(), - "enable-proposed-api": [ - "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", - ], + "disable-update-check": true, + folder: path.resolve(process.cwd(), "foobar"), + _: ["$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy"], }) }) it("should parse options with double-dash and multiple equal signs ", async () => { From 99454285aaf9d47cd9cd289c28deb830f2c6d1a4 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 18:11:10 -0400 Subject: [PATCH 10/27] Fix issues surrounding opening files within code-server's terminal. --- src/node/cli.ts | 9 +++--- src/node/constants.ts | 1 + src/node/entry.ts | 30 +++++++++++++------ src/node/wrapper.ts | 67 +------------------------------------------ 4 files changed, 28 insertions(+), 79 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index c199c23c63b5..baa2e6e31c0c 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -737,10 +737,11 @@ export const shouldOpenInExistingInstance = async (args: Args): Promise 0) { + // 1. Check if any unrelated flags are set + // 2. That a file or directory was passed + // 3. That the socket is active + // TODO: We check against 3 because some defaults always exist. This is fragile. + if (Object.keys(args).length === 3 && args._.length > 0) { const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH) if (socketPath && (await canConnect(socketPath))) { return socketPath diff --git a/src/node/constants.ts b/src/node/constants.ts index 343457a54256..b2e53dc90e73 100644 --- a/src/node/constants.ts +++ b/src/node/constants.ts @@ -18,6 +18,7 @@ export function getPackageJson(relativePath: string): JSONSchemaForNPMPackageJso const pkg = getPackageJson("../../package.json") +export const pkgName = pkg.name || "code-server" export const version = pkg.version || "development" export const commit = pkg.commit || "development" export const rootPath = path.resolve(__dirname, "../..") diff --git a/src/node/entry.ts b/src/node/entry.ts index fc37927343d3..4d8c7bad33ff 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -1,12 +1,17 @@ import { logger } from "@coder/logger" import { optionDescriptions, parse, readConfigFile, setDefaults, shouldOpenInExistingInstance } from "./cli" -import { commit, version } from "./constants" +import { commit, pkgName, version } from "./constants" import { openInExistingInstance, runCodeServer, runVsCodeCli, shouldSpawnCliProcess } from "./main" import { monkeyPatchProxyProtocols } from "./proxy_agent" -import { isChild, wrapper } from "./wrapper" +import { loadAMDModule } from "./util" +import { wrapper } from "./wrapper" + +const cliPipe = process.env["VSCODE_IPC_HOOK_CLI"] as string +const cliCommand = process.env["VSCODE_CLIENT_COMMAND"] as string async function entry(): Promise { monkeyPatchProxyProtocols() + const cliArgs = await parse(process.argv.slice(2)) // There's no need to check flags like --help or to spawn in an existing // instance for the child process because these would have already happened in @@ -14,15 +19,22 @@ async function entry(): Promise { // arguments from the parent so we don't have to parse twice and to account // for environment manipulation (like how PASSWORD gets removed to avoid // leaking to child processes). - if (isChild(wrapper)) { - const args = await wrapper.handshake() - wrapper.preventExit() - const server = await runCodeServer(args) - wrapper.onDispose(() => server.dispose()) + + if (cliPipe || cliCommand) { + const remoteAgentMain = await loadAMDModule("vs/server/remoteCli", "main") + + remoteAgentMain( + { + productName: pkgName, + version, + commit, + executableName: pkgName, + }, + process.argv.slice(2), + ) return } - const cliArgs = await parse(process.argv.slice(2)) const configArgs = await readConfigFile(cliArgs.config) const args = await setDefaults(cliArgs, configArgs) @@ -64,7 +76,7 @@ async function entry(): Promise { return openInExistingInstance(args, socketPath) } - return wrapper.start(args) + runCodeServer(args) } entry().catch((error) => { diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index c645fe83557d..424aecea5c76 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -143,64 +143,6 @@ abstract class Process { } } -/** - * Child process that will clean up after itself if the parent goes away and can - * perform a handshake with the parent and ask it to relaunch. - */ -class ChildProcess extends Process { - public logger = logger.named(`child:${process.pid}`) - - public constructor(private readonly parentPid: number) { - super() - - // Kill the inner process if the parent dies. This is for the case where the - // parent process is forcefully terminated and cannot clean up. - setInterval(() => { - try { - // process.kill throws an exception if the process doesn't exist. - process.kill(this.parentPid, 0) - } catch (_) { - // Consider this an error since it should have been able to clean up - // the child process unless it was forcefully killed. - this.logger.error(`parent process ${parentPid} died`) - this._onDispose.emit(undefined) - } - }, 5000) - } - - /** - * Initiate the handshake and wait for a response from the parent. - */ - public async handshake(): Promise { - this.send({ type: "handshake" }) - const message = await onMessage( - process, - (message): message is ParentHandshakeMessage => { - return message.type === "handshake" - }, - this.logger, - ) - return message.args - } - - /** - * Notify the parent process that it should relaunch the child. - */ - public relaunch(version: string): void { - this.send({ type: "relaunch", version }) - } - - /** - * Send a message to the parent. - */ - private send(message: ChildMessage): void { - if (!process.send) { - throw new Error("not spawned with IPC") - } - process.send(message) - } -} - /** * Parent process wrapper that spawns the child process and performs a handshake * with it. Will relaunch the child if it receives a SIGUSR1 or is asked to by @@ -346,14 +288,7 @@ export class ParentProcess extends Process { /** * Process wrapper. */ -export const wrapper = - typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" - ? new ChildProcess(parseInt(process.env.CODE_SERVER_PARENT_PID)) - : new ParentProcess(require("../../package.json").version) - -export function isChild(proc: ChildProcess | ParentProcess): proc is ChildProcess { - return proc instanceof ChildProcess -} +export const wrapper = new ParentProcess(require("../../package.json").version) // It's possible that the pipe has closed (for example if you run code-server // --version | head -1). Assume that means we're done. From 93adec731a33aec0c68e1d20465ce355c45aab29 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Thu, 4 Nov 2021 18:50:48 -0400 Subject: [PATCH 11/27] Bump vendor. --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 79c310a58465..1109c4d5e0b6 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#3610be0c6e7f3def890289c0927a3135ccfdb94d" + "code-oss-dev": "cdr/vscode#39ebef60dbbda4c23c8fdc0dc838e1d7484af612" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 3b3a16106eca..6d4779e2bcf9 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#3610be0c6e7f3def890289c0927a3135ccfdb94d: +code-oss-dev@cdr/vscode#39ebef60dbbda4c23c8fdc0dc838e1d7484af612: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/3610be0c6e7f3def890289c0927a3135ccfdb94d" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/39ebef60dbbda4c23c8fdc0dc838e1d7484af612" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 28e0d78da8723ee1c8407cdc987036f28bfc231b Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 5 Nov 2021 15:42:56 +0000 Subject: [PATCH 12/27] Update VS Code --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 1109c4d5e0b6..1e977ebc6b27 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#39ebef60dbbda4c23c8fdc0dc838e1d7484af612" + "code-oss-dev": "cdr/vscode#b93785c1438bc6e92f38c0842386afd8666c73f0" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 6d4779e2bcf9..c0abbd4d9d70 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#39ebef60dbbda4c23c8fdc0dc838e1d7484af612: +code-oss-dev@cdr/vscode#b93785c1438bc6e92f38c0842386afd8666c73f0: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/39ebef60dbbda4c23c8fdc0dc838e1d7484af612" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/b93785c1438bc6e92f38c0842386afd8666c73f0" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 76e6cccca4263bf19d6e9c39c01e3e8711924c92 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 5 Nov 2021 12:39:53 -0400 Subject: [PATCH 13/27] Readd parent wrapper for hot reload. --- src/node/entry.ts | 27 +++++++++++------- src/node/wrapper.ts | 67 ++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 11 deletions(-) diff --git a/src/node/entry.ts b/src/node/entry.ts index 4d8c7bad33ff..1f6d634429a5 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -4,21 +4,13 @@ import { commit, pkgName, version } from "./constants" import { openInExistingInstance, runCodeServer, runVsCodeCli, shouldSpawnCliProcess } from "./main" import { monkeyPatchProxyProtocols } from "./proxy_agent" import { loadAMDModule } from "./util" -import { wrapper } from "./wrapper" +import { isChild, wrapper } from "./wrapper" const cliPipe = process.env["VSCODE_IPC_HOOK_CLI"] as string const cliCommand = process.env["VSCODE_CLIENT_COMMAND"] as string async function entry(): Promise { monkeyPatchProxyProtocols() - const cliArgs = await parse(process.argv.slice(2)) - - // There's no need to check flags like --help or to spawn in an existing - // instance for the child process because these would have already happened in - // the parent and the child wouldn't have been spawned. We also get the - // arguments from the parent so we don't have to parse twice and to account - // for environment manipulation (like how PASSWORD gets removed to avoid - // leaking to child processes). if (cliPipe || cliCommand) { const remoteAgentMain = await loadAMDModule("vs/server/remoteCli", "main") @@ -35,6 +27,21 @@ async function entry(): Promise { return } + // There's no need to check flags like --help or to spawn in an existing + // instance for the child process because these would have already happened in + // the parent and the child wouldn't have been spawned. We also get the + // arguments from the parent so we don't have to parse twice and to account + // for environment manipulation (like how PASSWORD gets removed to avoid + // leaking to child processes). + if (isChild(wrapper)) { + const args = await wrapper.handshake() + wrapper.preventExit() + const server = await runCodeServer(args) + wrapper.onDispose(() => server.dispose()) + return + } + + const cliArgs = await parse(process.argv.slice(2)) const configArgs = await readConfigFile(cliArgs.config) const args = await setDefaults(cliArgs, configArgs) @@ -76,7 +83,7 @@ async function entry(): Promise { return openInExistingInstance(args, socketPath) } - runCodeServer(args) + return wrapper.start(args) } entry().catch((error) => { diff --git a/src/node/wrapper.ts b/src/node/wrapper.ts index 424aecea5c76..c645fe83557d 100644 --- a/src/node/wrapper.ts +++ b/src/node/wrapper.ts @@ -143,6 +143,64 @@ abstract class Process { } } +/** + * Child process that will clean up after itself if the parent goes away and can + * perform a handshake with the parent and ask it to relaunch. + */ +class ChildProcess extends Process { + public logger = logger.named(`child:${process.pid}`) + + public constructor(private readonly parentPid: number) { + super() + + // Kill the inner process if the parent dies. This is for the case where the + // parent process is forcefully terminated and cannot clean up. + setInterval(() => { + try { + // process.kill throws an exception if the process doesn't exist. + process.kill(this.parentPid, 0) + } catch (_) { + // Consider this an error since it should have been able to clean up + // the child process unless it was forcefully killed. + this.logger.error(`parent process ${parentPid} died`) + this._onDispose.emit(undefined) + } + }, 5000) + } + + /** + * Initiate the handshake and wait for a response from the parent. + */ + public async handshake(): Promise { + this.send({ type: "handshake" }) + const message = await onMessage( + process, + (message): message is ParentHandshakeMessage => { + return message.type === "handshake" + }, + this.logger, + ) + return message.args + } + + /** + * Notify the parent process that it should relaunch the child. + */ + public relaunch(version: string): void { + this.send({ type: "relaunch", version }) + } + + /** + * Send a message to the parent. + */ + private send(message: ChildMessage): void { + if (!process.send) { + throw new Error("not spawned with IPC") + } + process.send(message) + } +} + /** * Parent process wrapper that spawns the child process and performs a handshake * with it. Will relaunch the child if it receives a SIGUSR1 or is asked to by @@ -288,7 +346,14 @@ export class ParentProcess extends Process { /** * Process wrapper. */ -export const wrapper = new ParentProcess(require("../../package.json").version) +export const wrapper = + typeof process.env.CODE_SERVER_PARENT_PID !== "undefined" + ? new ChildProcess(parseInt(process.env.CODE_SERVER_PARENT_PID)) + : new ParentProcess(require("../../package.json").version) + +export function isChild(proc: ChildProcess | ParentProcess): proc is ChildProcess { + return proc instanceof ChildProcess +} // It's possible that the pipe has closed (for example if you run code-server // --version | head -1). Assume that means we're done. From efada23c4bc9fd52ed32cfd8bd781bb427de29b9 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 5 Nov 2021 15:39:16 -0400 Subject: [PATCH 14/27] Allow more errors. --- src/node/routes/vscode.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index 078e40bf7021..5ee56cd10f8d 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -63,7 +63,7 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise { req.on("error", (error: any) => { - if (error.code === "FileNotFound") { + if (error instanceof Error && ["EntryNotFound", "FileNotFound", "HttpError"].includes(error.message)) { next() } From 1f7e8a1ec75c91094584f91160f5803ee350db44 Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 5 Nov 2021 15:39:55 -0400 Subject: [PATCH 15/27] Bump vscode. --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 1e977ebc6b27..5707f99d6ae9 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#b93785c1438bc6e92f38c0842386afd8666c73f0" + "code-oss-dev": "cdr/vscode#f8de7134f6fdb8bb125b40a0ccae5a173d700056" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index c0abbd4d9d70..e992dcf40bd0 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#b93785c1438bc6e92f38c0842386afd8666c73f0: +code-oss-dev@cdr/vscode#f8de7134f6fdb8bb125b40a0ccae5a173d700056: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/b93785c1438bc6e92f38c0842386afd8666c73f0" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/f8de7134f6fdb8bb125b40a0ccae5a173d700056" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 914aad25cf50867ff742babf36200909d69ee0df Mon Sep 17 00:00:00 2001 From: Teffen Ellis Date: Fri, 5 Nov 2021 17:51:45 -0400 Subject: [PATCH 16/27] Fix issues surrounding Coder link. --- src/node/cli.ts | 2 ++ src/node/routes/vscode.ts | 10 +++++++++- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/node/cli.ts b/src/node/cli.ts index baa2e6e31c0c..19f5ebf1894a 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -158,6 +158,8 @@ const options: Options> = { // These two have been deprecated by bindAddr. host: { type: "string", description: "" }, port: { type: "string", description: "" }, + // Retained for VS Code compatibility. + protocol: { type: "string", description: "" }, socket: { type: "string", path: true, description: "Path to a socket (bind-addr will be ignored)." }, version: { type: "boolean", short: "v", description: "Display version information." }, diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index 5ee56cd10f8d..dccd15bcbd8c 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -45,7 +45,15 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise Date: Fri, 5 Nov 2021 22:05:55 +0000 Subject: [PATCH 17/27] Add dir creation and fix cli It seems VS Code explodes when certain directories do not exist so import the reh agent instead of the server component since it creates the directories (require patching thus the VS Code update). Also the CLI (for installing extensions) did not seem to be working so point that to the same place since it also exports a function for running that part of the CLI. --- src/node/entry.ts | 2 +- src/node/main.ts | 33 +++++---------------------------- src/node/routes/vscode.ts | 16 ++++++---------- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 5 files changed, 15 insertions(+), 42 deletions(-) diff --git a/src/node/entry.ts b/src/node/entry.ts index 1f6d634429a5..74276f95651e 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -75,7 +75,7 @@ async function entry(): Promise { } if (await shouldSpawnCliProcess(args)) { - return runVsCodeCli() + return runVsCodeCli(args) } const socketPath = await shouldOpenInExistingInstance(cliArgs) diff --git a/src/node/main.ts b/src/node/main.ts index bdc7eeec70ae..16a73f14f6ae 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -6,7 +6,7 @@ import { plural } from "../common/util" import { createApp, ensureAddress } from "./app" import { AuthType, DefaultedArgs, Feature } from "./cli" import { coderCloudBind } from "./coder_cloud" -import { commit, version, vsRootPath } from "./constants" +import { commit, version } from "./constants" import { register } from "./routes" import { humanPath, isFile, loadAMDModule, open } from "./util" @@ -26,37 +26,14 @@ export const shouldSpawnCliProcess = async (args: CodeServerLib.ServerParsedArgs * such as when managing extensions. * @deprecated This should be removed when code-server merges with lib/vscode. */ -export const runVsCodeCli = async (): Promise => { +export const runVsCodeCli = async (args: DefaultedArgs): Promise => { logger.debug("Running VS Code CLI") - // Delete `VSCODE_CWD` very early even before - // importing bootstrap files. We have seen - // reports where `code .` would use the wrong - // current working directory due to our variable - // somehow escaping to the parent shell - // (https://github.com/microsoft/vscode/issues/126399) - delete process.env["VSCODE_CWD"] - - const bootstrap = require(path.join(vsRootPath, "out", "bootstrap")) - const bootstrapNode = require(path.join(vsRootPath, "out", "bootstrap-node")) - const product = require(path.join(vsRootPath, "product.json")) - - // Avoid Monkey Patches from Application Insights - bootstrap.avoidMonkeyPatchFromAppInsights() - - // Enable portable support - bootstrapNode.configurePortable(product) - - // Enable ASAR support - bootstrap.enableASARSupport() - - // Signal processes that we got launched as CLI - process.env["VSCODE_CLI"] = "1" - - const cliProcessMain = await loadAMDModule("vs/code/node/cli", "initialize") + // See ../../vendor/modules/code-oss-dev/src/vs/server/main.js. + const spawnCli = await loadAMDModule("vs/server/remoteExtensionHostAgent", "spawnCli") try { - await cliProcessMain(process.argv) + await spawnCli(args) } catch (error: any) { logger.error("Got error from VS Code", error) } diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index dccd15bcbd8c..662d70177718 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -41,19 +41,15 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise( - "vs/server/remoteExtensionHostAgentServer", + "vs/server/remoteExtensionHostAgent", "createServer", ) - const codeServerMain = await createVSServer( - null, - { - ...args, - protocol: args["cert"] || args.link ? "https:" : "http:", - connectionToken: "0000", - }, - args["user-data-dir"], - ) + const codeServerMain = await createVSServer(null, { + ...args, + protocol: args["cert"] || args.link ? "https:" : "http:", + connectionToken: "0000", + }) const router = express.Router() const wsRouter = WsRouter() diff --git a/vendor/package.json b/vendor/package.json index 3e96d7c7905f..1a1cfd115c59 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#57c765fee66863583e48081244e18e7a79ca61a0" + "code-oss-dev": "cdr/vscode#122c0fa63769cf71c5fda4eee156e3c9fb57c3c0" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 4da2c57c41a1..0edcee65f692 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#57c765fee66863583e48081244e18e7a79ca61a0: +code-oss-dev@cdr/vscode#122c0fa63769cf71c5fda4eee156e3c9fb57c3c0: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/57c765fee66863583e48081244e18e7a79ca61a0" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/122c0fa63769cf71c5fda4eee156e3c9fb57c3c0" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 55d878e21ab587c7836a16360af383396b4ec125 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 8 Nov 2021 16:55:58 +0000 Subject: [PATCH 18/27] Remove hardcoded VSCODE_DEV=1 This causes VS Code to use the development HTML file. Move this to the watch command instead. I deleted the other stuff before it as well since in the latest main.js they do not have this code so I figure we should be safe to omit it. --- package.json | 2 +- src/node/routes/vscode.ts | 29 +---------------------------- 2 files changed, 2 insertions(+), 29 deletions(-) diff --git a/package.json b/package.json index 7babaa8e267e..d8a5fd891679 100644 --- a/package.json +++ b/package.json @@ -28,7 +28,7 @@ "lint": "./ci/dev/lint.sh", "test": "echo 'Run yarn test:unit or yarn test:e2e' && exit 1", "ci": "./ci/dev/ci.sh", - "watch": "VSCODE_IPC_HOOK_CLI= NODE_OPTIONS='--max_old_space_size=32384 --trace-warnings' ts-node ./ci/dev/watch.ts", + "watch": "VSCODE_DEV=1 VSCODE_IPC_HOOK_CLI= NODE_OPTIONS='--max_old_space_size=32384 --trace-warnings' ts-node ./ci/dev/watch.ts", "icons": "./ci/dev/gen_icons.sh", "coverage": "codecov" }, diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index 662d70177718..46a0d548db8d 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -1,7 +1,5 @@ import * as express from "express" -import path from "path" import { DefaultedArgs } from "../cli" -import { vsRootPath } from "../constants" import { ensureAuthenticated, authenticated, redirect } from "../http" import { loadAMDModule } from "../util" import { Router as WsRouter, WebsocketRouter } from "../wsRouter" @@ -14,32 +12,7 @@ export interface VSServerResult { } export const createVSServerRouter = async (args: DefaultedArgs): Promise => { - // Delete `VSCODE_CWD` very early even before - // importing bootstrap files. We have seen - // reports where `code .` would use the wrong - // current working directory due to our variable - // somehow escaping to the parent shell - // (https://github.com/microsoft/vscode/issues/126399) - delete process.env["VSCODE_CWD"] - - const bootstrap = require(path.join(vsRootPath, "out", "bootstrap")) - const bootstrapNode = require(path.join(vsRootPath, "out", "bootstrap-node")) - const product = require(path.join(vsRootPath, "product.json")) - - // Avoid Monkey Patches from Application Insights - bootstrap.avoidMonkeyPatchFromAppInsights() - - // Enable portable support - bootstrapNode.configurePortable(product) - - // Enable ASAR support - bootstrap.enableASARSupport() - - // Signal processes that we got launched as CLI - process.env["VSCODE_CLI"] = "1" - // Seems to be necessary to load modules properly. - process.env["VSCODE_DEV"] = "1" - + // See ../../../vendor/modules/code-oss-dev/src/vs/server/main.js. const createVSServer = await loadAMDModule( "vs/server/remoteExtensionHostAgent", "createServer", From d03c9f5dc9abecc2026fb092908e6f9e8ea2e52c Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 8 Nov 2021 17:14:14 +0000 Subject: [PATCH 19/27] Fix mismatching commit between client and server --- ci/build/build-release.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/build/build-release.sh b/ci/build/build-release.sh index 82e2585fdff8..7a13ded94c45 100755 --- a/ci/build/build-release.sh +++ b/ci/build/build-release.sh @@ -88,7 +88,7 @@ bundle_vscode() { cat << EOF { "enableTelemetry": true, - "commit": "$(git rev-parse HEAD)", + "commit": "$(cd "$VSCODE_SRC_PATH" && git rev-parse HEAD)", "quality": "stable", "date": $(jq -n 'now | todate') } From 8244463fb135970108e8c4d0bb8212caed376ae4 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 8 Nov 2021 21:26:10 +0000 Subject: [PATCH 20/27] Mostly restore command-line parity Restore most everything and remove the added server arguments. This will let us add and remove options after later so we can contain the number of breaking changes. To accomplish this a hard separation is added between the CLI arguments and the server arguments. The separation between user-provided arguments and arguments with defaults is also made more clear. The extra directory flags have been left out as they were buggy and should be implemented upstream although I think there are better solutions anyway. locale and install-source are unsupported with the web remote and are left removed. It is unclear whether they were used before anyway. Some restored flags still need to have their behavior re-implemented. --- src/node/app.ts | 2 +- src/node/cli.ts | 202 ++++++++++++++++---------------- src/node/entry.ts | 6 +- src/node/main.ts | 28 +++-- src/node/routes/vscode.ts | 5 +- test/unit/node/app.test.ts | 13 +-- test/unit/node/cli.test.ts | 213 +++++++++++++--------------------- test/unit/node/plugin.test.ts | 2 +- test/utils/integration.ts | 4 +- 9 files changed, 216 insertions(+), 259 deletions(-) diff --git a/src/node/app.ts b/src/node/app.ts index 238d74cf36de..1387135583d5 100644 --- a/src/node/app.ts +++ b/src/node/app.ts @@ -44,7 +44,7 @@ const listen = (server: http.Server, { host, port, socket }: ListenOptions) => { server.listen(socket, onListen) } else { // [] is the correct format when using :: but Node errors with them. - server.listen(parseInt(port, 10), host.replace(/^\[|\]$/g, ""), onListen) + server.listen(port, host.replace(/^\[|\]$/g, ""), onListen) } }) } diff --git a/src/node/cli.ts b/src/node/cli.ts index 19f5ebf1894a..d9bbca0e2658 100644 --- a/src/node/cli.ts +++ b/src/node/cli.ts @@ -39,7 +39,13 @@ export enum LogLevel { export class OptionalString extends Optional {} -export interface Args extends CodeServerLib.ServerParsedArgs { +/** + * Arguments that the user explicitly provided on the command line. All + * arguments must be optional. + * + * For arguments with defaults see DefaultedArgs. + */ +export interface UserProvidedArgs { config?: string auth?: AuthType password?: string @@ -47,25 +53,39 @@ export interface Args extends CodeServerLib.ServerParsedArgs { cert?: OptionalString "cert-host"?: string "cert-key"?: string - "disable-telemetry"?: boolean "disable-update-check"?: boolean enable?: string[] help?: boolean host?: string + port?: number json?: boolean log?: LogLevel open?: boolean "bind-addr"?: string socket?: string version?: boolean - force?: boolean - "show-versions"?: boolean "proxy-domain"?: string[] "reuse-window"?: boolean "new-window"?: boolean + "ignore-last-opened"?: boolean + link?: OptionalString verbose?: boolean + /* Positional arguments. */ + _?: string[] - link?: OptionalString + // VS Code flags. + "disable-telemetry"?: boolean + force?: boolean + "user-data-dir"?: string + "enable-proposed-api"?: string[] + "extensions-dir"?: string + "builtin-extensions-dir"?: string + "install-extension"?: string[] + "uninstall-extension"?: string[] + "list-extensions"?: boolean + "locate-extension"?: string[] + "show-versions"?: boolean + category?: string } interface Option { @@ -109,7 +129,7 @@ type Options = { [P in keyof T]: Option> } -const options: Options> = { +const options: Options> = { auth: { type: AuthType, description: "The type of authentication to use." }, password: { type: "string", @@ -157,9 +177,7 @@ const options: Options> = { // These two have been deprecated by bindAddr. host: { type: "string", description: "" }, - port: { type: "string", description: "" }, - // Retained for VS Code compatibility. - protocol: { type: "string", description: "" }, + port: { type: "number", description: "" }, socket: { type: "string", path: true, description: "Path to a socket (bind-addr will be ignored)." }, version: { type: "boolean", short: "v", description: "Display version information." }, @@ -171,15 +189,26 @@ const options: Options> = { "list-extensions": { type: "boolean", description: "List installed VS Code extensions." }, force: { type: "boolean", description: "Avoid prompts when installing VS Code extensions." }, "locate-extension": { type: "string[]" }, + category: { type: "string" }, "install-extension": { type: "string[]", description: "Install or update a VS Code extension by id or vsix. The identifier of an extension is `${publisher}.${name}`.\n" + "To install a specific version provide `@${version}`. For example: 'vscode.csharp@1.2.3'.", }, + "enable-proposed-api": { + type: "string[]", + description: + "Enable proposed API features for extensions. Can receive one or more extension IDs to enable individually.", + }, "uninstall-extension": { type: "string[]", description: "Uninstall a VS Code extension by id." }, "show-versions": { type: "boolean", description: "Show VS Code extension versions." }, "proxy-domain": { type: "string[]", description: "Domain used for proxying ports." }, + "ignore-last-opened": { + type: "boolean", + short: "e", + description: "Ignore the last opened directory or workspace in favor of an empty window.", + }, "new-window": { type: "boolean", short: "n", @@ -203,43 +232,6 @@ const options: Options> = { `, beta: true, }, - - connectionToken: { type: "string" }, - "connection-secret": { - type: "string", - description: - "Path to file that contains the connection token. This will require that all incoming connections know the secret.", - }, - "socket-path": { type: "string" }, - driver: { type: "string" }, - "start-server": { type: "boolean" }, - "print-startup-performance": { type: "boolean" }, - "print-ip-address": { type: "boolean" }, - "disable-websocket-compression": { type: "boolean" }, - - fileWatcherPolling: { type: "string" }, - - "enable-remote-auto-shutdown": { type: "boolean" }, - "remote-auto-shutdown-without-delay": { type: "boolean" }, - - "without-browser-env-var": { type: "boolean" }, - "extensions-download-dir": { type: "string" }, - "install-builtin-extension": { type: "string[]" }, - - category: { - type: "string", - description: "Filters installed extensions by provided category, when using --list-extensions.", - }, - "do-not-sync": { type: "boolean" }, - "force-disable-user-env": { type: "boolean" }, - - folder: { type: "string" }, - workspace: { type: "string" }, - "web-user-data-dir": { type: "string" }, - "use-host-proxy": { type: "string" }, - "enable-sync": { type: "boolean" }, - "github-auth": { type: "string" }, - logsPath: { type: "string" }, } export const optionDescriptions = (): string[] => { @@ -284,20 +276,16 @@ export function splitOnFirstEquals(str: string): string[] { return split } -export const createDefaultArgs = (): Args => { - return { - _: [], - workspace: "", - folder: "", - } -} - -export const parse = async ( +/** + * Parse arguments into UserProvidedArgs. This should not go beyond checking + * that arguments are valid types and have values when required. + */ +export const parse = ( argv: string[], opts?: { configFile?: string }, -): Promise => { +): UserProvidedArgs => { const error = (msg: string): Error => { if (opts?.configFile) { msg = `error reading ${opts.configFile}: ${msg}` @@ -306,7 +294,7 @@ export const parse = async ( return new Error(msg) } - const args: Args = createDefaultArgs() + const args: UserProvidedArgs = {} let ended = false for (let i = 0; i < argv.length; ++i) { @@ -320,17 +308,17 @@ export const parse = async ( // Options start with a dash and require a value if non-boolean. if (!ended && arg.startsWith("-")) { - let key: keyof Args | undefined + let key: keyof UserProvidedArgs | undefined let value: string | undefined if (arg.startsWith("--")) { const split = splitOnFirstEquals(arg.replace(/^--/, "")) - key = split[0] as keyof Args + key = split[0] as keyof UserProvidedArgs value = split[1] } else { const short = arg.replace(/^-/, "") const pair = Object.entries(options).find(([, v]) => v.short === short) if (pair) { - key = pair[0] as keyof Args + key = pair[0] as keyof UserProvidedArgs } } @@ -405,24 +393,11 @@ export const parse = async ( } // Everything else goes into _. - args._.push(arg) - } - - if (args.port && isNaN(parseInt(args.port, 10))) { - throw new Error("--port must be a number") - } - - if (args._.length && !args.folder && !args.workspace) { - const lastEntry = path.resolve(process.cwd(), args._[args._.length - 1]) - const entryIsFile = await isFile(lastEntry) - - if (entryIsFile && path.extname(lastEntry) === ".code-workspace") { - args.workspace = lastEntry - args._.pop() - } else if (!entryIsFile) { - args.folder = lastEntry - args._.pop() + if (typeof args._ === "undefined") { + args._ = [] } + + args._.push(arg) } // If a cert was provided a key must also be provided. @@ -435,19 +410,28 @@ export const parse = async ( return args } +/** + * User-provided arguments with defaults. The distinction between user-provided + * args and defaulted args exists so we can tell the difference between end + * values and what the user actually provided on the command line. + */ export interface DefaultedArgs extends ConfigArgs { auth: AuthType cert?: { value: string } host: string - port: string + port: number "proxy-domain": string[] verbose: boolean usingEnvPassword: boolean usingEnvHashedPassword: boolean "extensions-dir": string "user-data-dir": string + /* Positional arguments. */ + _: [] + folder: string + workspace: string } /** @@ -455,7 +439,7 @@ export interface DefaultedArgs extends ConfigArgs { * with the defaults set. Arguments from the CLI are prioritized over config * arguments. */ -export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promise { +export async function setDefaults(cliArgs: UserProvidedArgs, configArgs?: ConfigArgs): Promise { const args = Object.assign({}, configArgs || {}, cliArgs) if (!args["user-data-dir"]) { @@ -510,15 +494,15 @@ export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promi args.auth = AuthType.Password } - const addr = bindAddrFromAllSources(configArgs || createDefaultArgs(), cliArgs) + const addr = bindAddrFromAllSources(configArgs || {}, cliArgs) args.host = addr.host - args.port = addr.port.toString() + args.port = addr.port // If we're being exposed to the cloud, we listen on a random address and // disable auth. if (args.link) { args.host = "localhost" - args.port = "0" + args.port = 0 args.socket = undefined args.cert = undefined args.auth = AuthType.None @@ -551,8 +535,29 @@ export async function setDefaults(cliArgs: Args, configArgs?: ConfigArgs): Promi const proxyDomains = new Set((args["proxy-domain"] || []).map((d) => d.replace(/^\*\./, ""))) args["proxy-domain"] = Array.from(proxyDomains) + if (typeof args._ === "undefined") { + args._ = [] + } + + let workspace = "" + let folder = "" + if (args._.length) { + const lastEntry = path.resolve(process.cwd(), args._[args._.length - 1]) + const entryIsFile = await isFile(lastEntry) + + if (entryIsFile && path.extname(lastEntry) === ".code-workspace") { + workspace = lastEntry + args._.pop() + } else if (!entryIsFile) { + folder = lastEntry + args._.pop() + } + } + return { ...args, + workspace, + folder, usingEnvPassword, usingEnvHashedPassword, } as DefaultedArgs // TODO: Technically no guarantee this is fulfilled. @@ -577,7 +582,7 @@ cert: false ` } -interface ConfigArgs extends Args { +interface ConfigArgs extends UserProvidedArgs { config: string } @@ -617,9 +622,9 @@ export async function readConfigFile(configPath?: string): Promise { * parseConfigFile parses configFile into ConfigArgs. * configPath is used as the filename in error messages */ -export async function parseConfigFile(configFile: string, configPath: string): Promise { +export function parseConfigFile(configFile: string, configPath: string): ConfigArgs { if (!configFile) { - return { ...createDefaultArgs(), config: configPath } + return { config: configPath } } const config = yaml.load(configFile, { @@ -637,7 +642,7 @@ export async function parseConfigFile(configFile: string, configPath: string): P } return `--${optName}=${opt}` }) - const args = await parse(configFileArgv, { + const args = parse(configFileArgv, { configFile: configPath, }) return { @@ -666,7 +671,7 @@ interface Addr { * This function creates the bind address * using the CLI args. */ -export function bindAddrFromArgs(addr: Addr, args: Args): Addr { +export function bindAddrFromArgs(addr: Addr, args: UserProvidedArgs): Addr { addr = { ...addr } if (args["bind-addr"]) { addr = parseBindAddr(args["bind-addr"]) @@ -679,12 +684,12 @@ export function bindAddrFromArgs(addr: Addr, args: Args): Addr { addr.port = parseInt(process.env.PORT, 10) } if (args.port !== undefined) { - addr.port = parseInt(args.port, 10) + addr.port = args.port } return addr } -function bindAddrFromAllSources(...argsConfig: Args[]): Addr { +function bindAddrFromAllSources(...argsConfig: UserProvidedArgs[]): Addr { let addr: Addr = { host: "localhost", port: 8080, @@ -721,31 +726,34 @@ export async function readSocketPath(path: string): Promise /** * Determine if it looks like the user is trying to open a file or folder in an * existing instance. The arguments here should be the arguments the user - * explicitly passed on the command line, not defaults or the configuration. + * explicitly passed on the command line, *NOT DEFAULTS* or the configuration. */ -export const shouldOpenInExistingInstance = async (args: Args): Promise => { +export const shouldOpenInExistingInstance = async (args: UserProvidedArgs): Promise => { // Always use the existing instance if we're running from VS Code's terminal. if (process.env.VSCODE_IPC_HOOK_CLI) { + logger.debug("Found VSCODE_IPC_HOOK_CLI") return process.env.VSCODE_IPC_HOOK_CLI } // If these flags are set then assume the user is trying to open in an // existing instance since these flags have no effect otherwise. const openInFlagCount = ["reuse-window", "new-window"].reduce((prev, cur) => { - return args[cur as keyof Args] ? prev + 1 : prev + return args[cur as keyof UserProvidedArgs] ? prev + 1 : prev }, 0) if (openInFlagCount > 0) { + logger.debug("Found --reuse-window or --new-window") return readSocketPath(DEFAULT_SOCKET_PATH) } // It's possible the user is trying to spawn another instance of code-server. - // 1. Check if any unrelated flags are set - // 2. That a file or directory was passed - // 3. That the socket is active - // TODO: We check against 3 because some defaults always exist. This is fragile. - if (Object.keys(args).length === 3 && args._.length > 0) { + // 1. Check if any unrelated flags are set (this should only run when + // code-server is invoked exactly like this: `code-server my-file`). + // 2. That a file or directory was passed. + // 3. That the socket is active. + if (Object.keys(args).length === 1 && typeof args._ !== "undefined" && args._.length > 0) { const socketPath = await readSocketPath(DEFAULT_SOCKET_PATH) if (socketPath && (await canConnect(socketPath))) { + logger.debug("Found existing code-server socket") return socketPath } } diff --git a/src/node/entry.ts b/src/node/entry.ts index 74276f95651e..685f62f36149 100644 --- a/src/node/entry.ts +++ b/src/node/entry.ts @@ -41,7 +41,7 @@ async function entry(): Promise { return } - const cliArgs = await parse(process.argv.slice(2)) + const cliArgs = parse(process.argv.slice(2)) const configArgs = await readConfigFile(cliArgs.config) const args = await setDefaults(cliArgs, configArgs) @@ -74,12 +74,14 @@ async function entry(): Promise { return } - if (await shouldSpawnCliProcess(args)) { + if (shouldSpawnCliProcess(args)) { + logger.debug("Found VS Code arguments; spawning VS Code CLI") return runVsCodeCli(args) } const socketPath = await shouldOpenInExistingInstance(cliArgs) if (socketPath) { + logger.debug("Trying to open in existing instance") return openInExistingInstance(args, socketPath) } diff --git a/src/node/main.ts b/src/node/main.ts index 16a73f14f6ae..a4dfcee69082 100644 --- a/src/node/main.ts +++ b/src/node/main.ts @@ -4,20 +4,21 @@ import path from "path" import { Disposable } from "../common/emitter" import { plural } from "../common/util" import { createApp, ensureAddress } from "./app" -import { AuthType, DefaultedArgs, Feature } from "./cli" +import { AuthType, DefaultedArgs, Feature, UserProvidedArgs } from "./cli" import { coderCloudBind } from "./coder_cloud" import { commit, version } from "./constants" import { register } from "./routes" import { humanPath, isFile, loadAMDModule, open } from "./util" -export const shouldSpawnCliProcess = async (args: CodeServerLib.ServerParsedArgs): Promise => { +/** + * Return true if the user passed an extension-related VS Code flag. + */ +export const shouldSpawnCliProcess = (args: UserProvidedArgs): boolean => { return ( - !args["start-server"] && - (!!args["list-extensions"] || - !!args["install-extension"] || - !!args["install-builtin-extension"] || - !!args["uninstall-extension"] || - !!args["locate-extension"]) + !!args["list-extensions"] || + !!args["install-extension"] || + !!args["uninstall-extension"] || + !!args["locate-extension"] ) } @@ -33,7 +34,11 @@ export const runVsCodeCli = async (args: DefaultedArgs): Promise => { const spawnCli = await loadAMDModule("vs/server/remoteExtensionHostAgent", "spawnCli") try { - await spawnCli(args) + await spawnCli({ + ...args, + // For some reason VS Code takes the port as a string. + port: typeof args.port !== "undefined" ? args.port.toString() : undefined, + }) } catch (error: any) { logger.error("Got error from VS Code", error) } @@ -49,8 +54,9 @@ export const openInExistingInstance = async (args: DefaultedArgs, socketPath: st forceReuseWindow: args["reuse-window"], forceNewWindow: args["new-window"], } - for (let i = 0; i < args._.length; i++) { - const fp = path.resolve(args._[i]) + const paths = args._ || [] + for (let i = 0; i < paths.length; i++) { + const fp = path.resolve(paths[i]) if (await isFile(fp)) { pipeArgs.fileURIs.push(fp) } else { diff --git a/src/node/routes/vscode.ts b/src/node/routes/vscode.ts index 46a0d548db8d..76efb93402ed 100644 --- a/src/node/routes/vscode.ts +++ b/src/node/routes/vscode.ts @@ -19,9 +19,10 @@ export const createVSServerRouter = async (args: DefaultedArgs): Promise { let spy: jest.SpyInstance let unlinkSpy: jest.SpyInstance - let port: string + let port: number let tmpDirPath: string let tmpFilePath: string @@ -29,7 +29,7 @@ describe("createApp", () => { // then you can spy on those modules methods, like unlink. // See: https://github.com/aelbore/esbuild-jest/issues/26#issuecomment-893763840 unlinkSpy = jest.spyOn(promises, "unlink") - port = (await getAvailablePort()).toString() + port = await getAvailablePort() }) afterEach(() => { @@ -44,7 +44,6 @@ describe("createApp", () => { it("should return an Express app, a WebSockets Express app and an http server", async () => { const defaultArgs = await setDefaults({ - ...createDefaultArgs(), port, }) const app = await createApp(defaultArgs) @@ -61,7 +60,6 @@ describe("createApp", () => { it("should handle error events on the server", async () => { const defaultArgs = await setDefaults({ - ...createDefaultArgs(), port, }) @@ -82,9 +80,8 @@ describe("createApp", () => { it("should reject errors that happen before the server can listen", async () => { // We listen on an invalid port // causing the app to reject the Promise called at startup - const port = "2" + const port = 2 const defaultArgs = await setDefaults({ - ...createDefaultArgs(), port, }) @@ -105,7 +102,6 @@ describe("createApp", () => { it("should unlink a socket before listening on the socket", async () => { await promises.writeFile(tmpFilePath, "") const defaultArgs = await setDefaults({ - ...createDefaultArgs(), socket: tmpFilePath, }) @@ -119,7 +115,6 @@ describe("createApp", () => { const testCertificate = await generateCertificate("localhost") const cert = new OptionalString(testCertificate.cert) const defaultArgs = await setDefaults({ - ...createDefaultArgs(), port, cert, ["cert-key"]: testCertificate.certKey, diff --git a/test/unit/node/cli.test.ts b/test/unit/node/cli.test.ts index af4d15b1c636..0dcf24f4b8cc 100644 --- a/test/unit/node/cli.test.ts +++ b/test/unit/node/cli.test.ts @@ -4,24 +4,19 @@ import * as net from "net" import * as os from "os" import * as path from "path" import { - Args, + UserProvidedArgs, bindAddrFromArgs, - createDefaultArgs, defaultConfigFile, parse, + readSocketPath, setDefaults, shouldOpenInExistingInstance, splitOnFirstEquals, - readSocketPath, } from "../../../src/node/cli" import { shouldSpawnCliProcess } from "../../../src/node/main" import { generatePassword, paths } from "../../../src/node/util" import { useEnv, tmpdir } from "../../utils/helpers" -type Mutable = { - -readonly [P in keyof T]: T[P] -} - describe("parser", () => { beforeEach(() => { delete process.env.LOG_LEVEL @@ -33,24 +28,26 @@ describe("parser", () => { // values the user actually set. These are only set after explicitly calling // `setDefaults`. const defaults = { - ...createDefaultArgs(), auth: "password", host: "localhost", - port: "8080", + port: 8080, "proxy-domain": [], usingEnvPassword: false, usingEnvHashedPassword: false, "extensions-dir": path.join(paths.data, "extensions"), "user-data-dir": paths.data, + _: [], + workspace: "", + folder: "", } it("should parse nothing", async () => { - expect(await parse([])).toStrictEqual(createDefaultArgs()) + expect(parse([])).toStrictEqual({}) }) it("should parse all available options", async () => { expect( - await parse( + parse( [ ["--enable", "feature1"], ["--enable", "feature2"], @@ -59,9 +56,9 @@ describe("parser", () => { ["--auth", "none"], - ["--extensions-dir", "some/ext/dir"], + ["--extensions-dir", "path/to/ext/dir"], - ["--builtin-extensions-dir", "some/built/ext/dir"], + ["--builtin-extensions-dir", "path/to/builtin/ext/dir"], "1", "--verbose", @@ -77,12 +74,9 @@ describe("parser", () => { "3", - "--user-data-dir", - "some/user/dir", + ["--user-data-dir", "path/to/user/dir"], - "--cert=Foo", - "--cert-key", - "someCertKeyBar", + ["--cert=path/to/cert", "--cert-key", "path/to/cert/key"], "--version", @@ -94,19 +88,17 @@ describe("parser", () => { "4", "--", "--5", - "some/project/directory", ].flat(), ), ).toEqual({ _: ["1", "2", "3", "4", "--5"], auth: "none", - "builtin-extensions-dir": path.resolve("some/built/ext/dir"), - "extensions-dir": path.resolve("some/ext/dir"), - "user-data-dir": path.resolve("some/user/dir"), - folder: path.resolve(process.cwd(), "some/project/directory"), - "cert-key": path.resolve("someCertKeyBar"), + "builtin-extensions-dir": path.resolve("path/to/builtin/ext/dir"), + "extensions-dir": path.resolve("path/to/ext/dir"), + "user-data-dir": path.resolve("path/to/user/dir"), + "cert-key": path.resolve("path/to/cert/key"), cert: { - value: path.resolve("Foo"), + value: path.resolve("path/to/cert"), }, enable: ["feature1", "feature2"], help: true, @@ -114,32 +106,29 @@ describe("parser", () => { json: true, log: "error", open: true, - port: "8081", + port: 8081, socket: path.resolve("mumble"), verbose: true, version: true, "bind-addr": "192.169.0.1:8080", - workspace: "", }) }) it("should work with short options", async () => { - expect(await parse(["-vvv", "-v"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["-vvv", "-v"])).toEqual({ verbose: true, version: true, }) }) it("should use log level env var", async () => { - const args = await parse([]) - expect(args).toEqual(createDefaultArgs()) + const args = parse([]) + expect(args).toEqual({}) process.env.LOG_LEVEL = "debug" const defaults = await setDefaults(args) expect(defaults).toStrictEqual({ ...defaults, - _: [], log: "debug", verbose: false, }) @@ -150,7 +139,6 @@ describe("parser", () => { const updated = await setDefaults(args) expect(updated).toStrictEqual({ ...updated, - _: [], log: "trace", verbose: true, }) @@ -159,9 +147,8 @@ describe("parser", () => { }) it("should prefer --log to env var and --verbose to --log", async () => { - let args = await parse(["--log", "info"]) + let args = parse(["--log", "info"]) expect(args).toEqual({ - ...createDefaultArgs(), log: "info", }) @@ -169,7 +156,6 @@ describe("parser", () => { const defaults = await setDefaults(args) expect(defaults).toEqual({ ...defaults, - _: [], log: "info", verbose: false, }) @@ -180,16 +166,14 @@ describe("parser", () => { const updated = await setDefaults(args) expect(updated).toEqual({ ...defaults, - _: [], log: "info", verbose: false, }) expect(process.env.LOG_LEVEL).toEqual("info") expect(logger.level).toEqual(Level.Info) - args = await parse(["--log", "info", "--verbose"]) + args = parse(["--log", "info", "--verbose"]) expect(args).toEqual({ - ...createDefaultArgs(), log: "info", verbose: true, }) @@ -198,7 +182,6 @@ describe("parser", () => { const updatedAgain = await setDefaults(args) expect(updatedAgain).toEqual({ ...defaults, - _: [], log: "trace", verbose: true, }) @@ -208,84 +191,73 @@ describe("parser", () => { it("should ignore invalid log level env var", async () => { process.env.LOG_LEVEL = "bogus" - const defaults = await setDefaults(await parse([])) + const defaults = await setDefaults(parse([])) expect(defaults).toEqual({ ...defaults, - _: [], }) }) - it("should error if value isn't provided", async () => { - await expect(parse(["--auth"])).rejects.toThrowError(/--auth requires a value/) - await expect(parse(["--auth=", "--log=debug"])).rejects.toThrowError(/--auth requires a value/) - await expect(parse(["--auth", "--log"])).rejects.toThrowError(/--auth requires a value/) - await expect(parse(["--auth", "--invalid"])).rejects.toThrowError(/--auth requires a value/) - await expect(parse(["--bind-addr"])).rejects.toThrowError(/--bind-addr requires a value/) + it("should error if value isn't provided", () => { + expect(() => parse(["--auth"])).toThrowError(/--auth requires a value/) + expect(() => parse(["--auth=", "--log=debug"])).toThrowError(/--auth requires a value/) + expect(() => parse(["--auth", "--log"])).toThrowError(/--auth requires a value/) + expect(() => parse(["--auth", "--invalid"])).toThrowError(/--auth requires a value/) + expect(() => parse(["--bind-addr"])).toThrowError(/--bind-addr requires a value/) }) - it("should error if value is invalid", async () => { - await expect(parse(["--port", "foo"])).rejects.toThrowError(/--port must be a number/) - await expect(parse(["--auth", "invalid"])).rejects.toThrowError(/--auth valid values: \[password, none\]/) - await expect(parse(["--log", "invalid"])).rejects.toThrowError( - /--log valid values: \[trace, debug, info, warn, error\]/, - ) + it("should error if value is invalid", () => { + expect(() => parse(["--port", "foo"])).toThrowError(/--port must be a number/) + expect(() => parse(["--auth", "invalid"])).toThrowError(/--auth valid values: \[password, none\]/) + expect(() => parse(["--log", "invalid"])).toThrowError(/--log valid values: \[trace, debug, info, warn, error\]/) }) - it("should error if the option doesn't exist", async () => { - await expect(parse(["--foo"])).rejects.toThrowError(/Unknown option --foo/) + it("should error if the option doesn't exist", () => { + expect(() => parse(["--foo"])).toThrowError(/Unknown option --foo/) }) it("should not error if the value is optional", async () => { - expect(await parse(["--cert"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["--cert"])).toEqual({ cert: { value: undefined, }, }) }) - it("should not allow option-like values", async () => { - await expect(parse(["--socket", "--socket-path-value"])).rejects.toThrowError(/--socket requires a value/) + it("should not allow option-like values", () => { + expect(() => parse(["--socket", "--socket-path-value"])).toThrowError(/--socket requires a value/) // If you actually had a path like this you would do this instead: - expect(await parse(["--socket", "./--socket-path-value"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["--socket", "./--socket-path-value"])).toEqual({ socket: path.resolve("--socket-path-value"), }) - await expect(parse(["--cert", "--socket-path-value"])).rejects.toThrowError(/Unknown option --socket-path-value/) + expect(() => parse(["--cert", "--socket-path-value"])).toThrowError(/Unknown option --socket-path-value/) }) it("should allow positional arguments before options", async () => { - expect(await parse(["test", "some/project/directory", "--auth", "none"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["test", "--auth", "none"])).toEqual({ _: ["test"], auth: "none", - folder: path.resolve(process.cwd(), "some/project/directory"), }) }) it("should support repeatable flags", async () => { - expect(await parse(["--proxy-domain", "*.coder.com"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["--proxy-domain", "*.coder.com"])).toEqual({ "proxy-domain": ["*.coder.com"], }) - expect(await parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ - ...createDefaultArgs(), + expect(parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "test.com"])).toEqual({ "proxy-domain": ["*.coder.com", "test.com"], }) }) it("should enforce cert-key with cert value or otherwise generate one", async () => { - const args = await parse(["--cert"]) + const args = parse(["--cert"]) expect(args).toEqual({ - ...createDefaultArgs(), cert: { value: undefined, }, }) - await expect(parse(["--cert", "test"])).rejects.toThrowError(/--cert-key is missing/) + expect(() => parse(["--cert", "test"])).toThrowError(/--cert-key is missing/) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - ...createDefaultArgs(), ...defaults, cert: { value: path.join(paths.data, "localhost.crt"), @@ -295,19 +267,16 @@ describe("parser", () => { }) it("should override with --link", async () => { - const args = await parse( - "--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" "), - ) + const args = parse("--cert test --cert-key test --socket test --host 0.0.0.0 --port 8888 --link test".split(" ")) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ - ...createDefaultArgs(), ...defaults, auth: "none", host: "localhost", link: { value: "test", }, - port: "0", + port: 0, cert: undefined, "cert-key": path.resolve("test"), socket: undefined, @@ -316,15 +285,12 @@ describe("parser", () => { it("should use env var password", async () => { process.env.PASSWORD = "test" - const args = await parse([]) - expect(args).toEqual({ - ...createDefaultArgs(), - }) + const args = parse([]) + expect(args).toEqual({}) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], password: "test", usingEnvPassword: true, }) @@ -333,69 +299,54 @@ describe("parser", () => { it("should use env var hashed password", async () => { process.env.HASHED_PASSWORD = "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY" // test - const args = await parse([]) - expect(args).toEqual({ - ...createDefaultArgs(), - }) + const args = parse([]) + expect(args).toEqual({}) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], "hashed-password": "$argon2i$v=19$m=4096,t=3,p=1$0qR/o+0t00hsbJFQCKSfdQ$oFcM4rL6o+B7oxpuA4qlXubypbBPsf+8L531U7P9HYY", usingEnvHashedPassword: true, }) }) - it("should error if password passed in", async () => { - await expect(parse(["--password", "supersecret123"])).rejects.toThrowError( + it("should error if password passed in", () => { + expect(() => parse(["--password", "supersecret123"])).toThrowError( "--password can only be set in the config file or passed in via $PASSWORD", ) }) - it("should error if hashed-password passed in", async () => { - await expect(parse(["--hashed-password", "fdas423fs8a"])).rejects.toThrowError( + it("should error if hashed-password passed in", () => { + expect(() => parse(["--hashed-password", "fdas423fs8a"])).toThrowError( "--hashed-password can only be set in the config file or passed in via $HASHED_PASSWORD", ) }) it("should filter proxy domains", async () => { - const args = await parse([ - "--proxy-domain", - "*.coder.com", - "--proxy-domain", - "coder.com", - "--proxy-domain", - "coder.org", - ]) + const args = parse(["--proxy-domain", "*.coder.com", "--proxy-domain", "coder.com", "--proxy-domain", "coder.org"]) expect(args).toEqual({ - ...createDefaultArgs(), "proxy-domain": ["*.coder.com", "coder.com", "coder.org"], }) const defaultArgs = await setDefaults(args) expect(defaultArgs).toEqual({ ...defaults, - _: [], "proxy-domain": ["coder.com", "coder.org"], }) }) it("should allow '=,$/' in strings", async () => { - const args = await parse([ + const args = parse([ "--disable-update-check", "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", - "foobar", ]) expect(args).toEqual({ - ...createDefaultArgs(), "disable-update-check": true, - folder: path.resolve(process.cwd(), "foobar"), _: ["$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy"], }) }) it("should parse options with double-dash and multiple equal signs ", async () => { - const args = await parse( + const args = parse( [ "--hashed-password=$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", ], @@ -404,7 +355,6 @@ describe("parser", () => { }, ) expect(args).toEqual({ - ...createDefaultArgs(), "hashed-password": "$argon2i$v=19$m=4096,t=3,p=1$0qr/o+0t00hsbjfqcksfdq$ofcm4rl6o+b7oxpua4qlxubypbbpsf+8l531u7p9hyy", }) @@ -413,7 +363,6 @@ describe("parser", () => { describe("cli", () => { let testDir: string - let args: Mutable = createDefaultArgs() const vscodeIpcPath = path.join(os.tmpdir(), "vscode-ipc") beforeAll(async () => { @@ -424,45 +373,48 @@ describe("cli", () => { beforeEach(async () => { delete process.env.VSCODE_IPC_HOOK_CLI - args = createDefaultArgs() await fs.rmdir(vscodeIpcPath, { recursive: true }) }) it("should use existing if inside code-server", async () => { process.env.VSCODE_IPC_HOOK_CLI = "test" + const args: UserProvidedArgs = {} expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") - args.port = "8081" - args._.push("./file") + args.port = 8081 + args._ = ["./file"] expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") }) it("should use existing if --reuse-window is set", async () => { + const args: UserProvidedArgs = {} args["reuse-window"] = true await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual(undefined) await fs.writeFile(vscodeIpcPath, "test") await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") - args.port = "8081" + args.port = 8081 await expect(shouldOpenInExistingInstance(args)).resolves.toStrictEqual("test") }) it("should use existing if --new-window is set", async () => { + const args: UserProvidedArgs = {} args["new-window"] = true expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) await fs.writeFile(vscodeIpcPath, "test") expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") - args.port = "8081" + args.port = 8081 expect(await shouldOpenInExistingInstance(args)).toStrictEqual("test") }) it("should use existing if no unrelated flags are set, has positional, and socket is active", async () => { + const args: UserProvidedArgs = {} expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) - args._.push("./file") + args._ = ["./file"] expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) const socketPath = path.join(testDir, "socket") @@ -480,7 +432,7 @@ describe("cli", () => { expect(await shouldOpenInExistingInstance(args)).toStrictEqual(socketPath) - args.port = "8081" + args.port = 8081 expect(await shouldOpenInExistingInstance(args)).toStrictEqual(undefined) }) }) @@ -512,7 +464,7 @@ describe("splitOnFirstEquals", () => { describe("shouldSpawnCliProcess", () => { it("should return false if no 'extension' related args passed in", async () => { - const args = createDefaultArgs() + const args = {} const actual = await shouldSpawnCliProcess(args) const expected = false @@ -521,7 +473,6 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'list-extensions' passed in", async () => { const args = { - ...createDefaultArgs(), ["list-extensions"]: true, } const actual = await shouldSpawnCliProcess(args) @@ -532,7 +483,6 @@ describe("shouldSpawnCliProcess", () => { it("should return true if 'install-extension' passed in", async () => { const args = { - ...createDefaultArgs(), ["install-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -542,8 +492,7 @@ describe("shouldSpawnCliProcess", () => { }) it("should return true if 'uninstall-extension' passed in", async () => { - const args = { - ...createDefaultArgs(), + const args: UserProvidedArgs = { ["uninstall-extension"]: ["hello.world"], } const actual = await shouldSpawnCliProcess(args) @@ -555,7 +504,7 @@ describe("shouldSpawnCliProcess", () => { describe("bindAddrFromArgs", () => { it("should return the bind address", () => { - const args = createDefaultArgs() + const args: UserProvidedArgs = {} const addr = { host: "localhost", @@ -569,8 +518,7 @@ describe("bindAddrFromArgs", () => { }) it("should use the bind-address if set in args", () => { - const args = { - ...createDefaultArgs(), + const args: UserProvidedArgs = { ["bind-addr"]: "localhost:3000", } @@ -589,8 +537,7 @@ describe("bindAddrFromArgs", () => { }) it("should use the host if set in args", () => { - const args = { - ...createDefaultArgs(), + const args: UserProvidedArgs = { ["host"]: "coder", } @@ -612,7 +559,7 @@ describe("bindAddrFromArgs", () => { const [setValue, resetValue] = useEnv("PORT") setValue("8000") - const args = createDefaultArgs() + const args: UserProvidedArgs = {} const addr = { host: "localhost", @@ -630,9 +577,8 @@ describe("bindAddrFromArgs", () => { }) it("should set port if in args", () => { - const args = { - ...createDefaultArgs(), - port: "3000", + const args: UserProvidedArgs = { + port: 3000, } const addr = { @@ -653,9 +599,8 @@ describe("bindAddrFromArgs", () => { const [setValue, resetValue] = useEnv("PORT") setValue("8000") - const args = { - ...createDefaultArgs(), - port: "3000", + const args: UserProvidedArgs = { + port: 3000, } const addr = { diff --git a/test/unit/node/plugin.test.ts b/test/unit/node/plugin.test.ts index 4f9e14a3785f..5e23c24b67c3 100644 --- a/test/unit/node/plugin.test.ts +++ b/test/unit/node/plugin.test.ts @@ -34,7 +34,7 @@ describe("plugin", () => { _: [], auth: AuthType.None, host: "localhost", - port: "8080", + port: 8080, "proxy-domain": [], config: "~/.config/code-server/config.yaml", verbose: false, diff --git a/test/utils/integration.ts b/test/utils/integration.ts index e9a05c9c4a8e..5c4f0cc6aa7f 100644 --- a/test/utils/integration.ts +++ b/test/utils/integration.ts @@ -5,8 +5,8 @@ import * as httpserver from "./httpserver" export async function setup(argv: string[], configFile?: string): Promise { argv = ["--bind-addr=localhost:0", "--log=warn", ...argv] - const cliArgs = await parse(argv) - const configArgs = await parseConfigFile(configFile || "", "test/integration.ts") + const cliArgs = parse(argv) + const configArgs = parseConfigFile(configFile || "", "test/integration.ts") const args = await setDefaults(cliArgs, configArgs) const server = await runCodeServer(args) From d5ed30f87be326eb2f9adb13113ae919f53d71d7 Mon Sep 17 00:00:00 2001 From: Asher Date: Mon, 8 Nov 2021 21:28:17 +0000 Subject: [PATCH 21/27] Fix static endpoint not emitting 404s This fixes the last failing unit test. --- src/node/routes/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/node/routes/index.ts b/src/node/routes/index.ts index b02661841de4..7ca79fa7c486 100644 --- a/src/node/routes/index.ts +++ b/src/node/routes/index.ts @@ -121,6 +121,7 @@ export const register = async (app: App, args: DefaultedArgs): Promise Date: Mon, 8 Nov 2021 22:06:55 +0000 Subject: [PATCH 22/27] Update VS Code Fix a missing dependency, add some generic reverse proxy support for the protocol, and add back a missing nfpm fix. --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 1a1cfd115c59..7a3e3a3f2537 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#122c0fa63769cf71c5fda4eee156e3c9fb57c3c0" + "code-oss-dev": "cdr/vscode#0fd8653e99f74f13c37a5c1883a77acfbee33512" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 0edcee65f692..2d27657cf26f 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#122c0fa63769cf71c5fda4eee156e3c9fb57c3c0: +code-oss-dev@cdr/vscode#0fd8653e99f74f13c37a5c1883a77acfbee33512: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/122c0fa63769cf71c5fda4eee156e3c9fb57c3c0" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/0fd8653e99f74f13c37a5c1883a77acfbee33512" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From a281ccd284c81dd630d45df23bd5bf7517f4364e Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 9 Nov 2021 21:33:16 +0000 Subject: [PATCH 23/27] Import missing logError --- test/e2e/models/CodeServer.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 695d241a4782..38430ab56844 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -3,6 +3,7 @@ import * as cp from "child_process" import { promises as fs } from "fs" import * as path from "path" import { Page } from "playwright" +import { logError } from "../../../src/common/util" import { onLine } from "../../../src/node/util" import { PASSWORD, workspaceDir } from "../../utils/constants" import { idleTimer, tmpdir } from "../../utils/helpers" From 7d48b82db5e463c93462949ba2c250d2b27db521 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 9 Nov 2021 22:01:43 +0000 Subject: [PATCH 24/27] Fix 403 errors --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 7a3e3a3f2537..5dba3d8c0c63 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#0fd8653e99f74f13c37a5c1883a77acfbee33512" + "code-oss-dev": "cdr/vscode#620314b7c005ee471f99aa1b9ac9bc061810df57" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 2d27657cf26f..7e74184882f1 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#0fd8653e99f74f13c37a5c1883a77acfbee33512: +code-oss-dev@cdr/vscode#620314b7c005ee471f99aa1b9ac9bc061810df57: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/0fd8653e99f74f13c37a5c1883a77acfbee33512" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/620314b7c005ee471f99aa1b9ac9bc061810df57" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From fc94ac923829ab70b07c8801688bb962c04bdc91 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 9 Nov 2021 23:06:26 +0000 Subject: [PATCH 25/27] Add code-server version to about dialog --- vendor/package.json | 2 +- vendor/yarn.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/package.json b/vendor/package.json index 5dba3d8c0c63..bdf69c89768f 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -7,6 +7,6 @@ "postinstall": "./postinstall.sh" }, "devDependencies": { - "code-oss-dev": "cdr/vscode#620314b7c005ee471f99aa1b9ac9bc061810df57" + "code-oss-dev": "cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1" } } diff --git a/vendor/yarn.lock b/vendor/yarn.lock index 7e74184882f1..10acfc6576a8 100644 --- a/vendor/yarn.lock +++ b/vendor/yarn.lock @@ -296,9 +296,9 @@ clone-response@^1.0.2: dependencies: mimic-response "^1.0.0" -code-oss-dev@cdr/vscode#620314b7c005ee471f99aa1b9ac9bc061810df57: +code-oss-dev@cdr/vscode#d62e8db202f80db7a42233cd56d04e6806109fb1: version "1.61.1" - resolved "https://codeload.github.com/cdr/vscode/tar.gz/620314b7c005ee471f99aa1b9ac9bc061810df57" + resolved "https://codeload.github.com/cdr/vscode/tar.gz/d62e8db202f80db7a42233cd56d04e6806109fb1" dependencies: "@microsoft/applicationinsights-web" "^2.6.4" "@vscode/sqlite3" "4.0.12" From 89cc5a9d7824588d619b28edc71062f7aad1d10e Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 10 Nov 2021 00:39:11 +0000 Subject: [PATCH 26/27] Use user settings to disable welcome page The workspace setting seems to be recognized but if so it is having no effect. --- test/e2e/models/CodeServer.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/models/CodeServer.ts b/test/e2e/models/CodeServer.ts index 38430ab56844..12d6ef46180a 100644 --- a/test/e2e/models/CodeServer.ts +++ b/test/e2e/models/CodeServer.ts @@ -52,9 +52,9 @@ export class CodeServer { */ private async createWorkspace(): Promise { const dir = await tmpdir(workspaceDir) - await fs.mkdir(path.join(dir, ".vscode")) + await fs.mkdir(path.join(dir, "User")) await fs.writeFile( - path.join(dir, ".vscode/settings.json"), + path.join(dir, "User/settings.json"), JSON.stringify({ "workbench.startupEditor": "none", }), From e022788fd6c37a2f4358a1c382d9640d05fb914a Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 10 Nov 2021 03:37:57 +0000 Subject: [PATCH 27/27] Update VS Code cache step with new build directories --- .github/workflows/ci.yaml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index c0b1d6508ff4..6f69b6383591 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -142,9 +142,9 @@ jobs: path: | vendor/modules/code-oss-dev/.build vendor/modules/code-oss-dev/out-build - vendor/modules/code-oss-dev/out-vscode-server - vendor/modules/code-oss-dev/out-vscode-server-min - key: vscode-server-build-${{ steps.vscode-rev.outputs.rev }} + vendor/modules/code-oss-dev/out-vscode-reh-web + vendor/modules/code-oss-dev/out-vscode-reh-web-min + key: vscode-reh-build-${{ steps.vscode-rev.outputs.rev }} - name: Build vscode if: steps.cache-vscode.outputs.cache-hit != 'true'