Design linking scheme to make gunit testing of non-standalone blender libraries sane #40298

New Issue

Howard Trickey · 2014-05-21T17:13:07+02:00

Howard Trickey commented

2014-05-21 17:13:07 +02:00

We are trying to add automated unit test coverage using gunit, currently in the gtest-testing branch.

For CMake, there is now a Testing.cmake module that provides a BLENDER_TEST macro, where you give a test file and the extra libraries to link with.
This works well for standalone or close-to-standalone libraries like the ones in mathutils. But if you try to test something in a library that calls something in BKE, then you find that you need a very large fraction of all the of the libraries needed to link blender (creator) itself. Furthermore, that set of libraries depends on which features are enabled by WITH_xxx flags.

Longer term, we should refactor the libraries to be more independent. And / or make stub libraries to be used in many cases.

But for now, making macros in CMake that make it easier to bring in dependent libraries, and, transitively, indirectly dependent libraries, would make it possible to write tests for, say, bmesh primitives.

Open question: do we need to do the same for scons, or will automated testing be restricted for now to devs who build with CMake?

We are trying to add automated unit test coverage using gunit, currently in the gtest-testing branch. For CMake, there is now a Testing.cmake module that provides a BLENDER_TEST macro, where you give a test file and the extra libraries to link with. This works well for standalone or close-to-standalone libraries like the ones in mathutils. But if you try to test something in a library that calls something in BKE, then you find that you need a very large fraction of all the of the libraries needed to link blender (creator) itself. Furthermore, that set of libraries depends on which features are enabled by WITH_xxx flags. Longer term, we should refactor the libraries to be more independent. And / or make stub libraries to be used in many cases. But for now, making macros in CMake that make it easier to bring in dependent libraries, and, transitively, indirectly dependent libraries, would make it possible to write tests for, say, bmesh primitives. Open question: do we need to do the same for scons, or will automated testing be restricted for now to devs who build with CMake?

Howard Trickey commented

2014-05-21 17:13:07 +02:00

Changed status to: 'Open'

Howard Trickey commented

2014-05-21 17:13:07 +02:00

Added subscribers: @howardt, @ideasman42, @Sergey

Howard Trickey commented

2014-05-21 17:14:05 +02:00

Campbell wrote the following in email about this task:

I think the 'correct' solution would be to make each library list its
dependencies (we should probably do this anyway).

I looked into doing this a while ago and got it working (mostly), but
I didnt see any big benefits to commit at the time (ran into some
glitches linking cycles - but can be made to work if needed).

This has the advantage that CMake can figure out library deps for us,
and we dont have to maintain BLENDER_SORTED_LIBS which is kludgey.

See the attached patch as a reference, it applies to an old sha1:
8f1a6e26b6

The problem with running target_link_libraries explicitly on blender
libs is the blenderplayer since we want those same libs to use stubs
when used with the blenderplayer. and I couldn't find a nice way to
resolve that.
(Basically we need to support 2x different linking configurations for
some libs - and CMake doesn't do this easily).

Proposal:

We define re-usable mapping: "lib -> lib_deps" but defer calling
target_link_libraries
(can be defined in each CMakeLists.txt like SRC and INC).

eg: "bf_blenlib -> bf_intern_ghost;bf_intern_guardedalloc;extern_wcwidth"

When the library is used (by Blender / BlenderPlayer / Tests) use a
macro to request its lib deps to pass to target_link_libraries, and
a convenient way to clear after.

  blender_lib_init(bf_somelib)
  .... setup test.. exe... etc ...
  blender_lib_clear(bf_somelib)

We need this for the blenderplayer as mentioned before.

Optionally - Extend the BLENDER_TEST macro to check if any of the libs
are included in the linking map and run blender_lib_init/clear for
us (we may not want to do this in all cases since we might want to
intentionally stub out functions too... we could take an arg to
explicitly exclude some lib deps too).

Campbell wrote the following in email about this task: I think the 'correct' solution would be to make each library list its dependencies (we should probably do this anyway). I looked into doing this a while ago and got it working (mostly), but I didnt see any big benefits to commit at the time (ran into some glitches linking cycles - but can be made to work if needed). This has the advantage that CMake can figure out library deps for us, and we dont have to maintain BLENDER_SORTED_LIBS which is kludgey. See the attached patch as a reference, it applies to an old sha1: 8f1a6e26b6d32fd4b293ab06d22a34c8a8cd2505 The problem with running `target_link_libraries` explicitly on blender libs is the blenderplayer since we want those same libs to use stubs when used with the blenderplayer. and I couldn't find a nice way to resolve that. (Basically we need to support 2x different linking configurations for some libs - and CMake doesn't do this easily). ---- Proposal: - We define re-usable mapping: "lib -> lib_deps" but defer calling `target_link_libraries` (can be defined in each CMakeLists.txt like SRC and INC). eg: "bf_blenlib -> bf_intern_ghost;bf_intern_guardedalloc;extern_wcwidth" When the library is used (by Blender / BlenderPlayer / Tests) use a macro to request its lib deps to pass to `target_link_libraries`, and a convenient way to clear after. ``` blender_lib_init(bf_somelib) .... setup test.. exe... etc ... blender_lib_clear(bf_somelib) ``` We need this for the blenderplayer as mentioned before. Optionally - Extend the BLENDER_TEST macro to check if any of the libs are included in the linking map and run `blender_lib_init/clear` for us (we may not want to do this in all cases since we might want to intentionally stub out functions too... we could take an arg to explicitly exclude some lib deps too).

Campbell Barton commented

2014-05-22 01:41:54 +02:00

Attached the patch referenced in my previous message, posted by howardt : cmake_linking.diff

Attached the patch referenced in my previous message, posted by howardt : [cmake_linking.diff](https://archive.blender.org/developer/F89638/cmake_linking.diff)

Bastien Montagne commented

2014-05-23 09:03:56 +02:00

Added subscriber: @mont29

Sergey Sharybin commented

2014-05-23 09:32:25 +02:00

I'm not fully sure, is the main idea of the patch to maintain dependencies between libraries? For now it looks like it's kind of manual dependency specification between the libs which is not that nice..

Would rather try to detect dpeendencies automatically based on http://www.blender.org/bf/codelayout.jpg or make it an argument to blender_add_lib().

I'm not fully sure, is the main idea of the patch to maintain dependencies between libraries? For now it looks like it's kind of manual dependency specification between the libs which is not that nice.. Would rather try to detect dpeendencies automatically based on http://www.blender.org/bf/codelayout.jpg or make it an argument to blender_add_lib().

Campbell Barton commented

2014-05-23 13:17:11 +02:00

@Sergey, currently we already manually maintain lib deps BLENDER_SORTED_LIBS, worse, we have to solve deps by moving libs up and down and guessing in some cases, until it builds.

CMake can figure out library linking order for us, but we would have to manually list the libs each other lib depends on.

Not sure how this could be done automatic.

codelayout.jpg is nice, but in real life, its a lot more spaghetti (BKE_ -> RNA_ -> ED_). :S

I made a patch that added a lib argument to blender_add_lib(), a while ago, the nice thing with this is you can group lib setup with include's:
if(WITH_FFMPEG); list(append INC ...); list(append LIB...); endif()

@Sergey, currently we already manually maintain lib deps `BLENDER_SORTED_LIBS`, worse, we have to solve deps by moving libs up and down and guessing in some cases, until it builds. CMake can figure out library linking order for us, but we would have to manually list the libs each other lib depends on. Not sure how this could be done automatic. `codelayout.jpg` is nice, but in real life, its a lot more spaghetti (BKE_ -> RNA_ -> ED_). :S I made a patch that added a lib argument to blender_add_lib(), a while ago, the nice thing with this is you can group lib setup with include's: `if(WITH_FFMPEG); list(append INC ...); list(append LIB...); endif()`

Howard Trickey commented

2014-05-23 13:39:35 +02:00

I wrote a simple python program to find direct dependencies between a list of libxxx.a files. You'd think someone would have written such a think but I couldn't easily find one for linux (the things I found seem to all be about finding dependencies to shared libraries, not static ones; Windows has a Dependency Walker).

I will upload the script and the results of analyzing our libraries here, later today or tomorrow.

It would be possible to make such a think part of our build process -- that is, automatically detect the dependencies but

could be slow unless this were rewritten in C
seems a little silly to do exactly what the linker will do later in the build process anyway
there is value to making our CMake files explicitly, manually, declare the dependencies: the value is that it makes developers aware of the spaghetti interdependencies among the libraries, as opposed to the sane design envisioned by codelayout.jpg, and perhaps inspire working towards removing the dependencies, resulting in a cleaner design.

The downside of manually declaring the dependencies, of course, is that this will break when developers add new code that adds new dependencies and they don't remember to fix the CMakefiles. Especially if the only thing that breaks is test compilation and normal developers don't get in the habit of building the tests all the time.

I wrote a simple python program to find direct dependencies between a list of libxxx.a files. You'd think someone would have written such a think but I couldn't easily find one for linux (the things I found seem to all be about finding dependencies to shared libraries, not static ones; Windows has a Dependency Walker). I will upload the script and the results of analyzing our libraries here, later today or tomorrow. It would be possible to make such a think part of our build process -- that is, automatically detect the dependencies but 1) could be slow unless this were rewritten in C 2) seems a little silly to do exactly what the linker will do later in the build process anyway 3) there is value to making our CMake files explicitly, manually, declare the dependencies: the value is that it makes developers aware of the spaghetti interdependencies among the libraries, as opposed to the sane design envisioned by codelayout.jpg, and perhaps inspire working towards removing the dependencies, resulting in a cleaner design. The downside of manually declaring the dependencies, of course, is that this will break when developers add new code that adds new dependencies and they don't remember to fix the CMakefiles. Especially if the only thing that breaks is test compilation and normal developers don't get in the habit of building the tests all the time.

