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

cordova prepare misses updated files with same size and older timestamp #61

Open
brodycj opened this issue Jan 23, 2019 · 6 comments
Open
Labels

Comments

@brodycj
Copy link
Contributor

brodycj commented Jan 23, 2019

As reported by @knight9999 on Slack: if a file in www is updated to have the same size but an older timestamp compared to the corresponding file within platforms then cordova prepare will miss the file. Reproduction is given in: https://github.com/knight9999/sampleUnupdate

I think the best solution would be to check the md5 sum whenever the file in www and the corresponding file in platforms have the same size.

@brodycj brodycj added the bug label Jan 23, 2019
@janpio
Copy link
Member

janpio commented Jun 12, 2019

What code from common is actually involved here?

@brodycj
Copy link
Contributor Author

brodycj commented Jun 12, 2019

What code from common is actually involved here?

From the reproduction project I gave in the description:

if (sourceStats.mtime.getTime() >= targetStats.mtime.getTime() ||
sourceStats.size !== targetStats.size) {
log('copy ' + sourcePath + ' ' + targetPath + ' (updated file)');
fs.copySync(sourceFullPath, targetFullPath);
updated = true;
}

There may be a way to work around this in an options object, and there may or may not be a way to trigger this work around from the Cordova CLI. I will need some time to investigate.

@raphinesse
Copy link
Contributor

Do we know how it comes to be that the "latest" source version of the file has an older timestamp than a copy of an earlier version of it?

@brodycj
Copy link
Contributor Author

brodycj commented Dec 15, 2019

I suspect this could be triggered in one of 2 ways:

  • developer is manually moving old and new files back and forth on disk somehow
  • developer may be getting file with old timestamp using tar x (extract) command
  • some kind of custom file control system

I think git should not do this kind of thing.

@raphinesse
Copy link
Contributor

Thanks for providing that list @brodybits.

At least on Linux (and probably macOS) the following holds:

mtime changes when the file content changes. So if you move a file using mv the mtime stays the same. Moreover user space programs can easily modify mtime manually (e.g. 'touch'). So while we still should update the target if source.mtime >= target.mtime, the assumption that source.mtime < target.mtime implies source.data = target.data is faulty.

Unfortunately ctime does not really help us here either. While it changes any time mtime does and it cannot be changed by ordinary means, it also changes on a lot of other occasions that do not affect mtime, including when the file is read.

AFAICT the only way to know if the files are different is actually reading the contents. However, check sums don't make much sense here. You would have to read both source and target, compute the checksum on each, and if they differ read source again and write it to target. It should be much more efficient to read chunks of a fixed size (e.g. 4 KB) from both files until you have found the offset of the first difference and then copy only the bytes from that offset to the end. That way you read source once and either read or write each byte in target once.

However, even with the optimized approach above you would have to read every unchanged file twice in its entirety. As unchanged files probably make up the majority of files, I don't think we want to do that.

After thinking about this long and hard, I asked myself "What would make do?". To know what to recompile, they have to figure out all changed files too. According to this SO question they also just use timestamps. I would like to have a more reliable source on that though.

To conclude, I think we should not start comparing the file contents in FileUpdater but instead continue to rely on the file stats. As unsatisfactory as this might be, but I think if the timestamps get out of sync you have to do a clean build. I usually do rm -rf platforms plugins && cordova prepare.

PS: There is a copyAll option that will skip the stat comparison and simply copy all files. I don't think that we should add a --force parameter to cordova prepare though.

@githubDiversity
Copy link

githubDiversity commented Jan 3, 2020

Although not directly related to the issue.

I ended up changing line 100 in

if (sourceStats.mtime.getTime() >= targetStats.mtime.getTime() ||
sourceStats.size !== targetStats.size) {
log('copy ' + sourcePath + ' ' + targetPath + ' (updated file)');
fs.copySync(sourceFullPath, targetFullPath);
updated = true;
}

to check if source time > target time rather than >=

In my scenario I have many static assets that never change that kept being copied because the source and target time were the same. Which is to be expected of static assets so I am not sure why the >= check

Also I use a live reload approach so there are often updates with only minor changes often in a single file only thus the speed of prepare needs to be as fast as possible as to assure an effective live reload experience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants