Skip to content

Commit

Permalink
Remove a layer of indirection causing race condition (#1172)
Browse files Browse the repository at this point in the history
  • Loading branch information
ooeygui authored Oct 18, 2023
1 parent cb167c5 commit 4c6d533
Show file tree
Hide file tree
Showing 12 changed files with 1,444 additions and 53 deletions.
1,394 changes: 1,382 additions & 12 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -483,6 +483,7 @@
"timers-browserify": "^2.0.12",
"tmp": "^0.2.1",
"tslib": "^2.5.0",
"vsce": "^2.15.0",
"xmlrpc": "^1.3.2"
},
"devDependencies": {
Expand Down
3 changes: 0 additions & 3 deletions src/build-tool/build-tool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,15 +96,12 @@ BuildTool.current = new NotImplementedBuildTool();
export async function determineBuildTool(dir: string): Promise<boolean> {
while (dir && path.dirname(dir) !== dir) {
if (await CatkinMakeBuildTool.isApplicable(dir)) {
extension.setBaseDir(dir);
BuildTool.current = new CatkinMakeBuildTool();
return true;
} else if (await CatkinToolsBuildTool.isApplicable(dir)) {
extension.setBaseDir(dir);
BuildTool.current = new CatkinToolsBuildTool();
return true;
} else if (await ColconBuildTool.isApplicable(dir)) {
extension.setBaseDir(dir);
BuildTool.current = new ColconBuildTool();
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/build-tool/catkin-tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as common from "./common";
import * as rosShell from "./ros-shell";

function makeCatkin(command: string, args: string[], category?: string): vscode.Task {
const task = rosShell.make({type: command, command, args: ['--workspace', extension.baseDir, ...args]}, category)
const task = rosShell.make({type: command, command, args: ['--workspace', vscode.workspace.rootPath, ...args]}, category)
task.problemMatchers = ["$catkin-gcc"];

return task;
Expand Down
2 changes: 1 addition & 1 deletion src/build-tool/catkin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import * as common from "./common";
import * as rosShell from "./ros-shell";

function makeCatkin(command: string, args: string[], category?: string): vscode.Task {
const task = rosShell.make({type: command, command, args: ['--directory', extension.baseDir, '-DCMAKE_BUILD_TYPE=RelWithDebInfo',...args]}, category)
const task = rosShell.make({type: command, command, args: ['--directory', vscode.workspace.rootPath, '-DCMAKE_BUILD_TYPE=RelWithDebInfo',...args]}, category)
task.problemMatchers = ["$catkin-gcc"];

return task;
Expand Down
2 changes: 1 addition & 1 deletion src/build-tool/colcon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ function makeColcon(command: string, verb: string, args: string[], category?: st
installType = '--merge-install';
}

const task = rosShell.make({type: command, command, args: [verb, installType, '--event-handlers', 'console_cohesion+', '--base-paths', extension.baseDir, `--cmake-args`, `-DCMAKE_BUILD_TYPE=RelWithDebInfo`,...args]},
const task = rosShell.make({type: command, command, args: [verb, installType, '--event-handlers', 'console_cohesion+', '--base-paths', vscode.workspace.rootPath, `--cmake-args`, `-DCMAKE_BUILD_TYPE=RelWithDebInfo`,...args]},
category)
task.problemMatchers = ["$catkin-gcc"];

Expand Down
2 changes: 1 addition & 1 deletion src/build-tool/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function _createPackage(createPkgCommand: (dependencies: string, na
return;
}

const cwd = typeof uri !== "undefined" ? uri.fsPath : `${extension.baseDir}/src`;
const cwd = typeof uri !== "undefined" ? uri.fsPath : `${vscode.workspace.rootPath}/src`;
const opts = { cwd, env: extension.env };

child_process.exec(createPkgCommand(dependencies, name), opts, (err, stdout, stderr) => {
Expand Down
2 changes: 1 addition & 1 deletion src/debugger/configuration/resolvers/attach.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class AttachResolver implements vscode.DebugConfigurationProvider {
try {
if (os.platform() === "win32") {
const processOptions: child_process.ExecOptions = {
cwd: extension.baseDir,
cwd: vscode.workspace.rootPath,
env: await extension.resolvedEnv(),
};

Expand Down
81 changes: 52 additions & 29 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,6 @@ import * as debug_manager from "./debugger/manager";
import * as debug_utils from "./debugger/utils";
import { registerRosShellTaskProvider } from "./build-tool/ros-shell";

/**
* The catkin workspace base dir.
*/
export let baseDir: string;

export function setBaseDir(dir: string) {
baseDir = dir;
}

/**
* The sourced ROS environment.
*/
Expand Down Expand Up @@ -192,7 +183,6 @@ export async function activate(context: vscode.ExtensionContext) {
await activateEnvironment(context);

return {
getBaseDir: () => baseDir,
getEnv: () => env,
onDidChangeEnv: (listener: () => any, thisArg: any) => onDidChangeEnv(listener, thisArg),
};
Expand Down Expand Up @@ -242,6 +232,8 @@ async function activateEnvironment(context: vscode.ExtensionContext) {
return;
}

outputChannel.appendLine(`Determining build tool for workspace: ${vscode.workspace.rootPath}`);

// Determine if we're in a catkin workspace.
let buildToolDetected = await buildtool.determineBuildTool(vscode.workspace.rootPath);

Expand All @@ -257,6 +249,9 @@ async function activateEnvironment(context: vscode.ExtensionContext) {
subscriptions.push(rosApi.activateCoreMonitor());
if (buildToolDetected) {
subscriptions.push(...buildtool.BuildTool.registerTaskProvider());
} else {
outputChannel.appendLine(`Build tool NOT detected`);

}
subscriptions.push(...registerRosShellTaskProvider());

Expand Down Expand Up @@ -302,6 +297,8 @@ async function sourceRosAndWorkspace(): Promise<void> {
// Wait to atomicly switch by composing a new environment block then switching at the end.
let newEnv = undefined;

outputChannel.appendLine("Sourcing ROS and Workspace");

const kWorkspaceConfigTimeout = 30000; // ms

let setupScriptExt: string;
Expand Down Expand Up @@ -330,6 +327,8 @@ async function sourceRosAndWorkspace(): Promise<void> {
try {
newEnv = await ros_utils.sourceSetupFile(rosSetupScript, newEnv);

outputChannel.appendLine(`Sourced ${rosSetupScript}`);

attemptWorkspaceDiscovery = false;
} catch (err) {
vscode.window.showErrorMessage(`A ROS setup script was provided, but could not source "${rosSetupScript}". Attempting standard discovery.`);
Expand All @@ -341,22 +340,39 @@ async function sourceRosAndWorkspace(): Promise<void> {
let distro = config.get("distro", "");

// Is there a distro defined either by setting or environment?
if (!distro && !process.env.ROS_DISTRO)
outputChannel.appendLine(`Current ROS_DISTRO environment variable: ${process.env.ROS_DISTRO}`);
if (!distro)
{
// No? Try to find one.
const installedDistros = await ros_utils.getDistros();
if (!installedDistros.length) {
outputChannel.appendLine(`No distros found.`);

throw new Error("ROS has not been found on this system.");
} else if (installedDistros.length === 1) {
outputChannel.appendLine(`Only one distro, selecting ${installedDistros[0]}`);

// if there is only one distro installed, directly choose it
config.update("distro", installedDistros[0]);
distro = installedDistros[0];
} else {
outputChannel.appendLine(`Multiple distros found, prompting user to select one.`);
// dump installedDistros to outputChannel
outputChannel.appendLine(`Installed distros: ${installedDistros}`);

const message = "Unable to determine ROS distribution, please configure this workspace by adding \"ros.distro\": \"<ROS Distro>\" in settings.json";
await vscode.window.setStatusBarMessage(message, kWorkspaceConfigTimeout);
}
}

if (process.env.ROS_DISTRO && process.env.ROS_DISTRO !== distro) {
outputChannel.appendLine(`ROS_DISTRO environment variable (${process.env.ROS_DISTRO}) does not match configured distro (${distro}).`);

outputChannel.appendLine(`Overriding the configured distro with the environment variable.`);

distro = process.env.ROS_DISTRO;
}

if (distro) {
let setupScript: string;
try {
Expand All @@ -371,6 +387,8 @@ async function sourceRosAndWorkspace(): Promise<void> {
name: "setup",
ext: setupScriptExt,
});

outputChannel.appendLine(`Sourcing ROS Distro: ${setupScript}`);
newEnv = await ros_utils.sourceSetupFile(setupScript, newEnv);
} catch (err) {
vscode.window.showErrorMessage(`Could not source ROS setup script at "${setupScript}".`);
Expand All @@ -381,30 +399,35 @@ async function sourceRosAndWorkspace(): Promise<void> {
}

let workspaceOverlayPath: string = "";
if (baseDir !== undefined) {
// Source the workspace setup over the top.
// TODO: we should test what's the build tool (catkin vs colcon).
workspaceOverlayPath = path.join(`${baseDir}`, "devel_isolated");
// Source the workspace setup over the top.

if (newEnv.ROS_VERSION === "1") {
workspaceOverlayPath = path.join(`${vscode.workspace.rootPath}`, "devel_isolated");
if (!await pfs.exists(workspaceOverlayPath)) {
workspaceOverlayPath = path.join(`${baseDir}`, "devel");
workspaceOverlayPath = path.join(`${vscode.workspace.rootPath}`, "devel");
}
} else { // FUTURE: Revisit if ROS_VERSION changes - not clear it will be called 3
if (!await pfs.exists(workspaceOverlayPath)) {
workspaceOverlayPath = path.join(`${baseDir}`, "install");
workspaceOverlayPath = path.join(`${vscode.workspace.rootPath}`, "install");
}
let wsSetupScript: string = path.format({
dir: workspaceOverlayPath,
name: "setup",
ext: setupScriptExt,
});
}

if (newEnv && typeof newEnv.ROS_DISTRO !== "undefined" && await pfs.exists(wsSetupScript)) {
try {
newEnv = await ros_utils.sourceSetupFile(wsSetupScript, newEnv);
} catch (_err) {
vscode.window.showErrorMessage("Failed to source the workspace setup file.");
}
}
let wsSetupScript: string = path.format({
dir: workspaceOverlayPath,
name: "setup",
ext: setupScriptExt,
});

if (await pfs.exists(wsSetupScript)) {
outputChannel.appendLine(`Workspace overlay path: ${wsSetupScript}`);

try {
newEnv = await ros_utils.sourceSetupFile(wsSetupScript, newEnv);
} catch (_err) {
vscode.window.showErrorMessage("Failed to source the workspace setup file.");
}
} else {
outputChannel.appendLine(`Not sourcing workspace does not exist yet: ${wsSetupScript}. Need to build workspace.`);
}

env = newEnv;
Expand Down
2 changes: 1 addition & 1 deletion src/ros/build-env-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export async function updateCppProperties(context: vscode.ExtensionContext): Pro
*/
async function updateCppPropertiesInternal(): Promise<void> {
let includes = await rosApi.getIncludeDirs();
const workspaceIncludes = await rosApi.getWorkspaceIncludeDirs(extension.baseDir);
const workspaceIncludes = await rosApi.getWorkspaceIncludeDirs(vscode.workspace.rootPath);
includes = includes.concat(workspaceIncludes);

if (process.platform === "linux") {
Expand Down
2 changes: 1 addition & 1 deletion src/ros/ros1/core-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export function startCore(context: vscode.ExtensionContext) {

let launchCoreCommand: string = "roscore";
let processOptions: child_process.SpawnOptions = {
cwd: extension.baseDir,
cwd: vscode.workspace.rootPath,
env: extension.env,
};

Expand Down
4 changes: 2 additions & 2 deletions src/ros/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export function sourceSetupFile(filename: string, env?: any): Promise<any> {
}

let processOptions: child_process.ExecOptions = {
cwd: extension.baseDir,
cwd: vscode.workspace.rootPath,
env: env,
};
child_process.exec(exportEnvCommand, processOptions, (error, stdout, _stderr) => {
Expand All @@ -49,7 +49,7 @@ export function sourceSetupFile(filename: string, env?: any): Promise<any> {
export function xacro(filename: string): Promise<any> {
return new Promise((resolve, reject) => {
let processOptions = {
cwd: extension.baseDir,
cwd: vscode.workspace.rootPath,
env: extension.env,
windowsHide: false,
};
Expand Down

0 comments on commit 4c6d533

Please sign in to comment.