kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

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 <john.j.beard@xxxxxxxxx>
> Date: Thu, Nov 8, 2018 at 12:09 PM
> Subject: Re: [Kicad-developers] [PATCH]/Question TITLE_BLOCK tests
> To: Wayne Stambaugh <stambaughw@xxxxxxxxx>
> 
> 
> 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 <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
>>>
>>>
>>> _______________________________________________
>>> Mailing list: https://launchpad.net/~kicad-developers
>>> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>>> Unsubscribe : https://launchpad.net/~kicad-developers
>>> More help   : https://help.launchpad.net/ListHelp