Sergey Sharybin commented

2014-05-23 13:42:55 +02:00

Why not just link tests against all libs? It should still be pretty fast and doesn't add complexity to the build system maintaince.

Campbell Barton commented

2014-05-23 14:47:19 +02:00

@Sergey, we could, and to get things working probably its a good option.

I would rather not do this long term though, it means we accept blender is a ball of code we can't manage deps with, and I think its not that big a deal to list deps for each lib.

@Sergey, we could, and to get things working probably its a good option. I would rather not do this long term though, it means we accept blender is a ball of code we can't manage deps with, and I think its not _that_ big a deal to list deps for each lib.

Sergey Sharybin commented

2014-05-23 14:54:40 +02:00

Original issue was caused by bmesh using stuff from all over the place afair? Then that's the issue which is to be solved. Adding even more complexity to cmake is not best solution for the original issue, imo.

As for the long term solution i don't see why we can't make it so all the modules are strictly following hierarchy, avoid cross-references from all over the place and pass list of libs which are needed to particular group of tests.

Original issue was caused by bmesh using stuff from all over the place afair? Then that's the issue which is to be solved. Adding even more complexity to cmake is not best solution for the original issue, imo. As for the long term solution i don't see why we can't make it so all the modules are strictly following hierarchy, avoid cross-references from all over the place and pass list of libs which are needed to particular group of tests.

Howard Trickey commented

2014-05-23 19:21:12 +02:00

I am thinking now that I should try to first get a method that links a test with all libraries, just to get going.

To Campbell's point: yes, this means we are accepting that blender is a ball of code we can't manage deps with. For now, even if we list the direct dependencies, by the time you take the transitive closure, pretty much all libs will come in anyway.

To Sergey's point about solving the problem of bmesh using stuff from all over the place: while bmesh has a couple strange dependencies (on bullet, for example), it is the very reasonable dependency on blenkernel that leads to the main problem: blenkernel has way too many outside dependencies. That is because it calls out to all manner of modules directly. Here is the set of dependencies for blenkernel:

bf_avi(AVI_open_compress)
bf_blenfont(BLF_translate_do_iface)
bf_blenlib(dist_squared_to_plane_v3)
bf_blenloader(BLO_write_file)
bf_bmesh(bmiter__edge_of_mesh_begin)
bf_editor_screen(stipple_quarttone)
bf_gpu(GPU_build_bmesh_pbvh_buffers)
bf_ikplugin(BIK_release_tree)
bf_imbuf(imb_ext_image_qt)
bf_imbuf_openexr(IMB_exr_close)
bf_intern_audaspace(AUD_load)
bf_intern_guardedalloc(MEM_get_memory_in_use)
bf_intern_mikktspace(genTangSpaceDefault)
bf_intern_raskter(PLX_raskterize)
bf_intern_rigidbody(RB_constraint_set_limits_hinge)
bf_intern_smoke(smoke_dissolve_wavelet)
bf_modifiers(modifier_type_init)
bf_nodes(register_node_type_cmp_pixelate)
bf_python(BPY_context_update)
bf_python_bmesh(bpy_bm_generic_invalidate)
bf_python_ext(IDP_spit)
bf_render(RE_SwapResult)
bf_rna(RNA_def_struct_duplicate_pointers)
bf_windowmanager(WM_cursor_wait)
extern_bullet(plNearestPoints)
extern_libmv(libmv_reprojectionErrorForImage)
extern_lzma(LzmaCompress)
extern_minilzo(lzo1x_1_compress)
extern_recastnavigation(recast_buildMeshAdjacency)
c(fread)
gomp(GOMP_loop_end_nowait)
m(log)
pthread(pthread_mutex_lock)
z(gzread)
GLEW(__glewVertexAttrib4fvARB)
fftw3(fftw_execute)
GL(glDrawArrays)
avformat(avio_seek)
avcodec(av_init_packet)
avutil(av_dict_free)
swscale(sws_scale)

