Skip to content

Commit

Permalink
Always write both .h/.rs headers; check header diff on single linux
Browse files Browse the repository at this point in the history
… job; disable version r/w
  • Loading branch information
Bromeon committed Apr 1, 2023
1 parent 120a0dd commit c20197f
Show file tree
Hide file tree
Showing 7 changed files with 126 additions and 76 deletions.
30 changes: 19 additions & 11 deletions .github/composite/godot-itest/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -75,24 +80,26 @@ 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 ""
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
Expand Down Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions .github/workflows/full-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
49 changes: 46 additions & 3 deletions godot-bindings/res/tweak.patch
Original file line number Diff line number Diff line change
Expand Up @@ -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;
74 changes: 32 additions & 42 deletions godot-bindings/src/godot_exe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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)
Expand Down Expand Up @@ -135,41 +127,39 @@ 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);
cmd.current_dir(cwd)
.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);
cmd.current_dir(cwd)
.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;
Expand All @@ -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()
)
});
}
Expand Down
16 changes: 8 additions & 8 deletions godot-bindings/src/header_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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"),
)
Expand All @@ -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
)
});
Expand Down
22 changes: 13 additions & 9 deletions godot-bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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");
}
}

Expand Down
Loading

0 comments on commit c20197f

Please sign in to comment.