Move most "drivers" as toggleable "modules" and split their thirdparty libraries in an own tree #6830

akien-mga · 2016-10-14T20:34:30Z

Should address most of #6157.

While working on this, I got a bit more scons experience and could isolate most modules (and some of the remaining drivers) in their own environment, which means that their necessary defines and include paths can now be passed without polluting the global env. It means we could drop some dirty hacks we put in several thirdparty sources to hardcode include paths so that they match our repo, etc. There is also no more risk of confusion between headers of different projects that share the same name.

Most libraries that are typically available on a Linux distribution can be unbundled (i.e. you can just delete their thirdparty/$libname folder and build with libname=system). Some more work will be needed to put some order in the scons options as it's a bit messy now, but I'll do all this in a later PR. Currently only platform/x11 has instructions to link against system libraries when system options are chosen, but it could be extended for other relevant platforms.

To disable a module, the syntax is now automatic: module_$module_enable=yes|no (see scons --help). We might have to document this a bit better in the official compilation docs, and also rename the current arguments I used to specify the use of builtin or system libraries for more clarity (e.g. openssl=system could become system_openssl=yes or builtin_openssl=no.

All in all, this was quite a lot of work and I might have made a few mistakes along the road. Therefore I'd appreciate:

extensive testing on all supported platforms and whenever possible also with some exotic options (e.g. disabling an important module and check that the rest of the engine still builds and runs ok).
code review especially of the various SCsub files.

That should be a first step towards making it much easier to maintain our thirdparty code, and also to get Godot packaged in Linux distributions.

akien-mga · 2016-10-15T08:17:14Z

Alright, as expected it doesn't build out of the box :) Should have tried to remove some system headers to ensure built-in libraries are ok. It looks like having made the ogg module build in its own env hid it for theora/vorbis/opus which need it, will fix.

Uses the new structure agreed upon in godotengine#6157, but the thirdparty/ folder does not behave following a logic similar to that of modules/ yet. The png driver can't be moved to a module as discussed in godotengine#6157, as it's required by core together with a few other ImageLoader implementations (see drivers/register_driver_types.cpp:register_core_driver_types()) Dropped the possibility to disable PNG support, it's a core component of Godot.

Similar rationale as in previous commit.

Building against shared libraries only implemented for Linux X11 so far. TODO: Document Godot's modifications of upstream enet.

They are not particularly packaged in Linux distros so we do not facilitate unbundling via SCons. There could be done if/when there is interest. Also s/pnm/pbm/, long-lived typo :)

Note that there are two Godot-specific changes made to libwebp for the javascript/HTML5 platform. They are documented in the README.md.

Took the opportunity to undo the Godot changed made to the opus source. The opus module should eventually be built in its own environment to avoid polluting others with too many include dirs and defines. TODO: Fix the platform/ stuff for opus.

Same rationale as the previous commits.

