kicad-developers team mailing list archive

[John Beard <john.j.beard@xxxxxxxxx>]

Hi Wayne,

I have no ideas why that would be, but the QA stuff does have a funny
way of exposing strange link behaviours. In this case, we have:

target_link_libraries( qa_common
    common
    polygon
    bitmaps
    ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}
    ${wxWidgets_LIBRARIES} # doesn't this provide wxcolourbase??
)

I have no explanation for why wxColourBase::FromString doesn't link
here. Perhaps it's a similar reason for whatever prompted this being
necessary for pcb_test_window:

target_link_libraries( test_window
    polygon
    pnsrouter
    common
    pcbcommon
    bitmaps
    polygon
    pnsrouter
    common
    pcbcommon
    bitmaps
    polygon
    pnsrouter
    common
    pcbcommon
    bitmaps
    polygon
    pnsrouter
    common
    pcbcommon
    3d-viewer
    bitmaps
    gal
    pcad2kicadpcb
    common
    pcbcommon
    ${GITHUB_PLUGIN_LIBRARIES}
    common
    pcbcommon
    ${Boost_FILESYSTEM_LIBRARY}
    ${Boost_SYSTEM_LIBRARY}
    ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY}
    ${wxWidgets_LIBRARIES}
)

Cheers,

John
On Mon, Oct 8, 2018 at 5:39 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
>
> Hi John,
>
> I am getting the following build error on windows with this patch:
>
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(color4d.cpp.obj):color4d.cpp:(.text+0x1c9):
> undefined reference to `wxColourBase::FromString(wxString const&)'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(color4d.cpp.obj):color4d.cpp:(.text+0x32e):
> undefined reference to `wxColourBase::GetAsString(long) const'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(color4d.cpp.obj):color4d.cpp:(.text+0x80c):
> undefined reference to `wxColourBase::GetAsString(long) const'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(gal_display_options.cpp.obj):gal_display_options.cpp:(.text+0x22b):
> undefined reference to `wxConfigBase::Read(wxString const&, long*, long)
> const'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(gal_display_options.cpp.obj):gal_display_options.cpp:(.text+0x2b8):
> undefined reference to `wxConfigBase::Read(wxString const&, long*, long)
> const'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(gal_display_options.cpp.obj):gal_display_options.cpp:(.text+0x347):
> undefined reference to `wxConfigBase::Read(wxString const&, double*,
> double) const'
> C:/msys64/mingw32/bin/../lib/gcc/i686-w64-mingw32/7.3.0/../../../../i686-w64-mingw32/bin/ld.exe:
> ../../common/libgal.a(gal_display_options.cpp.obj):gal_display_options.cpp:(.text+0x398):
> undefined reference to `wxConfigBase::Read(wxString const&, double*,
> double) const'
> collect2.exe: error: ld returned 1 exit status
> make[2]: *** [qa/common/CMakeFiles/qa_common.dir/build.make:217:
> qa/common/qa_common.exe] Error 1
> make[1]: *** [CMakeFiles/Makefile2:3111:
> qa/common/CMakeFiles/qa_common.dir/all]
>
>
>
> On 10/6/2018 3:48 PM, John Beard wrote:
> > Hi Wayne,
> >
> > On Sat, Oct 6, 2018 at 5:29 PM Wayne Stambaugh <stambaughw@xxxxxxxxx> wrote:
> >> No problem.  I will test your patch once you post it.
> >
> > Patch attached. Indeed, it was a missing include in the header that
> > probably worked until now due to serendipitous include ordering in the
> > existing CPPs.
> >
> > This actually illustrates one nice thing about having unit tests CPPs
> > that include only a single KiCad header - they make sure that header
> > and any transitive includes are compilable even when the test CPP only
> > includes that one header (i.e. as recommended by the code guidelines:
> > 6.2 Headers Without Unsatisfied Dependencies [1]).
> >
> > Another way this can improved is when you have a .cpp/.h pair for a
> > self-contained class, the .cpp should include the corresponding .h
> > first before any other include. This ensures the header is compilable
> > by itself, and has no implicit inclusion criteria for use. This is
> > sometimes, but not always true in KiCad. (In this case, there is no
> > .cpp anyway, so the unit test was the first CPP to attempt to include
> > by itself).
> >
> >> The problem is all of the inline functions defined in convert_to_biu.h.
> >> Until they are replaced with per application equivalents, we will
> >> continue to have to compile every common source file multiple times that
> >> use anything in this file.
> >
> > Right, but in the case of dialog_page_settings.cpp in particular, the
> > only definition actually needed from here was IU_PER_MILS,
> > which can be equivalently retrieved, via the virtual BASE_SCREEN
> > interface, from the derived class, where IU_PER_MILS is properly set
> > for each compiled target (eeschema, pl_editor, pcbnew).
> >
> >> This is odd as they are both in the COMMON_SRCS list.  Actually,
> >> color.cpp is included twice.  Once in KICAD_SRCS and once in COMMON_SRCS
> >> which includes KICAD_SRCS.  We should remove color.cpp from either
> >> KICAD_SRCS or COMMON_SRCS.  I see no need to build it twice.
> >
> > Even more odd. Removing one of the colors.cpp from the
> > common/CMakeLists.cpp didn't stop qa_common having to specifiy it
> > again, though.
> >
> > For colors.cpp specifically, this is an issue which will be resolved
> > when, as promised in colors.h, the removal of legacy support will make
> > the global colour table obsolete. So it's perhaps more of a curiosity
> > than an issue?
> >
> > Regardless, if anyone can shed light on this, I'd appreciate it, as it
> > probably indicates something fishy somewhere?
> >
> > Cheers,
> >
> > John
> >
> > [1]: http://docs.kicad-pcb.org/doxygen/md_Documentation_development_coding-style-policy.html#header_depends
> >