RFC: Update the Gem Templates to support a Public/Private API split about sig-core

[cgalvan]

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]

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]

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}.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.

• {GemName}
• {GemName}.API
• {GemName}.Private.Object
• {GemName}.Editor
• {GemName}.Editor.API
• {GemName}.Editor.Private.Object
• {GemName}.Tests
• {GemName}.Editor.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.

[lemonade-dm]

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]

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

• I have fixed the links to the CppToolGem and the DefaultGem templates
• I updated the end of Suggested Design Description section to mention that the gem's Editor STATIC library target would also be converted an OBJECT library target.
• I have added an example of a TestGem added using the new public .API targets with the new .Private.Object targets.
• I have also clarified the disadvantages section to indicate that existing gems may not have the .API targets. This better emphasizes that the change is a "convention" to be followed and not an enforcement of specific targets in case the gem users are surprised by older gems not have the .API targets.

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.

2. 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

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]

Closing this out as the split has been performed