This allows to pass include paths and flags only to a given thirdparty library, thus preventing conflicts between their files (e.g. between opus and openssl which both provide modes.h. This also has the nice effect of making the compilation command smaller for each module as it no longer related to all other modules, only the final linking brings them together. This however requires adding manually the ogg include path in opus and vorbis when building against the builtin ogg, since it is no longer in the global env. Also simplified template 'thirdparty_<module>_sources' to 'thirdparty_sources'. "Core" modules like cscript, gdscript, gridmap, ik and virtual_script still use the main env_modules, but it could be changed if need be.

Same rationale as the previous commits.

akien-mga · 2016-10-15T09:54:40Z

Rebased to fix a few things:

png: "png" option renamed to "libpng", can't be disabled anymore (only "system" or "builtin", no "no", as it makes little sense to use Godot without png support)
ogg/vorbis/opus/theora: fixed including ogg headers for vorbis/opus/theora and vorbis headers for theora when those are builtin
theora: fixed thirdparty/libtheora where headers were placed in the wrong folder which did not match system wide includes (also added missing description of the thirdparty folder in thirdparty/README.md)
rtaudio: undid changes to isolate the env as platform/windows includes this driver and therefore needs the include path to the thirdparty header
squish: tried a new trick to disable the module in config.py when not building tools (it seems to failed building on iphone due to a half-initialized module, let's see how it goes)

Sources are untouched, tarball from https://sourceforge.net/projects/libsquish

Not fully happy about the way this one interacts with the various platforms. Maybe the platform_config.h should be generated by the SCsub instead of passing a define just to know where is the header.

akien-mga · 2016-10-15T14:29:07Z

After a few hours of trial and error, it seems like it now builds fine for all platforms on our travis tests. I'd love to have people testing the PR on Windows though, both using MSVC and MinGW.

Comment out the weird workaround for building on Windows at it might not be needed anymore. Testing needed to confirm.

The reordering of the SConscript includes allows to ensure that stuff like the builtin zlib headers will be available for libpng. Also moved glew back into global env, otherwise windows seems not to find it... Kind of shooting in the dark with this multi-env setup.

akien-mga · 2016-10-15T16:12:01Z

Fixed a wrong include path for winrt. According to @vnen it now builds fine with MSVC for both windows and winrt.

akien-mga · 2016-10-15T19:54:32Z

I'm testing a build of all templates using this PR: https://travis-ci.org/GodotBuilder/godot-builds/builds/167937894 https://ci.appveyor.com/project/GodotBuilder/godot-builds/build/1.0.90
And even for winrt: https://ci.appveyor.com/project/GodotBuilder/godot-builds/build/1.0.98

In parallel, I'm also building the current master branch for comparison: https://travis-ci.org/GodotBuilder/godot-builds/builds/167937619 https://ci.appveyor.com/project/GodotBuilder/godot-builds/build/1.0.89

akien-mga · 2016-10-16T07:57:23Z

It built properly on all platforms (apart from javascript which is timing out, but same for current master, and OSX which has timed out while master managed to build in timeout - 2 min :D).

I'd propose to merge now that we know that it doesn't introduce big buildsystem regressions, and then fix issues as they come (as there should probably be a couple ones along the way).

akien-mga · 2016-10-16T08:42:28Z

(Actually I finally got the javascript build to fail, but it looks like emscripten is going crazy: https://travis-ci.org/GodotBuilder/godot-builds/jobs/167937903)

Edit: Same issues for the master branch, it's just random. Will have to investigate more for official binaries, but it shouldn't stop this PR.

zaps166 · 2016-10-16T13:06:40Z

@akien-mga

armv7_opt_gcc is not used, other *_opt_gcc are only for libtheora, so maybe rename it *_theora_opt_gcc (and the same for *_opt_vc)?
isn't better to add SCsub files for every thirdparty library (or only for shared thirdparty libs like libogg and libvorbis) for static build (if not system is specified)? E.g. both theroa and vorbis modules use libogg and libvorbis. Currently compiling theora module without vorbis module (module_vorbis_enabled=no) is not possible.
Is ogg module necessary? theora and vorbis modules can notify SCons that they want to use libogg thirdparty or system library/ Currently ogg module exists only for building libogg builtin dependency.

akien-mga · 2016-10-16T13:46:45Z

@zaps166 Yes to both. For the former I have no clue about it, but please make a PR :). For the latter, I also thought about it and had no strong opinion for either way, but yes I think it would likely be best to put the SCsubs in thirdparty and have the modules that need them include them. Not sure yet how it would best be done, so I went for what I knew was working for a first step :)

zaps166 · 2016-10-16T14:16:00Z

@akien-mga Ok, so maybe I'll try later :)

akien-mga added enhancement topic:buildsystem topic:thirdparty needs testing labels Oct 14, 2016

akien-mga added this to the 3.0 milestone Oct 14, 2016

akien-mga force-pushed the thirdparty branch from a497ac0 to 8098d96 Compare October 15, 2016 09:50

akien-mga added 10 commits October 15, 2016 11:50

jpg: Make it a module and split jpgd thirdparty files

16ba665

Similar rationale as in previous commit.

enet: Split enet thirdparty files and allow unbundling

c31ad71

Building against shared libraries only implemented for Linux X11 so far. TODO: Document Godot's modifications of upstream enet.

dds/etc1/pbm/pvr: Make those modules and split thirdparty files

b1e8889

They are not particularly packaged in Linux distros so we do not facilitate unbundling via SCons. There could be done if/when there is interest. Also s/pnm/pbm/, long-lived typo :)

