Use target/godot-gen for generated files; streamline godot-bindings API

Author: Bromeon

.github/composite/godot-itest/action.yml

@@ -60,42 +60,43 @@ runs:
         echo "GODOT_BUILT_FROM=_Built from [\`$godotVer\`](https://github.com/godotengine/godot/commit/$gitSha)._" >> $GITHUB_ENV
       shell: bash
 
-    # Note: if this fails, run `git diff -R > tweaks.patch` after updating the file manually
+    - name: "Install Rust"
+      uses: ./.github/composite/rust
+      with:
+        rust: ${{ inputs.rust-toolchain }}
+        with-llvm: ${{ inputs.with-llvm }}
+
+    - name: "Build gdext (itest)"
+      run: |
+        cargo build -p itest ${{ inputs.rust-extra-args }}
+      shell: bash
+      env:
+        RUSTFLAGS: ${{ inputs.rust-env-rustflags }}
+
+    # Note: no longer fails, as we expect header to be forward-compatible; instead issues a warning
     - name: "Copy and compare GDExtension header"
       if: inputs.artifact-name == 'godot-linux'
+      working-directory: target/godot-gen
       run: |
-        mkdir -p godot-codegen/input
-        cp $RUNNER_DIR/godot_bin/gdextension_interface.h godot-codegen/input/gdextension_interface.h
-        git apply godot-codegen/input/tweak.patch -v
-        git diff --exit-code --quiet || {
+        mv $RUNNER_DIR/godot_bin/gdextension_interface.h gdextension_interface.h.bak
+        git apply ../../godot-bindings/res/tweaks.patch
+        git diff --no-index --exit-code --quiet gdextension_interface.h gdextension_interface.h.bak || {
             echo "OUTCOME=header-diff" >> $GITHUB_ENV
-            echo "gdextension_interface.h is not up-to-date; abort."
+            echo "::warning::gdextension_interface.h is not up-to-date."
             echo ""
-            
-            echo "### :x: Outdated GDExtension API header" >> $GITHUB_STEP_SUMMARY
+        
+            echo "### :warning: Outdated GDExtension API header" >> $GITHUB_STEP_SUMMARY
             echo "gdextension_interface.h contains the following differences:" >> $GITHUB_STEP_SUMMARY
             echo "\`\`\`diff" >> $GITHUB_STEP_SUMMARY
-            git diff >> $GITHUB_STEP_SUMMARY
+            git diff --no-index gdextension_interface.h gdextension_interface.h.bak >> $GITHUB_STEP_SUMMARY
             echo "\`\`\`" >> $GITHUB_STEP_SUMMARY
             echo "After manually updating file, run:   \`git diff -R > tweak.patch\`." >> $GITHUB_STEP_SUMMARY
 
-        	exit 1
+            mv gdextension_interface.h.bak gdextension_interface.h
+        	#exit 1
         }
       shell: bash
 
-    - name: "Install Rust"
-      uses: ./.github/composite/rust
-      with:
-        rust: ${{ inputs.rust-toolchain }}
-        with-llvm: ${{ inputs.with-llvm }}
-
-    - name: "Build godot-rust"
-      run: |
-        cargo build -p itest ${{ inputs.rust-extra-args }}
-      shell: bash
-      env:
-        RUSTFLAGS: ${{ inputs.rust-env-rustflags }}
-
     - name: "Run Godot integration tests"
       # Aborts immediately if Godot outputs certain keywords (would otherwise stall until CI runner times out).
       # Explanation:

.github/workflows/full-ci.yml

@@ -205,6 +205,14 @@ jobs:
             godot-binary: godot.linuxbsd.editor.dev.double.x86_64
             rust-extra-args: --features double-precision
 
+          - name: linux-bindgen
+            artifact-name: linux
+            os: ubuntu-20.04
+            rust-toolchain: stable
+            godot-binary: godot.linuxbsd.editor.dev.x86_64
+            rust-extra-args: --features custom-godot
+
+
           # Special Godot binaries compiled with AddressSanitizer/LeakSanitizer to detect UB/leaks.
           # Additionally, the Godot source is patched to make dlclose() a no-op, as unloading dynamic libraries loses stacktrace and
           # cause false positives like println!. See https://github.com/google/sanitizers/issues/89.
@@ -231,7 +239,7 @@ jobs:
       - name: "Run Godot integration test"
         uses: ./.github/composite/godot-itest
         with:
-          artifact-name: godot-${{ matrix.name }}
+          artifact-name: godot-${{ matrix.artifact-name || matrix.name }}
           godot-binary: ${{ matrix.godot-binary }}
           godot-args: ${{ matrix.godot-args }}
           rust-extra-args: ${{ matrix.rust-extra-args }}

godot-bindings/Cargo.toml

@@ -8,10 +8,13 @@ keywords = ["gamedev", "godot", "engine", "ffi", "sys"]
 categories = ["game-engines", "graphics"]
 
 [features]
+default = ["prebuilt-godot"]
+prebuilt-godot = ["dep:godot4-prebuilt"]
 custom-godot = ["dep:bindgen", "dep:regex", "dep:which"]
+custom-godot-extheader = []
 
 [dependencies]
-godot4-artifacts = { git = "https://github.com/godot-rust/godot4-artifacts", branch = "4.0" }
+godot4-prebuilt = { optional = true, git = "https://github.com/godot-rust/godot4-artifacts", branch = "4.0" }
 
 # Version >= 1.5.5 for security: https://blog.rust-lang.org/2022/03/08/cve-2022-24713.html
 # 'unicode-gencat' needed for \d, see: https://docs.rs/regex/1.5.5/regex/#unicode-features

godot-bindings/res/tweak.patch

@@ -1,7 +1,7 @@
-diff --git b/godot-bindings/gen/gdextension_interface.h a/godot-bindings/gen/gdextension_interface.h
+diff --git b/target/godot-gen/gdextension_interface.h a/target/godot-gen/gdextension_interface.h
 index 0b7615f..6db266e 100644
---- b/godot-bindings/gen/gdextension_interface.h
-+++ a/godot-bindings/gen/gdextension_interface.h
+--- b/target/godot-gen/gdextension_interface.h
++++ a/target/godot-gen/gdextension_interface.h
 @@ -139,22 +139,22 @@ typedef enum {
 
  } GDExtensionVariantOperator;

godot-bindings/src/godot_exe.rs

@@ -4,17 +4,28 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
+//! Commands related to Godot executable
+
 use crate::godot_version::parse_godot_version;
 use crate::header_gen::generate_rust_binding;
 use crate::watch::StopWatch;
 use std::path::{Path, PathBuf};
-use std::process::Command;
-
-/// Commands related to Godot executable
-
-const GODOT_VERSION_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/gen/godot_version.txt");
-const JSON_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/gen/extension_api.json");
-const HEADER_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/gen/gdextension_interface.h");
+use std::process::{Command, Output};
+
+// Note: CARGO_BUILD_TARGET_DIR and CARGO_TARGET_DIR are not set.
+// OUT_DIR would be standing to reason, but it's an unspecified path that cannot be referenced by CI.
+const GODOT_VERSION_PATH: &str = concat!(
+    env!("CARGO_MANIFEST_DIR"),
+    "/../target/godot-gen/godot_version.txt"
+);
+const JSON_PATH: &str = concat!(
+    env!("CARGO_MANIFEST_DIR"),
+    "/../target/godot-gen/extension_api.json"
+);
+const HEADER_PATH: &str = concat!(
+    env!("CARGO_MANIFEST_DIR"),
+    "/../target/godot-gen/gdextension_interface.h"
+);
 const RES_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/res");
 
 pub fn load_gdextension_json(watch: &mut StopWatch) -> String {
@@ -31,7 +42,7 @@ pub fn load_gdextension_json(watch: &mut StopWatch) -> String {
     dump_extension_api(&godot_bin, json_path);
     update_version_file(&version);
 
-    watch.record("dump_gdextension_json");
+    watch.record("dump_json");
     // }
 
     let result = std::fs::read_to_string(json_path)
@@ -41,34 +52,40 @@ pub fn load_gdextension_json(watch: &mut StopWatch) -> String {
     result
 }
 
-pub fn load_gdextension_header_rs(rust_out_path: &Path, watch: &mut StopWatch) -> String {
-    let c_header_path = Path::new(HEADER_PATH);
-    let resource_path = Path::new(RES_PATH);
-    rerun_on_changed(c_header_path);
-
-    let godot_bin = locate_godot_binary();
-    rerun_on_changed(&godot_bin);
-    watch.record("locate_godot");
+pub fn load_gdextension_header_rs(
+    c_in_path: Option<&Path>,
+    rust_out_path: &Path,
+    watch: &mut StopWatch,
+) {
+    let c_header_path;
 
-    // Regenerate API JSON if first time or Godot version is different
-    let version = read_godot_version(&godot_bin);
-    // if !c_header_path.exists() || has_version_changed(&version) {
-    dump_header_file(&godot_bin, c_header_path);
-    update_version_file(&version);
+    if let Some(c_in_path) = c_in_path {
+        // External C header file provided; we don't invoke Godot
+        c_header_path = c_in_path;
+    } else {
+        // No external C header file: Godot binary is present, we use it to dump C header
+        let godot_bin = locate_godot_binary();
+        rerun_on_changed(&godot_bin);
+        watch.record("locate_godot");
+
+        // Regenerate API JSON if first time or Godot version is different
+        let version = read_godot_version(&godot_bin);
+        c_header_path = Path::new(HEADER_PATH);
+
+        // if !c_header_path.exists() || has_version_changed(&version) {
+        dump_header_file(&godot_bin, c_header_path);
+        update_version_file(&version);
+        // }
+    };
+    rerun_on_changed(c_header_path);
 
-    watch.record("dump_gdextension_header");
-    // }
+    watch.record("dump_header_c");
 
-    patch_c_header(&resource_path.join("tweak.patch"));
+    let tweak_path = Path::new(RES_PATH).join("tweak.patch");
+    patch_c_header(c_header_path, &tweak_path);
     generate_rust_binding(c_header_path, rust_out_path);
 
-    watch.record("read_header_file");
-    std::fs::read_to_string(rust_out_path).unwrap_or_else(|_| {
-        panic!(
-            "failed to read generated Rust file {}",
-            rust_out_path.display()
-        )
-    })
+    watch.record("generate_header_rs");
 }
 
 #[allow(dead_code)]
@@ -151,21 +168,30 @@ fn dump_header_file(godot_bin: &Path, out_file: &Path) {
     println!("Generated {}/extension_api.json.", cwd.display());
 }
 
-fn patch_c_header(tweak_path: &Path) {
+fn patch_c_header(c_header_path: &Path, tweak_path: &Path) {
+    // The C header path *must* be passed in by the invoking crate, as the path cannot be relative to this crate.
+    // Otherwise, it can be something like `/home/runner/.cargo/git/checkouts/gdext-76630c89719e160c/efd3b94/godot-bindings`.
+    let cwd = c_header_path.parent().unwrap().parent().unwrap();
+    println!("cwd: {}", cwd.display());
+
     // Note: patch must have paths relative to Git root (aka top-level dir), so cwd is root
-    let cwd = tweak_path.parent().unwrap().parent().unwrap();
     rerun_on_changed(tweak_path);
 
     let git = locate_git_binary();
     let mut cmd = Command::new(&git);
     cmd.current_dir(cwd).arg("apply").arg("-v").arg(tweak_path);
 
-    execute(cmd, "apply Git patch");
+    let output = execute(cmd, "apply Git patch");
+    let stderr = String::from_utf8(output.stderr).expect("convert Git patch output to UTF-8");
+
+    // `git patch` returns 0 even if it skips a patch because it's not applicable -- treat this as error
+    assert!(!stderr.contains("Skipped"), "Git patch was skipped");
 }
 
 fn locate_godot_binary() -> PathBuf {
     if let Ok(string) = std::env::var("GODOT4_BIN") {
         println!("Found GODOT4_BIN with path to executable: '{string}'");
+        println!("cargo:rerun-if-env-changed=GODOT4_BIN");
         PathBuf::from(string)
     } else if let Ok(path) = which::which("godot4") {
         println!("Found 'godot4' executable in PATH: {}", path.display());
@@ -193,12 +219,23 @@ fn locate_git_binary() -> PathBuf {
     }
 }
 
-fn execute(mut cmd: Command, error_message: &str) {
+fn execute(mut cmd: Command, error_message: &str) -> Output {
     let output = cmd
         .output()
         .unwrap_or_else(|_| panic!("failed to execute command: {error_message}"));
 
-    if !output.status.success() {
+    if output.status.success() {
+        println!(
+            "[stdout] {}",
+            String::from_utf8(output.stdout.clone()).unwrap()
+        );
+        println!(
+            "[stderr] {}",
+            String::from_utf8(output.stderr.clone()).unwrap()
+        );
+        println!("[status] {}", output.status);
+        output
+    } else {
         println!("[stdout] {}", String::from_utf8(output.stdout).unwrap());
         println!("[stderr] {}", String::from_utf8(output.stderr).unwrap());
         println!("[status] {}", output.status);
@@ -208,5 +245,4 @@ fn execute(mut cmd: Command, error_message: &str) {
 
 fn rerun_on_changed(path: &Path) {
     println!("cargo:rerun-if-changed={}", path.display());
-    println!("cargo:rerun-if-env-changed=GODOT4_BIN");
 }

godot-bindings/src/header_gen.rs

@@ -7,8 +7,8 @@
 use std::env;
 use std::path::Path;
 
-pub(crate) fn generate_rust_binding(c_header_path: &Path, out_rust_path: &Path) {
-    let c_header_path = c_header_path.display().to_string();
+pub(crate) fn generate_rust_binding(in_c_path: &Path, out_rust_path: &Path) {
+    let c_header_path = in_c_path.display().to_string();
     println!("cargo:rerun-if-changed={}", c_header_path);
 
     let builder = bindgen::Builder::default()
@@ -27,12 +27,24 @@ pub(crate) fn generate_rust_binding(c_header_path: &Path, out_rust_path: &Path)
 
     let bindings = configure_platform_specific(builder)
         .generate()
-        .expect("failed generate gdextension_interface.h bindings");
+        .unwrap_or_else(|err| {
+            panic!(
+                "bindgen generate failed\n    c: {}\n   rs: {}\n  err: {}\n",
+                in_c_path.display(),
+                out_rust_path.display(),
+                err
+            )
+        });
 
     // Write the bindings to the $OUT_DIR/bindings.rs file.
-    bindings
-        .write_to_file(out_rust_path)
-        .expect("failed write gdextension_interface.h bindings to file");
+    bindings.write_to_file(out_rust_path).unwrap_or_else(|err| {
+        panic!(
+            "bindgen write failed\n    c: {}\n   rs: {}\n  err: {}\n",
+            in_c_path.display(),
+            out_rust_path.display(),
+            err
+        )
+    });
 }
 
 //#[cfg(target_os = "macos")]