cross-language-cpp / djinni

A tool for generating cross-language type declarations and interface bindings.
https://djinni.xlcpp.dev
29 stars 2 forks source link

CMake function wrapper for djinni #10

Open jothepro opened 4 years ago

jothepro commented 4 years ago

As a CMake user I would like to call djinni by simply calling a CMake function that I imported from djinni. The call should look something like this:

# calls djinni on given IDL.
# calls `message(FATAL_ERROR ...)` and aborts configuration when something goes wrong during generation
djinni(path/to/idl.djinni
   JAVA_OUT ${JAVA_OUTPUT_FOLDER} # expose all djinni options as cmake function parameters
   JAVA_PACKAGE com.example.jnigenpackage
   IDENT_JAVA_FIELD mFooBar
   CPP_OUT ${CPP_OUTPUT_FOLDER}
   # ... more djinni arguments
)

This makes my CMake code cleaner and more readable because I don't have to call djinni myself with execute_process and I don't need to implement error handling (what to do in case djinni generation fails?) myself.

jothepro commented 4 years ago

@a4z @bojanin do you use CMake in your project configuration? How do you call djinni in your build-configuration?

a4z commented 4 years ago

yes, and I have something somehow similar . lit looks somehow like that

process_djinni_idl(
    FILE ${CMAKE_CURRENT_SOURCE_DIR}/idl/version.djinni
    OUTDIR ${PROJECT_SOURCE_DIR}/gen 
    VARIABLE_PREFIX DJINNI_GEN   
    NAMESPACE example
    JAVA_PACKAGE a4z.example.cpp
)

then there is of core the problem of informing cmake what new files exists ... I do not like globbing, so there is some regex chaos going on striping out the class names and get the files names from that.

It would of course be nice to find a standard way of handling all that!

jothepro commented 4 years ago

What is the result of your function? Does it fill variables with lists of the generated files? Or does it generate targets ready to go? My current wrapper implementation calls djinni, globs for all generated files & generates targets from it (following a naming convention). The intention of this issue was to just wrap the execute_process call as a first step, but now I realize that this might not be enough to be helpful.

a4z commented 4 years ago

yes, it generates variables, with the VARIABLE_PREFIX , prefix in the function it looks like thisset(${_idldef_VARIABLE_PREFIX}_DJINNI_CPP_INCLUDE_DIR ${_idldef_OUTDIR}/cpp ) ..... so with the example from above where DJINNI_GEN was the prefix, you need to include DJINNI_GEN_DJINNI_CPP_INCLUDE_DIR and then variables for the source files are also created, other include dirs (objc) , ....

so internal knowledge, or reading the function is required. and there are also problems, like, when a class renames, getting rid of old files, ....

nice build system integration, maybe with focus in cmake to narrow the possibilities a bit, is for sure a topic that requires some thinking.

a4z commented 4 years ago

some more thoughts: the only one who knows what files are generated is the djinni generator, so the generator should provide (optional) a list of generated files

This would eliminate the need of file globbing in cmake, or working with workarounds like parsing a djinni file with regex in cmake to get class names and therefore file names. But this is also difficult, since this means there need to be some kind of context, created before the first dinni invocation and finalized to write out the files

maybe we can do am mapping between input idl file and file with list of generated files with the same name

jothepro commented 4 years ago

hm, thats a great idea! My take on it would be:

execute_process(COMMAND djinni
        --idl project-a.djinni
        # print a \n separated list of all generated files to stdout
        --print-generated-files 
    OUTPUT_VARIABLE PROJECT_A_GENERATED_FILES)

execute_process(COMMAND djinni
        --idl project-b.djinni
        --print-generated-files 
    OUTPUT_VARIABLE PROJECT_B_GENERATED_FILES)

# aggregate a list of all generated files
list(APPEND ALL_GENERATED_FILES 
    ${PROJECT_A_GENERATED_FILES}
    ${PROJECT_B_GENERATED_FILES})

add_library(myproject
    ${ALL_GENERATED_FILES}
)

With this take one would not need to read from a generated file or something else.

Do you think that would work?

a4z commented 4 years ago

Using console output seem risky for various reasons, one is there might be additional info/warn/debug messages and that would make users very unhappy

It might make more sense to add a command line argument that takes a file and writes the generated files into that something like this (kind of pseudo code but I think it should work )

execute_process(COMMAND djinni
        --idl project-b.djinni
        --generated-files-sink  project-b.djinnigenearted.files
 # .... lots of other settings
  )
file(STRINGS project-b.djinnigenearted.files B-DJINNI_FILES)
set_source_files_properties(${B-DJINNI_FILES}
                            PROPERTIES GENERATED TRUE)
add_libarary(bdjinni ${B-DJINNI_FILES})

This would also be a good motivator to look at the command line parsing, that is .... interesting, as it is.

jothepro commented 3 years ago

