o3de / sig-core

5 stars 6 forks source link

RFC: Update the Gem Templates to support a Public/Private API split #37

Closed lemonade-dm closed 2 years ago

lemonade-dm commented 2 years ago

Summary:

The O3DE Template system provides several gem templates that users instantiate in order to start iteration on a new gem. Those Gems templates need expose several CMake targets that allows a gem to build it's code into a library that is loaded by O3DE applications as a "plugin", as well as targets for sharing a gem public API with outside dependencies(other gems and libraries) as well as private targets for sharing code among the gem "plugin" and test targets.

Currently as it stands , the gem templates exposes a ${GemName}.Static library target whose purpose is to share source files and include directories among the gem plugin targets. It is meant to be a private target not used outside of gem. Additionally the gem templates exposed a ${GemName} and ${GemName}.Editor target that are a MODULE library type in non-monolithic builds and a static library in monolithic builds. CMake enforces that MODULE library targets can't be linked into other targets can only be loaded dynamically using dlopen functionality. See CMake add_library documentation for more details.

Since monolithic builds are meant for release of a game or simulation application, all the shared libraries targets are converted to the static library targets and linked into the final application in order to prevent shared library dependency issues, reduce the size of the overall binary folder and to allow the linker to perform de-duplication and optimizations over all the code.

What is the motivation for this suggestion?

Why is this important? Having a convention for separating a private vs public API within the gem templates allow authors of gems to enforce boundaries as to what functionality a dependents libraries should use and what functionality is internal functionality that is subject to change among versions. Furthermore a public API convention allows users of gem to learn a recommended approach for using other how to depend on the code of other gems

What are the use cases for this suggestion? The primary use case is for dependent gems and other libraries to be able to know which targets they can depend on as part of the public API and therefore have an expectation of gem functionality that is meant to be stable. This also empowers authors of gem to separate their internal implementation from their public exposed API, which allows them to set user expectations of updates they make to their gems. Gem authors would be allowed to change their internal implementation with little push back as public users wouldn't depend on that implementation. When it comes to updating the public API gem authors have to be cognizant of changes they make as to not break other gems using that public API. This creates an unspoken contract between the gem maintainer and the gem user, that the gem maintainer is allowed to change the internal implementation in anyway they would like without external insight, while any public API changes can and should receive scrutiny from users of that gem

What should the outcome be if this suggestion is implemented? The following Gem Templates should be updated as of 2022-04-26 CppToolGem DefaultGem PythonToolGem

Suggested design description:

The first update to those gem would be to add a new ${GemName}.API and ${GemName}.Editor.API INTERFACE target that exposes the public API of the gem. The those targets are meant to transfer over the interface include directories, compile definitions and options, and build dependencies that any outside gem/library using that gem needs to depend on.

By default the following gem variant API aliases will be added to facilitate users depending on the Gem API of variants instead of the runtime and editor API targets. Here is the list of new variant API aliases

  1. ${GemName}.Clients.API
  2. ${GemName}.Servers.API
  3. ${GemName}.Tools.API
  4. ${GemName}.Builders.API

The next update is to create Gem variant aliases for the API targets. This allows a gem to expose a specific API based on the Gem variants that it is used with. For example a gem could expose a different API to the AssetProcessor than the Editor by making a separate ${GemName}.Builders.API INTERFACE target vs ${GemName}.Editor.API INTERFACE target.

Next the ${GemName}.Static STATIC library target should change to an OBJECT target be renamed to a private ${GemName}.Private.Object as part of the Gem template(this also applies to the ${GemName}.Editor.Static being renamed to ${GemName}.Editor.Private.Object).

Why switch the STATIC library over to a OBJECT target?

An OBJECT target in CMake allow developers to compile source files into a collection of object files without creating a static or shared library. More information on an OBJECT target can be found here. This has the major benefit that unnecessary copies of object files aren't stored in a static library, thereby saving disk space. Furthermore this de-clutters the SDK lib directory as it would only contain public static libraries that aren't meant to be used by other targets.

