Godot: Incremental compilation borked on 3.2 branch

[lawnjelly]

Godot version:
3.2 branch commit 1339976
Scons 3.1.2
Linux Mint

Issue description:
Incremental compilation is broken, I have bisected it down to be being caused by the above commit. It works in the commit before.

Touching a cpp or simply rerunning Scons repeatedly, without editing any files, compiles a large number of files.
It doesn't appear to be a full rebuild, it seems to be compiling most or all files in modules and thirdparty directories every time.

Further info - I was changing drivers/gles2/rasterizer_canvas_gles2.cpp, in case which cpp affects whether this happens - however it now appears that touching a file is not necessary.

Steps to reproduce:
Pull the commit mentioned, rebuild, then run Scons again (or touch a cpp first).

[akien-mga]

CC @Xrayez

[Xrayez]

I'm not really sure why this would happen at first thought. Perhaps the list iterating the modules is unordered, but nonetheless it's sorted in:

https://github.com/godotengine/godot/blob/133997654cf77ba8effdde0f4f11fdf65cb32e72/SConstruct#L457

Yet maybe this list can be sorted even earlier:

https://github.com/godotengine/godot/blob/133997654cf77ba8effdde0f4f11fdf65cb32e72/SConstruct#L192-L213

I'll test this locally, but I have to say I've had incremental compilation issues long before 1339976.

[Xrayez]

Can't reproduce on Windows, SCons v3.1.2, Python 3.6.4.

Compiled four times: before the commit, and with commit, while changing drivers/gles2/rasterizer_canvas_gles2.cpp.

Same while compiling on x11 WSL (Ubuntu 18.04.2 LTS), with Python 2.7.

[lawnjelly]

Just to update: We seem to have pinned this down to the version of python.

My PC

Linux Mint 18.3
I have Scons 3.1.2 installed, old version is uninstalled.
Python 2.7.12 is installed and default, and advice is not to upgrade the default because of conflicts with the OS.
Looking in usr/lib
It seems I have python 2.6, 2.7, 3 and 3.5 installed (3.5 was installed in order to compile Godot 4.x).

Python

I had changed the shebang (first line) of scons in usr/local/bin to python3 (this seemed to be the recommended way to get it to override the default and work to compile Godot 4.x).

_However_ for some reason this was causing this issue to exhibit with this PR. We found that if I changed the shebang BACK to just python, the compilation would take place as expected.

So it is possible there is some conflict occurring as a result of having python 2 (default) and 3 on the same system.

This is just straight feedback, I am very much a linux beginner, and not familiar with python or scons. :+1:

[Xrayez]

I've discovered some recent issue regarding incremental build: #25403, might worth checking out for hints.

[lawnjelly]

Following the hints in #25403 I've run with --debug=explain --taskmastertrace=trace.log.

The debug explain shows that it thinks that lots of files have changed when they haven't. This is just a sample, it could well be thinking all files have changed:

Compiling ==> thirdparty/libtheora/cpu.c
scons: rebuilding `thirdparty/libtheora/decinfo.x11.tools.64.llvm.o' because:
           `thirdparty/libtheora/decinfo.c' changed
           `thirdparty/libtheora/decint.h' changed
           `thirdparty/libtheora/bitpack.h' changed
           `thirdparty/libtheora/dequant.h' changed
           `thirdparty/libtheora/huffdec.h' changed
           `thirdparty/libtheora/internal.h' changed
           `thirdparty/libtheora/theora/theoradec.h' changed
           `thirdparty/libtheora/quant.h' changed
           `thirdparty/libtheora/huffman.h' changed
           `thirdparty/libtheora/ocintrin.h' changed
           `thirdparty/libtheora/theora/codec.h' changed
           `thirdparty/libtheora/theora/theora.h' changed
           `thirdparty/libogg/ogg/ogg.h' changed
           `thirdparty/libogg/ogg/os_types.h' changed
           `thirdparty/libogg/ogg/config_types.h' changed
           `core/int_types.h' changed
           `/usr/bin/clang' changed
