eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
945 stars 396 forks source link

CMake: Remove Glue libraries? #1999

Open dnakamura opened 6 years ago

dnakamura commented 6 years ago

Problem:

Currently the way we handle the glue (having the consumer define an interface library and passing us its name), has some issues

Proposal:

Eliminate OMR_X_GLUE_TARGET variables, have consumers manually inject glue code into libraries.

Example:

(only the gc glue is shown for brevity):

Current method :

...
add_library(my_gc_glue INTERFACE)
target_add_sources(my_gc_glue 
    INTERFACE
        ${CMAKE_CURRENT_SOURCE_DIR}/foo_1.cpp
        ${CMAKE_CURRENT_SOURCE_DIR}/foo_2.cpp
        ...
)
target_include_directories(my_gc_glue
    INTERFACE
        ${CMAKE_CURRENT_SOURCE_DIR}/glue_public_includes
        ${CMAKE_CURRENT_SOURCE_DIR}/glue_private_includes
)

set(OMR_GC_GLUE_TARGET my_gc_glue)
add_subdirectory(omr)
...

New method:

...
add_subdirectory(omr)

target_sources(omrgc
    PRIVATE
        foo_1.cpp
        foo_2.cpp
)

target_include_directories(omrgc
    PRIVATE
        glue_private_includes
    PUBLIC
        glue_public_includes
)
...

Pros:

Cons:

dnakamura commented 6 years ago

Thoughts? @rwy0717 @mgaudet @charliegracie

mgaudet commented 6 years ago

I mean, the 'new method' example seems pretty compelling to me. The ordering dependence doesn't seem particularly convincing argument against, so... LGTM?

rwy7 commented 6 years ago

This looks good to me, it makes OMR's build system less complicated/"frameworky", by pushing more code out to the consumers.

charliegracie commented 6 years ago

@dnakamura is this something we still want to do or something that has been completed?

dnakamura commented 6 years ago

Yes I would still like to do this. However we probably need the openJ9 cmake stuff to go live so we can see if it actually makes sense / works the way I think it should