An example where this helps is with the LyShine.Static library target which should be private to the LyShine Gem. In a build of the source engine, there would be two copies of the object files that get build for it, one on disk and one inside of the "LyShine.Static.a" archive file, but if the target is an OBJECT target, then there would be no "LyShine.Static.a" file. Because there is no "LyShine.Static.a" file, when creating an SDK layout, there is no copying of that private library to the SDK

What are the advantages of the suggestion?

The gem maintainer, has a framework in which to expose public functionality, while reserving a section of their gem from making changes to the implementation without worrying that they are breaking external users. This make it easier to implement a version scheme for code changes within a gem. Perhaps API breaking changes are major version bumps, while internal implementation changes are minor changes

Users of newer gems will now have a known target name they can depend on in their libraries to access the functionality of the gem.

What are the disadvantages of the suggestion?

  1. The disadvantage is that existing gems will not have the new API targets. The expectation for users will be that to add a dependency on a gem API, they would only need to a dependency on ${GemName}.<variant>.API target. User could be surprised when coming across gems that don't use the convention. This creates a bit of an inconsistency between older gems and newer gems.

  2. Using the updated Gem Template would result in 1 to 2 more TARGETS being created out of the box by default. If using an IDE such as Visual Studio with a Gem created from the Default Gem template, this will result in 8 .vcxproj being loaded instead of 6. This is balanced with the .Static vcxproj is being split into a smaller .API and .Private.Object vcxproj targets which loads faster.

How will this be work within the O3DE project?

This should be seamless. Users would use the same o3de.py create-gem command to create their gems with the new public API and private object targets.

Are there any alternatives to this suggestion?

There is a feature request with CMake to allow specification of DIRECTORY scope for regular CMake Targets: https://gitlab.kitware.com/cmake/cmake/-/issues/23433

That would allow enforcement through cmake other gems couldn't depend on the private ${GemName}.Static targets making it a lot easier to make sure an internal library isn't used outside of the Gem's root directory.

What is the strategy for adoption?

The first step is to update the Gem Templates to expose the new .API INTERFACE targets with the public include directories, compile option and build dependencies.

The next step is to update existing gems with the O3DE engine repo: https://github.com/o3de/o3de/tree/development/Gems with gem variant aliases for ${GemName}.<variant>.API targets. Ideally the existing ${GemName}.Static targets inside of gems that aren't a dependency of another gem are changed into OBJECT targets and renamed ${GemName}.Private.Object.

Gems with the engine which are depending on other gems ${GemName}.Static target should be updated to instead depend on the ${GemName}.API target. The ${GemName}.API target for the gem that is being depended on can be an alias of ${GemName}.Static in this case.

Example: New Gem within .API targets

The following is an example of a TestGem created using the new Public/Private API split


# Currently we are in the Code folder: ${CMAKE_CURRENT_LIST_DIR}
# Get the platform specific folder ${pal_dir} for the current folder: ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME}
# Note: o3de_pal_dir will take care of the details for us, as this may be a restricted platform
#       in which case it will see if that platform is present here or in the restricted folder.
#       i.e. It could here in our gem : Gems/TestGem/Code/Platform/<platorm_name>  or
#            <restricted_folder>/<platform_name>/Gems/TestGem/Code
o3de_pal_dir(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME} "${gem_restricted_path}" "${gem_path}" "${gem_parent_relative_path}")

# Now that we have the platform abstraction layer (PAL) folder for this folder, thats where we will find the
# traits for this platform. Traits for a platform are defines for things like whether or not something in this gem
# is supported by this platform.
include(${pal_dir}/PAL_${PAL_PLATFORM_NAME_LOWERCASE}.cmake)

# The TestGem.API target declares the common interface that users of this gem should depend on in their targets
ly_add_target(
    NAME TestGem.API INTERFACE
    NAMESPACE Gem
    FILES_CMAKE
        testgem_api_files.cmake
        ${pal_dir}/testgem_api_files.cmake
    INCLUDE_DIRECTORIES
        INTERFACE
            Include
    BUILD_DEPENDENCIES
        INTERFACE
           AZ::AzCore
)

