Skip to content

Commit 8546f4f

Browse files
code-ashercode-asher
authored
Improve fetching Coder binary (#222)
1 parent 6407437 commit 8546f4f

File tree

6 files changed

+436
-176
lines changed

6 files changed

+436
-176
lines changed

‎fixtures/bin.bash

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
#!/usr/bin/env bash
2+
3+
echo '$ECHO'

‎fixtures/bin.old.bash

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#!/usr/bin/env bash
2+
3+
if [[ $* == *--output* ]] ; then
4+
>&2 echo -n '$STDERR'
5+
exit 1
6+
else
7+
echo -n '$STDOUT'
8+
fi

‎src/cliManager.test.ts

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
import fs from "fs/promises"
2+
import os from "os"
3+
import path from "path"
4+
import { beforeAll, describe, expect, it } from "vitest"
5+
import * as cli from "./cliManager"
6+
7+
describe("cliManager", () => {
8+
const tmp = path.join(os.tmpdir(), "vscode-coder-tests")
9+
10+
beforeAll(async () => {
11+
// Clean up from previous tests, if any.
12+
await fs.rm(tmp, { recursive: true, force: true })
13+
await fs.mkdir(tmp, { recursive: true })
14+
})
15+
16+
it("name", () => {
17+
expect(cli.name().startsWith("coder-")).toBeTruthy()
18+
})
19+
20+
it("stat", async () => {
21+
const binPath = path.join(tmp, "stat")
22+
expect(await cli.stat(binPath)).toBeUndefined()
23+
24+
await fs.writeFile(binPath, "test")
25+
expect((await cli.stat(binPath))?.size).toBe(4)
26+
})
27+
28+
it("rm", async () => {
29+
const binPath = path.join(tmp, "rm")
30+
await cli.rm(binPath)
31+
32+
await fs.writeFile(binPath, "test")
33+
await cli.rm(binPath)
34+
})
35+
36+
// TODO: CI only runs on Linux but we should run it on Windows too.
37+
it("version", async () => {
38+
const binPath = path.join(tmp, "version")
39+
await expect(cli.version(binPath)).rejects.toThrow("ENOENT")
40+
41+
const binTmpl = await fs.readFile(path.join(__dirname, "../fixtures/bin.bash"), "utf8")
42+
await fs.writeFile(binPath, binTmpl.replace("$ECHO", "hello"))
43+
await expect(cli.version(binPath)).rejects.toThrow("EACCES")
44+
45+
await fs.chmod(binPath, "755")
46+
await expect(cli.version(binPath)).rejects.toThrow("Unexpected token")
47+
48+
await fs.writeFile(binPath, binTmpl.replace("$ECHO", "{}"))
49+
await expect(cli.version(binPath)).rejects.toThrow("No version found in output")
50+
51+
await fs.writeFile(
52+
binPath,
53+
binTmpl.replace(
54+
"$ECHO",
55+
JSON.stringify({
56+
version: "v0.0.0",
57+
}),
58+
),
59+
)
60+
expect(await cli.version(binPath)).toBe("v0.0.0")
61+
62+
const oldTmpl = await fs.readFile(path.join(__dirname, "../fixtures/bin.old.bash"), "utf8")
63+
const old = (stderr: string, stdout: string): string => {
64+
return oldTmpl.replace("$STDERR", stderr).replace("$STDOUT", stdout)
65+
}
66+
67+
// Should fall back only if it says "unknown flag".
68+
await fs.writeFile(binPath, old("foobar", "Coder v1.1.1"))
69+
await expect(cli.version(binPath)).rejects.toThrow("foobar")
70+
71+
await fs.writeFile(binPath, old("unknown flag: --output", "Coder v1.1.1"))
72+
expect(await cli.version(binPath)).toBe("v1.1.1")
73+
74+
// Should trim off the newline if necessary.
75+
await fs.writeFile(binPath, old("unknown flag: --output\n", "Coder v1.1.1\n"))
76+
expect(await cli.version(binPath)).toBe("v1.1.1")
77+
78+
// Error with original error if it does not begin with "Coder".
79+
await fs.writeFile(binPath, old("unknown flag: --output", "Unrelated"))
80+
await expect(cli.version(binPath)).rejects.toThrow("unknown flag")
81+
82+
// Error if no version.
83+
await fs.writeFile(binPath, old("unknown flag: --output", "Coder"))
84+
await expect(cli.version(binPath)).rejects.toThrow("No version found")
85+
})
86+
87+
it("rmOld", async () => {
88+
const binDir = path.join(tmp, "bins")
89+
expect(await cli.rmOld(path.join(binDir, "bin1"))).toStrictEqual([])
90+
91+
await fs.mkdir(binDir, { recursive: true })
92+
await fs.writeFile(path.join(binDir, "bin.old-1"), "echo hello")
93+
await fs.writeFile(path.join(binDir, "bin.old-2"), "echo hello")
94+
await fs.writeFile(path.join(binDir, "bin.temp-1"), "echo hello")
95+
await fs.writeFile(path.join(binDir, "bin.temp-2"), "echo hello")
96+
await fs.writeFile(path.join(binDir, "bin1"), "echo hello")
97+
await fs.writeFile(path.join(binDir, "bin2"), "echo hello")
98+
99+
expect(await cli.rmOld(path.join(binDir, "bin1"))).toStrictEqual([
100+
{
101+
fileName: "bin.old-1",
102+
error: undefined,
103+
},
104+
{
105+
fileName: "bin.old-2",
106+
error: undefined,
107+
},
108+
{
109+
fileName: "bin.temp-1",
110+
error: undefined,
111+
},
112+
{
113+
fileName: "bin.temp-2",
114+
error: undefined,
115+
},
116+
])
117+
118+
expect(await fs.readdir(path.join(tmp, "bins"))).toStrictEqual(["bin1", "bin2"])
119+
})
120+
121+
it("ETag", async () => {
122+
const binPath = path.join(tmp, "hash")
123+
124+
await fs.writeFile(binPath, "foobar")
125+
expect(await cli.eTag(binPath)).toBe("8843d7f92416211de9ebb963ff4ce28125932878")
126+
127+
await fs.writeFile(binPath, "test")
128+
expect(await cli.eTag(binPath)).toBe("a94a8fe5ccb19ba61c4c0873d391e987982fbbd3")
129+
})
130+
})

‎src/cliManager.ts

+167
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import { execFile, type ExecFileException } from "child_process"
2+
import * as crypto from "crypto"
3+
import { createReadStream, type Stats } from "fs"
4+
import fs from "fs/promises"
5+
import os from "os"
6+
import path from "path"
7+
import { promisify } from "util"
8+
9+
/**
10+
* Stat the path or undefined if the path does not exist. Throw if unable to
11+
* stat for a reason other than the path not existing.
12+
*/
13+
export async function stat(binPath: string): Promise<Stats | undefined> {
14+
try {
15+
return await fs.stat(binPath)
16+
} catch (error) {
17+
if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
18+
return undefined
19+
}
20+
throw error
21+
}
22+
}
23+
24+
/**
25+
* Remove the path. Throw if unable to remove.
26+
*/
27+
export async function rm(binPath: string): Promise<void> {
28+
try {
29+
await fs.rm(binPath, { force: true })
30+
} catch (error) {
31+
// Just in case; we should never get an ENOENT because of force: true.
32+
if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") {
33+
throw error
34+
}
35+
}
36+
}
37+
38+
// util.promisify types are dynamic so there is no concrete type we can import
39+
// and we have to make our own.
40+
type ExecException = ExecFileException & { stdout?: string; stderr?: string }
41+
42+
/**
43+
* Return the version from the binary. Throw if unable to execute the binary or
44+
* find the version for any reason.
45+
*/
46+
export async function version(binPath: string): Promise<string> {
47+
let stdout: string
48+
try {
49+
const result = await promisify(execFile)(binPath, ["version", "--output", "json"])
50+
stdout = result.stdout
51+
} catch (error) {
52+
// It could be an old version without support for --output.
53+
if ((error as ExecException)?.stderr?.includes("unknown flag: --output")) {
54+
const result = await promisify(execFile)(binPath, ["version"])
55+
if (result.stdout?.startsWith("Coder")) {
56+
const v = result.stdout.split(" ")[1]?.trim()
57+
if (!v) {
58+
throw new Error("No version found in output: ${result.stdout}")
59+
}
60+
return v
61+
}
62+
}
63+
throw error
64+
}
65+
66+
const json = JSON.parse(stdout)
67+
if (!json.version) {
68+
throw new Error("No version found in output: ${stdout}")
69+
}
70+
return json.version
71+
}
72+
73+
export type RemovalResult = { fileName: string; error: unknown }
74+
75+
/**
76+
* Remove binaries in the same directory as the specified path that have a
77+
* .old-* or .temp-* extension. Return a list of files and the errors trying to
78+
* remove them, when applicable.
79+
*/
80+
export async function rmOld(binPath: string): Promise<RemovalResult[]> {
81+
const binDir = path.dirname(binPath)
82+
try {
83+
const files = await fs.readdir(binDir)
84+
const results: RemovalResult[] = []
85+
for (const file of files) {
86+
const fileName = path.basename(file)
87+
if (fileName.includes(".old-") || fileName.includes(".temp-")) {
88+
try {
89+
await fs.rm(path.join(binDir, file), { force: true })
90+
results.push({ fileName, error: undefined })
91+
} catch (error) {
92+
results.push({ fileName, error })
93+
}
94+
}
95+
}
96+
return results
97+
} catch (error) {
98+
// If the directory does not exist, there is nothing to remove.
99+
if ((error as NodeJS.ErrnoException)?.code === "ENOENT") {
100+
return []
101+
}
102+
throw error
103+
}
104+
}
105+
106+
/**
107+
* Return the etag (sha1) of the path. Throw if unable to hash the file.
108+
*/
109+
export async function eTag(binPath: string): Promise<string> {
110+
const hash = crypto.createHash("sha1")
111+
const stream = createReadStream(binPath)
112+
return new Promise((resolve, reject) => {
113+
stream.on("end", () => {
114+
hash.end()
115+
resolve(hash.digest("hex"))
116+
})
117+
stream.on("error", (err) => {
118+
reject(err)
119+
})
120+
stream.on("data", (chunk) => {
121+
hash.update(chunk)
122+
})
123+
})
124+
}
125+
126+
/**
127+
* Return the binary name for the current platform.
128+
*/
129+
export function name(): string {
130+
const os = goos()
131+
const arch = goarch()
132+
let binName = `coder-${os}-${arch}`
133+
// Windows binaries have an exe suffix.
134+
if (os === "windows") {
135+
binName += ".exe"
136+
}
137+
return binName
138+
}
139+
140+
/**
141+
* Returns the Go format for the current platform.
142+
* Coder binaries are created in Go, so we conform to that name structure.
143+
*/
144+
export function goos(): string {
145+
const platform = os.platform()
146+
switch (platform) {
147+
case "win32":
148+
return "windows"
149+
default:
150+
return platform
151+
}
152+
}
153+
154+
/**
155+
* Return the Go format for the current architecture.
156+
*/
157+
export function goarch(): string {
158+
const arch = os.arch()
159+
switch (arch) {
160+
case "arm":
161+
return "armv7"
162+
case "x64":
163+
return "amd64"
164+
default:
165+
return arch
166+
}
167+
}

‎src/remote.ts

+8-4
Original file line numberDiff line numberDiff line change
@@ -465,7 +465,14 @@ export class Remote {
465465
//
466466
// If we didn't write to the SSH config file, connecting would fail with
467467
// "Host not found".
468-
await this.updateSSHConfig(authorityParts[1], hasCoderLogs)
468+
try {
469+
await this.updateSSHConfig(authorityParts[1], hasCoderLogs)
470+
} catch (error) {
471+
// TODO: This is a bit weird, because even if we throw an error VS Code
472+
// still tries to connect. Can we stop it?
473+
this.storage.writeToCoderOutputChannel(`Failed to configure SSH: ${error}`)
474+
throw error
475+
}
469476

470477
this.findSSHProcessID().then((pid) => {
471478
// Once the SSH process has spawned we can reset the timeout.
@@ -587,9 +594,6 @@ export class Remote {
587594
binaryPath = await this.storage.fetchBinary()
588595
}
589596
}
590-
if (!binaryPath) {
591-
throw new Error("Failed to fetch the Coder binary!")
592-
}
593597

594598
const escape = (str: string): string => `"${str.replace(/"/g, '\\"')}"`
595599

0 commit comments

Comments
 (0)