Add `sbe_generate()` cmake-function

[ngrodzitski]

The change adds sbe_generate() function which enables cmake projects to use sbepp-compiler by using a function which accepts TARGET_NAME argument. The specified target can be used later by lib/exe cmake-targets in order to run all necessary code generation. This hides all the trickery with custom commands. Similar approach is used in Protobuf library.

Example:

sbepp_generate(
    SCHEMA_NAME ${schema_name}
    SCHEMA_FILE ${CMAKE_CURRENT_LIST_DIR}/${schema_name}.xml
    OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}
    TARGET_NAME compile_benchmark_schema
)

# ...

add_executable(${target})
add_dependencies(${target} compile_benchmark_schema)

[OleksandrKvl]

Hi @ngrodzitski, thanks for your effort but you have to better explain what problem this PR is trying to solve. I will give it a deeper look later but at the moment it looks like all these changes have been made only to replace add_custom_command with sbepp_generate and I don't understand what's the problem with the former.

I also see that besides sbepp_generate you've added FetchContent, again, what are you trying to solve? I don't believe that in 2023 CMake is the right tool for dependency management.

[ngrodzitski]

Hi @OleksandrKvl, This change is sort of a usability feature. It doesn't fix anything. But does all necessary trickery within a single function call.

Take for example the following:

set(schema_file "${CMAKE_CURRENT_LIST_DIR}/${schema_name}.xml")
set(output_file "${CMAKE_CURRENT_BINARY_DIR}/${schema_name}/${schema_name}.hpp")

add_custom_command(
    OUTPUT ${output_file}
    COMMAND $<TARGET_FILE:sbepp::sbeppc> "${schema_file}"
    DEPENDS sbepp::sbeppc "${schema_file}"
)

add_custom_target(compile_benchmark_schema DEPENDS ${output_file})
# ...
add_dependencies(${target} compile_benchmark_schema)

