Circular CMake dependency: gal <-> common

Bug #1832229 reported by John Beard on 2019-06-10
14
This bug affects 3 people
Affects Status Importance Assigned to Milestone
KiCad
Triaged
Low
Ian McInerney

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

no longer affects: kicad/6.0
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
Jeff Young (jeyjey) wrote :

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

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.

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.

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)
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers