Skip to content

make livesync work with renamed files and folders #3042

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

Merged
merged 8 commits into from
Aug 17, 2017
Merged

make livesync work with renamed files and folders #3042

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
94eeebb
added special check for file changes on Mac
Plamen5kov Aug 9, 2017
d7f06d2
fix missing imports in tests
Plamen5kov Aug 9, 2017
3692d8b
add dir check, modified file check
Plamen5kov Aug 10, 2017
c861d06
check isDirModified before anything else to save time
Plamen5kov Aug 10, 2017
3b8cb02
updates after review
Plamen5kov Aug 11, 2017
3c2713c
bring back check for symlinks
Plamen5kov Aug 14, 2017
9020a86
add check for modification in bot files and folders
Plamen5kov Aug 14, 2017
f99697d
fixed lint
Plamen5kov Aug 14, 2017
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
19 changes: 14 additions & 5 deletions lib/services/project-changes-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export class ProjectChangesService implements IProjectChangesService {
path.join(projectData.projectDir, NODE_MODULES_FOLDER_NAME, "tns-ios-inspector"),
projectData,
this.fileChangeRequiresBuild);

if (this._newFiles > 0) {
this._changesInfo.modulesChanged = true;
}
Expand Down Expand Up @@ -218,6 +219,10 @@ export class ProjectChangesService implements IProjectChangesService {
}

private containsNewerFiles(dir: string, skipDir: string, projectData: IProjectData, processFunc?: (filePath: string, projectData: IProjectData) => boolean): boolean {
if (this.isDirectoryModified(dir)) {
return true;
}

let files = this.$fs.readDirectory(dir);
for (let file of files) {
let filePath = path.join(dir, file);
Expand All @@ -227,11 +232,8 @@ export class ProjectChangesService implements IProjectChangesService {

let fileStats = this.$fs.getFsStats(filePath);

let changed = fileStats.mtime.getTime() >= this._outputProjectMtime || fileStats.ctime.getTime() >= this._outputProjectCTime;
if (!changed) {
let lFileStats = this.$fs.getLsStats(filePath);
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this code deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's unnecessary to do this.$fs.getLsStats(filePath) twice and the logic is kept the same in the refactored version

Copy link
Contributor

Choose a reason for hiding this comment

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

getLstStats and getFsStats are different. This code is used when the file/dir is symlink - we need to check the symlink itself (for example if it is recreated) and this is done by getLsStats.

Copy link
Contributor

Choose a reason for hiding this comment

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

ping @Plamen5kov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will bring back the code, so we continue working with synlinked files.

changed = lFileStats.mtime.getTime() >= this._outputProjectMtime || lFileStats.ctime.getTime() >= this._outputProjectCTime;
}
let changed = fileStats.mtime.getTime() > this._outputProjectMtime ||
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this without = sign, but the next check has it? I believe you need the = in both cases, as the _outputProjectMtime is set after the prepare. In case it is equal with a file's mtime, this means the file has been modified when the prepare is finishing (exactly at the same time), so we need to prepare the project again.

fileStats.ctime.getTime() >= this._outputProjectCTime;

if (changed) {
if (processFunc) {
Expand All @@ -250,10 +252,17 @@ export class ProjectChangesService implements IProjectChangesService {
return true;
}
}

}
return false;
}

private isDirectoryModified(dirPath: string): boolean {
const dirPathStat = this.$fs.getFsStats(dirPath);
Copy link
Contributor

Choose a reason for hiding this comment

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

The code is the same as the one used on line 235. Also the method is called isDirectortyModified, but the implementation does not have any code specific to directories only.
I suggest renaming the method and use it on line 235.

return dirPathStat.mtime.getTime() > this._outputProjectMtime ||
dirPathStat.ctime.getTime() >= this._outputProjectCTime;
}

private fileChangeRequiresBuild(file: string, projectData: IProjectData) {
if (path.basename(file) === "package.json") {
return true;
Expand Down
5 changes: 4 additions & 1 deletion test/project-changes-service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { PlatformsData } from "../lib/platforms-data";
import { ProjectChangesService } from "../lib/services/project-changes-service";
import * as Constants from "../lib/constants";
import { FileSystem } from "../lib/common/file-system";
import * as HostInfoLib from "../lib/common/host-info";
import * as ErrorsLib from "../lib/common/errors";

// start tracking temporary folders/files
temp.track();
Expand All @@ -26,7 +28,8 @@ class ProjectChangesServiceTest extends BaseServiceTest {
this.injector.register("platformsData", PlatformsData);
this.injector.register("androidProjectService", {});
this.injector.register("iOSProjectService", {});

this.injector.register("hostInfo", HostInfoLib.HostInfo);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not used and shouldn't be added

this.injector.register("errors", ErrorsLib.Errors);
Copy link
Contributor

Choose a reason for hiding this comment

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

this is not used and shouldn't be added

this.injector.register("fs", FileSystem);
this.injector.register("devicePlatformsConstants", {});
this.injector.register("devicePlatformsConstants", {});
Expand Down