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

Preserve files on exit (aka hot exit) #12400

Merged
merged 58 commits into from
Oct 20, 2016
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
c410c94
Add files.hotExit setting
Tyriar Sep 16, 2016
ce42316
Start building out backup file code
Tyriar Sep 17, 2016
90e9796
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Sep 20, 2016
33e6a47
Shuffle backup code around
Tyriar Sep 20, 2016
39faa28
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Sep 21, 2016
9c9c0b2
Perform backups on content change not dirty change
Tyriar Sep 21, 2016
9c0d19a
Back up files to the user data directory
Tyriar Sep 21, 2016
a8b4f27
Backup to a unique location for this workspace
Tyriar Sep 21, 2016
9e48499
Discard backups on exit
Tyriar Sep 21, 2016
038f756
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Sep 21, 2016
2ded2a7
Discard backups when the file is no longer dirty
Tyriar Sep 21, 2016
24adcf7
Fix tests
Tyriar Sep 21, 2016
08fad0b
Move backup logic into models
Tyriar Sep 21, 2016
54108a5
Use unique backup path for file and untitled schemes
Tyriar Sep 22, 2016
d118598
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Sep 22, 2016
c860d62
Add new workspaces to backups as they are created on main process
Tyriar Sep 23, 2016
a638f02
Remove workspace from workspaces.json when not last window
Tyriar Sep 23, 2016
be58446
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 5, 2016
836cbac
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 6, 2016
61d2696
Get hot exit working end-to-end (very rough)
Tyriar Oct 7, 2016
163dfff
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 7, 2016
2cac437
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 11, 2016
f7ea530
Make workspaces.json loading more resilient
Tyriar Oct 11, 2016
660f6bd
Open restored files pinned
Tyriar Oct 11, 2016
86e1c20
Fix tests
Tyriar Oct 11, 2016
4928801
Get untitled files restoring from backup
Tyriar Oct 13, 2016
701fe8c
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 13, 2016
e4f0dec
Remove usage of fs in common/ layer
Tyriar Oct 13, 2016
52c8af5
Perform a last minute backup when closing the last window
Tyriar Oct 13, 2016
550da5e
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 13, 2016
e055ced
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 13, 2016
5f230ed
Move workspaces.json path to IEnvironmentService
Tyriar Oct 13, 2016
4fcfdc6
Replace all {} with Object.create(null)
Tyriar Oct 13, 2016
a822c88
Remove unused service
Tyriar Oct 13, 2016
1a29d14
Improve IBackupService names and add jsdoc
Tyriar Oct 14, 2016
3387b6c
Move BackupService to vs/platform/backup/node/
Tyriar Oct 14, 2016
c280e0c
Merge main and renderer BackupServices
Tyriar Oct 14, 2016
10aa709
Disable hot exit on empty workspaces, fix uncaught exceptions
Tyriar Oct 14, 2016
69a6a18
Remove empty line after function
Tyriar Oct 14, 2016
9d4ec99
Remove IWorkspaceContextService as dep on BackupService
Tyriar Oct 14, 2016
a002932
Clean up pass
Tyriar Oct 14, 2016
dcd624c
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 17, 2016
2b0a65a
Remove async calls in all but critical window/workbench init
Tyriar Oct 17, 2016
9aa7b9c
Improve exit logic when errors occur
Tyriar Oct 17, 2016
1a46093
Fix unit tests
Tyriar Oct 17, 2016
084764d
Add null checks to fix integration tests
Tyriar Oct 17, 2016
da56a72
Push backup resource aquisition to FileEditorInput
Tyriar Oct 17, 2016
e53f008
Fix sync-related backupservice issues, add tests
Tyriar Oct 18, 2016
9e5a282
Merge remote-tracking branch 'origin/master' into tyriar/101_hot_exit
Tyriar Oct 19, 2016
f9cc78f
Fix duplicate instances being restored on non-Linux
Tyriar Oct 19, 2016
ecc83b8
Use Uri exclusively to deal with paths in BackupService
Tyriar Oct 19, 2016
30bb0b7
Fix tests to work on Windows
Tyriar Oct 19, 2016
eb2a8e1
Fix some empty workspace edge cases on Mac
Tyriar Oct 19, 2016
a040237
Disable hot exit on Mac
Tyriar Oct 19, 2016
36f1d70
jsdoc and comment cleanup
Tyriar Oct 19, 2016
e9be330
Add backup tests for FileService
Tyriar Oct 19, 2016
0c48efc
Add untitled backup tests to fileService
Tyriar Oct 19, 2016
4e34bfc
Use a temp folder for backupservice tests
Tyriar Oct 20, 2016
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
85 changes: 85 additions & 0 deletions src/vs/code/electron-main/backup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*---------------------------------------------------------------------------------------------
Copy link
Member

