project-chip
Use build.rs instead of import!() for matter clusters
Currently the import!() macro processes globals, but if the macro is used more than once, things will break because there will be more than 1 globals generated (in >= matter 1.4)
Additionally, if we do a glob import!() (i.e. all clusters), the generated token stream is massive and results in:
macro invocation exceeds token limit: produced 2759294 tokens, limit is 2097152
As such, import!() is unlikely to be a viable long-term approach to generating all clusters, e.g. for an all-clusters-app
Instead, we should use build.rs with cargo:rerun-if-changed=[source.matter], so that we can output globals and clusters to separate files while also caching outputs if input .matter file hasn't changed
This offers the additional benefit of being able to see the actual cluster code, as it will generate an actual rust file in generated/
Additionally, we may want to move this to a separate crate e.g. rs-matter-dm (likely including all its deps in said crate, such as TLV & core types), because compilation of all the clusters adds > 10s, and moving to a separate crate allows caching - we should do this only if there is a regression in build speed vs import!()
See detailed discussion in https://github.com/project-chip/rs-matter/pull/338#discussion_r2473222598_
As such, import!() is unlikely to be a viable long-term approach to generating all clusters, e.g. for an all-clusters-app
You can instead do multiple imports. As in:
import!(cluster1 cluster2 clusterN...);
import!(clusterN+1);
import!(clusterN+2, ...);
I.e. the above restriction of RA pertains to a single proc-macro invocation only.
This is not to say that all issues with import are suddenly solved (globals are still an issue). Just that this particular one has an easy workaround.
Instead, we should use build.rs with cargo:rerun-if-changed=[source.matter], so that we can output globals and clusters to separate files while also caching outputs if input .matter file hasn't changed
This offers the additional benefit of being able to see the actual cluster code, as it will generate an actual rust file in generated/
I agree to both and I think we should at least try the build.rs approach to see what is the outcome and compare with the current import! one. (which can be extended to support globals, but it won't be that pretty).
As noted earlier - in the linked PR - this would generate gobs of code though so we might want to have a feature for each cluster (which would however be very, very, very verbose!!!) and then hide each generated cluster behind its feature toggle. Or else we are at the mercy of the LTO / linker to remove all unused code - which might work but we don't know until we try.
Additionally, we may want to move this to a separate crate e.g. rs-matter-dm (likely including all its deps in said crate, such as TLV & core types), because compilation of all the clusters adds > 10s, and moving to a separate crate allows caching.
I'm not convinced we need that. Once the code is generated, I do hope the compiler would just cache it and not re-compile (or more importantly, re-check in the IDE) each time. Something that is unfortunately not happening with the import! macro. But then again, we don't know until we try.
I like the build.rs approach, mostly for the fact that users can easily navigate the generated code. However, we should avoid having the generated/ path checked in git. This is effectively what exits in the c++ SDK and small changes tended to generate a lot of noise in PRs. I don't think that this has been suggested but I wanted to point it out in case the temptation arises.
@hicklin oh god yes no doubt - beauty of the codegen being rust is that it will automatically be generating the files before the build
The only reason these are checked in to C++ sdk (we want to move away from that) is that zap, responsible for the generation, is javascript, and we can't have that running as part of the build step (because not all build envs support running JS) - whereas obviously if you're building rust you have to support build.rs
@ivmarkov agreed on not pulling out into separate crate unless there is a regression in build time going from import! -> this