webp: Make it a module and unbundle libwebp thirdparty files

ee3cf21

Note that there are two Godot-specific changes made to libwebp for the javascript/HTML5 platform. They are documented in the README.md.

openssl: Move to a module and split thirdparty lib

4221967

Same rationale as the previous commits.

theora: Move to a module and split thirdparty lib

cfcc8a2

Same rationale as the previous commits.

mpc: Move to a module and split thirdparty libmpcdec

5c12c9e

akien-mga force-pushed the thirdparty branch from 8098d96 to 20f9852 Compare October 15, 2016 09:50

akien-mga force-pushed the thirdparty branch from 20f9852 to 808155f Compare October 15, 2016 10:01

akien-mga added 4 commits October 15, 2016 12:01

rtaudio: Split thirdparty files

8981ff8

squish: Move to a module and split thirdparty lib

8311a78

squish: Update to upstream 1.14

249836e

Sources are untouched, tarball from https://sourceforge.net/projects/libsquish

glew: Split thirdparty files and isolate env

36738dd

Not fully happy about the way this one interacts with the various platforms. Maybe the platform_config.h should be generated by the SCsub instead of passing a define just to know where is the header.

akien-mga force-pushed the thirdparty branch from 808155f to 3e9940d Compare October 15, 2016 10:01

akien-mga added 2 commits October 15, 2016 12:20

zlib: Split thirdparty files, simplify scons option

cbf5260

chibi: Move to a module

e6dc51a

akien-mga force-pushed the thirdparty branch 3 times, most recently from c812d06 to 0fcb765 Compare October 15, 2016 12:06

akien-mga force-pushed the thirdparty branch 3 times, most recently from 5239d6b to 91448b6 Compare October 15, 2016 14:06

akien-mga added 3 commits October 15, 2016 18:10

freetype: Make it a module and split thirdparty library

edbc0c0

Comment out the weird workaround for building on Windows at it might not be needed anymore. Testing needed to confirm.

png: Try to fix neon issue on iphone armv7

e57042e

akien-mga force-pushed the thirdparty branch from 91448b6 to e57042e Compare October 15, 2016 16:10

akien-mga merged commit 8913222 into godotengine:master Oct 16, 2016

akien-mga deleted the thirdparty branch October 16, 2016 09:52

akien-mga mentioned this pull request Oct 16, 2016

Split third party libraries in their own folder, with unmodified upstream versions #6157

Closed

zaps166 mentioned this pull request Oct 16, 2016

Theora: Don't compile unnecessary files, rename "x86_opt_*" #6848

Merged

This was referenced Sep 23, 2017

Why Godot isn't in Debian repository #8983

Closed

Bullet physics engine implementation #10013

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move most "drivers" as toggleable "modules" and split their thirdparty libraries in an own tree #6830

Move most "drivers" as toggleable "modules" and split their thirdparty libraries in an own tree #6830

akien-mga commented Oct 14, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016

akien-mga commented Oct 15, 2016

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 16, 2016

akien-mga commented Oct 16, 2016 •

edited

Loading

zaps166 commented Oct 16, 2016

akien-mga commented Oct 16, 2016

zaps166 commented Oct 16, 2016

Move most "drivers" as toggleable "modules" and split their thirdparty libraries in an own tree #6830

Move most "drivers" as toggleable "modules" and split their thirdparty libraries in an own tree #6830

Conversation

akien-mga commented Oct 14, 2016 • edited Loading

akien-mga commented Oct 15, 2016 • edited Loading

akien-mga commented Oct 15, 2016 • edited Loading

akien-mga commented Oct 15, 2016

akien-mga commented Oct 15, 2016

akien-mga commented Oct 15, 2016 • edited Loading

akien-mga commented Oct 16, 2016

akien-mga commented Oct 16, 2016 • edited Loading

zaps166 commented Oct 16, 2016

akien-mga commented Oct 16, 2016

zaps166 commented Oct 16, 2016

akien-mga commented Oct 14, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 15, 2016 •

edited

Loading

akien-mga commented Oct 16, 2016 •

edited

Loading