diff --git a/.github/composite/godot-itest/action.yml b/.github/composite/godot-itest/action.yml index 4bc66841f..0e999fdda 100644 --- a/.github/composite/godot-itest/action.yml +++ b/.github/composite/godot-itest/action.yml @@ -19,6 +19,11 @@ inputs: default: '' description: "Command-line arguments passed to Godot" + godot-check-header: + required: false + default: 'false' + description: "Should the job check against latest gdextension_interface.h, and warn on difference" + rust-toolchain: required: false default: 'stable' @@ -75,12 +80,13 @@ runs: # 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: godot-ffi/src/gen + if: inputs.godot-check-header == 'true' run: | - 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 || { + mv godot-ffi/src/gen/gdextension_interface.h godot-ffi/src/gen/gdextension_interface_prebuilt.h + mv $RUNNER_DIR/godot_bin/gdextension_interface.h godot-ffi/src/gen/gdextension_interface.h + git apply godot-bindings/res/tweak.patch + cd godot-ffi/src/gen + git diff --no-index --exit-code --quiet gdextension_interface_prebuilt.h gdextension_interface.h || { echo "OUTCOME=header-diff" >> $GITHUB_ENV echo "::warning::gdextension_interface.h is not up-to-date." echo "" @@ -88,11 +94,12 @@ runs: 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 --no-index gdextension_interface.h gdextension_interface.h.bak >> $GITHUB_STEP_SUMMARY + git diff --no-index gdextension_interface_prebuilt.h gdextension_interface.h >> $GITHUB_STEP_SUMMARY || true echo "\`\`\`" >> $GITHUB_STEP_SUMMARY echo "After manually updating file, run: \`git diff -R > tweak.patch\`." >> $GITHUB_STEP_SUMMARY - mv gdextension_interface.h.bak gdextension_interface.h + # Undo modifications + mv gdextension_interface_prebuilt.h gdextension_interface.h #exit 1 } shell: bash @@ -121,19 +128,20 @@ runs: run: | if grep -q "ObjectDB instances leaked at exit" "${{ runner.temp }}/log.txt"; then echo "OUTCOME=godot-leak" >> $GITHUB_ENV - exit 2 + exit 3 fi shell: bash - name: "Conclusion" if: always() run: | - echo "Evaluate conclusion ($OUTCOME)" + echo "Evaluate conclusion: $OUTCOME" case $OUTCOME in "success") - echo "### :heavy_check_mark: Godot integration tests passed" > $GITHUB_STEP_SUMMARY - echo "$GODOT_BUILT_FROM" >> $GITHUB_STEP_SUMMARY + # Do not output success for now, to keep summary focused on warnings/errors + #echo "### :heavy_check_mark: Godot integration tests passed" > $GITHUB_STEP_SUMMARY + #echo "$GODOT_BUILT_FROM" >> $GITHUB_STEP_SUMMARY ;; "godot-runtime") diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 24762bda0..cfa805a37 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -260,6 +260,7 @@ jobs: rust-toolchain: ${{ matrix.rust-toolchain }} rust-env-rustflags: ${{ matrix.rust-env-rustflags }} with-llvm: ${{ matrix.with-llvm }} + godot-check-header: ${{ matrix.name == 'linux' }} license-guard: diff --git a/godot-bindings/res/tweak.patch b/godot-bindings/res/tweak.patch index 7553caf6b..9aad26029 100644 --- a/godot-bindings/res/tweak.patch +++ b/godot-bindings/res/tweak.patch @@ -27,14 +27,57 @@ index 0b7615f..6db266e 100644 +typedef const struct __GdextObject *GDExtensionConstObjectPtr; +typedef struct __GdextType *GDExtensionTypePtr; +typedef const struct __GdextType *GDExtensionConstTypePtr; -+typedef struct __GdextMethodBind *GDExtensionMethodBindPtr; ++typedef const struct __GdextMethodBind *GDExtensionMethodBindPtr; typedef int64_t GDExtensionInt; typedef uint8_t GDExtensionBool; typedef uint64_t GDObjectInstanceID; -typedef void *GDExtensionRefPtr; -typedef const void *GDExtensionConstRefPtr; -+typedef struct __GdextExtensionRef *GDExtensionRefPtr; -+typedef const struct __GdextExtensionRef *GDExtensionConstRefPtr; ++typedef struct __GdextRef *GDExtensionRefPtr; ++typedef const struct __GdextRef *GDExtensionConstRefPtr; /* VARIANT DATA I/O */ +@@ -203,7 +203,7 @@ typedef struct { + + /* EXTENSION CLASSES */ + +-typedef void *GDExtensionClassInstancePtr; ++typedef struct __GdextClassInstance *GDExtensionClassInstancePtr; + + typedef GDExtensionBool (*GDExtensionClassSet)(GDExtensionClassInstancePtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionConstVariantPtr p_value); + typedef GDExtensionBool (*GDExtensionClassGet)(GDExtensionClassInstancePtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionVariantPtr r_ret); +@@ -266,7 +266,7 @@ typedef struct { + void *class_userdata; // Per-class user data, later accessible in instance bindings. + } GDExtensionClassCreationInfo; + +-typedef void *GDExtensionClassLibraryPtr; ++typedef struct __GdextClassLibrary *GDExtensionClassLibraryPtr; + + /* Method */ + +@@ -323,7 +323,7 @@ typedef struct { + + /* SCRIPT INSTANCE EXTENSION */ + +-typedef void *GDExtensionScriptInstanceDataPtr; // Pointer to custom ScriptInstance native implementation. ++typedef struct __GdextScriptInstanceData *GDExtensionScriptInstanceDataPtr; // Pointer to custom ScriptInstance native implementation. + + typedef GDExtensionBool (*GDExtensionScriptInstanceSet)(GDExtensionScriptInstanceDataPtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionConstVariantPtr p_value); + typedef GDExtensionBool (*GDExtensionScriptInstanceGet)(GDExtensionScriptInstanceDataPtr p_instance, GDExtensionConstStringNamePtr p_name, GDExtensionVariantPtr r_ret); +@@ -353,13 +353,13 @@ typedef GDExtensionBool (*GDExtensionScriptInstanceRefCountDecremented)(GDExtens + typedef GDExtensionObjectPtr (*GDExtensionScriptInstanceGetScript)(GDExtensionScriptInstanceDataPtr p_instance); + typedef GDExtensionBool (*GDExtensionScriptInstanceIsPlaceholder)(GDExtensionScriptInstanceDataPtr p_instance); + +-typedef void *GDExtensionScriptLanguagePtr; ++typedef struct __GdextScriptLanguage *GDExtensionScriptLanguagePtr; + + typedef GDExtensionScriptLanguagePtr (*GDExtensionScriptInstanceGetLanguage)(GDExtensionScriptInstanceDataPtr p_instance); + + typedef void (*GDExtensionScriptInstanceFree)(GDExtensionScriptInstanceDataPtr p_instance); + +-typedef void *GDExtensionScriptInstancePtr; // Pointer to ScriptInstance. ++typedef struct __GdextScriptInstance *GDExtensionScriptInstancePtr; // Pointer to ScriptInstance. + + typedef struct { + GDExtensionScriptInstanceSet set_func; diff --git a/godot-bindings/src/godot_exe.rs b/godot-bindings/src/godot_exe.rs index 759e79f95..916104b00 100644 --- a/godot-bindings/src/godot_exe.rs +++ b/godot-bindings/src/godot_exe.rs @@ -17,12 +17,8 @@ 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"), "/src/gen/godot_version.txt"); +// const GODOT_VERSION_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src/gen/godot_version.txt"); const JSON_PATH: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/src/gen/extension_api.json"); -const HEADER_PATH: &str = concat!( - env!("CARGO_MANIFEST_DIR"), - "/src/gen/gdextension_interface.h" -); pub fn load_gdextension_json(watch: &mut StopWatch) -> String { let json_path = Path::new(JSON_PATH); @@ -33,57 +29,52 @@ pub fn load_gdextension_json(watch: &mut StopWatch) -> String { watch.record("locate_godot"); // Regenerate API JSON if first time or Godot version is different - let version = read_godot_version(&godot_bin); + let _version = read_godot_version(&godot_bin); // if !json_path.exists() || has_version_changed(&version) { dump_extension_api(&godot_bin, json_path); - update_version_file(&version); + // update_version_file(&version); - watch.record("dump_json"); + watch.record("dump_api_json"); // } let result = fs::read_to_string(json_path) .unwrap_or_else(|_| panic!("failed to open file {}", json_path.display())); - watch.record("read_json_file"); + watch.record("read_api_json"); result } -pub fn load_gdextension_header_rs( - c_in_path: Option<&Path>, - rust_out_path: &Path, +pub fn write_gdextension_headers( + inout_h_path: &Path, + out_rs_path: &Path, + is_h_provided: bool, watch: &mut StopWatch, ) { - let c_header_path; - - 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 { + if !is_h_provided { // 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); + 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); + dump_header_file(&godot_bin, inout_h_path); + // update_version_file(&version); + watch.record("dump_header_h"); // } }; - rerun_on_changed(c_header_path); - watch.record("dump_header_c"); - - patch_c_header(c_header_path); - generate_rust_binding(c_header_path, rust_out_path); + rerun_on_changed(inout_h_path); + patch_c_header(inout_h_path); + watch.record("patch_header_h"); + generate_rust_binding(inout_h_path, out_rs_path); watch.record("generate_header_rs"); } -#[allow(dead_code)] +/* fn has_version_changed(current_version: &str) -> bool { let version_path = Path::new(GODOT_VERSION_PATH); @@ -97,9 +88,10 @@ fn update_version_file(version: &str) { let version_path = Path::new(GODOT_VERSION_PATH); rerun_on_changed(version_path); - std::fs::write(version_path, version) + fs::write(version_path, version) .unwrap_or_else(|_| panic!("write Godot version to file {}", version_path.display())); } +*/ fn read_godot_version(godot_bin: &Path) -> String { let output = Command::new(godot_bin) @@ -135,7 +127,7 @@ fn read_godot_version(godot_bin: &Path) -> String { fn dump_extension_api(godot_bin: &Path, out_file: &Path) { let cwd = out_file.parent().unwrap(); - std::fs::create_dir_all(cwd).unwrap_or_else(|_| panic!("create directory '{}'", cwd.display())); + fs::create_dir_all(cwd).unwrap_or_else(|_| panic!("create directory '{}'", cwd.display())); println!("Dump GDExtension API JSON to dir '{}'...", cwd.display()); let mut cmd = Command::new(godot_bin); @@ -143,14 +135,13 @@ fn dump_extension_api(godot_bin: &Path, out_file: &Path) { .arg("--headless") .arg("--dump-extension-api"); - execute(cmd, "dump Godot header file"); - - println!("Generated {}/gdextension_interface.h.", cwd.display()); + execute(cmd, "dump Godot JSON file"); + println!("Generated {}/extension_api.json.", cwd.display()); } fn dump_header_file(godot_bin: &Path, out_file: &Path) { let cwd = out_file.parent().unwrap(); - std::fs::create_dir_all(cwd).unwrap_or_else(|_| panic!("create directory '{}'", cwd.display())); + fs::create_dir_all(cwd).unwrap_or_else(|_| panic!("create directory '{}'", cwd.display())); println!("Dump GDExtension header file to dir '{}'...", cwd.display()); let mut cmd = Command::new(godot_bin); @@ -158,18 +149,17 @@ fn dump_header_file(godot_bin: &Path, out_file: &Path) { .arg("--headless") .arg("--dump-gdextension-interface"); - execute(cmd, "dump Godot JSON file"); - - println!("Generated {}/extension_api.json.", cwd.display()); + execute(cmd, "dump Godot header file"); + println!("Generated {}/gdextension_interface.h.", cwd.display()); } -fn patch_c_header(c_header_path: &Path) { +fn patch_c_header(inout_h_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`. // Read the contents of the file into a string - let c = fs::read_to_string(c_header_path) - .unwrap_or_else(|_| panic!("failed to read C header file {}", c_header_path.display())); + let c = fs::read_to_string(inout_h_path) + .unwrap_or_else(|_| panic!("failed to read C header file {}", inout_h_path.display())); // Use single regex with independent "const"/"Const", as there are definitions like this: // typedef const void *GDExtensionMethodBindPtr; @@ -180,10 +170,10 @@ fn patch_c_header(c_header_path: &Path) { println!("Patched contents:\n\n{}\n\n", c.as_ref()); // Write the modified contents back to the file - fs::write(c_header_path, c.as_ref()).unwrap_or_else(|_| { + fs::write(inout_h_path, c.as_ref()).unwrap_or_else(|_| { panic!( "failed to write patched C header file {}", - c_header_path.display() + inout_h_path.display() ) }); } diff --git a/godot-bindings/src/header_gen.rs b/godot-bindings/src/header_gen.rs index 8df5726d1..c9a30b8bc 100644 --- a/godot-bindings/src/header_gen.rs +++ b/godot-bindings/src/header_gen.rs @@ -7,8 +7,8 @@ use std::env; use std::path::Path; -pub(crate) fn generate_rust_binding(in_c_path: &Path, out_rust_path: &Path) { - let c_header_path = in_c_path.display().to_string(); +pub(crate) fn generate_rust_binding(in_h_path: &Path, out_rs_path: &Path) { + let c_header_path = in_h_path.display().to_string(); println!("cargo:rerun-if-changed={}", c_header_path); let builder = bindgen::Builder::default() @@ -19,7 +19,7 @@ pub(crate) fn generate_rust_binding(in_c_path: &Path, out_rust_path: &Path) { .prepend_enum_name(false); std::fs::create_dir_all( - out_rust_path + out_rs_path .parent() .expect("bindgen output file has parent dir"), ) @@ -30,18 +30,18 @@ pub(crate) fn generate_rust_binding(in_c_path: &Path, out_rust_path: &Path) { .unwrap_or_else(|err| { panic!( "bindgen generate failed\n c: {}\n rs: {}\n err: {}\n", - in_c_path.display(), - out_rust_path.display(), + in_h_path.display(), + out_rs_path.display(), err ) }); // Write the bindings to the $OUT_DIR/bindings.rs file. - bindings.write_to_file(out_rust_path).unwrap_or_else(|err| { + bindings.write_to_file(out_rs_path).unwrap_or_else(|err| { panic!( "bindgen write failed\n c: {}\n rs: {}\n err: {}\n", - in_c_path.display(), - out_rust_path.display(), + in_h_path.display(), + out_rs_path.display(), err ) }); diff --git a/godot-bindings/src/lib.rs b/godot-bindings/src/lib.rs index bdd9df7ac..8cbf4a334 100644 --- a/godot-bindings/src/lib.rs +++ b/godot-bindings/src/lib.rs @@ -29,19 +29,17 @@ mod custom { pub(crate) mod godot_version; pub(crate) mod header_gen; - // #[cfg(not(feature = "custom-godot-extheader"))] pub fn load_gdextension_json(watch: &mut StopWatch) -> String { godot_exe::load_gdextension_json(watch) } - // #[cfg(not(feature = "custom-godot-extheader"))] - pub fn write_gdextension_header_rs(to: &Path, watch: &mut StopWatch) { - godot_exe::load_gdextension_header_rs(None, to, watch); + pub fn write_gdextension_headers(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) { + godot_exe::write_gdextension_headers(h_path, rs_path, false, watch); } #[cfg(feature = "custom-godot-extheader")] - pub fn write_gdextension_header_rs_from_c(from: &Path, to: &Path, watch: &mut StopWatch) { - godot_exe::load_gdextension_header_rs(Some(from), to, watch); + pub fn write_gdextension_headers_from_c(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) { + godot_exe::write_gdextension_headers(h_path, rs_path, true, watch); } } @@ -60,11 +58,17 @@ mod prebuilt { godot4_prebuilt::load_gdextension_json() } - pub fn write_gdextension_header_rs(to: &Path, _watch: &mut StopWatch) { + pub fn write_gdextension_headers(h_path: &Path, rs_path: &Path, watch: &mut StopWatch) { // Note: prebuilt artifacts just return a static str. - let header_rs = godot4_prebuilt::load_gdextension_header_rs(); - std::fs::write(to, header_rs) + let h_contents = godot4_prebuilt::load_gdextension_header_h(); + std::fs::write(h_path, h_contents) + .unwrap_or_else(|e| panic!("failed to write gdextension_interface.h: {e}")); + watch.record("write_header_h"); + + let rs_contents = godot4_prebuilt::load_gdextension_header_rs(); + std::fs::write(rs_path, rs_contents) .unwrap_or_else(|e| panic!("failed to write gdextension_interface.rs: {e}")); + watch.record("write_header_rs"); } } diff --git a/godot-ffi/build.rs b/godot-ffi/build.rs index d5ca18cbe..d1ab57d5b 100644 --- a/godot-ffi/build.rs +++ b/godot-ffi/build.rs @@ -14,12 +14,16 @@ fn main() { // It would be better to generate this in /.generated or /target/godot-gen, however IDEs currently // struggle with static analysis when symbols are outside the crate directory (April 2023). let gen_path = Path::new(concat!(env!("CARGO_MANIFEST_DIR"), "/src/gen")); - let header_rs_path = gen_path.join("gdextension_interface.rs"); + + // C header is not strictly required, however it is generated for debugging, and to allow CI + // to check for differences (tweak.patch). + let h_path = gen_path.join("gdextension_interface.h"); + let rs_path = gen_path.join("gdextension_interface.rs"); godot_bindings::clear_dir(gen_path, &mut watch); - godot_bindings::write_gdextension_header_rs(&header_rs_path, &mut watch); + godot_bindings::write_gdextension_headers(&h_path, &rs_path, &mut watch); godot_codegen::generate_sys_files(gen_path, &mut watch); - watch.write_stats_to(&gen_path.join("godot-ffi-stats.txt")); + watch.write_stats_to(&gen_path.join("ffi-stats.txt")); }