Archive compiled core and use on subsequent compilations #213
Conversation
| } | ||
|
|
||
| func GetCoreArchivePath(fqbnToUnderscore string) string { | ||
| return os.TempDir() + "/core_" + fqbnToUnderscore + ".a" |
There was a problem hiding this comment.
Is it a good idea to put these files directly in the tempdir? Also, shouldn't these files be cleaned up at some point (e.g. when the IDE exits)?
There was a problem hiding this comment.
This is only a temporary way to store them 😄 A proper db (or folder structure) is surely the way to go
| @@ -258,6 +289,24 @@ func nonEmptyString(s string) bool { | |||
| return s != constants.EMPTY_STRING | |||
| } | |||
|
|
|||
| func CheckIfRecompileIsAvoidable(corePath string, archiveFile string) bool { | |||
There was a problem hiding this comment.
Should this not also archive and check the build options used? It's a bit of a corner case, but if the platform.txt is now changed (or any other source of compile options), the old .a file is still used. I guess this could happen when updating the core through the boards manager. The sketch itself is rebuilt ("Build options changed, rebuilding all..." or something like that), but I think this is bypassed by this archiving.
There was a problem hiding this comment.
Correct, the timestamps are only extracted from core subfolder, while they should be applied to the whole folder. I'm pushing another commit to fix this
There was a problem hiding this comment.
How does this interplay with referenced cores? E.g. when a platform has a platform.txt but does not contain the actual core, but instead references (e.g.) the arduino core? I believe compilation options could then come from either platform?
In general, I wonder if this can be more thoroughly covered? Ideally, a compiled core would keep a file (like the .d files for each .o file) that shows what files were used to create the .a file, so these could all be checked for modifications, as well as noting the compile options / build properties used to build the core. Even more general would be to log the complete compiler commands executed to build a .a file, and then compare that to the commands that would be executed on a subsequent run, and only use them if the commands are exactly equal? This is a bit similar to the caching for include detection that is now used.
There was a problem hiding this comment.
There are some tricky details, indeed, which should be tackled in a saner way. The inter-dependencies problem is one (and could be solved by tracking also the parent core). Note that the current builder does not wipe the environment if platform.txt changes (because it's not part of the .d dance). Tracking dependencies via .d files is really handy for libraries or mutable code, while the core is not really mutable (in Create it's even "static"). To be even more conservative, one could track all the modification dates on all core folders and wipe if even one file is newer than the precompiled core (this could be time consuming in case of slow disks or lots of cores).
| @@ -70,6 +70,7 @@ func (s *SetupBuildProperties) Run(ctx *types.Context) error { | |||
| buildProperties[constants.BUILD_PROPERTIES_RUNTIME_PLATFORM_PATH] = targetPlatform.Folder | |||
| buildProperties[constants.BUILD_PROPERTIES_RUNTIME_HARDWARE_PATH] = filepath.Join(targetPlatform.Folder, "..") | |||
| buildProperties[constants.BUILD_PROPERTIES_RUNTIME_IDE_VERSION] = ctx.ArduinoAPIVersion | |||
| buildProperties[constants.BUILD_PROPERTIES_FQBN] = strings.Replace(ctx.FQBN, ":", "_", -1) | |||
There was a problem hiding this comment.
Why is this replacement done? If it is so that it can occur in a filename, I think the property name should reflect that and the FQBN property should just contain the original, unmodified fqbn, to prevent confusion.
There was a problem hiding this comment.
You are right, buildProperties[constants.BUILD_PROPERTIES_FQBN] should contain the actual fqbn, however I'm going to apply the substitution later to make sure the filename is valid on all OS
|
Is there an @ArduinoBot build? I'd like to give this a try here, but I don't have the Go stuff set up. On Teensy, I make much more use of defining things from boards.txt and the custom menus, which are then referenced by platform.txt. The entire core library really needs to be rebuilt when those change. I know most 3rd party boards just copy platform.txt with only minimal changes, so maybe this is a corner case... but I'm pretty concerned about core.a not rebuilding... |
|
The bot is still in a coma, I'm pasting here the link for the artefacts |
|
@PaulStoffregen if the custom menus selection changes the fqbn (like |
|
@ArduinoBot build this please. |
Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
Some tests require that the core object files are recreated, but it doesn't happen when the prebuilt core is used.
Wipe the workspace if any of the core files has changed; this solves a long-standing problem for core developers about changes in platform.txt requiring a manual wipe to be correctly applied
Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
…tion Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
Now that caching is optional old tests may run as before (without the need to cleanup cache). Signed-off-by: Cristian Maglie <c.maglie@arduino.cc>
cmaglie
force-pushed
the
archiveCompiledCore
branch
from
b99aae5 to
4591ba3
Compare
Build options json is not the optimal way to discover if something has changed in the build. Commit arduino@19751fe#diff-f796a16acff0c78b88ffe6618745edc6R67 enforces the check and also wipes if the core has changed "in place". Wiping more often gives better confidence that the actual compliation results reflect the backing storage situation.
|
✅ Build completed. ⬇️ Build URL: ℹ️ To test this build:
|
Very WIP but almost working good