Choose a reason for hiding this comment

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

So this guy is the one that manages all backups across all workspaces and the vs/platform/backup is the one per workspace for individual files?

Copy link
Member Author

Choose a reason for hiding this comment

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

IBackupService manages all I/O with what is now the IEnvironmentService.backupHome directory.

* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

'use strict';

import * as fs from 'original-fs';
import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';

export const IBackupService = createDecorator<IBackupService>('backupService');

export interface IBackupService {
getBackupWorkspaces(): string[];
clearBackupWorkspaces(): void;
pushBackupWorkspaces(workspaces: string[]): void;
Copy link
Member

Choose a reason for hiding this comment

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

Not quite clear here what workspaces is about? Is this an identifier or absolute path?

Copy link
Member Author

Choose a reason for hiding this comment

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

"workspaces" here is an absolute path to a "folderWorkspaces" entry within workspaces.json. For example in the following:

{
  "folderWorkspaces": {
    "/home/daimms/dev/Microsoft/vscode/src": [
      "/home/daimms/dev/Microsoft/vscode/src/bootstrap.js"
    ]
  }
}

/home/daimms/dev/Microsoft/vscode/src is a workspace. I can see how the name could be confusing, how about getWorkspaceBackupPath, clearWorkspaceBackupPaths, etc.?

getBackupFiles(workspace: string): string[];
}

interface IBackupFormat {
folderWorkspaces?: {
[workspacePath: string]: string[]
};
}

export class BackupService implements IBackupService {

private fileContent: IBackupFormat;

constructor(
@IEnvironmentService private environmentService: IEnvironmentService
) {
}

public getBackupWorkspaces(): string[] {
if (!this.fileContent) {
this.load();
}
return Object.keys(this.fileContent.folderWorkspaces || Object.create(null));
}

public clearBackupWorkspaces(): void {
this.fileContent = {
folderWorkspaces: Object.create(null)
};
this.save();
}

public pushBackupWorkspaces(workspaces: string[]): void {
this.load();
workspaces.forEach(workspace => {
if (!this.fileContent.folderWorkspaces[workspace]) {
this.fileContent.folderWorkspaces[workspace] = [];
}
});
this.save();
}

public getBackupFiles(workspace: string): string[] {
this.load();
return this.fileContent.folderWorkspaces[workspace];
}

private load(): void {
try {
this.fileContent = JSON.parse(fs.readFileSync(this.environmentService.backupWorkspacesPath).toString()); // invalid JSON or permission issue can happen here
} catch (error) {
this.fileContent = Object.create(null);
}
if (Array.isArray(this.fileContent) || typeof this.fileContent !== 'object') {
this.fileContent = Object.create(null);
}
if (!this.fileContent.folderWorkspaces) {
this.fileContent.folderWorkspaces = Object.create(null);
}
}

private save(): void {
try {
fs.writeFileSync(this.environmentService.backupWorkspacesPath, JSON.stringify(this.fileContent));
} catch (error) {
}
}
}
8 changes: 5 additions & 3 deletions src/vs/code/electron-main/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { ServiceCollection } from 'vs/platform/instantiation/common/serviceColle
import { SyncDescriptor } from 'vs/platform/instantiation/common/descriptors';
import { ILogService, MainLogService } from 'vs/code/electron-main/log';
import { IStorageService, StorageService } from 'vs/code/electron-main/storage';
import { IBackupService, BackupService } from 'vs/code/electron-main/backup';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { EnvironmentService } from 'vs/platform/environment/node/environmentService';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
Expand Down Expand Up @@ -255,11 +256,11 @@ function main(accessor: ServicesAccessor, mainIpcServer: Server, userEnv: platfo

// Open our first window
if (environmentService.args['new-window'] && environmentService.args._.length === 0) {
windowsService.open({ cli: environmentService.args, forceNewWindow: true, forceEmpty: true }); // new window if "-n" was used without paths
windowsService.open({ cli: environmentService.args, forceNewWindow: true, forceEmpty: true, restoreBackups: true }); // new window if "-n" was used without paths
} else if (global.macOpenFiles && global.macOpenFiles.length && (!environmentService.args._ || !environmentService.args._.length)) {
windowsService.open({ cli: environmentService.args, pathsToOpen: global.macOpenFiles }); // mac: open-file event received on startup
windowsService.open({ cli: environmentService.args, pathsToOpen: global.macOpenFiles, restoreBackups: true }); // mac: open-file event received on startup
} else {
windowsService.open({ cli: environmentService.args, forceNewWindow: environmentService.args['new-window'], diffMode: environmentService.args.diff }); // default: read paths from cli
windowsService.open({ cli: environmentService.args, forceNewWindow: environmentService.args['new-window'], diffMode: environmentService.args.diff, restoreBackups: true }); // default: read paths from cli
}
}

