lime
lime copied to clipboard
openfl
Stale output from previous builds can cause errors
Lime tends to avoid unnecessary file operations where possible, which is good. Avoiding unnecessary file IO is faster, and some external build systems rely on file timestamps to determine whether a file has been modified. However, since the output folder is not cleaned before each build, this can lead to problems due to stale files remaining in the build output.
For a real-world example:
- Take a lime project that has Stencyl's iap extension enabled.
- Build the project for Android. The export folder now contains Java source files from the extension.
- Update the extension past this commit (notice that a number of Java source files have been deleted).
- Build the project for Android again. The lime output / gradle input folder now contains a mixture of the old Java source files and the new ones, causing a compilation error during the gradle build.
Of course the easiest way to get a correct build in this situation is to just do a clean build. But I'd like to explore options that allow us to have a correct build without sacrificing the cache, even in cases like this. There should be some mechanism in place that allows the build process to clean out stale files from old builds.
I've prepared one potential approach, and have put it to use in the Android platform, which I'll post soon.
Also worth noting that the vast majority of an Android/iOS build is spent compiling C++ code, which is cached in the obj folder. Deleting the bin folder wouldn't slow compilation that much, and would also fix this. I'm still happy to talk about the automated approach, but should Lime also add a -clear-templates flag? (Or some other name?)
I do think there's a better way. While looking at the code, I realized a few key things.
- There's no need to call
markFileAsTouched()if the file is overwritten. Instead, we can check the timestamp. - All non-template assets are included in an asset manifest. We can read the manifest for an up-to-date list.
- It is possible for asset manifests themselves to be stale, but we can tell by checking the timestamp.
- All templates are ultimately handled by
hxp.System.copyFile().- Custom source files like the ones in Stencyl's iap extension are all processed as templates.
That last one is extra important, because it means we don't even need to worry about ordinary assets. We could clean them up if we had to, but they aren't going to break anything.
Pull requests pending.
Hey Justin, do you plan to continue working on your pulls, or should we proceed with mine?
Sorry about taking so long to reply to this. I wanted to take a closer look at how templates are handled to determine whether I could get behind your assertions or not, and I didn't put time aside for that until now.
Also worth noting that the vast majority of an Android/iOS build is spent compiling C++ code, which is cached in the obj folder. Deleting the bin folder wouldn't slow compilation that much, and would also fix this. I'm still happy to talk about the automated approach, but should Lime also add a
-clear-templatesflag? (Or some other name?)
I'm not sure that would be helpful, because it requires users to know why their build is suddenly failing, and that using that flag is the appropriate course of action.
- There's no need to call
markFileAsTouched()if the file is overwritten. Instead, we can check the timestamp.
The issue is that many files are purposefully not overwritten, even though they're not stale. For text files, overwriting is avoided if the file content is unchanged, and for other types of files, overwriting is avoided if the timestamp is newer in the destination. This is why everything is tracked on a per-file-operation basis in my PR.
- All non-template assets are included in an asset manifest. We can read the manifest for an up-to-date list.
Yes, perhaps this would be a better way to handle the assets folder. Asset handling is one of the weaknesses of my solution.
I included assets in my PR since the template output folder encompasses the assets as well, at least for Android and iOS targets, and they're all part of the same update step.
A notable disadvantage of my current approach is the potential of AssetHelper.processLibraries to call external asset handlers, leaving the filesystem in an unknown state, in which case I just call System.markAllFilesStateAsUnknown and give up. I don't know what exactly external asset processors are expected to do, how widespread their use is, or if the ones in use today are well-behaved in respect to updating the project's asset listing.
Ignoring the assets folder (just like how the "app/build" and "app/src/main/jniLibs" folders are ignored in AndroidPlatform in my PR), and instead using the manifest to determine what should still exist there may be a better solution, certainly if it works with external asset processors.
- All templates are ultimately handled by
hxp.System.copyFile().
- Custom source files like the ones in Stencyl's iap extension are all processed as templates.
I do think there's a pleasant simplicity to your idea, and it would certainly solve the issue I posed as an example.
It leaves out everything other than processed templates, though. Leaving other types of stale files might not cause process-stopping compilation errors, but including files that the user didn't intend to keep is an error in its own right. The problem of stale files in general is what I hope to address here.
For example, during the update step, other types of resources are also copied without using the copyTemplate functions at all, such as application icons, splash screens, webfonts, javascript dependencies, and so on.
A notable disadvantage of my current approach is the potential of
AssetHelper.processLibrariesto call external asset handlers, leaving the filesystem in an unknown state, in which case I just callSystem.markAllFilesStateAsUnknownand give up. I don't know what exactly external asset processors are expected to do, how widespread their use is, or if the ones in use today are well-behaved in respect to updating the project's asset listing.
I agree that Lime shouldn't mess with files added by external asset handlers, but we can definitely do better than giving up on all files.
It leaves out everything other than processed templates, though. Leaving other types of stale files might not cause process-stopping compilation errors, but including files that the user didn't intend to keep is an error in its own right. The problem of stale files in general is what I hope to address here.
I don't think they'll cause runtime errors either. That said, there are at least a few targets where they'll make it into the final package, increasing download size, which isn't great.
I took another look at your code and realized you do this:
System.markAllFilesInFolderAsUntouched(destination, ["app/build", "app/src/main/jniLibs"]);
...ensuring that all of Gradle's build files will be deleted and Gradle will have to start from scratch, even though I'm nearly certain it can manage app/build just fine if we leave it alone. I tested this in DisplayingABitmap, and deleting app/build added about 2.5 seconds to the build time.
But this got me thinking. The only remaining problem seems to be filesize, and that only really matters when distributing the binary to the public. And we already have a flag for ready-for-distribution builds: -final.
So what if final builds deleted Export/
This isn't only about correctness of final builds (which is important of course!), but correctness of testing builds as well. I would expect that modifying source files and then testing a game again will make all the changes necessary so that the output folder accurately reflects the inputs.
I took another look at your code and realized you do this:
System.markAllFilesInFolderAsUntouched(destination, ["app/build", "app/src/main/jniLibs"]);
Right, I do that, and that's precisely what I referred to in my previous post. It's two folders which are ignored. It's probably counterintuitive, but it does the opposite of what you think. It ensures that those folders aren't considered in the stable/live check, and not deleted in the end either.
As you said, gradle should be trusted to do its job. I said as much as well in my lime PR:
app/src/main/jniLibsis ignored because that folders isn't modified as part of theupdatestep, but later on as part of thebuildstep.app/buildis ignored because that's the gradle build folder. We don't directly touch it ourselves, and gradle can hopefully be trusted to keep that in order as long as we provide the correct inputs to the gradle build.
In the same way that those folders are ignored, I think that ignoring the assets folder as well and letting it be cleaned by a different procedure, one that takes the asset manifest into account, may be a good idea, especially if that cleanly addresses the weakness in my proposal.
This isn't only about correctness of final builds (which is important of course!), but correctness of testing builds as well.
Can you give an example of a real-world use case where this would interfere with testing?