Circular CMake dependency: gal <-> common

Bug #1832229 reported by John Beard
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
KiCad
Expired
Low

Bug Description

These two libraries have circular dependencies. This manifests if you link something only to 'common' and not 'gal', even though 'common' has a public 'gal' dependency.

'common' depends on 'gal', but 'gal' depends on the following things (at least) in 'common':

* DPI_SCALING
* UTF8
* DisplayError()
* g_ColorRefs

This is a tricky one, as some things (UTF8) can break out to a small "core" library, but some things like "DPI_SCALING" and "DisplayError" seem a bit "advanced" for that.

---

What happens when you no longer include 'gal' in kicad-ogltest CMake:

/usr/bin/ld: ../../common/libgal.a(gal_display_options.cpp.o): in function `KIGFX::GAL_DISPLAY_OPTIONS::GAL_DISPLAY_OPTIONS()':
/home/john/src/kicad/common/gal/gal_display_options.cpp:73: undefined reference to `DPI_SCALING::GetDefaultScaleFactor()'
/usr/bin/ld: ../../common/libgal.a(gal_display_options.cpp.o): in function `KIGFX::GAL_DISPLAY_OPTIONS::ReadCommonConfig(wxConfigBase&, wxWindow*)':
/home/john/src/kicad/common/gal/gal_display_options.cpp:113: undefined reference to `DPI_SCALING::DPI_SCALING(wxConfigBase*, wxWindow const*)'
/usr/bin/ld: /home/john/src/kicad/common/gal/gal_display_options.cpp:114: undefined reference to `DPI_SCALING::GetScaleFactor() const'
/usr/bin/ld: ../../common/libgal.a(opengl_gal.cpp.o): in function `KIGFX::OPENGL_GAL::DrawPolygon(SHAPE_POLY_SET const&)':
/home/john/src/kicad/common/gal/opengl/opengl_gal.cpp:1000: undefined reference to `SHAPE_POLY_SET::IsTriangulationUpToDate() const'
/usr/bin/ld: ../../common/libgal.a(opengl_gal.cpp.o): in function `KIGFX::OPENGL_GAL::BitmapText(wxString const&, VECTOR2<double> const&, double)':
/home/john/src/kicad/common/gal/opengl/opengl_gal.cpp:1128: undefined reference to `UTF8::UTF8(wxString const&)'
/usr/bin/ld: ../../common/libgal.a(opengl_gal.cpp.o): in function `UTF8::uni_iter::operator++()':
/home/john/src/kicad/include/utf8.h:235: undefined reference to `UTF8::uni_forward(unsigned char const*, unsigned int*)'
/usr/bin/ld: ../../common/libgal.a(opengl_gal.cpp.o): in function `UTF8::uni_iter::operator*() const':
/home/john/src/kicad/include/utf8.h:264: undefined reference to `UTF8::uni_forward(unsigned char const*, unsigned int*)'
/usr/bin/ld: ../../common/libgal.a(opengl_gal.cpp.o): in function `KIGFX::GAL::StrokeText(wxString const&, VECTOR2<double> const&, double)':
/home/john/src/kicad/include/gal/graphics_abstraction_layer.h:342: undefined reference to `UTF8::UTF8(wxString const&)'
/usr/bin/ld: ../../common/libgal.a(vertex_manager.cpp.o): in function `KIGFX::VERTEX_MANAGER::Reserve(unsigned int)':
/home/john/src/kicad/common/gal/opengl/vertex_manager.cpp:77: undefined reference to `DisplayError(wxWindow*, wxString const&, int)'
/usr/bin/ld: ../../common/libgal.a(vertex_manager.cpp.o): in function `KIGFX::VERTEX_MANAGER::Vertex(float, float, float)':
/home/john/src/kicad/common/gal/opengl/vertex_manager.cpp:115: undefined reference to `DisplayError(wxWindow*, wxString const&, int)'
/usr/bin/ld: ../../common/libgal.a(vertex_manager.cpp.o): in function `KIGFX::VERTEX_MANAGER::Vertices(KIGFX::VERTEX const*, unsigned int)':
/home/john/src/kicad/common/gal/opengl/vertex_manager.cpp:140: undefined reference to `DisplayError(wxWindow*, wxString const&, int)'
/usr/bin/ld: ../../common/libgal.a(gpu_manager.cpp.o): in function `KIGFX::GPU_MANAGER::SetShader(KIGFX::SHADER&)':
/home/john/src/kicad/common/gal/opengl/gpu_manager.cpp:69: undefined reference to `DisplayError(wxWindow*, wxString const&, int)'
/usr/bin/ld: ../../common/libgal.a(utils.cpp.o): in function `checkGlError(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, bool)':
/home/john/src/kicad/common/gal/opengl/utils.cpp:116: undefined reference to `DisplayErrorMessage(wxWindow*, wxString const&, wxString const&)'
/usr/bin/ld: ../../common/libgal.a(text_utils.cpp.o): in function `ProcessOverbars(UTF8 const&)':
/home/john/src/kicad/common/text_utils.cpp:49: undefined reference to `UTF8::operator+=(unsigned int)'
/usr/bin/ld: ../../common/libgal.a(color4d.cpp.o): in function `KIGFX::COLOR4D::COLOR4D(EDA_COLOR_T)':
/home/john/src/kicad/common/gal/color4d.cpp:40: undefined reference to `g_ColorRefs'
/usr/bin/ld: /home/john/src/kicad/common/gal/color4d.cpp:41: undefined reference to `g_ColorRefs'
/usr/bin/ld: /home/john/src/kicad/common/gal/color4d.cpp:42: undefined reference to `g_ColorRefs'
/usr/bin/ld: ../../common/libgal.a(hidpi_gl_canvas.cpp.o): in function `HIDPI_GL_CANVAS::HIDPI_GL_CANVAS(wxWindow*, int, int const*, wxPoint const&, wxSize const&, long, wxString const&, wxPalette const&)':
/home/john/src/kicad/common/gal/hidpi_gl_canvas.cpp:36: undefined reference to `DPI_SCALING::GetDefaultScaleFactor()'
/usr/bin/ld: ../../common/libgal.a(stroke_font.cpp.o): in function `UTF8::UTF8(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)':
/home/john/src/kicad/include/utf8.h:94: undefined reference to `IsUTF8(char const*)'
collect2: error: ld returned 1 exit status

Tags: cleanup
no longer affects: kicad/6.0
Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

It seems to me that gal should just be rolled into common given that gal is mandatory pretty much everywhere or am I missing something here?

Changed in kicad:
status: New → Triaged
milestone: none → 6.0.0-rc1
Revision history for this message
Jeff Young (jeyjey) wrote :

Did this get fixed, or was that a different circular CMake dep that was breaking the builds recently?

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

@Jeff, AFAIK it did not get fixed. Given that GAL is used almost everywhere within KiCad, why not just build gal into the common library rather than a separate static library. This should resolve the circular dependency.

Revision history for this message
Seth Hillbrand (sethh) wrote :

@Wayne-

libcommon is pretty unwieldy already. If possible, could we keep them separate?

The functions John mentions can be broken out into GAL-specific calls that are then registered by the referencing instance.

Revision history for this message
Wayne Stambaugh (stambaughw) wrote :

@Seth, I don't have a strong opinion one way or the other. Everything required to build gal is already defined in the common/CMakeList.txt file, so it seemed like the path of least resistance. This fix will ensure that we don't end up with this discussion being circular. Given that someone added dependencies from common to the gal source, it's highly likely that it will happen again.

Changed in kicad:
assignee: nobody → Ian McInerney (imcinerney)
Revision history for this message
KiCad Janitor (kicad-janitor) wrote :

KiCad bug tracker has moved to Gitlab. This report is now available here: https://gitlab.com/kicad/code/kicad/-/issues/1906

Changed in kicad:
status: Triaged → Expired
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.