Skip to content

Commit 675c6ad

Browse files
committed
fix: some variants of launching production builds can result in infinite loops and bad spawn paths
Our logic for finding the "app path" was incomplete.
1 parent 9751cf0 commit 675c6ad

File tree

4 files changed

+62
-36
lines changed

4 files changed

+62
-36
lines changed

Diff for: plugins/plugin-codeflare/src/controller/guide.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@ import respawnCommand from "./respawn"
2727
*
2828
* see bin/codeflare; we are mostly copying bits from there
2929
*/
30-
export default function guide(args: Arguments) {
30+
export default async function guide(args: Arguments) {
3131
setTabReadonly(args)
3232

33-
const { argv, env } = respawnCommand(
33+
const { argv, env } = await respawnCommand(
3434
args.command.replace(/--type=renderer/, "").replace(/^codeflare\s+gui\s+guide/, "codeflare")
3535
)
3636

Diff for: plugins/plugin-codeflare/src/controller/respawn.ts

+32-21
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,27 @@
1717
import { join } from "path"
1818
import { encodeComponent } from "@kui-shell/core"
1919

20+
async function getAppPath() {
21+
try {
22+
const { app } = await import("electron")
23+
if (app) {
24+
const appPath = app.getAppPath()
25+
if (appPath) {
26+
return appPath
27+
}
28+
}
29+
} catch (err) {
30+
console.error("Error fetching app path", err)
31+
}
32+
33+
const appPath = process.argv.find((_) => /app-path/.test(_))
34+
if (appPath) {
35+
return appPath.replace(/^--app-path=/, "")
36+
} else {
37+
return "."
38+
}
39+
}
40+
2041
/**
2142
* In electron production builds, if the user launches by double
2243
* clicking, or via spotlight (macos), then the CODEFLARE_HEADLESS and
@@ -33,44 +54,34 @@ import { encodeComponent } from "@kui-shell/core"
3354
* @return the absolute path to the directory that includes the
3455
* headless bundle.js.
3556
*/
36-
function electronProductionBuildHeadlessRoot() {
37-
const appPath = process.argv.find((_) => /app-path/.test(_))
38-
if (appPath) {
39-
return join(appPath.replace(/^--app-path=/, ""), "dist/headless")
40-
} else {
41-
return "."
42-
}
57+
async function electronProductionBuildHeadlessRoot() {
58+
return join(await getAppPath(), "dist/headless")
4359
}
4460

4561
/**
4662
* @return same as with `electronProductionBuildHeadlessRoot()`, except
4763
* returning the guidebook store absolute path
4864
*/
49-
function electronProductionBuildGuidebookStore() {
50-
const appPath = process.argv.find((_) => /app-path/.test(_))
51-
if (appPath) {
52-
return join(appPath.replace(/^--app-path=/, ""), "store")
53-
} else {
54-
return ""
55-
}
65+
async function electronProductionBuildGuidebookStore() {
66+
return join(await getAppPath(), "store")
5667
}
5768

5869
/** @return the absolute path to the directory that contains the headless bundle.js */
59-
function headlessRoot() {
60-
return process.env.CODEFLARE_HEADLESS || electronProductionBuildHeadlessRoot()
70+
async function headlessRoot() {
71+
return process.env.CODEFLARE_HEADLESS || (await electronProductionBuildHeadlessRoot())
6172
}
6273

6374
/** @return the absolute path to the directory that contains the guidebook store for this build */
64-
function guidebookStore() {
65-
return process.env.GUIDEBOOK_STORE || electronProductionBuildGuidebookStore()
75+
async function guidebookStore() {
76+
return process.env.GUIDEBOOK_STORE || (await electronProductionBuildGuidebookStore())
6677
}
6778

6879
/** Fill in the given command line to spawn ourselves as a subprocess */
69-
export default function respawnCommand(cmdline: string | string[]) {
80+
export default async function respawnCommand(cmdline: string | string[]) {
7081
return {
7182
argv: [
7283
encodeComponent(process.argv[0]),
73-
encodeComponent(headlessRoot() + "/codeflare.min.js"),
84+
encodeComponent((await headlessRoot()) + "/codeflare.min.js"),
7485
"--",
7586
...(typeof cmdline === "string" ? [cmdline] : cmdline),
7687
],
@@ -79,7 +90,7 @@ export default function respawnCommand(cmdline: string | string[]) {
7990
KUI_HEADLESS: "true",
8091
KUI_HEADLESS_WEBPACK: "true",
8192
ELECTRON_RUN_AS_NODE: "true",
82-
GUIDEBOOK_STORE: guidebookStore(),
93+
GUIDEBOOK_STORE: await guidebookStore(),
8394
DEBUG: process.env.DEBUG || "",
8495
HOME: process.env.HOME || "",
8596
PATH: process.env.PATH || "",

Diff for: plugins/plugin-codeflare/src/controller/terminal.tsx

+24-9
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,9 @@ import "allotment/dist/style.css"
3232
/**
3333
* This is a command handler that opens up a plain terminal that shows a login session using the user's $SHELL.
3434
*/
35-
export function shell(args: Arguments) {
35+
export async function shell(args: Arguments) {
3636
// respawn, meaning launch it with codeflare
37-
const { argv, env } = respawn(["$SHELL", "-l"])
37+
const { argv, env } = await respawn(["$SHELL", "-l"])
3838
const cmdline = argv.map((_) => encodeComponent(_)).join(" ")
3939

4040
return {
@@ -43,7 +43,7 @@ export function shell(args: Arguments) {
4343
}
4444

4545
type Props = Pick<BaseProps, "tab" | "repl">
46-
type State = Pick<BaseProps, "cmdline" | "env"> & { error?: boolean; selectedProfile?: string }
46+
type State = Partial<Pick<BaseProps, "cmdline" | "env">> & { error?: boolean; selectedProfile?: string }
4747
class TaskTerminal extends React.PureComponent<Props, State> {
4848
/** Allotment initial split sizes */
4949
private readonly sizes = [35, 65]
@@ -53,13 +53,25 @@ class TaskTerminal extends React.PureComponent<Props, State> {
5353
public constructor(props: Props) {
5454
super(props)
5555

56-
// respawn, meaning launch it with codeflare
57-
const { argv, env } = respawn(this.tasks[0].argv)
58-
const cmdline = argv.map((_) => encodeComponent(_)).join(" ")
56+
this.init()
57+
this.state = {}
58+
}
5959

60-
this.state = {
61-
cmdline,
62-
env,
60+
private async init() {
61+
try {
62+
// respawn, meaning launch it with codeflare
63+
const { argv, env } = await respawn(this.tasks[0].argv)
64+
const cmdline = argv.map((_) => encodeComponent(_)).join(" ")
65+
66+
this.setState({
67+
cmdline,
68+
env,
69+
})
70+
} catch (error) {
71+
console.error("Error initializing command line", error)
72+
this.state = {
73+
error: true,
74+
}
6375
}
6476
}
6577

@@ -75,7 +87,10 @@ class TaskTerminal extends React.PureComponent<Props, State> {
7587
public render() {
7688
if (this.state.error) {
7789
return "Internal Error"
90+
} else if (!this.state.cmdline || !this.state.env) {
91+
return <Loading />
7892
}
93+
7994
return (
8095
<Allotment defaultSizes={this.sizes} snap>
8196
<Allotment.Pane className="flex-fill flex-layout flex-align-stretch" minSize={400}>

Diff for: plugins/plugin-codeflare/src/tray/watchers/profile/status.ts

+4-4
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import respawnCommand from "../../../controller/respawn"
2424
export default class ProfileStatusWatcher {
2525
private headReadiness = "pending"
2626
private workerReadiness = "pending"
27-
private readonly _job: ReturnType<typeof spawn>
27+
// private readonly _job: ReturnType<typeof spawn>
2828

2929
public constructor(
3030
/** The profile to watch */
@@ -33,7 +33,7 @@ export default class ProfileStatusWatcher {
3333
/** How to update the Tray menu with changes*/
3434
private readonly updateFunction: UpdateFunction
3535
) {
36-
this._job = this.initJob(profile)
36+
/* this._job = */ this.initJob(profile)
3737
}
3838

3939
public get head() {
@@ -52,8 +52,8 @@ export default class ProfileStatusWatcher {
5252
return this.initJob(profile)
5353
}
5454

55-
private initJob(profile: string) {
56-
const { argv, env } = respawnCommand([
55+
private async initJob(profile: string) {
56+
const { argv, env } = await respawnCommand([
5757
"guide",
5858
"-q",
5959
"-y",

0 commit comments

Comments
 (0)