# Add the TestGem.Private.Object target
# Note: We include the common files and the platform specific files which are set in
# 1.testgem_private_files.cmake
# 2.${pal_dir}/testgem_private_files.cmake
ly_add_target(
    NAME TestGem.Private.Object OBJECT
    NAMESPACE Gem
    FILES_CMAKE
        testgem_private_files.cmake
        ${pal_dir}/testgem_private_files.cmake
    INCLUDE_DIRECTORIES
        PRIVATE
            Include
            Source
    BUILD_DEPENDENCIES
        PUBLIC
            AZ::AzCore
            AZ::AzFramework
)

# Here add TestGem target, it depends on the Private Object library and Public API interface
ly_add_target(
    NAME TestGem ${PAL_TRAIT_MONOLITHIC_DRIVEN_MODULE_TYPE}
    NAMESPACE Gem
    FILES_CMAKE
        testgem_shared_files.cmake
        ${pal_dir}/testgem_shared_files.cmake
    INCLUDE_DIRECTORIES
        PUBLIC
            Include
        PRIVATE
            Source
    BUILD_DEPENDENCIES
        PUBLIC
            Gem::TestGem.API
        PRIVATE
            Gem::TestGem.Private.Object
)

# By default, we will specify that the above target TestGem would be used by
# Client and Server type targets when this gem is enabled.  If you don't want it
# active in Clients or Servers by default, delete one of both of the following lines:
ly_create_alias(NAME TestGem.Clients NAMESPACE Gem TARGETS Gem::TestGem)
ly_create_alias(NAME TestGem.Servers NAMESPACE Gem TARGETS Gem::TestGem)

# For the Client and Server variants of TestGem Gem, an alias to the TestGem.API target will be made
ly_create_alias(NAME TestGem.Clients.API NAMESPACE Gem TARGETS Gem::TestGem.API)
ly_create_alias(NAME TestGem.Servers.API NAMESPACE Gem TARGETS Gem::TestGem.API)

# If we are on a host platform, we want to add the host tools targets like the TestGem.Editor MODULE target
if(PAL_TRAIT_BUILD_HOST_TOOLS)
    # The TestGem.Editor.API target can be used by other gems that want to interact with the TestGem.Editor module
    ly_add_target(
        NAME TestGem.Editor.API INTERFACE
        NAMESPACE Gem
        FILES_CMAKE
            testgem_editor_api_files.cmake
            ${pal_dir}/testgem_editor_api_files.cmake
        INCLUDE_DIRECTORIES
            INTERFACE
                Include
        BUILD_DEPENDENCIES
            INTERFACE
                AZ::AzToolsFramework
    )

    # The TestGem.Editor.Private.Object target is an internal target
    # which is only to be used by this Gem's CMakeLists.txt and any Subdirectories
    # Other Gems should not use this target
    ly_add_target(
        NAME TestGem.Editor.Private.Object OBJECT
        NAMESPACE Gem
        FILES_CMAKE
            testgem_editor_private_files.cmake
        INCLUDE_DIRECTORIES
            PRIVATE
                Include
                Source
        BUILD_DEPENDENCIES
            PUBLIC
                AZ::AzToolsFramework
                $<TARGET_OBJECTS:Gem::TestGem.Private.Object>
    )

    ly_add_target(
        NAME TestGem.Editor GEM_MODULE
        NAMESPACE Gem
        AUTOMOC
        FILES_CMAKE
            testgem_editor_shared_files.cmake
        INCLUDE_DIRECTORIES
            PRIVATE
                Source
            PUBLIC
                Include
        BUILD_DEPENDENCIES
            PUBLIC
                Gem::TestGem.Editor.API
            PRIVATE
                Gem::TestGem.Editor.Private.Object
    )

    # By default, we will specify that the above target TestGem would be used by
    # Tool and Builder type targets when this gem is enabled.  If you don't want it
    # active in Tools or Builders by default, delete one of both of the following lines:
    ly_create_alias(NAME TestGem.Tools    NAMESPACE Gem TARGETS Gem::TestGem.Editor)
    ly_create_alias(NAME TestGem.Builders NAMESPACE Gem TARGETS Gem::TestGem.Editor)

    # For the Tools and Builders variants of TestGem Gem, an alias to the TestGem.Editor API target will be made
    ly_create_alias(NAME TestGem.Tools.API NAMESPACE Gem TARGETS Gem::TestGem.Editor.API)
    ly_create_alias(NAME TestGem.Builders.API NAMESPACE Gem TARGETS Gem::TestGem.Editor.API)

