Feature custom-godot: unchanged code is recompiled

[Bromeon]

When using the feature custom-godot and compiling itest, it looks like the crates godot-ffi and godot-core are always recompiled, even if none of the code has changed.

Commenting out both the following cargo:rerun-if-changed stops this from happening:

• the gdextension_interface.h header
• the function including any generated file.
  instead of disabling this, it's enough to only disable 2 of its callers:
  • the JSON file
  • the header file

Only turning either of the two main bullet points (or only one of the sub-points) does not do the job. So this could be a start to look for.

It also looks like gdextension_interface.h is added twice. This could be addressed as well; just make sure it still works without custom-godot.

Maybe we can do what the library did originally: store the Godot version in a file and only run codegen when that changes. I also think auto-detecting Godot version changes isn't important, it's OK to expect a user to use cargo clean or similar. However, what's not OK is re-compiling entire unchanged crates for several seconds during incremental compilation.

[lilizoey]

Uncommenting both the following cargo:rerun-if-changed stops this from happening:

Do you mean commenting? because they are already currently uncommented.

[Bromeon]

I meant "commenting-out". There should be an easier word in English 😬

[lilizoey]

i made a thing that seems to fix it which is basically just another crate that will update a version file if the godot version changes, and ffi and core just do rerun if changed on that. idk if that's what we actually want to do though.

[Bromeon]

Is that more or less what we had before (the commented-out code here)?

gdext/godot-bindings/src/godot_exe.rs

Lines 33 to 38 in 8990464

	// if !json_path.exists() || has_version_changed(&version) {
	dump_extension_api(&godot_bin, json_path);
	// update_version_file(&version);
	watch.record("dump_api_json");
	// }

The problem is that the code to read that file itself isn't run unless someone triggers it (so it would need to run godot --version every time). And we should ideally not require a Godot binary to be present just to compile gdext; that's somewhat the idea of the prebuilt artifacts.

[lilizoey]

No it's in a second crate, so we have

godot-bindings
godot-version 
godot-core 
godot-ffi

and in godot-version's build script we check the version and if it's mismatched we update it. since we don't emit a rebuild-if-changed, then it will always run every build no matter what. but it'll only change the file if the version is different. so godot-core and godot-ffi can just rebuild if that file is changed. and that file will be changed whenever the version is different since the build-script runs every build. but since nothing is different if the version is the same, then core and ffi wont be rebuilt.

this does mean we need to run the build-script every build. but it's a very cheap build script to run as it only checks the contents of the file against the output of godot --version. possibly overriding a file.

[TCROC]

My PR over here should fix this issue as well: #469

[Bromeon]

Just to clarify, as already stated here and here, this issue is NOT fixed.

The detection of Godot changes in presence of custom-godot is not robust. I switched branches a few times and it didn't pick them up. We basically traded one issue (needless recompilation) for another (no change detection).