Compilation cleanup and automatic library dependencies #2174
Conversation
This simplifies upcoming changes.
Nobody was using it anymore, except for checking against specific extensions, which is easily done against the filename itself. This prepares for some simplification of Sketch.load next.
By using FileUtils.listFiles, this function can be greatly simplified, without changing functionality in any way.
Previously, the useRecursion and srcFolders were filled on library creation, based on the existence of the src folder. Now, a layout variable is set, and the useRecursion() and getSrcFolder() methods change their return value based on the layout in use.
Instead of doing three passes over the filesystem to collect source files, it now does a single pass (using the newly introduced FileUtils.listFiles) and later splits between .S, .c and .cpp files. This allows sharing more code between the three file types and allows removing Compiler.findFilesInFolder. Additionally, this splits compileFiles into a version that compiles all files in a folder and one that only compiles selected files. This prepares for later refactoring of the Library class. This has the side-effect of calling isAlreadyCompiled for .S files as well (which didn't happen previously). However, since the assembler command does not produce any .d files, .S files are still always recompiled, just like before. Finally, this has the side effect of handling file extensions in a case insensitive manner during compilation (instead of just during load), which means that e.g. .CPP files are not just loaded, but also compiled.
This function allows making a path relative. However, unlike the existing FileUtils.relativePath, this function only works to make a path relative to one of its parent paths (e.g. strip a prefix), which allows it to be a lot simpler.
Since Compiler.compileFiles already knows how to recursively compile files, it seems pointless to keep these around. Because compileFiles uses a one-off recursion to list files to compile at the start, it has to be slightly smarter about creating directories recursively when needed and needs to fiddle with the filenames a bit (so that the directory structure within a new-style library is maintained in the build directory), but those are only minor changes to compileFiles.
This prevents duplicating these lists in multiple places. As a side-effect, .S files are now handled in sketches as well (instead of just in libraries), fixing arduino#1616.
Previously, the Compiler class had some special casing for new-style and old-style libraries and hardcoded source and include paths for those. Now, this info is moved into the Library class, simplifying the Compiler class and keeping the new vs old-style library stuff in one place.
…esolver class This offers a more consistent interface to the library autodetection that can next be expanded to also detect inter-library dependencies. As a side effect, the resolver now looks through all sketch code, including any .c, .cpp and .h files. Previously, only the .ino and .pde files were inspected during preprocessing. This fixes arduino#636. This commit is based on code by Christian Maglie.
Previously, any libraries #included by the sketch were added to the include path and link, but any libraries included by other libraries were not. This meant that all libraries used, even indirectly by other libraries, must be explicitely included in the sketch. This commit changes the dependency resolution to become recursive - it also includes the libraries included by other libraries in the build. This commit is based on code by Christian Maglie and fixes arduino#236.
|
I've done basic testing, which all seems to work. For more extensive testing, I'd like to let this code recompile all examples shipped with Arduino and compare the resulting hex files - but I'm still working on a script to automate this (which I'll publish separately, seems like a useful tool for other testing as well). |
matthijskooijman
added
Component: Compilation
matthijskooijman
changed the title
|
The library dependency part of this PR is more elegantly solved by #2792. Some of the cleanups have been included in the IDE refactor from a while ago, and the rest is probably broken by that. However, some cleanups might still be useful, so I'll have to go over this PR soon and salvage the useful parts. |
|
@matthijskooijman we did a lot of work since this PR, including library dependencies. Are there any part of it which could go to a separate PR? |
|
IMHO the only commit that is still worth merging (or cherry-picking) is 8bece7c - Simplify Sketch.load (Sketch.load is now in SketchData.load) All the rest (AFAICS) have been already merged or implemented in another way. |
|
I haven't looked very closely at the current compilation code (after the refactoring done earlier and the introduction of arduino-builder), so I'm not sure what is still relevant (IIRC some of these commits were already merged manually as part of that refactoring as well). I won't have time soon to rebase that one commit, so perhaps @cmaglie or @ffissore could pick it up directly? |
|
The changes in the "Simplify Sketch.load" commit have been included as part of another commit in #4363, and all the other commits in this PR are either included, or no longer relevant with arduino-builder, so I'm closing this one. |
This pullrequest is based on and replaces #1726. While rebasing that PR I also moved some code around for a more sensible structure, which again exposed some opportunities for cleanup, meaning this pullrequest now moves a lot of code around, while only the last commit actually changes the detection of dependencies.
The other commits move and clean up code, which does improve consistency in a few places, fixing a few bugs as a side effect.