Merge #838

838: Godot version check + workaround for API generation bug r=Bromeon a=Bromeon

As mentioned in #833, Godot versions < 3.3.1 were subject to [a bug](godotengine/godot#48081) that caused non-deterministic crashes during the command:
```
godot --gdnative-generate-json-api api.json
```

For users working with affected Godot versions (e.g. 3.2), this makes the feature flag `custom-godot` annoying to use, since they can't rely on the API generation to succeed -- let alone use it for automation/CI. This PR works around that by retrying the command up to 10 times (magic number). I changed the minimum supported Godot version in our own CI again to 3.2, meaning that if it _doesn't_ work, we have to suffer, too 😬

Additionally, this PR parses the Godot version and emits a meaningful error message if an unsupported version is detected (3.1 or 4.0).

Co-authored-by: Jan Haller <[email protected]>

Author: bors[bot]

.github/workflows/full-ci.yml

@@ -244,10 +244,9 @@ jobs:
             godot: "3.4.1"
             postfix: ' (msrv 1.51)'
 
-          # Test with older engine version
-          # Note: headless versions of Godot <= 3.3 may crash with a bug, see feature description in lib.rs
+          # Test with oldest supported engine version
           - rust: stable
-            godot: "3.3.1"
+            godot: "3.2"
             postfix: ''
             build_args: '--features custom-godot'
 

bindings_generator/Cargo.toml

@@ -12,14 +12,16 @@ edition = "2018"
 
 [features]
 debug = []
+custom-godot = ["which"]
 
 [dependencies]
 heck = "0.4.0"
-roxmltree = "0.14.1"
+memchr = "2.4.1"
+miniserde = "0.1.15"
 proc-macro2 = "1.0.30"
 quote = "1.0.10"
+regex = "1.5.4"
+roxmltree = "0.14.1"
 syn = { version = "1.0.80", features = ["full", "extra-traits", "visit"] }
-miniserde = "0.1.15"
 unindent = "0.1.7"
-regex = "1.5.4"
-memchr = "2.4.1"
+which = { optional = true, version = "4.2.2" }

bindings_generator/src/godot_api_json.rs

@@ -0,0 +1,70 @@
+use crate::godot_version;
+use std::path::PathBuf;
+use std::process::Command;
+
+pub fn generate_json_if_needed() -> bool {
+    let godot_bin: PathBuf = if let Ok(string) = std::env::var("GODOT_BIN") {
+        println!("Found GODOT_BIN with path to executable: '{}'", string);
+        PathBuf::from(string)
+    } else if let Ok(path) = which::which("godot") {
+        println!("Found 'godot' executable in PATH: {}", path.display());
+        path
+    } else {
+        panic!(
+            "Feature 'custom-godot' requires an accessible 'godot' executable or \
+             a GODOT_BIN environment variable (with the path to the executable)."
+        );
+    };
+
+    let version = exec(1, Command::new(&godot_bin).arg("--version"));
+
+    let has_generate_bug = match godot_version::parse_godot_version(&version) {
+        Ok(parsed) => {
+            assert!(
+                parsed.major == 3 && parsed.minor >= 2,
+                "Only Godot versions >= 3.2 and < 4.0 are supported; found version {}.",
+                version.trim()
+            );
+
+            // bug for versions < 3.3.1
+            parsed.major == 2 || parsed.major == 3 && parsed.minor == 0
+        }
+        Err(e) => {
+            // Don't treat this as fatal error
+            eprintln!("Warning, failed to parse version: {}", e);
+            true // version not known, conservatively assume bug
+        }
+    };
+
+    // Workaround for Godot bug, where the generate command crashes the engine.
+    // Try 10 times (should be reasonably high confidence that at least 1 run succeeds).
+    println!("Found Godot version < 3.3.1 with potential generate bug; trying multiple times...");
+
+    exec(
+        if has_generate_bug { 10 } else { 1 },
+        Command::new(&godot_bin)
+            .arg("--gdnative-generate-json-api")
+            .arg("api.json"),
+    );
+
+    true
+}
+
+/// Executes a command and returns stdout. Panics on failure.
+fn exec(attempts: i32, command: &mut Command) -> String {
+    let command_line = format!("{:?}", command);
+
+    for _attempt in 0..attempts {
+        match command.output() {
+            Ok(output) => return String::from_utf8(output.stdout).expect("parse UTF8 string"),
+            Err(err) => {
+                eprintln!(
+                    "Godot command failed:\n  command: {}\n  error: {}",
+                    command_line, err
+                )
+            }
+        }
+    }
+
+    panic!("Could not execute Godot command (see above).")
+}

bindings_generator/src/godot_version.rs

