Skip to content

Instantly share code, notes, and snippets.

@fingolfin
Created May 4, 2011 11:55
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save fingolfin/955116 to your computer and use it in GitHub Desktop.
Save fingolfin/955116 to your computer and use it in GitHub Desktop.
Comments on the ScummVM configure

In the following, I go through the ScummVM configure script sequentially, and roughly describe what each part does. Three of these sections are highlighted (dubbed sections A, B and C). These three do somewhat similar things, namely set settings based on _host_os, _host resp. _backends. They are the primary reason for this analysis: Right now, they are used in a very inconsistent fashion, people currently mostly guess when they decide which of the three sections to use. I would like to analyze what each is really good for. Also, possibly move section C further up (right after section B if possible), so that its settings also can affect detection of libraries.

The line numbers refer to this revision.

Anyway, here we go:

line 1

general setup, default settings, functions, etc.

line 625

user greetings, parameter parsing

line 977

  • host detection via config.guess and --host=HOST
  • custom host handling based on _host
  • most hosts just override _host_os, _host_cpu, _host_alias
  • some also change debug/release mode defaults
  • webos also overrides datarootdir, datadir, docdir -- why?
  • but some do more:
    • dreamcast sets CXXFLAGS, LDFLAGS
    • psp sets PSPDEV (if missing) and LDFLAGS
    • why? can we move the FLAGS* to sections ABC? the PSPDEV default could be moved to the code in line 1214

line 1177

debug/release flags are handled, values for HOSTEXEPRE, HOSTEXEEXT, SEPARATOR computed

line 1214

platform specific sanity checks (based on _host_os), i.e. check whether vars like ANDROID_SDK, DEVKITPRO etc. are set, abort if not

line 1324

  • determine c++ compiler
  • this check is very early on, no custom compiler flags set yet,
  • also checks compiler version
  • checks whether -Wglobal-constructors works
  • TODO: much later on (line 3143) we check for other -WFOO flags; can / should we unify this?

