[Scummvm-devel] enabled engines and dependency tracking

Max Horn max at quendi.de
Wed Oct 13 17:22:08 CEST 2010


Am 13.10.2010 um 16:33 schrieb Willem Jan Palenstijn:

> Hi,
> 
> In the following scenario I get a link error at the end:
> 
> ./configure
> make clean
> make
> ./configure --enable-all-engines
> make
> 
> 
> The reason seems to be that graphics/video/codecs/qdm2.{cpp,h} are
> conditionally compiled based on if the Mohawk engine is built. They don't
> include config.h, so aren't automatically rebuilt when reconfiguring. (I'm not
> implying they should be; they use nothing in config.h.)
> 
> Any suggestions? The easiest way of working around this might be to include
> config.h or scummsys.h anyway in qdm2.h, to force a rebuild after
> reconfiguring, but that doesn't seem very elegant...
> 
> (And maybe there are other conditional audio/video components that would cause
> similar problems if engines are enabled/disabled in different combinations.)

Our compilation process, and the way we define global defines, is a bit messy and inconsistent. Some stuff we declare by adding -DFOO to CPPFLAGS, others we put in config.h. The advantage of putting things into the CPPFLAGS is that they are visible everywhere; the drawback is that, as it happened here, changes there do not trigger a rebuild of affected files.

On the other hand, if we define stuff in config.h, then all files that use it automatically get recompiled when necessary. But then another typical mistake happens, namely people writing
  #ifdef USE_OPTIONAL_FEATURE
    #include "common/scummsys.h"
    ..
  #endif
instead of having the #include before the #ifdef (we had an example of that today in the commit logs, but certainly not the first).


Using CPPFLAGS also has the drawback (in my eyes) of causing enormous compiler commands, which I find quite annoying; take this example that comes from the the Lua code in sword25:

cc  -DHAVE_CONFIG_H -DMACOSX -DUNIX -DDATA_PATH=\"/usr/local/share/scummvm\" -DPLUGIN_DIRECTORY=\"/usr/local/lib/scummvm\" -DSDL_BACKEND -DENABLE_SCUMM=STATIC_PLUGIN -DENABLE_SCUMM_7_8 -DENABLE_HE -DENABLE_AGI=STATIC_PLUGIN -DENABLE_AGOS=STATIC_PLUGIN -DENABLE_AGOS2 -DENABLE_CINE=STATIC_PLUGIN -DENABLE_CRUISE=STATIC_PLUGIN -DENABLE_DRACI=STATIC_PLUGIN -DENABLE_DRASCULA=STATIC_PLUGIN -DENABLE_GOB=STATIC_PLUGIN -DENABLE_GROOVIE=STATIC_PLUGIN -DENABLE_GROOVIE2 -DENABLE_HUGO=STATIC_PLUGIN -DENABLE_KYRA=STATIC_PLUGIN -DENABLE_LOL -DENABLE_LURE=STATIC_PLUGIN -DENABLE_M4=STATIC_PLUGIN -DENABLE_MADE=STATIC_PLUGIN -DENABLE_MOHAWK=STATIC_PLUGIN -DENABLE_PARALLACTION=STATIC_PLUGIN -DENABLE_QUEEN=STATIC_PLUGIN -DENABLE_SAGA=STATIC_PLUGIN -DENABLE_IHNM -DENABLE_SAGA2 -DENABLE_SCI=STATIC_PLUGIN -DENABLE_SCI32 -DENABLE_SKY=STATIC_PLUGIN -DENABLE_SWORD1=STATIC_PLUGIN -DENABLE_SWORD2=STATIC_PLUGIN -DENABLE_SWORD25=STATIC_PLUGIN -DENABLE_TESTBED=STATIC_PLUGIN -DENABLE_TEENAGENT=STATIC_PLUGIN -DENABLE_TINSEL=STATIC_PLUGIN -DENABLE_TOON=STATIC_PLUGIN -DENABLE_TOUCHE=STATIC_PLUGIN -DENABLE_TUCKER=STATIC_PLUGIN -I. -I. -I./engines -I/sw/include/SDL -D_GNU_SOURCE=1 -D_THREAD_SAFE  -c -o engines/sword25/util/pluto/plzio.o engines/sword25/util/pluto/plzio.c



There are several other kinds of issues that are caused by "forgetting" to include scummsys.h (or not including it as first thing). So I actually think we should always #include scummsys.h in *all* files at the top. Elegant or not, this is the only way to really ensure we see the same base #defines and typedefs in *all* source files, and to ensure a a proper full rebuild after a reconfigure.

If there weren't those portability issues (when using compilers other than GCC), I would actually suggest that we use 
  -include common/scummsys.h
in our CPPFLAGS and be done with this once and for all ;).


In this particular case, though, I guess we could also avoid the issue by adding a check analogous to
  #if defined(ENABLE_MOHAWK) || defined(DYNAMIC_MODULES)
in video/module.mk, compiling qdm2.cpp only conditionally. I haven't verified this, though.


Bye,
Max



More information about the Scummvm-devel mailing list