Expand Down Expand Up @@ -474,6 +475,7 @@ function start(): void {
services.set(IConfigurationService, new SyncDescriptor(ConfigurationService));
services.set(IRequestService, new SyncDescriptor(RequestService));
services.set(IUpdateService, new SyncDescriptor(UpdateManager));
services.set(IBackupService, new SyncDescriptor(BackupService));
services.set(IURLService, new SyncDescriptor(URLService, args['open-url']));

const instantiationService = new InstantiationService(services);
Expand Down
23 changes: 22 additions & 1 deletion src/vs/code/electron-main/windows.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import * as types from 'vs/base/common/types';
import * as arrays from 'vs/base/common/arrays';
import { assign, mixin } from 'vs/base/common/objects';
import { EventEmitter } from 'events';
import { IBackupService } from 'vs/code/electron-main/backup';
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
import { IStorageService } from 'vs/code/electron-main/storage';
import { IPath, VSCodeWindow, ReadyState, IWindowConfiguration, IWindowState as ISingleWindowState, defaultWindowState, IWindowSettings } from 'vs/code/electron-main/window';
Expand Down Expand Up @@ -49,6 +50,7 @@ export interface IOpenConfiguration {
forceEmpty?: boolean;
windowToUse?: VSCodeWindow;
diffMode?: boolean;
restoreBackups?: boolean;
}

interface IWindowState {
Expand Down Expand Up @@ -166,7 +168,8 @@ export class WindowsManager implements IWindowsService {
@IEnvironmentService private environmentService: IEnvironmentService,
@ILifecycleService private lifecycleService: ILifecycleService,
@IUpdateService private updateService: IUpdateService,
@IConfigurationService private configurationService: IConfigurationService
@IConfigurationService private configurationService: IConfigurationService,
@IBackupService private backupService: IBackupService
) { }

onOpen(clb: (path: IPath) => void): () => void {
Expand Down Expand Up @@ -602,6 +605,18 @@ export class WindowsManager implements IWindowsService {
iPathsToOpen = this.cliToPaths(openConfig.cli, ignoreFileNotFound);
}

// Add any existing backup workspaces
if (openConfig.restoreBackups) {
// TODO: Ensure the workspaces being added actually have backups
this.backupService.getBackupWorkspaces().forEach(ws => {
Copy link
Member

Choose a reason for hiding this comment

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

What if a window is already opened with a workspace that has backup files, I think you need to avoid opening those again (since we prevent opening the same workspace twice we would just focus that window).

Copy link
Member Author

Choose a reason for hiding this comment

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

Provided I understand correctly, does opening the same folder twice restore backups? This should never happen as openConfig.restoreBackups is only ever true on the first window launch. https://github.com/Microsoft/vscode/pull/12400/files#diff-d8dfcbab1cae48b2cbc3cabc682c9cd4R258

iPathsToOpen.push(this.toIPath(ws));
});
// Get rid of duplicates
iPathsToOpen = arrays.distinct(iPathsToOpen, path => {
return path.workspacePath;
});
}

let filesToOpen: IPath[] = [];
let filesToDiff: IPath[] = [];
let foldersToOpen = iPathsToOpen.filter(iPath => iPath.workspacePath && !iPath.filePath);
Expand Down Expand Up @@ -730,6 +745,12 @@ export class WindowsManager implements IWindowsService {
// Emit events
iPathsToOpen.forEach(iPath => this.eventEmitter.emit(EventTypes.OPEN, iPath));

// Add to backups
Copy link
Member

Choose a reason for hiding this comment

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

Why are we adding these workspaces again to backup service?

Copy link
Member Author

Choose a reason for hiding this comment

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

The intent here is for workspaces.json to contain a key for each workspace currently opened in order to support the crash recovery use case, so it's being added on the main process side when the window is launched.

This is also used to determine whether a single window is opened (there is probably another way to do this) in order to decide whether the backups can be discarded safely, but that's not the main reason for it.

console.log('iPathsToOpen', iPathsToOpen);
this.backupService.pushBackupWorkspaces(iPathsToOpen.map((path) => {
return path.workspacePath;
}));

return arrays.distinct(usedWindows);
}

Expand Down
25 changes: 25 additions & 0 deletions src/vs/platform/backup/common/backup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

'use strict';

import { createDecorator } from 'vs/platform/instantiation/common/instantiation';
import Uri from 'vs/base/common/uri';

export const IBackupService = createDecorator<IBackupService>('backupService');

export interface IBackupService {
_serviceBrand: any;

getBackupWorkspaces(): string[];
Copy link
Member

Choose a reason for hiding this comment

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

Now I am confused why this guy reaches into the territory of vs/electron-main/backup

Copy link
Member Author

Choose a reason for hiding this comment

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

As discussed, I'll merge these services as there is no reason for them not to share code.

clearBackupWorkspaces(): void;
removeWorkspace(workspace: string): void;

registerBackupFile(resource: Uri): void;
deregisterBackupFile(resource: Uri): void;
getBackupFiles(workspace: string): string[];
getBackupUntitledFiles(workspace: string): string[];
getBackupResource(resource: Uri): Uri;
}
3 changes: 3 additions & 0 deletions src/vs/platform/environment/common/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ export interface IEnvironmentService {
appSettingsPath: string;
appKeybindingsPath: string;

backupHome: string;
backupWorkspacesPath: string;

disableExtensions: boolean;
extensionsPath: string;
extensionDevelopmentPath: string;
Expand Down
6 changes: 6 additions & 0 deletions src/vs/platform/environment/node/environmentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,12 @@ export class EnvironmentService implements IEnvironmentService {
@memoize
get appKeybindingsPath(): string { return path.join(this.appSettingsHome, 'keybindings.json'); }

@memoize
get backupHome(): string { return path.join(this.userDataPath, 'Backups'); }

@memoize
get backupWorkspacesPath(): string { return path.join(this.backupHome, 'workspaces.json'); }

@memoize
get extensionsPath(): string { return path.normalize(this._args.extensionHomePath || path.join(this.userHome, 'extensions')); }

Expand Down
22 changes: 22 additions & 0 deletions src/vs/platform/files/common/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,27 @@ export interface IFileService {
*/
del(resource: URI, useTrash?: boolean): TPromise<void>;

/**
* Backs up the provided file to a temporary directory to be used by the hot
* exit feature and crash recovery.
*/
backupFile(resource: URI, content: string): TPromise<IFileStat>;
Copy link
Member

Choose a reason for hiding this comment

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

I think all of this should move into textfile.ts (ITextFileService) because in here we talk about all files but for hot exit we only care about text files.

Copy link
Member Author

@Tyriar Tyriar Oct 13, 2016

Choose a reason for hiding this comment

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

For hot exit we care about untitled files as well, I believe you told me to put it in IFileService because of this?

Copy link
Member

Choose a reason for hiding this comment

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

I meanwhile moved ITextFileService out of file contrib land into the workbench so there was a chance of layer breaking. But now that the service is in workbench core it is on the same level as untitled files.

Copy link
Member Author

Choose a reason for hiding this comment

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

To clarify, so I should move the backup logic to ITextFileService?


/**
* Discard the backup for the resource specified.
*/
discardBackup(resource: URI): TPromise<void>;

/**
* Discards all backups associated with this session.
*/
discardBackups(): TPromise<void>;

/**
* Whether hot exit is enabled.
*/
isHotExitEnabled(): boolean;

/**
* Imports the file to the parent identified by the resource.
*/
Expand Down Expand Up @@ -475,6 +496,7 @@ export interface IFilesConfiguration {
autoSave: string;
autoSaveDelay: number;
eol: string;
hotExit: boolean;
};
}

Expand Down
18 changes: 16 additions & 2 deletions src/vs/test/utils/servicesTestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { Storage, InMemoryLocalStorage } from 'vs/workbench/node/storage';
import { IEditorGroup, ConfirmResult } from 'vs/workbench/common/editor';
import Event, { Emitter } from 'vs/base/common/event';
import Severity from 'vs/base/common/severity';
import {IBackupService} from 'vs/platform/backup/common/backup';
import { IConfigurationService, getConfigurationValue, IConfigurationValue } from 'vs/platform/configuration/common/configuration';
import { IStorageService, StorageScope } from 'vs/platform/storage/common/storage';
import { IQuickOpenService } from 'vs/workbench/services/quickopen/common/quickOpenService';
Expand Down Expand Up @@ -110,9 +111,10 @@ export class TestTextFileService extends TextFileService {
@IEditorGroupService editorGroupService: IEditorGroupService,
@IFileService fileService: IFileService,
@IUntitledEditorService untitledEditorService: IUntitledEditorService,
@IInstantiationService instantiationService: IInstantiationService
@IInstantiationService instantiationService: IInstantiationService,
@IBackupService backupService: IBackupService
) {
super(lifecycleService, contextService, configurationService, telemetryService, editorGroupService, editorService, fileService, untitledEditorService, instantiationService);
super(lifecycleService, contextService, configurationService, telemetryService, editorGroupService, editorService, fileService, untitledEditorService, instantiationService, backupService);
}

public setPromptPath(path: string): void {
Expand Down Expand Up @@ -572,6 +574,18 @@ export const TestFileService = {
name: paths.basename(res.fsPath)
};
});
},

discardBackup: function (resource: URI) {
return TPromise.as(void 0);
},

discardBackups: function () {
return TPromise.as(void 0);
},

isHotExitEnabled: function () {
return false;
}
};

Expand Down
Loading