eclipse-cyclonedds / cyclonedds-cxx

Other
96 stars 75 forks source link

Feature Request: Bring directory reconstruction in to 0.10.x series #500

Open matthock opened 3 months ago

matthock commented 3 months ago

418 implemented a feature to match a feature supported by idlc and the C cyclonedds in 0.10 - idl files in nested directories. While it was implemented in master, it was never merged in to 0.10 to match the 0.10 C feature. Would it be possible to pull those changes in to 0.10?

eboasson commented 3 months ago

If a back port is trivial we can consider it ...

At first sight it looks like

git cherry-pick --no-commit 12b4d8a c67fbc7 aab7156 bbb3214

appears to work (as it: cherry picks cleanly and builds cleanly). Maybe you give it a try?

matthock commented 3 months ago

I had to cherry pick 12b4d8aafb and 9485a9389 instead (possibly the others got squashed in the PR merge and are only in dev checkouts), pulling in to tag 0.10.5, but using those two did get it working as expected, so it does look like it's a trivial backport.

edit Hold on that a bit. It's doing some odd unnecessary rebuilding of objects. I'm going to see if that's a problem with my configuration or if the PR is missing a necessary commit.

matthock commented 3 months ago

So getting into what I'm seeing - it is regenerating/rebuilding everything in idlcxx_generate for every target it's linked in to. Given that I have a half dozen targets and hundreds of idl items, this takes a while ;)

That said, I believe the problem is actually in the core cyclonedds idlc, and is fixed in master there but just hasn't been backported into 0.10. It's also more of an annoyance than a blocker and one that can be worked around (build the idlcxx_generate target into an intermediate static library with private linking and then link that to each target that needs it). As a result, I don't believe it has an impact on this issue, it's just something I also saw while testing the potential PR changeset.