@@ -0,0 +1,69 @@
+//#![allow(unused_variables, dead_code)]
+
+use regex::Regex;
+use std::error::Error;
+
+pub struct GodotVersion {
+    pub major: u8,
+    pub minor: u8,
+    pub patch: u8,         //< 0 if none
+    pub stability: String, // stable|beta|dev
+}
+
+pub fn parse_godot_version(version_str: &str) -> Result<GodotVersion, Box<dyn Error>> {
+    let regex = Regex::new("(\\d+)\\.(\\d+)(?:\\.(\\d+))?\\.(stable|beta|dev)")?;
+
+    let caps = regex.captures(version_str).ok_or("Regex capture failed")?;
+
+    let fail = || {
+        format!(
+            "Version substring could not be matched in '{}'",
+            version_str
+        )
+    };
+
+    Ok(GodotVersion {
+        major: caps.get(1).ok_or_else(fail)?.as_str().parse::<u8>()?,
+        minor: caps.get(2).ok_or_else(fail)?.as_str().parse::<u8>()?,
+        patch: caps
+            .get(3)
+            .map(|m| m.as_str().parse::<u8>())
+            .transpose()?
+            .unwrap_or(0),
+        stability: caps.get(4).ok_or_else(fail)?.as_str().to_string(),
+    })
+}
+
+#[test]
+fn test_godot_versions() {
+    let good_versions = [
+        ("3.0.stable.official", 3, 0, 0, "stable"),
+        ("3.0.1.stable.official", 3, 0, 1, "stable"),
+        ("3.2.stable.official", 3, 2, 0, "stable"),
+        ("3.37.stable.official", 3, 37, 0, "stable"),
+        ("3.4.stable.official.206ba70f4", 3, 4, 0, "stable"),
+        ("3.4.1.stable.official.aa1b95889", 3, 4, 1, "stable"),
+        ("3.5.beta.custom_build.837f2c5f8", 3, 5, 0, "beta"),
+        ("4.0.dev.custom_build.e7e9e663b", 4, 0, 0, "dev"),
+    ];
+
+    let bad_versions = [
+        "4.0.unstable.custom_build.e7e9e663b", // "unstable"
+        "4.0.3.custom_build.e7e9e663b",        // no stability
+        "3.stable.official.206ba70f4",         // no minor
+    ];
+
+    // From Rust 1.56: 'for (...) in good_versions'
+    for (full, major, minor, patch, stability) in good_versions.iter().cloned() {
+        let parsed: GodotVersion = parse_godot_version(full).unwrap();
+        assert_eq!(parsed.major, major);
+        assert_eq!(parsed.minor, minor);
+        assert_eq!(parsed.patch, patch);
+        assert_eq!(parsed.stability, stability);
+    }
+
+    for full in bad_versions.iter() {
+        let parsed = parse_godot_version(full);
+        assert!(parsed.is_err());
+    }
+}

bindings_generator/src/lib.rs

@@ -13,29 +13,42 @@
 //! must be taken to ensure that the version of the generator matches the one specified in
 //! the `Cargo.toml` of the `gdnative` crate exactly, even for updates that are considered
 //! non-breaking in the `gdnative` crate.
-use proc_macro2::TokenStream;
-
-use quote::{format_ident, quote};
 
-pub mod api;
 mod class_docs;
 mod classes;
-pub mod dependency;
 mod documentation;
 mod methods;
 mod special_methods;
 
-pub use crate::api::*;
-pub use crate::class_docs::*;
+#[cfg(feature = "custom-godot")]
+mod godot_api_json;
+mod godot_version;
+
+pub mod api;
+pub mod dependency;
+
 use crate::classes::*;
-pub use crate::dependency::*;
 use crate::documentation::*;
 use crate::methods::*;
 use crate::special_methods::*;
-
+use proc_macro2::TokenStream;
+use quote::{format_ident, quote};
 use std::collections::HashMap;
 use std::io;
 
+pub use api::*;
+pub use class_docs::*;
+pub use dependency::*;
+
+#[cfg(feature = "custom-godot")]
+pub use godot_api_json::*;
+pub use godot_version::*;
+
+#[cfg(not(feature = "custom-godot"))]
+pub fn generate_json_if_needed() -> bool {
+    false
+}
+
 pub type GeneratorResult<T = ()> = Result<T, io::Error>;
 
 pub struct BindingResult<'a> {

gdnative-bindings/Cargo.toml

@@ -13,15 +13,13 @@ edition = "2018"
 [features]
 formatted = []
 one-class-one-file = []
-custom-godot = ["which"]
+custom-godot = ["gdnative_bindings_generator/custom-godot"]
 
 [dependencies]
-gdnative-sys = { path = "../gdnative-sys", version = "0.9.3" }
-gdnative-core = { path = "../gdnative-core", version = "=0.9.3" }
+gdnative-sys = { path = "../gdnative-sys" }
+gdnative-core = { path = "../gdnative-core" }
 libc = "0.2.104"
 bitflags = "1.3.2"
 
 [build-dependencies]
-heck = "0.4.0"
-gdnative_bindings_generator = { path = "../bindings_generator", version = "=0.9.3" }
-which = { optional = true, version = "4.2.2" }
+gdnative_bindings_generator = { path = "../bindings_generator" }