Compiling ==> thirdparty/libtheora/decinfo.c
scons: rebuilding `thirdparty/libtheora/decode.x11.tools.64.llvm.o' because:
           `thirdparty/libtheora/decode.c' changed
           `thirdparty/libtheora/decint.h' changed
           `thirdparty/libpng/png.h' changed
           `thirdparty/libogg/ogg/ogg.h' changed
           `thirdparty/libtheora/bitpack.h' changed
           `thirdparty/libtheora/dequant.h' changed
           `thirdparty/libtheora/huffdec.h' changed
           `thirdparty/libtheora/internal.h' changed
           `thirdparty/libtheora/theora/theoradec.h' changed
           `thirdparty/libpng/pngconf.h' changed
           `thirdparty/libpng/pnglibconf.h' changed
           `thirdparty/libogg/ogg/os_types.h' changed
           `thirdparty/libtheora/quant.h' changed
           `thirdparty/libtheora/huffman.h' changed
           `thirdparty/libtheora/ocintrin.h' changed
           `thirdparty/libtheora/theora/codec.h' changed
           `thirdparty/libtheora/theora/theora.h' changed
           `thirdparty/libogg/ogg/config_types.h' changed
           `core/int_types.h' changed
           `/usr/bin/clang' changed
Compiling ==> thirdparty/libtheora/decode.c
scons: rebuilding `thirdparty/libtheora/dequant.x11.tools.64.llvm.o' because:
           `thirdparty/libtheora/dequant.c' changed
           `thirdparty/libtheora/decint.h' changed
           `thirdparty/libtheora/dequant.h' changed
           `thirdparty/libogg/ogg/ogg.h' changed
           `thirdparty/libtheora/bitpack.h' changed
           `thirdparty/libtheora/huffdec.h' changed
           `thirdparty/libtheora/internal.h' changed
           `thirdparty/libtheora/theora/theoradec.h' changed
           `thirdparty/libtheora/quant.h' changed
           `thirdparty/libogg/ogg/os_types.h' changed
           `thirdparty/libtheora/huffman.h' changed
           `thirdparty/libtheora/ocintrin.h' changed
           `thirdparty/libtheora/theora/codec.h' changed
           `thirdparty/libtheora/theora/theora.h' changed
           `thirdparty/libogg/ogg/config_types.h' changed
           `core/int_types.h' changed
           `/usr/bin/clang' changed

It even thinks clang has changed every compile...

If it thinks all files have changed, it is interesting that it only seems to be rebuilding modules and thirdparty, and not everything. And that it has been triggered by the PR, but was not occurring before that. Could it be something to do with paths?

If we go with the idea that it is somehow using a combination of python2 and 3, I had a quick look at the differences to see if there might be any differences in paths.
https://www.guru99.com/python-2-vs-python-3.html
One thing I noticed is that Python3 defaults to unicode strings:

Unicode | In Python 3, default storing of strings is Unicode. | Python2 - To store Unicode string value, you require to define them with "u".

This is just an excerpt from the trace.log, it was too big for pastebin (23 megs) but mostly it is similar:
https://pastebin.com/JTb9vWVf

[Xrayez]

Already fixed by #39121 on 3.2, waiting for the same patch to be available for 4.0 with #39125.

But I wanted to say that, somehow even while testing this with Python 2.7, I couldn't really reproduce the issue, but I think that I've got lucky enough for the same list of modules to remain in order every build, preventing full rebuild. Or might certainly be implementation-defined behavior specifically exhibited on (Windows) interpreter, who knows. :eyes:

[akien-mga]

But I wanted to say that, somehow even while testing this with Python 2.7, I couldn't really reproduce the issue, but I think that I've got lucky enough for the same list of modules to remain in order every build, preventing full rebuild.

Well that's because, again, @lawnjelly's issue was with Python 3.5 :) https://github.com/godotengine/godot/pull/39121#issuecomment-635393280
Python 2.7 works fine, it likely had the bug fixed or is using a completely different system for dictionaries which doesn't exhibit this de-ordering issue in the first place.

So you'd need to test with Python 3.5 (possibly also earlier 3.x) to reproduce it.

[bdbaddog]

Is it possible it was someone switching back and forth between py 2.7 and 3.5?

[lawnjelly]

Is it possible it was someone switching back and forth between py 2.7 and 3.5?

Well it _could_ have been something to do with intermediate files, but I don't think that was the theory used to fix it.

Situation in the end was that it worked when the Scons file 1st line shebang just contained python, but didn't work when it contained python3.

I think though the conclusion was that there was some python feature used that was 3.6-3.7 only in the PR, and it wasn't present in 3.5 (rather than a problem with Scons itself). But that is just my understanding, @Xrayez and @akien-mga would be able to tell you more.

There's more info on the IRC logs here:
https://godot.eska.me/irc-logs/devel/2020-05-28.log

I did do various cleans (passing -c in the command line, and manually deleting .scons* files) before running the tests, on instructions from the others. Which would seem to backup the theory of it just being the python version rather than to do with intermediates.

[Xrayez]

1. A list of modules was refactored to a dictionary of modules in 1339976.
2. Developing the feature on Python 3.6 couldn't allow me to detect the limitations of dictionaries not guaranteeing the order of insertion... (and tbh I've never had to extensively develop in older versions of Python).
3. The order determines the contents of modules/modules_enabled.gen.h which is included in some sources and modules/register_modules_types.gen.cpp. Obviously shuffling the order makes SCons think that the files are changed, so they need to be rebuilt.

I wonder whether it could be useful to determine whether those files are modified by writing a custom env.Decider function, like len(modules_contents) == previous_length, but that's probably more hacky than #39121.

[bdbaddog]

@Xrayez - yup that'd be more hacky. Dict insertion order maintenance starts in py 3.6.. so likely that was the root of your issue. Glad you found a solution.