modern-cmake / cppfront

CMake wrapper around the cppfront repository
BSD 3-Clause "New" or "Revised" License
69 stars 16 forks source link

Make directory-level enabling recursive #122

Open FeignClaims opened 8 months ago

FeignClaims commented 8 months ago

Currently when enabling a directory by setting CPPFRONT_NO_MAGIC to false, only targets right in CMAKE_CURRENT_SOURCE_DIR will get automatic cpp2-to-cpp translation.

This conflict with the behavior that find_package makes the package available for all subdirectories inside the directory. That is, if users already call find_package in src, they are not required to find_package again in src/app.

The pr fix the above conflict by

  1. Making the directory enabling (cppfront_enable_directories) recursive.
  2. Making the paths of generated cpp files consistent with the origin cpp2 files. For instance, ${CMAKE_SOURCE_DIR}/app/src/main.cpp2 to ${CMAKE_BINARY_DIR}/_cppfront/app/src/main.cpp.

The second change enables users to navigate the corresponding cpp files. I've tested that the cpp2 file target_source-ed by multiple targets won't translate more than once, so I think this change is ok compared to the hash-and-rename.


A better solution might be making the cpp2-to-cpp translation registration behave like handling uic automatically for cmake-qt, but I failed to learn how that works.

alexreinking commented 8 months ago

Thanks for your contribution! I will look at it and review this weekend.

FeignClaims commented 4 months ago

Hi @alexreinking, just wanted to bring attention to this PR. Would you mind taking a look when you have a chance? Thanks!

alexreinking commented 1 week ago

We're going to need to think about how to make this work with CXX_MODULE and HEADER_SET file sets that specify a BASE_DIR. For sources in those properties, we need to

  1. Identify the base dir they are below. CMake enforces that base dirs do not contain each other, so there is a unique one.
  2. Add new base dirs that preserve the relative path of the generated file.

Basically, if someone has a base dir of include/project with files like include/project/component/header.h2, we need to make sure that we add a generated base dir that will still allow component/header.h to be included.

FeignClaims commented 1 week ago

We're going to need to think about how to make this work with CXX_MODULE and HEADER_SET file sets that specify a BASE_DIR.

I see, but I'm afraid I won't have time to modify this PR in the coming month. Let's change this PR to a draft PR for now, and I'll set it to "Ready for review" once I've made the necessary changes.

alexreinking commented 1 week ago

Oh boy and while we're working on total correctness, we need to make sure that translating generated cpp2 sources works correctly