I think this is the correct way of doing things, and it seems like something we can do incrementally. So as far as I'm concerned it's fine to go ahead with this.

I am missing description of what TARGET is and how it is supposed to be used and when.

In D8864#217205, @Sergey Sharybin (sergey) wrote:

I am missing description of what TARGET is

This gets defined in the very first paragraph in the cmake documentation i did not think rehashing it here would add any value, so i left it out

and how it is supposed to be used and when.

There are various resources available online (and in hard copy) that give good guidance on what "modern cmake" is and how it should be used,

in no particular order and by far not exhaustive:

• https://cliutils.gitlab.io/modern-cmake/
• https://pabloariasal.github.io/2018/02/19/its-time-to-do-cmake-right/
• https://leimao.github.io/blog/CMake-Public-Private-Interface/

This also looks generally fine to me. What would be the minimal CMake version now? Cannot even find what is our current minimal version, but think it is fairly old one (pre-3.0)…

we bumped the lower bound fairly recently, to 3.10

In D8864#225308, @Brecht Van Lommel (brecht) wrote:

I assumed this was only a conversation starter and didn't need to be accepted since you planned more work, but from devtalk it seems this is waiting so I'll accept it since it also seems fine as a first step to commit directly and then continue adding more libraries in master.

I'll be honest, I was stalling a little bit on devtalk, i have no intentions of landing this so close to bcon3, also I was waiting to give @Campbell Barton (campbellbarton) the chance to chime in.

My plan was to more actively pursue this particular diff and getting it accepted when we are back in bcon1

In D8864#226047, @Campbell Barton (campbellbarton) wrote:

Don't see any issues with this.

Once fully implemented I'd assume this would avoid recent bugs caused by moving code?

• rB4bb15c8eecbf: VSE: Fix audio strips
• rB0bf12cb025a3: Fix error in recent change "run UserDef versioning from readfile.c"

---

Note, would it be better to do:

set(TARGETS
  PRIVATE
  foo
  bar

  PUBLIC
  baz
)

Otherwise we can't auto-indent.

AFAIK (unnecessarily) building lists in CMake is an antipattern. Processing lists in CMake is quite tricky and not natively supported by a datatype.

Thus the canonical way of doing

list(APPEND LIB
  ${AUDASPACE_C_LIBRARIES}
  ${AUDASPACE_PY_LIBRARIES}
)

is

target_link_libraries(bf_foo
  PRIVATE
  ${AUDASPACE_C_LIBRARIES}
  ${AUDASPACE_PY_LIBRARIES}
)

Respectively

set(TARGETS
  PRIVATE
  foo
  bar

  PUBLIC
  baz
)

is canonically

target_link_libraries(bf_foo
  PRIVATE
  foo
  bar
  PUBLIC
  baz
)

As a result, anyone who ever had any exposure to modern CMake immediately knows what is going on without any abstraction.
The signature extending of blender_add_lib will stop working as soon as you want to add support for PRIVATE, PUBLIC and INTERFACE includes or definitions.
To me blender_add_lib is already to overloaded to begin with.

As a result, anyone who ever had any exposure to modern CMake immediately knows what is going on without any abstraction.

Which is very for for us as a project,

We have developers with various degrees of experience, but for a few exceptions in the platforms teams, cmake is just one of those things that most of them dread, it's the thing they have to battle to get their code building.

Now to make this a little less painful we have across the build system identical ways to set up a library, every single CMakeLists.txt has the same variables (LIB/INC/etc) and the same call to blender_add_lib

I deliberately chose to extend blender_add_lib for the consumer side of things only, since this is the part most less experienced developers among us will be exposed to, they are just going "I need function X from lib Y, oh I'll just add it to this list here" this barrier needs to stay as low as possible, yes from a cmake point of view there is no technical need for blender_add_lib to even exist, but from a developer perspective the picture looks quite differently. right now the list of libraries is there in LIB, sometimes empty sometimes not, but devs recognize what is there, and know what to do.

Now for defining properties on targets, there we actually CAN require the platform & build maintainers to be a little bit more knowledgeable and expose them to cmake details , and those properties are directly set with target_xxx calls in the cmakelists.txt for both clog and guardedalloc. There is no desire to extend blender_add_lib to include this functionality.

So while there may not be a technical reason, there definitely is a people reason to do it the way we are doing it, you may not like it as a cmake purist, but putting the split of having to know intimate cmake details at the boundary between "i need to use a lib" and "i need to define a lib" is the right place and I have yet to see a convincing reason why it wouldn't be.