Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove a layer of indirection causing race condition #1172

Merged
merged 1 commit into from
Oct 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading