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 <stambau...@gmail.com> 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@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp