Hi John, I merged your patch.
Thanks, Wayne On 11/8/2018 7:10 AM, John Beard wrote: > Forwarding to the list (sorry!). > > ---------- Forwarded message --------- > From: John Beard <[email protected]> > Date: Thu, Nov 8, 2018 at 12:09 PM > Subject: Re: [Kicad-developers] [PATCH]/Question TITLE_BLOCK tests > To: Wayne Stambaugh <[email protected]> > > > Hi Wayne, > > Coming back to this now the fuzzing stuff seems to have settled in. > > This is pretty much the same patch as before with only very minor > tweaks, but according to Jenkins, it builds on both Msys2 and MSVC. So > perhaps something has settled down after the recent merges? > > As before, the tests introduced are trivial tests on TITLE_BLOCK - the > real aim is to get the libcommon mocks building, so new tests should > just be "drop-in" additions of .cpp files. > > This patch depends on the legacy_gal patch posted yesterday, as > qa_common uses legacy_gal (which is why this is patch #0002). > > Cheers, > > John > On Mon, Oct 8, 2018 at 5:39 PM Wayne Stambaugh <[email protected]> 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 <[email protected]> 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 >>> >>> >>> _______________________________________________ >>> Mailing list: https://launchpad.net/~kicad-developers >>> Post to : [email protected] >>> Unsubscribe : https://launchpad.net/~kicad-developers >>> More help : https://help.launchpad.net/ListHelp _______________________________________________ Mailing list: https://launchpad.net/~kicad-developers Post to : [email protected] Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