endif()

################################################################################
# Tests
################################################################################
# See if globally, tests are supported
if(PAL_TRAIT_BUILD_TESTS_SUPPORTED)
    # We globally support tests, see if we support tests on this platform for TestGem.Tests
    if(PAL_TRAIT_TESTGEM_TEST_SUPPORTED)
        # We support TestGem.Tests on this platform, add dependency on the Private Object target
        ly_add_target(
            NAME TestGem.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
            NAMESPACE Gem
            FILES_CMAKE
                testgem_tests_files.cmake
            INCLUDE_DIRECTORIES
                PRIVATE
                    Tests
                    Source
            BUILD_DEPENDENCIES
                PRIVATE
                    AZ::AzTest
                    AZ::AzFramework
                    Gem::TestGem.Private.Object
        )

        # Add TestGem.Tests to googletest
        ly_add_googletest(
            NAME Gem::TestGem.Tests
        )
    endif()

    # If we are a host platform we want to add tools test like editor tests here
    if(PAL_TRAIT_BUILD_HOST_TOOLS)
        # We are a host platform, see if Editor tests are supported on this platform
        if(PAL_TRAIT_TESTGEM_EDITOR_TEST_SUPPORTED)
            # We support TestGem.Editor.Tests on this platform, add TestGem.Editor.Tests target which depends on
            # private TestGem.Editor.Private.Object target
            ly_add_target(
                NAME TestGem.Editor.Tests ${PAL_TRAIT_TEST_TARGET_TYPE}
                NAMESPACE Gem
                FILES_CMAKE
                    testgem_editor_tests_files.cmake
                INCLUDE_DIRECTORIES
                    PRIVATE
                        Tests
                        Source
                BUILD_DEPENDENCIES
                    PRIVATE
                        AZ::AzTest
                        Gem::TestGem.Private.Object
            )

            # Add TestGem.Editor.Tests to googletest
            ly_add_googletest(
                NAME Gem::TestGem.Editor.Tests
            )
        endif()
    endif()
endif()

If using the CMake Tools extension for VSCode, this is how a new Gem Targets would look image

Example: Existing Gem using an API target

The following is a contrived example of the NvCloth gem using the API target of the TestGem from the example above. The gem maintainer of the NvCloth gem would add a build dependency on the TestGem.${API target for the runtime API target.

ly_add_target(
    NAME NvCloth.API INTERFACE
    NAMESPACE Gem
    FILES_CMAKE
        nvcloth_api_files.cmake
    INCLUDE_DIRECTORIES
        INTERFACE
            Include
    BUILD_DEPENDENCIES
        INTERFACE
            3rdParty::NvCloth
            AZ::AzFramework
            Gem::AtomLyIntegration_CommonFeatures.API
            Gem::TestGem.API
            Gem::EMotionFX.API
)

Other Concerns

The O3DE repo currently contains 80 top level gems with an AutomatedTesting project which contains a single gem. The Atom Gem contains several sub gems(Atom_RHI, Atom_RPI, etc...) as well which pushes the total number of gems to over 100. The list of targets in gems vary based on the needs of that gem what it wishes to expose. Some gems have 0 targets others have 2~3 targets based on if Testing is enabled, while others might have 9+ targets that will show up in an IDE such as Visual Studio.

Gems are plugins, that will often have disjoint authors, that should not need to worry about any issues with a particular when authoring their gems.

For example a gem author who creates a Particle system gem that has 10 targets and host it as part of an external repo, does not need to worry if their gem can load within the engine's build solution, as only gems registered with the the current project and the engine have their CMakeLists.txt visited.

But someone adding a new gem within the engine repo, will have that gem's targets be part of the generated build solution for whatever generator was used(Ninja, Xcode, Unix Makefiles, etc...).

A subset of gems registered within the engine and resides within the O3DE repo should be moved to another repo, since they are not core to the engine itself.

The following is a list of gems that might not be core to the engine itself can be moved to another repo

#### Gem List ``` Achievements AssetValidation AtomContent AtomTressFX AudioEngineWwise AWSClientAuth AWSCore AWSGameLift AWSMetrics BarrierInput Blast CertificateManager CustomAssetExample DevTextures ExpressionEvaluation GameState GameStateSamples Gestures HttpRequestor InAppPurchases LocalUser LyShineExamples MessagePopup Metastream Microphone MotionMatching NvCloth PhysXDebug Presence PrimitiveAssets SaveData SceneLoggingExample ScriptCanvasDeveloper ScriptCanvasPhysics ScriptCanvasTesting ScriptedEntityTweener ScriptEvents SliceFavorites Stars StartingPointCamera StartingPointInput StartingPointMovement TestAssetBuilder TickBusOrderViewer Twitch UiBasics VideoPlaybackFramework VirtualGamepad ``` That would leave the engine repo with set of "core" gems ``` Atom AtomLyIntegration AudioSystem Camera CameraFramework CrashReporting DebugDraw EditorPythonBindings EMotionFX FastNoise GradientSignal GraphCanvas GraphModel ImGui LandscapeCanvas LmbrCentral LyShine Maestro Multiplayer PhysX Prefab Profiler PythonAssetBuilder QtForPython SceneProcessing ScriptCanvas SurfaceData Terrain TextureAtlas Vegetation WhiteBox ``` That would reduce the number of gems that come with the engine from 80 to 32.
cgalvan commented 2 years ago

I think this is a great idea. It will be a cleaner separation of concerns, and will make users more cognizant of the potential side-effects when making CMake changes to their gems.

I also agree we should start by updating our templates first, and then do a sweep of updating our existing gems. This will likely need input from various gem owners to determine which things can/should be in their Public vs. Private APIs.

AMZN-alexpete commented 2 years ago

I like the idea. Couple question:

  1. How will the addition of a new target (potentially for every gem) cause an increase in configuration time, Visual Studio solution generation time and solution load time - I'm assuming compile time would not be affected much since they're just headers, but we could be looking at a significant number of new vs projects.

  2. How will the additional target lead to added confusion for customers who want to do something common like adding a component with an ebus/interface and need to choose between the targets:

    • gem
    • gem.API
    • gem.Editor
    • gem.Editor.static
    • gem.Static
    • gem.Tests

Could we be increasing the difficulty for customers to learn how to add content to gems?

moraaar commented 2 years ago

Thanks for writing this RFC @lumberyard-employee-dm.

Overall I'm onboard with adding the new API targets to template gems. It paves the way for Gem authors to think on how they are going to present publicly their functionality while keeping the implementation private.

Feedback on the document

Question

Concerns

  1. Dependency and API readiness When a target on gem A depends on a API target on gem B, that's not going to guarantee that the API of gem B is going to be ready before gem A calls it. The document doesn't explain how this runtime dependency works and what steps the users has to do to guarantee that an API is ready for when their dependents use it.

  2. Very high number of projects The major concern would be the total number of projects we have with O3DE. Currently there are 526 projects on the VS solution. A normal Gem with runtime and editor functionality it's currently formed of 6 targets:

    • {GemName}
    • {GemName}.static
    • {GemName}.Editor
    • {GemName}.Editor.static
    • {GemName}.Tests
    • {GemName}.Editor.Tests

On top of this now we are going to add 2 more projects, making 8 targets per Gem. And this is ignoring the Benchmark target, which we should be encouraging to do as well.

This comes with the risk of potential disadvantages that the RFC does not mention. If O3DE scales in number of gems, the VS solution could become very large, slow, difficult to navigate, difficult to search and find things. I think at least the RFC should touch on this subject and mention a mitigation if there is any.

Alternative

There is an alternative that the RFC document doesn't mention, which is to keep using the static and module targets and educate the user to make a better usage of the visibility properties PUBLIC and PRIVATE on the Includes and Dependencies. The static target can have their includes PRIVATE, that way anybody outside the Gem that tries to depend on them won't find the headers. Then the module target will make the Include directory PUBLIC and everything else PRIVATE (including build dependencies). There is an example of this in NvCloth gem. This will avoid adding 2 new targets per Gem.

I still prefer the proposed solution of adding API targets, because it's more explicit. But if something goes wrong it's good that the RFC mentions this as an alternative.

lemonade-dm commented 2 years ago

I like the idea. Couple question:

1. How will the addition of a new target (potentially for every gem) cause an increase in configuration time, Visual Studio solution generation time and solution load time - I'm assuming compile time would not be affected much since they're just headers, but we could be looking at a significant number of new vs projects.

2. How will the additional target lead to added confusion for customers who want to do something common like adding a component with an ebus/interface and need to choose between the targets:

* gem

* gem.API

* gem.Editor

* gem.Editor.static

* gem.Static

* gem.Tests

Could we be increasing the difficulty for customers to learn how to add content to gems?

For your first question: The addition of new targets to Visual Studio shouldnot cause a large increase in solution loading time. The addition of the .API .vcxproj projects is balanced against the fact the at the .Static becoming a .Private.Object target is smaller and does not contain the files from the Include directory.

Compilation time doesn't change at all.

Onto the second question: The additional target will simplify users adding and EBus interface or AZ interface to their gem. Those interfaces would go into the .API target if the interface is meant to be used by other gems, otherwise they should be placed into the .Private.Object target. Component themselves should only be placed in the .Private.Object target as they shouldn't be directly available to gem users

lemonade-dm commented 2 years ago

Thanks for writing this RFC @lumberyard-employee-dm.

Overall I'm onboard with adding the new API targets to template gems. It paves the way for Gem authors to think on how they are going to present publicly their functionality while keeping the implementation private.

Feedback on the document

* It's a very nice intro. I miss in its second paragraph a mention of `${GemName}.Editor.Static` target, which is a very common static library our gems have as well.

* The links to CppToolGem and DefaultGem seem to be broken.

* I'm missing in this RFC an example of a `CMakeList.txt` file of a common Gem with the normal targets plus the new API targets, with comments on each target explaining their scope and visibility. It'd help understanding better how it would look.

* It'd also be great to show a Gem's `CMakeList.txt` file having a dependency on the API of another gem.

* The first sentence in the disadvantages section is "_The disadvantage is that existing gems will not new API targets exposed by new gems._", there is some error in the sentence around "will not new API targets" that I don't grasp what it means. Is this meant to say that existing gems won't be able to depend on this new API targets?

Question

* A gem is meant to be a compendium of libraries and we should not be limiting the user in what types of libraries they can add to it. Just to be clear, this proposal of adding API targets will be encouraged but optional, right? If the user still wants to create static libraries inside the Gem they can still do it, correct?

Concerns

1. Dependency and API readiness
   When a target on gem A depends on a API target on gem B, that's not going to guarantee that the API of gem B is going to be ready before gem A calls it. The document doesn't explain how this runtime dependency works and what steps the users has to do to guarantee that an API is ready for when their dependents use it.

2. Very high number of projects
   The major concern would be the total number of projects we have with O3DE. Currently there are 526 projects on the VS solution. A normal Gem with runtime and editor functionality it's currently formed of 6 targets:

* {GemName}

* {GemName}.static

* {GemName}.Editor

* {GemName}.Editor.static

* {GemName}.static.Tests

* {GemName}.Editor.static.Tests

On top of this now we are going to add 2 more projects, making 8 targets per Gem. And this is ignoring the Benchmark target, which we should be encouraging to do as well.

* {GemName}

* {GemName}.API

* {GemName}.Private.Object

* {GemName}.Editor

* {GemName}.Editor.API

* {GemName}.Editor.Private.Object

* {GemName}.static.Tests

* {GemName}.Editor.static.Tests

This comes with the risk of potential disadvantages that the RFC does not mention. If O3DE scales in number of gems, the VS solution could become very large, slow, difficult to navigate, difficult to search and find things. I think at least the RFC should touch on this subject and mention a mitigation if there is any.

Alternative

There is an alternative that the RFC document doesn't mention, which is to keep using the static and module targets and educate the user to make a better usage of the visibility properties PUBLIC and PRIVATE on the Includes and Dependencies. The static target can have their includes PRIVATE, that way anybody outside the Gem that tries to depend on them won't find the headers. Then the module target will make the Include directory PUBLIC and everything else PRIVATE (including build dependencies). There is an example of this in NvCloth gem. This will avoid adding 2 new targets per Gem.

I still prefer the proposed solution of adding API targets, because it's more explicit. But if something goes wrong it's good that the RFC mentions this as an alternative.

Answers: Feedback on the document

Answers: Question.

* A gem is meant to be a compendium of libraries and we should not be limiting the user in what types of libraries they can add to it. Just to be clear, this proposal of adding API targets will be encouraged but optional, right? If the user still wants to create static libraries inside the Gem they can still do it, correct?

To answer your question, the Gem Template is only meant has a starting place for users to create there own gems. There is no CMake code in the O3DE build system to require users to use a specific target naming scheme or how many targets a gem can have.

Now what a gem is really a package with a descriptor "gem.json" file and a CMakeLists.txt script that allows hooking into the O3DE Build System as a plugin. A gem doesn't even need to have any libraries at all. This is the case for an Asset only gem or a python script only gem which provides a list of files that the AssetProcessor or Editor can use without their being any built code.

Answers: Concerns

  1. Dependency and API readiness

That is out of scope for this document. The Gem systems requires that gems can be loaded in any order. Gems are initialized in several phases, where first the gems are all loaded and their component descriptors registered, followed by the required system components of all loaded gems being created. Finally the activation of the System component uses a topological sort to order the gems based on their service dependencies. That is covered in the Component Services doc.

Now for runtime dependencies that is actually explained in the Settings Registry Developer documentation on how gems are loaded. That documentation for the Settings Registry is not yet brought over.

  1. Very high number of projects The users control how many targets there gem exposes. A user can create a gem using one of our Gem Templates and reduce it down to a single target or zero target or they can add 20 targets to a gem.

Also Visual Studio isn't the only IDE out there, that exposes CMake targets as projects(Xcode, VSCode CMake extension, CLion, etc...). For Visual Studio at least, the cost of loading a project and indexing is based on the content within the .vcxproj files. In this case we aren't adding any new code files, we are splitting up a target into two smaller targets, which has near the same total load time. Furthermore additional source files aren't being indexed, since the total number of files haven't increased, they are just in different .vcxprojs

The reason the Windows Visual Studio project has so many projects is because we have 80 gems within the engine. We are actually trying to move gems out out of the repo which would reduce the number of Visual Studio Projects that are added to the o3de Engine solution.

The main mitigation for the high number of projects is to move to reduce the number of gems that come with the engine. Using an external gem with an O3DE project only hooks that gem's CMakeLists.txt into the build system if that gem is used.

Answers: Alternative

For the alternative approach you mentioned, static library targets still have the issue that it duplicates the object files into a .lib and .a archive file which is results in gigabytes of disk space being used for non-SDK libraries.

Here it can be seen that we have 11GiB of STATIC libraries just for the non-monolithic profile configuration alone, most of which are private targets only used within the engine and not meant for consumption for non-engine gems. So at the very least they should be transitioned over to OBJECT targets image

Furthermore I think the naming scheme of ${GemName}.API better conveys the intent that the target is meant for public consumption. The ${GemName}.Static target is not meant as a public external target for other gems to depend on. Its purpose is to share code among the other targets internal to reduce the need to compile the same source file multiple times. Primarily it is meant as a way for the ${GemName}, ${GemName}.Editor, ${GemName}.Test and ${GemName}.Editor.Test to re-use the object files that are built as part of the gem among them.

lemonade-dm commented 2 years ago

Closing this out as the split has been performed

nick-l-o3de commented 10 hours ago

I would like to add that CMake and OBJECT libraries have additional dependency problems as we've discovered. The RFC is still valid, but I'd also recommend against using any macros such as $<TARGET_OBJECTS> and make sure that any OBJECT libraries are used only directly, and only in PRIVATE, and only in ways that don't complicate SDK install layouts.

(We should not be shipping OBJECT libraries in the install, they should not be considered portable).

This essentially means that an OBJECT library needs to be declared itself as an OBJECT library, not as a STATIC or SHARED, and others importing it need to import it in their PRIVATE section, by name, not using $<TARGET_OBJECTS>.

Another reason not to use $<TARGET_OBJECTS> is that cmake appears to expand that macro to the literal full absolute path to the list of actual build artifact file names, and loses the concept of targets, similar to how if you tell it to link blah.lib instead of linking the target named blah.