While working on the documentation i found out that the suggested command line argument already exists:

--list-out-files <list-out-files> Optional file in which to write the list of output files produced.

🥳

a4z commented 3 years ago

awesome catch Johannes, will have to test that tomorrow if it works as advertised

delaitre commented 3 years ago

The way we have it setup in my company is to use --list-out-files to get the list of generated files, then parse that file and for each file listed we add the filename to a java_files, cpp_files, jni_files or objc_files CMake variable depending on the filename extension. We then generate a new CMakeLists.txt in the build folder defining CMake variables like so:

    file(WRITE "${output_dir}/CMakeLists.txt"
        "set(DJINNI_GENERATED_JAVA_FILES\n" ${java_files} ")\n\n"
        "set(DJINNI_GENERATED_CPP_FILES\n"  ${cpp_files}  ")\n\n"
        "set(DJINNI_GENERATED_JNI_FILES\n"  ${jni_files}  ")\n\n"
        "set(DJINNI_GENERATED_OBJC_FILES\n" ${objc_files} ")\n\n"
        "if(BUILD_FOR_ANDROID)\n"
        "    set(DJINNI_GENERATED_SOURCES\n"
        "        \${DJINNI_GENERATED_CPP_FILES}\n"
        "        \${DJINNI_GENERATED_JNI_FILES}\n"
        "        # Always include the djinni_main.cpp from the support lib so that the resulting\n"
        "        # library contains the JNI_OnLoad and JNI_OnUnload functions.\n"
        "        ${djinni_main_cpp}\n"
        "    )\n"
        "elseif(BUILD_FOR_IOS)\n"
        "    set(DJINNI_GENERATED_SOURCES\n"
        "        \${DJINNI_GENERATED_CPP_FILES}\n"
        "        \${DJINNI_GENERATED_OBJC_FILES}\n"
        "    )\n"
        "else()\n"
        "    set(DJINNI_GENERATED_SOURCES \${DJINNI_GENERATED_CPP_FILES})\n"
        "endif()\n"

We also generate a module.modulemap file automatically here for iOS (I suspect this is quite similar to the bridging header generated using the finn fork with swift support).

All that script is in a RunDjinni.cmake script.

We have another CMake file defining some function to add djinni support to a CMake target so that you can call: add_djinni_support(foo) and it will extract all the .djinni files from the SOURCES property of the target foo and it will generate a new djinni-foo target that you can run to run djinni on all those .djinni files (you could add a dependency to do that automatically when building foo but we haven't done that). This add_djinni_support function also include the CMakeLists.txt that RunDjinni.cmake has generated in the build directory, which then allows add_djinni_support to add ${DJINNI_GENERATED_SOURCES} to foo's SOURCES target property.

a4z commented 3 years ago

Thanks for the feedback @delaitre !

I play also around with reading the generated file list. I transform that into a cmake list that gets included and provides the variables which based on the file location I like to separate files. Djinni can generate an umbrella header, see --objc-swift-bridging-header The module map can be generated by cmake.

Sooner or later I will put that here as an example. This will be very opinionated, because how/where files are generated, named, ... is very individual, but it will be better to have some example than none.

I would be more than happy, once this is up , if you check that to see if you find points that can be improved.

richey-v commented 3 years ago

@delaitre - correct me if I'm wrong, but the strategy you mention above creates DJINNI_GENERATED_SOURCES at cmake generate time, right? Do you have a way to get cmake to run djinni and create the list of outputs at build time? I have been struggling with getting cmake to do that.

a4z commented 3 years ago

it does not work on build time, it needs to be done a cmake generation time but you can set a property on a directory

  set_property(DIRECTORY APPEND PROPERTY CMAKE_CONFIGURE_DEPENDS
    my_djinni_file.djinni
  )

If my my_djinni_file.djinni changes, cmake will re-generate, and the cmake code to do the stuff runs again ...

To me it seems bit counterintuitive that this can not be done at build time, and I struggled also at first, but since I moved everything to cmake config time, it works

delaitre commented 3 years ago

@richey-v : I am not 100% sure anymore, I wrote that stuff 4 years ago :)

I think at generation time, the .djinni files are extracted from the target SOURCES and the djinni-foo target is created. Then, when I run make djinni-foo (so, at build time), djinni is ran, which generates the new CMakeLists.txt in the build folder. But since the CMakeLists.txt is included already (basically, it is empty until you run make djinni-foo, and then it is filled up with actualy useful info) by add_djinni_support, CMake detects the content of a CMakeLists.txt changed and will happily takes everything into accout next time you run make.

My workflow is always to run make djinni-foo, and the make. Ideally you wouldn't have to run djinni-foo explicitly and it would run automatically when a djinni file is modified but we did not go down that route for several reason:

I will see with my company if I can post our CMake code to do all that as an example to get the ball rolling.