1. It takes extra variable: `output_file, which required only to set params for custom command and deps arrangement.
2. Strictly speaking, the value of output_file is not known in advance (as it might be defined in xml).
3. Requires user to learn sbepp-compiler command.
4. If need to handle more then one schema needs an extra variable to gather all output_file in a list.

With this function needs 2 steps.

sbepp_generate(
    SCHEMA_NAME ${schema_name}
    SCHEMA_FILE ${CMAKE_CURRENT_LIST_DIR}/${schema_name}.xml
    OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}
    TARGET_NAME compile_benchmark_schema
)
# ...
add_dependencies(${target} compile_benchmark_schema)

Looks very self explaining to me. Maybe you have an idea of better names for parameters.

And in general it suggests a similar workflow to Protobuf, which is quite a mainstream.

Regarding fetch-content. On a system that doesn't have fmt, pugixml, gtest installed find_package() fails. And to make the project buildable I've added fetch-content as a mini-dependency management. Also, I moved all find-package/fetch-content to root cmake file which allows to integrate it to another project as a submodule: one can add add_subdirectory(<sbepp_submodule_dir>/sbepp) skipping the root. With current master it has find_package() in it which may conflicts with the way one gets dependencies (git submodules).

[OleksandrKvl]

Sorry, I'm quite busy these days and can't respond quickly.

1. Let's first deal with FetchContent. Neither making itself buildable, nor using it as a submodule, nor using add_subdirectory(<sbepp_submodule_dir>/sbepp) is my current goal. Just because something is possible to do doesn't mean it should be done. Especially without the actual issue from someone. That being said, this PR won't be merged with those changes, and also with moved installation commands (probably you did that for the same reasons). If you don't want to do that, I will close this PR, create another issue about sbepp_generate only and use your solution as a basis for that one, let me know.
2. I generally like sbepp_generate idea but need some free time to check all the details. One question I have right now: is it possible to omit the creation of .gen files? For example if sbeppc reports all the filenames it creates during generation, can they be used to setup file deps?

[ngrodzitski]

1. All machinery with fetch content was necessary for me so I can implement sbepp_generate(), otherwise I'm unable to build the sbepp-compiler in the first place. So on my end it is a part of a change that introduces sbepp_generate(). Since it is not your goal (no argue on that, I pretty sure current master works well for you, and you don't need another ) I didn't ask you to solve my problem for me via creating an issue, I've just implemented what makes it usable for me and I believe they are not breaking for the way you use it.

2. I'm afraid it is not possible with SCHEMA_NAME parameter optional. Extending compiler with sub-command that emits names will not help because when cmake generate runs there is no compiler yet. On the other hand if SCHEMA_NAME is mandatory then you have a certain filename to rely on: ${SCHEMA_NAME}.hpp and you can use it instead of .gen files. Basically, any file that has a predictable name will work (with least surprise principle).

[ngrodzitski]

Closing as discussed above.

[OleksandrKvl]

Ok, I'll open separate issue for this feature and refer to your initial implementation there. And just in case you will want another feature, please don't mix multiple things in a single PR and create and issue first to discuss a solution.

[OleksandrKvl]

@ngrodzitski have you ever tested it? I finally got my hands on it, refactored things and at the end discovered that the whole thing doesn't work because DEPENDS option for add_custom_command add_custom_target doesn't support generator expressions so this trick DEPENDS $<TARGET_PROPERTY:${arg_TARGET_NAME}, SBEPPC_ANCHORS> cannot work.

[ngrodzitski]

@OleksandrKvl Definitely works in my environment.

$ cmake --version
cmake version 3.25.1

From here: https://cmake.org/cmake/help/latest/command/add_custom_command.html:

New in version 3.1: Arguments to DEPENDS may use generator expressions.

[OleksandrKvl]

Yes but I'm talking about add_custom_target, not about add_custom_command.

[OleksandrKvl]

Sorry, my bad, I named wrong command in the original message.

[ngrodzitski]

In what way it doesn't support generator expression?

I've just did a check.

cmake -B_build_test . -DCMAKE_BUILD_TYPE=Debug -DSBEPP_BUILD_TESTS=ON -DSBEPP_BUILD_BENCHMARK=ON -DSBEPP_DEV_MODE=ON
cmake --build _build_test -j1 --target sbepp-benchmark --verbose

So I specifically build only exe target and it behaves as expected building the deps first:

And in output I see what is expected:

[ 25%] Generating benchmark_schema.xml.gen
cd /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark && ../sbeppc/sbeppc --schema-name benchmark_schema --output-dir /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark /home/ngrodzitski/github/ngrodzitski/sbepp/benchmark/benchmark_schema.xml
cd /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark && /usr/bin/cmake -E touch /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark/benchmark_schema.xml.gen
make[3]: Leaving directory '/home/ngrodzitski/github/ngrodzitski/sbepp/_build_test'

Also checked the following:

1. manually update schema file to see if it causes rerun of generator (saw the same message as above).
2. Removed anchor .gen file (the same as above)
3. Spoil the schema and try to rerun:

   [ 25%] Generating benchmark_schema.xml.gen
   cd /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark && ../sbeppc/sbeppc --schema-name benchmark_schema --output-dir /home/ngrodzitski/github/ngrodzitski/sbepp/_build_test/benchmark /home/ngrodzitski/github/ngrodzitski/sbepp/benchmark/benchmark_schema.xml
   Error: /home/ngrodzitski/github/ngrodzitski/sbepp/benchmark/benchmark_schema.xml:74:3: XML parsing error: Start-end tags mismatch

The only downside I see now is that running clean doesn't remove generated files. The later can be addressed within compiler. For example it will have an option to pu all the generated code into a directory we set in sbepp_generate function and so it can tell cmake to remove it (just an idea, don't know exactly how it can be done).

[OleksandrKvl]

Look here: https://cmake.org/cmake/help/latest/command/add_custom_target.html, there's not a single word that DEPENDS supports it (unlike a couple of others like COMMENT). On my macOs + Ninja it fails because generated Ninja build.ninja file contains:

# Phony custom command for test/CMakeFiles/compile_test_schemas
build test/CMakeFiles/compile_test_schemas | ${cmake_ninja_workdir}test/CMakeFiles/compile_test_schemas: phony test/$$<TARGET_PROPERTY$:compile_test_schemas, test/SBEPPC_ANCHORS>

And the build results in:

[build] ninja: error: ‘test/$<TARGET_PROPERTY:compile_test_schemas,’, needed by ‘test/CMakeFiles/compile_test_schemas’, missing and no known rule to make it

You see, at the end CMake just put raw generator-expression without expanding it and breaks everything else. What platform/generator are you using?

[ngrodzitski]

That was a default (Makefile) generator on Linux (ubuntu 2020). Similar results with Ninja. It does run generator before compiling smth from benchmark target.

[OleksandrKvl]

I had extra whitespace inside generator-expression in my version... Now it works but I still need to understand if generator expressions are officially supported in DEPENDS, will ask CMake people about it. Thanks for you help.

[OleksandrKvl]

It turns out that add_custom_target generates custom command under the hood, that's why generator expressions are implicitly supported for DEPENDS. But I still want to get some official confirmation from CMake devs that I can rely on this functionality.

[OleksandrKvl]

@ngrodzitski I've implemented the core functionality in #18. It's still a draft since it requires documentation and other non-essential stuff but you can check it out and give some feedback if any.

There are 2 functions: the first creates custom target, similar to your original solution and the second creates INTERFACE library which makes integration even simpler. I replaced some generator expressions with normal code wherever possible. Anchor file name is now a hash of the full schema path, just as an extra protection against 2 schemas with the same file name.