Repository navigation
tools: fix incremental build of topologies. Make incremental build the default. - #6342
Conversation
This makes it possible to find stuff in the output of `ninja help`. Zero effect on the build artefacts. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Make prints a line for every topology that is already up to date and not rebuilt. It's impossible to see what was done. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Modifying abi.h now rebuilds topologies v1 Signed-off-by: Marc Herbert <marc.herbert@intel.com>
It's now possible to rebuild just only one modified topology without rebuilding all of them from scratch. Building topologies v2 has two steps: 1. concatenation of abi.conf with topo.conf 2. run alsatplg on the concatenation Concatenation step 1. was wrongly implemented at configuration time, not at build time. So when the topology source of step 1 (topo.conf) was modified, the dependency of step 2 (= the concatenation) was not affected and the topology was not rebuilt. Fix this by moving the concatenation to build time. Fixes commit 308a24a ("topology2: Add build support") Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Move abi.conf generation to build time. Changing abi.h now rebuilds all v2 topologies. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
Faster build and focused and readable logs at last. Add a warning not to surprise anyone. Signed-off-by: Marc Herbert <marc.herbert@intel.com>
ab6ae3a to
6eb06d2
Compare
|
I do like this, but some testing does feel needed. @dbaluta can you please check that rebuilding a topology after changing the M4 file works correctly without cleaning the folder/deleting the conf/tplg files? (with this PR, of course) |
|
Thanks @paulstelian97 for taking a look. For the record I did test incremental builds - the main feature in this PR - quite thoroughly but of course you can never have too much testing[*] and I would love more testing, especially from someone else in a different environment. [*] given infinite time and resources |
aborisovich
left a comment
aborisovich
left a comment
There was a problem hiding this comment.
Looks ok, not sure whether that dependency system with abi.h, abi_v1 etc. cannot be simplified somehow...
| # The 'abi.h' file is the real dependency that triggers | ||
| # a rebuild. 'abi_v1' is the target that avoids a race | ||
| # to rebuild abi.h. See the CMake FAQ or (better) Sam | ||
| # Thursfield's blog about custom targets. | ||
| DEPENDS abi_v1 ${CMAKE_CURRENT_BINARY_DIR}/abi.h |
There was a problem hiding this comment.
Have you tried using set_source_files_properties and setting GENERATED on abi.h?
I did not analyzed whole dependency system here, maybe it can't be used - just a suggestion.
There was a problem hiding this comment.
Thanks for the review! No I did not trys that.
https://cmake.org/cmake/help/latest/prop_sf/GENERATED.html says GENERATED is automatically set but... works only in the current directory.
I followed https://samthursfield.wordpress.com/2015/11/21/cmake-dependencies-between-targets-and-files-and-custom-commands/ and tested it quite thoroughly. Seems OK.
|
Please check that tplg2 .conf dependencies are handled correctly. I didn't seem to get a rebuild for a small edit there. Tplg1 seemed to be OK. |
Sorry my bad, I edited a tplg2 file that is not used in build: tools/topology/topology2/cavs/cavs-mixin-mixout-hda.conf . In other test for tplg2 no issues. My edits were noticed and rebuilt correctly. I see no issues with this patch. Thanks! |
|
https://sof-ci.01.org/sofpr/PR6342/build1659/devicetest/?model=TGLU_UP_HDA_ZEPHYR&testcase=verify-kernel-boot-log failed like this: You may have seen this failure in other pull requests. I investigated and just found it seems caused by the following combination:
Investigation and discussion still in progress, please help at Off-topic here sorry. |
|
@dbaluta @paulstelian97 @LaurentiuM1234 good for you ? |
|
@dbaluta any inputs before merge ? |
|
@dbaluta we can revert/fix if there is a problem. |
Faster and no more noise. Run
./scripts/build-tools.shtwice: the second time builds the minimum required and prints almost nothing when nothing changed.You really want to review these commits separately from each other.