Fixing this mess probably requires splitting blenkernel up some. And/or having some parts of Blender 'register' with the blenkernel via an API to provide function pointers (to simulate dynamic binding).

For your interests and future reference, here is the program I used to generate the direct dependencies:
getdeps.py
And here is the shell script to drive it. The extra libraries supplied were somewhat specific to being on Linux, my build configuration (this one has pretty much WITH_everything), and where some of the software is on my system.
getalldeps.sh
And here is the output of running getalldeps.sh at the root of my build directory
getalldeps.txt

I am thinking now that I should try to first get a method that links a test with all libraries, just to get going. To Campbell's point: yes, this means we are accepting that blender is a ball of code we can't manage deps with. For now, even if we list the direct dependencies, by the time you take the transitive closure, pretty much all libs will come in anyway. To Sergey's point about solving the problem of bmesh using stuff from all over the place: while bmesh has a couple strange dependencies (on bullet, for example), it is the very reasonable dependency on blenkernel that leads to the main problem: blenkernel has way too many outside dependencies. That is because it calls out to all manner of modules directly. Here is the set of dependencies for blenkernel: ``` bf_avi(AVI_open_compress) bf_blenfont(BLF_translate_do_iface) bf_blenlib(dist_squared_to_plane_v3) bf_blenloader(BLO_write_file) bf_bmesh(bmiter__edge_of_mesh_begin) bf_editor_screen(stipple_quarttone) bf_gpu(GPU_build_bmesh_pbvh_buffers) bf_ikplugin(BIK_release_tree) bf_imbuf(imb_ext_image_qt) bf_imbuf_openexr(IMB_exr_close) bf_intern_audaspace(AUD_load) bf_intern_guardedalloc(MEM_get_memory_in_use) bf_intern_mikktspace(genTangSpaceDefault) bf_intern_raskter(PLX_raskterize) bf_intern_rigidbody(RB_constraint_set_limits_hinge) bf_intern_smoke(smoke_dissolve_wavelet) bf_modifiers(modifier_type_init) bf_nodes(register_node_type_cmp_pixelate) bf_python(BPY_context_update) bf_python_bmesh(bpy_bm_generic_invalidate) bf_python_ext(IDP_spit) bf_render(RE_SwapResult) bf_rna(RNA_def_struct_duplicate_pointers) bf_windowmanager(WM_cursor_wait) extern_bullet(plNearestPoints) extern_libmv(libmv_reprojectionErrorForImage) extern_lzma(LzmaCompress) extern_minilzo(lzo1x_1_compress) extern_recastnavigation(recast_buildMeshAdjacency) c(fread) gomp(GOMP_loop_end_nowait) m(log) pthread(pthread_mutex_lock) z(gzread) GLEW(__glewVertexAttrib4fvARB) fftw3(fftw_execute) GL(glDrawArrays) avformat(avio_seek) avcodec(av_init_packet) avutil(av_dict_free) swscale(sws_scale) ``` Fixing this mess probably requires splitting blenkernel up some. And/or having some parts of Blender 'register' with the blenkernel via an API to provide function pointers (to simulate dynamic binding). For your interests and future reference, here is the program I used to generate the direct dependencies: [getdeps.py](https://archive.blender.org/developer/F90481/getdeps.py) And here is the shell script to drive it. The extra libraries supplied were somewhat specific to being on Linux, my build configuration (this one has pretty much WITH_everything), and where some of the software is on my system. [getalldeps.sh](https://archive.blender.org/developer/F90482/getalldeps.sh) And here is the output of running getalldeps.sh at the root of my build directory [getalldeps.txt](https://archive.blender.org/developer/F90483/getalldeps.txt)

Howard Trickey commented

2014-05-26 02:02:39 +02:00

The "link with everything" method could be done as in patch D551: Making libraries needed to link Blender visible to testing, and bmesh_core_test that uses this..

The "link with everything" method could be done as in patch [D551: Making libraries needed to link Blender visible to testing, and bmesh_core_test that uses this.](https://archive.blender.org/developer/D551).

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Design linking scheme to make gunit testing of non-standalone blender libraries sane #40298