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.

Sign up for GitHub

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

Jump to bottom

Fix for automatic folderOpen tasks with Remote SSH extension #204008

Merged
merged 1 commit into from
Feb 1, 2024
Merged

Fix for automatic folderOpen tasks with Remote SSH extension #204008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/vs/workbench/contrib/tasks/browser/abstractTaskService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
public onDidChangeTaskSystemInfo: Event<void> = this._onDidChangeTaskSystemInfo.event;
private _onDidReconnectToTasks: Emitter<void> = new Emitter();
public onDidReconnectToTasks: Event<void> = this._onDidReconnectToTasks.event;
private _onDidChangeTaskConfig: Emitter<void> = new Emitter();
public onDidChangeTaskConfig: Event<void> = this._onDidChangeTaskConfig.event;
public get isReconnected(): boolean { return this._tasksReconnected; }

constructor(
Expand Down Expand Up @@ -289,7 +291,7 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
this._updateSetup(folderSetup);
return this._updateWorkspaceTasks(TaskRunSource.FolderOpen);
}));
this._register(this._configurationService.onDidChangeConfiguration((e) => {
this._register(this._configurationService.onDidChangeConfiguration(async (e) => {
if (!e.affectsConfiguration('tasks') || (!this._taskSystem && !this._workspaceTasksPromise)) {
return;
}
Expand All @@ -306,7 +308,8 @@ export abstract class AbstractTaskService extends Disposable implements ITaskSer
}

this._setTaskLRUCacheLimit();
return this._updateWorkspaceTasks(TaskRunSource.ConfigurationChange);
await this._updateWorkspaceTasks(TaskRunSource.ConfigurationChange);
this._onDidChangeTaskConfig.fire();
}));
this._taskRunningState = TASK_RUNNING_STATE.bindTo(_contextKeyService);
this._onDidStateChange = this._register(new Emitter());
Expand Down
34 changes: 28 additions & 6 deletions src/vs/workbench/contrib/tasks/browser/runAutomaticTasks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,34 @@ export class RunAutomaticTasks extends Disposable implements IWorkbenchContribut
this._logService.trace('RunAutomaticTasks: Awaiting task system info.');
await Event.toPromise(Event.once(this._taskService.onDidChangeTaskSystemInfo));
}
const workspaceTasks = await this._taskService.getWorkspaceTasks(TaskRunSource.FolderOpen);
let workspaceTasks = await this._taskService.getWorkspaceTasks(TaskRunSource.FolderOpen);
this._logService.trace(`RunAutomaticTasks: Found ${workspaceTasks.size} automatic tasks`);
await this._runWithPermission(this._taskService, this._configurationService, workspaceTasks);

let autoTasks = this._findAutoTasks(this._taskService, workspaceTasks);
this._logService.trace(`RunAutomaticTasks: taskNames=${JSON.stringify(autoTasks.taskNames)}`);

// As seen in some cases with the Remote SSH extension, the tasks configuration is loaded after we have come
// to this point. Let's give it some extra time.
if (autoTasks.taskNames.length === 0) {
const updatedWithinTimeout = await Promise.race([
new Promise<boolean>((resolve) => {
Event.toPromise(Event.once(this._taskService.onDidChangeTaskConfig)).then(() => resolve(true));
}),
new Promise<boolean>((resolve) => {
const timer = setTimeout(() => { clearTimeout(timer); resolve(false); }, 10000);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting on some timeout is suspicious to me. Seems like you should just be able to know for sure when the task configuration is loaded. But I reloaded the window a few times and it worked when I tried it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks for your review!

I fully agree that this isn't the prettiest solution. In my opinion, it would have been better if TaskService waited with firing the existing onDidChangeTaskSystemInfo event until it had tried to fetch the task configuration, if any. I suspect this is where the hidden timing issue really is. But I know too little about the interactions involved in this code base, also together with the remote SSH extension, to look into that right now. And debugging all this was really quite complex when I couldn't use the remote SSH extension with my custom Code OSS build.

The timer is just there to not wait forever if there is no task configuration at all. Removing the timer (and not waiting just once for the new event) could be a way to support running automatic tasks that are configured manually after having opened the folder. But that's new functionality and not something I'd put in a bug fix.

Anyway, I just tested the latest Insiders build on Windows, which includes this commit. I can no longer reproduce the issues I had.

})]);

if (!updatedWithinTimeout) {
this._logService.trace(`RunAutomaticTasks: waited some extra time, but no update of tasks configuration`);
return;
}

workspaceTasks = await this._taskService.getWorkspaceTasks(TaskRunSource.FolderOpen);
autoTasks = this._findAutoTasks(this._taskService, workspaceTasks);
this._logService.trace(`RunAutomaticTasks: updated taskNames=${JSON.stringify(autoTasks.taskNames)}`);
}

this._runWithPermission(this._taskService, this._configurationService, autoTasks.tasks, autoTasks.taskNames);
}

private _runTasks(taskService: ITaskService, tasks: Array<Task | Promise<Task | undefined>>) {
Expand Down Expand Up @@ -124,10 +149,7 @@ export class RunAutomaticTasks extends Disposable implements IWorkbenchContribut
return { tasks, taskNames, locations };
}

private async _runWithPermission(taskService: ITaskService, configurationService: IConfigurationService, workspaceTaskResult: Map<string, IWorkspaceFolderTaskResult>) {

const { tasks, taskNames } = this._findAutoTasks(taskService, workspaceTaskResult);

private async _runWithPermission(taskService: ITaskService, configurationService: IConfigurationService, tasks: (Task | Promise<Task | undefined>)[], taskNames: string[]) {
if (taskNames.length === 0) {
return;
}
Expand Down
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/tasks/common/taskService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export interface ITaskService {

registerTaskSystem(scheme: string, taskSystemInfo: ITaskSystemInfo): void;
onDidChangeTaskSystemInfo: Event<void>;
onDidChangeTaskConfig: Event<void>;
readonly hasTaskSystemInfo: boolean;
registerSupportedExecutions(custom?: boolean, shell?: boolean, process?: boolean): void;

Expand Down