Skip to content

Commit e60f4d9

Browse files
DanTupDanTup
committed
Reduce the amount of Snap-specific code requires to set up Flutter
Fixes #2853.
1 parent 393763b commit e60f4d9

6 files changed

Lines changed: 89 additions & 98 deletions

File tree

‎src/extension/sdk/flutter.ts‎

Lines changed: 41 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { ProgressLocation, window } from "vscode";
2-
import { initializingFlutterMessage, noAction, showLogAction, yesAction } from "../../shared/constants";
2+
import { initializingFlutterMessage, showLogAction } from "../../shared/constants";
33
import { LogCategory } from "../../shared/enums";
44
import { Logger } from "../../shared/interfaces";
55
import { logProcess } from "../../shared/logging";
@@ -8,52 +8,47 @@ import { ringLog } from "../extension";
88
import { openLogContents } from "../utils";
99
import { safeToolSpawn } from "../utils/processes";
1010

11-
export async function initializeFlutterSdk(logger: Logger, flutterScript: string, promptText?: string): Promise<void> {
12-
const selectedItem = promptText ? await window.showInformationMessage(promptText, yesAction, noAction) : yesAction;
13-
if (selectedItem === yesAction) {
14-
logger.info(`Flutter is not initialized, running 'flutter config --machine'...`);
15-
try {
16-
await window.withProgress(
17-
{
18-
location: ProgressLocation.Notification,
19-
title: initializingFlutterMessage,
20-
},
21-
async (progress, cancellationToken) => {
22-
const proc = safeToolSpawn(undefined, flutterScript, ["doctor", "-v"]);
11+
export async function initializeFlutterSdk(logger: Logger, flutterScript: string): Promise<void> {
12+
logger.info(`Flutter is not initialized, running 'flutter doctor' to force...`);
13+
try {
14+
await window.withProgress(
15+
{
16+
location: ProgressLocation.Notification,
17+
title: initializingFlutterMessage,
18+
},
19+
async (progress, cancellationToken) => {
20+
const proc = safeToolSpawn(undefined, flutterScript, ["doctor", "-v"]);
2321

24-
// Show the output in an output channel so if it gets stuck the user can see it.
25-
const channel = channels.getOutputChannel(`flutter doctor`, true);
26-
channel.show();
27-
channels.runProcessInOutputChannel(proc, channel);
22+
// Show the output in an output channel so if it gets stuck the user can see it.
23+
const channel = channels.getOutputChannel(`flutter doctor`);
24+
channel.show();
25+
channels.runProcessInOutputChannel(proc, channel);
2826

29-
cancellationToken.onCancellationRequested((e) => {
30-
logger.info(`User canceled!`);
31-
proc.kill();
32-
});
33-
// Log this to general as it's startup stuff that can't be captured with
34-
// Capture Logs so log it to the main log file.
35-
logProcess(logger, LogCategory.General, proc);
36-
return new Promise<void>((resolve, reject) => proc.on("exit", (code) => {
37-
if (code) {
38-
const ringLogContents = ringLog.toString();
39-
logger.error(`Failed to initialize Flutter: Process exited with code ${code}.`);
40-
window.showErrorMessage(`Failed to initialize Flutter: Process exited with code ${code}.`, showLogAction).then((chosenAction) => {
41-
if (chosenAction === showLogAction)
42-
openLogContents(undefined, ringLogContents);
43-
});
44-
reject();
45-
} else {
46-
channel.hide();
47-
resolve();
48-
}
49-
}));
50-
},
51-
);
52-
logger.info(`Flutter initialized!`);
53-
} catch (e) {
54-
logger.warn(`Flutter initialization failed, proceeding without!`);
55-
}
56-
} else {
57-
logger.info(`User cancelled Flutter initialization`);
27+
cancellationToken.onCancellationRequested((e) => {
28+
logger.info(`User canceled!`);
29+
proc.kill();
30+
});
31+
// Log this to general as it's startup stuff that can't be captured with
32+
// Capture Logs so log it to the main log file.
33+
logProcess(logger, LogCategory.General, proc);
34+
return new Promise<void>((resolve, reject) => proc.on("exit", (code) => {
35+
if (code) {
36+
const ringLogContents = ringLog.toString();
37+
logger.error(`Failed to initialize Flutter: Process exited with code ${code}.`);
38+
window.showErrorMessage(`Failed to initialize Flutter: Process exited with code ${code}.`, showLogAction).then((chosenAction) => {
39+
if (chosenAction === showLogAction)
40+
openLogContents(undefined, ringLogContents);
41+
});
42+
reject();
43+
} else {
44+
channel.hide();
45+
resolve();
46+
}
47+
}));
48+
},
49+
);
50+
logger.info(`Flutter initialized!`);
51+
} catch (e) {
52+
logger.warn(`Flutter initialization failed, proceeding without!`);
5853
}
5954
}

‎src/extension/sdk/utils.ts‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import * as fs from "fs";
2-
import * as os from "os";
32
import * as path from "path";
43
import { commands, ExtensionContext, window } from "vscode";
5-
import { analyzerSnapshotPath, dartPlatformName, dartVMPath, DART_DOWNLOAD_URL, executableNames, flutterPath, flutterSnapScript, FLUTTER_CREATE_PROJECT_TRIGGER_FILE, FLUTTER_DOWNLOAD_URL, initializeSnapPrompt, isLinux, isMac, isWin, showLogAction } from "../../shared/constants";
6-
import { Logger, Sdks, WorkspaceConfig } from "../../shared/interfaces";
4+
import { analyzerSnapshotPath, dartPlatformName, dartVMPath, DART_DOWNLOAD_URL, executableNames, flutterPath, FLUTTER_CREATE_PROJECT_TRIGGER_FILE, FLUTTER_DOWNLOAD_URL, isLinux, isMac, isWin, showLogAction, snapBinaryPath, snapFlutterBinaryPath } from "../../shared/constants";
5+
import { Logger, Sdks, SdkSearchResults, WorkspaceConfig } from "../../shared/interfaces";
76
import { nullLogger } from "../../shared/logging";
87
import { PackageMap } from "../../shared/pub/package_map";
98
import { flatMap, isDartSdkFromFlutter, notUndefined } from "../../shared/utils";
109
import { fsPath, getSdkVersion, hasPubspec } from "../../shared/utils/fs";
1110
import { resolvedPromise } from "../../shared/utils/promises";
12-
import { processBazelWorkspace, processFlutterSnap, processFuchsiaWorkspace, processKnownGitRepositories } from "../../shared/utils/workspace";
11+
import { processBazelWorkspace, processFuchsiaWorkspace, processKnownGitRepositories } from "../../shared/utils/workspace";
1312
import { envUtils, getAllProjectFolders, getDartWorkspaceFolders } from "../../shared/vscode/utils";
1413
import { WorkspaceContext } from "../../shared/workspace";
1514
import { Analytics } from "../analytics";
@@ -106,7 +105,7 @@ export class SdkUtils {
106105

107106
public async showSdkActivationFailure(
108107
sdkType: string,
109-
search: (path: string[]) => string | undefined,
108+
search: (path: string[]) => Promise<SdkSearchResults>,
110109
downloadUrl: string,
111110
saveSdkPath: (path: string) => Thenable<void>,
112111
commandToReRun?: string,
@@ -127,9 +126,9 @@ export class SdkUtils {
127126
const selectedFolders =
128127
await window.showOpenDialog({ canSelectFolders: true, openLabel: `Set ${sdkType} SDK folder` });
129128
if (selectedFolders && selectedFolders.length > 0) {
130-
const matchingSdkFolder = search(selectedFolders.map(fsPath));
131-
if (matchingSdkFolder) {
132-
await saveSdkPath(matchingSdkFolder);
129+
const matchingSdkFolder = await search(selectedFolders.map(fsPath));
130+
if (matchingSdkFolder.sdkPath) {
131+
await saveSdkPath(matchingSdkFolder.sdkPath);
133132
await promptToReloadExtension();
134133
if (commandToReRun) {
135134
commands.executeCommand(commandToReRun);
@@ -237,7 +236,6 @@ export class SdkUtils {
237236
// TODO: Remove this lambda when the preview flag is removed.
238237
await processWorkspaceType(findBazelWorkspaceRoot, (l, c, b) => processBazelWorkspace(l, c, b, config.previewBazelWorkspaceCustomScripts));
239238
const fuchsiaRoot = await processWorkspaceType(findFuchsiaRoot, processFuchsiaWorkspace);
240-
await processWorkspaceType(findFlutterSnapSdkRoot, processFlutterSnap);
241239

242240
if (fuchsiaRoot) {
243241
this.logger.info(`Found Fuchsia root at ${fuchsiaRoot}`);
@@ -261,7 +259,20 @@ export class SdkUtils {
261259
"~/.flutter-sdk",
262260
].concat(paths).filter(notUndefined);
263261

264-
const flutterSdkPath = this.findFlutterSdk(flutterSdkSearchPaths);
262+
let flutterSdkResult = await this.findFlutterSdk(flutterSdkSearchPaths);
263+
264+
// Handle the case where the Flutter snap has not been initialised.
265+
if (!flutterSdkResult.sdkPath && flutterSdkResult.candidatePaths.includes(snapBinaryPath)) {
266+
// Trigger initialization.
267+
this.logger.info(`No Flutter SDK found, but a 'flutter' binary did point at ${snapBinaryPath} so attempting to initialize...`);
268+
await initializeFlutterSdk(this.logger, snapFlutterBinaryPath);
269+
270+
// Then search again.
271+
this.logger.info(`Snap initialization completed, searching for Flutter SDK again...`);
272+
flutterSdkResult = await this.findFlutterSdk(flutterSdkSearchPaths);
273+
}
274+
275+
const flutterSdkPath = flutterSdkResult.sdkPath;
265276

266277
// Since we just blocked on a lot of sync FS, yield.
267278
await resolvedPromise;
@@ -296,7 +307,8 @@ export class SdkUtils {
296307
// Since we just blocked on a lot of sync FS, yield.
297308
await resolvedPromise;
298309

299-
const dartSdkPath = this.findDartSdk(dartSdkSearchPaths);
310+
const dartSdkResult = await this.findDartSdk(dartSdkSearchPaths);
311+
const dartSdkPath = dartSdkResult.sdkPath;
300312

301313
// Since we just blocked on a lot of sync FS, yield.
302314
await resolvedPromise;
@@ -330,7 +342,7 @@ export class SdkUtils {
330342
return fs.existsSync(fullPath) && fs.statSync(fullPath).isFile();
331343
}
332344

333-
public searchPaths(paths: string[], executableFilename: string, postFilter?: (s: string) => boolean): string | undefined {
345+
public async searchPaths(paths: string[], executableFilename: string, postFilter?: (s: string) => boolean): Promise<SdkSearchResults> {
334346
this.logger.info(`Searching for ${executableFilename}`);
335347

336348
let sdkPaths =
@@ -362,7 +374,7 @@ export class SdkUtils {
362374
this.logger.info(` ${p}`);
363375

364376
// Convert all the paths to their resolved locations.
365-
sdkPaths = sdkPaths.map((p) => {
377+
sdkPaths = await Promise.all(sdkPaths.map(async (p) => {
366378
const fullPath = path.join(p, executableFilename);
367379

368380
// In order to handle symlinks on the binary (not folder), we need to add the executableName before calling realpath.
@@ -371,9 +383,16 @@ export class SdkUtils {
371383
if (realExecutableLocation.toLowerCase() !== fullPath.toLowerCase())
372384
this.logger.info(`Following symlink: ${fullPath} ==> ${realExecutableLocation}`);
373385

386+
// If the symlink resolves to the Snap binary, it's not a real SDK
387+
// and we should return as-is rather than walk up two levels, as we
388+
// may want to use the presence of this to trigger initialisation.
389+
if (realExecutableLocation === snapBinaryPath) {
390+
return snapBinaryPath;
391+
}
392+
374393
// Then we need to take the executable name and /bin back off
375394
return path.dirname(path.dirname(realExecutableLocation));
376-
});
395+
}));
377396

378397
// Now apply any post-filters.
379398
this.logger.info(" Candidate paths to be post-filtered:");
@@ -384,9 +403,10 @@ export class SdkUtils {
384403
if (sdkPath)
385404
this.logger.info(` Found at ${sdkPath}`);
386405

406+
387407
this.logger.info(` Returning SDK path ${sdkPath} for ${executableFilename}`);
388408

389-
return sdkPath;
409+
return { sdkPath, candidatePaths: sdkPaths };
390410
}
391411
}
392412

@@ -461,25 +481,6 @@ async function findGitRoot(logger: Logger, folder: string): Promise<string | und
461481
return findRootContaining(folder, ".git");
462482
}
463483

464-
async function findFlutterSnapSdkRoot(logger: Logger, folder: string): Promise<string | undefined> {
465-
if (isLinux && fs.existsSync(flutterSnapScript)) {
466-
logger.info(`Found Flutter snap script`);
467-
const snapSdkRoot = path.join(os.homedir(), "/snap/flutter/common/flutter");
468-
469-
if (!fs.existsSync(snapSdkRoot + "/.git")) {
470-
logger.info(`Flutter snap is not initialized, showing prompt`);
471-
await initializeFlutterSdk(logger, flutterSnapScript, initializeSnapPrompt);
472-
}
473-
474-
if (fs.existsSync(snapSdkRoot + "/.git")) {
475-
logger.info(`Returning ${snapSdkRoot} as Flutter snap SDK root`);
476-
return snapSdkRoot;
477-
}
478-
}
479-
return undefined;
480-
}
481-
482-
483484
function findRootContaining(folder: string, childName: string, expectFile = false): string | undefined {
484485
if (folder) {
485486
// Walk up the directories from the workspace root, and see if there

‎src/shared/constants.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ export const pubPath = "bin/" + executableNames.pub;
3030
export const flutterPath = "bin/" + executableNames.flutter;
3131
export const pubSnapshotPath = "bin/snapshots/pub.dart.snapshot";
3232
export const analyzerSnapshotPath = "bin/snapshots/analysis_server.dart.snapshot";
33-
export const flutterSnapScript = "/snap/flutter/current/flutter.sh";
3433
export const androidStudioPaths = androidStudioExecutableNames.map((s) => "bin/" + s);
3534
export const DART_DOWNLOAD_URL = "https://dart.dev/get-dart";
3635
export const FLUTTER_DOWNLOAD_URL = "https://flutter.dev/setup/";
@@ -81,10 +80,11 @@ export const takeSurveyAction = "Take Survey";
8180
export const skipThisSurveyAction = "Skip This Survey";
8281

8382
export const modifyingFilesOutsideWorkspaceInfoUrl = "https://dartcode.org/docs/modifying-files-outside-workspace/";
84-
85-
export const initializeSnapPrompt = "The Flutter snap is installed but not initialized. Would you like to initialize it now?";
8683
export const initializingFlutterMessage = "Initializing Flutter. This may take a few minutes.";
8784

85+
export const snapBinaryPath = "/usr/bin/snap";
86+
export const snapFlutterBinaryPath = "/snap/bin/flutter";
87+
8888
// Minutes.
8989
export const fiveMinutesInMs = 1000 * 60 * 5;
9090
export const tenMinutesInMs = 1000 * 60 * 10;

‎src/shared/interfaces.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ import * as f from "./flutter/daemon_interfaces";
66
import { UnknownResponse } from "./services/interfaces";
77
import { WorkspaceContext } from "./workspace";
88

9+
export interface SdkSearchResults {
10+
sdkPath: string | undefined;
11+
candidatePaths: string[];
12+
}
13+
914
export interface Sdks {
1015
readonly dart?: string;
1116
readonly dartVersion?: string;

‎src/shared/utils/workspace.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import * as fs from "fs";
22
import * as path from "path";
3-
import { flutterSnapScript, isWin } from "../constants";
3+
import { isWin } from "../constants";
44
import { CustomScript, Logger, WritableWorkspaceConfig } from "../interfaces";
55

66
export function processKnownGitRepositories(logger: Logger, config: WritableWorkspaceConfig, gitRoot: string) {
@@ -12,11 +12,6 @@ export function processKnownGitRepositories(logger: Logger, config: WritableWork
1212
}
1313
}
1414

15-
export function processFlutterSnap(logger: Logger, config: WritableWorkspaceConfig, snapSdkRoot: string) {
16-
config.flutterSdkHome = snapSdkRoot;
17-
config.flutterScript = { replacesArgs: 0, script: flutterSnapScript };
18-
}
19-
2015
export function processFuchsiaWorkspace(logger: Logger, config: WritableWorkspaceConfig, fuchsiaRoot: string) {
2116
config.disableAutomaticPackageGet = true;
2217
config.disableSdkUpdateChecks = true;

‎src/test/flutter_snap/extension.test.ts‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ import * as assert from "assert";
22
import * as os from "os";
33
import * as path from "path";
44
import * as vs from "vscode";
5-
import { initializeSnapPrompt, isLinux, yesAction } from "../../shared/constants";
5+
import { isLinux } from "../../shared/constants";
66
import { fsPath } from "../../shared/utils/fs";
7-
import { activate, extApi, logger, sb } from "../helpers";
7+
import { activate, extApi, logger } from "../helpers";
88
import sinon = require("sinon");
99

1010
describe("test environment", () => {
@@ -25,23 +25,18 @@ describe("extension", () => {
2525
});
2626

2727
it("initializes the snap and locates the SDK", async () => {
28-
// Automatically approve the initialization.
29-
const showInformationMessage = sb.stub(vs.window, "showInformationMessage");
30-
const initializeSnapMessagePrompt = showInformationMessage.withArgs(initializeSnapPrompt, sinon.match.any, sinon.match.any).resolves(yesAction);
31-
3228
await activate();
3329

34-
assert.ok(initializeSnapMessagePrompt.calledOnce);
35-
3630
const workspaceContext = extApi.workspaceContext;
3731

3832
assert.ok(workspaceContext.sdks);
3933
assert.ok(workspaceContext.sdks.dart);
4034
assert.equal(workspaceContext.sdks.flutter, `${path.join(os.homedir(), "/snap/flutter/common/flutter")}`);
35+
assert.equal(workspaceContext.sdks.dart, `${path.join(os.homedir(), "/snap/flutter/common/flutter/bin/cache/dart-sdk")}`);
4136
assert.ok(workspaceContext.config);
4237
assert.equal(workspaceContext.config?.dartSdkHomeLinux, undefined);
4338
assert.equal(workspaceContext.config?.dartSdkHomeMac, undefined);
44-
assert.deepStrictEqual(workspaceContext.config?.flutterScript, { script: "/snap/flutter/current/flutter.sh", replacesArgs: 0 });
39+
assert.equal(workspaceContext.config?.flutterScript, undefined);
4540
logger.info(" " + JSON.stringify(workspaceContext, undefined, 8).trim().slice(1, -1).trim());
4641
});
4742
});

0 commit comments

Comments
 (0)