line 1430

  • endian check
  • check data type sizes
  • detect x86 (based on _host_cpu)
  • TODO: unify _have_x86 and _arm_asm ? Both seem to have a similar meaning (represent the "host cpu type"), but:
    • their names follow different patterns. Some possible solutions:
      • rename _have_x86 -> _x86_asm (don't like this)
      • rename _arm_asm -> _have_arm (better)
      • replace both by _host_cputype = arm or x86 (most flexible)
    • _have_x86 is computed based on _host_cpu, while _arm_asm is hardcoded based on _host. The former approach seems superior to me: It will automatically cover new ARM based targets, without the porter having to know about _arm_asm at all. Moreover, for e.g. Android there are rumors of x86 based android devices. An important lesson to learn from autoconf: Always detect features, never assume a certain OS / hosttype / etc. implies a feature. So, using _host_cpu seems to be the better approach (even if we hardcode _host_cpu when cross compiling right now, too)

line 1517 (SECTION A)

  • determine build settings -- based on _host_os

  • most ports sets some LDFLAGS and CXXFLAGS here, some add to DEFINES, LIBS, ASFLAGS

    NOTE: sometimes -D defines are added to DEFINES, sometimes to CXXFLAGS... But cc_check* only uses LDFLAGS + CXXFLAGS, but not DEFINES nor LIBS This means that while for some things it doesn't matter whether you put them into e.g. DEFINES or CXXFLAGS, for others this is very important, specifically when it can affect whether libraries can be correctly detected or not.

    typically added defines:

    • system specifiers like -DMACOSX, -D__DC__
    • enable portdefs.h usage: -DNONSTANDARD_PORT
  • enable/disable features (such as _unix, _seq_midi,, type_4_byte, ...)

  • TODO: Distinguish between flags that are

    • specific to the compile environment (such as include paths) vs.
    • specific to the host os (such as -DMACOSX) or modifying behavior (such as -DREDUCE_MEMORY_USAGE)

line 1706 (SECTION B)

  • cross compiler settings, based on $_host (so only performed if --host=HOST was used)

  • TODO / WEIRD: Many systems occur here again that already occured in section A, including many that are likely to only ever be cross compiled. E.g. android, dreamcast, gamecube, n64, ps2, psp, webos.

    This is one of the points where I think we need to think about what should be in each of section A, B and C.

  • TODO: As in section A, some ports set _unix, _seq_midi -> this overlap should be investigated and either removed, or explained

  • some settings made here are indeed clearly specific to cross compilers such as encoding a fixed endianess (however, is that really necessary? shouldn't our automatic test have picked that up already anyway?), and further custom values (_need_memalign, _arm_asm, ...)

  • more importantly, often _backend and _port_mk are set here

  • finally, some ports sets DEFINES like -DGPH_DEVICE here -> once more overlap with section A (and C)

line 2066

if NOT cross compiling, compute _need_memalign

line 2117

enable 16bit gfx support based on _backend

line 2134

  • update config.h based on values of _endian, _have_x86, _need_memalign
  • update config.mk based on values of _unix, _verbose_build

line 2167

setup stuff for plugins / dynamic modules based on _host_os

line 2364

more config.h/config.mk updates, based on _mt32emu, _16bit, _build_scalers, _build_hq_scalers, _arm_asm

line 2393

  • en-/disable indeo3 decoder depending on whether gob is being built or not
  • TODO: unify this with other code that only specific engines use (e.g. various other audio, graphics and video decoders). Either all of these features should be controlled by configure, or none. In the latter case, we could still use #ifdef's for the same purpose, with all associated pros and cons.

line 2406

  • check for libs: math, vorbis, tremor, flac, mad, alsa, png, theora, seq, timidity, zlib, libmpeg2, libfluidsynth, readline
  • TODO: Readline is only tested for if _text_console = yes; now the default for _text_console is no, can be enabled by param but n64 forces it off (why?)
  • NOTE: All these checks usually crucially depend on CXXFLAGS and LDFLAGS being set right (esp. for cross compilation). So having this after sections A and B (and C??) is indeed the right place

line 2744

  • add "#define USE_TEXT_CONSOLE" to config.h if _text_console = yes
  • FIXME: the possible confusion with the similarly named but completely unrelated DISABLE_TEXT_CONSOLE should be resolved by renaming either / both.
  • also, why is this check in this particular spot? it also lacks a comment, so is easily missed (I assume it is here, right after the readline check, for historical reasons, but nowadays I would recommend adding a comment, and moving this together with other stuff that updates config.h / config.mk (other stuff different from library checks, that is)

line 2746

opengl checks

line 2830

nasm check (but only if _have_x86 = yes)

line 2891

Enable vkeybd / keymapper, handle translation support, figure out installation directories, set variables for profiling,

line 2953

print some detection results

line 2997 (SECTION C)

  • backend specific stuff, based on _backend

  • NOTE: This is after libraries have been detected; this has the drawback that any CXXFLAGS and LDFLAGS crucial for library detection must be set before this point, i.e. likely in sections A and B

  • considering this, it is surprising how many CXXFLAGS and LDFLAGS are only set here.

  • In addition, tons of LIBS, DEFINES, INCLUDES are set

  • TODO: All (?) SDL based ports run find_sdlconfig here -- except for the WebOS and WinCE ports -- why? shouldn't they also use find_sdlconfig?

  • most SDL based backends add -DSDL_BACKEND to DEFINES, except for OpenPandora. Why?

  • TODO: could this section be moved much more upward, maybe more or less right after section B? That would simplify cleaning up the three sections; right now, many *FLAGS cannot be set in section C, even if they logically fit here (in the "backend specific" section) best.

Such a move could result in some unforseen complications, though, and hence needs to be tested carefully.

line 3143

  • Comment say: "Do CXXFLAGS now that we know the compiler version"
  • we actually checked the compiler versions about 2000 lines before this one ;) so "now that we know ..." seems a bit inappropriate :) Indeed, it seems that here we decide upon compiler specific CXXFLAGS here, for en-/disabling warnings, -ansi, -pedantic, ... so a better comment for ths section seems in order.
  • TODO: we checked for -Wglobal-constructors support roughly 2000 lines ago, now we adjust for other -W flags here -> unify this? Indeed, maybe all the compiler specific code should be together up there -- or is there a reason for the split? Maybe the extra -W flags cause issues with the library etc. detection? In that case, we could add a new WARNINGS env var, similar to DEFINES etc.

line 3184

engine selection

line 3265

create config.h / config.mk

line 3347

create Makefile in case we are building out-of-tree

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment