Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary checks when exporting gdextension binaries and allow using a prefix to auto-detect files #67906

Merged
merged 2 commits into from
Dec 6, 2022
Merged

Remove unnecessary checks when exporting gdextension binaries and allow using a prefix to auto-detect files #67906

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
36bcb82
Remove uncessecary checks when exporting gdextension binaries
groud Oct 26, 2022
fa4143c
Allow specifying a prefix to automatically detect library files for g…
groud Oct 26, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 107 additions & 23 deletions core/extension/native_extension.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@

#include "native_extension.h"
#include "core/config/project_settings.h"
#include "core/io/config_file.h"
#include "core/io/dir_access.h"
#include "core/object/class_db.h"
#include "core/object/method_bind.h"
#include "core/os/os.h"
Expand All @@ -39,6 +39,111 @@ String NativeExtension::get_extension_list_config_file() {
return ProjectSettings::get_singleton()->get_project_data_path().path_join("extension_list.cfg");
}

String NativeExtension::find_extension_library(const String &p_path, Ref<ConfigFile> p_config, std::function<bool(String)> p_has_feature, PackedStringArray *r_tags) {
// First, check the explicit libraries.
if (p_config->has_section("libraries")) {
List<String> libraries;
p_config->get_section_keys("libraries", &libraries);

// Iterate the libraries, finding the best matching tags.
String best_library_path;
Vector<String> best_library_tags;
for (const String &E : libraries) {
Vector<String> tags = E.split(".");
bool all_tags_met = true;
for (int i = 0; i < tags.size(); i++) {
String tag = tags[i].strip_edges();
if (!p_has_feature(tag)) {
all_tags_met = false;
break;
}
}

if (all_tags_met && tags.size() > best_library_tags.size()) {
best_library_path = p_config->get_value("libraries", E);
best_library_tags = tags;
}
}

if (!best_library_path.is_empty()) {
if (best_library_path.is_relative_path()) {
best_library_path = p_path.get_base_dir().path_join(best_library_path);
}
if (r_tags != nullptr) {
r_tags->append_array(best_library_tags);
}
return best_library_path;
}
}

// Second, try to autodetect
String autodetect_library_prefix;
if (p_config->has_section_key("configuration", "autodetect_library_prefix")) {
autodetect_library_prefix = p_config->get_value("configuration", "autodetect_library_prefix");
}
if (!autodetect_library_prefix.is_empty()) {
String autodetect_path = autodetect_library_prefix;
if (autodetect_path.is_relative_path()) {
autodetect_path = p_path.get_base_dir().path_join(autodetect_path);
}

// Find the folder and file parts of the prefix.
String folder;
String file_prefix;
if (DirAccess::dir_exists_absolute(autodetect_path)) {
folder = autodetect_path;
} else if (DirAccess::dir_exists_absolute(autodetect_path.get_base_dir())) {
folder = autodetect_path.get_base_dir();
file_prefix = autodetect_path.get_file();
} else {
ERR_FAIL_V_MSG(String(), vformat("Error in extension: %s. Could not find folder for automatic detection of libraries files. autodetect_library_prefix=\"%s\"", p_path, autodetect_library_prefix));
}

// Open the folder.
Ref<DirAccess> dir = DirAccess::open(folder);
ERR_FAIL_COND_V_MSG(!dir.is_valid(), String(), vformat("Error in extension: %s. Could not open folder for automatic detection of libraries files. autodetect_library_prefix=\"%s\"", p_path, autodetect_library_prefix));

// Iterate the files and check the prefixes, finding the best matching file.
String best_file;
Vector<String> best_file_tags;
dir->list_dir_begin();
String file_name = dir->_get_next();
while (file_name != "") {
if (!dir->current_is_dir() && file_name.begins_with(file_prefix)) {
// Check if the files matches all requested feature tags.
String tags_str = file_name.trim_prefix(file_prefix);
tags_str = tags_str.trim_suffix(tags_str.get_extension());

Vector<String> tags = tags_str.split(".", false);
bool all_tags_met = true;
for (int i = 0; i < tags.size(); i++) {
String tag = tags[i].strip_edges();
if (!p_has_feature(tag)) {
all_tags_met = false;
break;
}
}

// If all tags are found in the feature list, and we found more tags than before, use this file.
if (all_tags_met && tags.size() > best_file_tags.size()) {
best_file_tags = tags;
best_file = file_name;
}
}
file_name = dir->_get_next();
}

if (!best_file.is_empty()) {
String library_path = folder.path_join(best_file);
if (r_tags != nullptr) {
r_tags->append_array(best_file_tags);
}
return library_path;
}
}
return String();
}

class NativeExtensionMethodBind : public MethodBind {
GDNativeExtensionClassMethodCall call_func;
GDNativeExtensionClassMethodPtrCall ptrcall_func;
Expand Down Expand Up @@ -415,28 +520,7 @@ Ref<Resource> NativeExtensionResourceLoader::load(const String &p_path, const St

String entry_symbol = config->get_value("configuration", "entry_symbol");

List<String> libraries;

config->get_section_keys("libraries", &libraries);

String library_path;

for (const String &E : libraries) {
Vector<String> tags = E.split(".");
bool all_tags_met = true;
for (int i = 0; i < tags.size(); i++) {
String tag = tags[i].strip_edges();
if (!OS::get_singleton()->has_feature(tag)) {
all_tags_met = false;
break;
}
}

if (all_tags_met) {
library_path = config->get_value("libraries", E);
break;
}
}
String library_path = NativeExtension::find_extension_library(p_path, config, [](String p_feature) { return OS::get_singleton()->has_feature(p_feature); });

if (library_path.is_empty()) {
if (r_error) {
Expand Down
4 changes: 4 additions & 0 deletions core/extension/native_extension.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@
#ifndef NATIVE_EXTENSION_H
#define NATIVE_EXTENSION_H

#include <functional>

#include "core/extension/gdnative_interface.h"
#include "core/io/config_file.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could possibly be forward declared? Not very important though.

#include "core/io/resource_loader.h"
#include "core/object/ref_counted.h"

Expand Down Expand Up @@ -65,6 +68,7 @@ class NativeExtension : public Resource {

public:
static String get_extension_list_config_file();
static String find_extension_library(const String &p_path, Ref<ConfigFile> p_config, std::function<bool(String)> p_has_feature, PackedStringArray *r_tags = nullptr);

Error open_library(const String &p_path, const String &p_entry_symbol);
void close_library();
Expand Down
18 changes: 12 additions & 6 deletions core/os/os.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -353,20 +353,26 @@ bool OS::has_feature(const String &p_feature) {
if (p_feature == "debug") {
return true;
}
#endif // DEBUG_ENABLED

#ifdef TOOLS_ENABLED
if (p_feature == "editor") {
return true;
}
#else
if (p_feature == "release") {
if (p_feature == "template") {
return true;
}
#endif
#ifdef TOOLS_ENABLED
if (p_feature == "editor") {
#ifdef DEBUG_ENABLED
if (p_feature == "template_debug") {
return true;
}
#else
if (p_feature == "standalone") {
if (p_feature == "template_release" || p_feature == "release") {
return true;
}
#endif
#endif // DEBUG_ENABLED
#endif // TOOLS_ENABLED

if (sizeof(void *) == 8 && p_feature == "64") {
return true;
Expand Down
5 changes: 3 additions & 2 deletions editor/export/editor_export_platform.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,10 +442,11 @@ HashSet<String> EditorExportPlatform::get_features(const Ref<EditorExportPreset>
result.insert(E);
}

result.insert("template");
if (p_debug) {
result.insert("debug");
result.insert("template_debug");
} else {
result.insert("release");
result.insert("template_release");
}

if (!p_preset->get_custom_features().is_empty()) {
Expand Down
4 changes: 2 additions & 2 deletions editor/export/editor_export_platform_pc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ bool EditorExportPlatformPC::has_valid_export_configuration(const Ref<EditorExpo

// Look for export templates (first official, and if defined custom templates).
String arch = p_preset->get("binary_format/architecture");
bool dvalid = exists_export_template(get_template_file_name("debug", arch), &err);
bool rvalid = exists_export_template(get_template_file_name("release", arch), &err);
bool dvalid = exists_export_template(get_template_file_name("template_debug", arch), &err);
bool rvalid = exists_export_template(get_template_file_name("template_release", arch), &err);
Comment on lines +84 to +85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This broke looking up the templates. Is that needed for GDExtension or was just a search-and-replace mistake?


if (p_preset->get("custom_template/debug") != "") {
dvalid = FileAccess::exists(p_preset->get("custom_template/debug"));
Expand Down
95 changes: 36 additions & 59 deletions editor/plugins/gdextension_export_plugin.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,58 +54,36 @@ void GDExtensionExportPlugin::_export_file(const String &p_path, const String &p

String entry_symbol = config->get_value("configuration", "entry_symbol");

List<String> libraries;

config->get_section_keys("libraries", &libraries);

bool could_export = false;
for (const String &E : libraries) {
Vector<String> tags = E.split(".");
bool all_tags_met = true;
for (int i = 0; i < tags.size(); i++) {
String tag = tags[i].strip_edges();
if (!p_features.has(tag)) {
all_tags_met = false;
break;
}
}

if (all_tags_met) {
String library_path = config->get_value("libraries", E);
if (!library_path.begins_with("res://")) {
print_line("Skipping export of out-of-project library " + library_path);
continue;
}
add_shared_object(library_path, tags);

if (p_features.has("iOS") && (library_path.ends_with(".a") || library_path.ends_with(".xcframework"))) {
String additional_code = "extern void register_dynamic_symbol(char *name, void *address);\n"
"extern void add_ios_init_callback(void (*cb)());\n"
"\n"
"extern \"C\" void $ENTRY();\n"
"void $ENTRY_init() {\n"
" if (&$ENTRY) register_dynamic_symbol((char *)\"$ENTRY\", (void *)$ENTRY);\n"
"}\n"
"struct $ENTRY_struct {\n"
" $ENTRY_struct() {\n"
" add_ios_init_callback($ENTRY_init);\n"
" }\n"
"};\n"
"$ENTRY_struct $ENTRY_struct_instance;\n\n";
additional_code = additional_code.replace("$ENTRY", entry_symbol);
add_ios_cpp_code(additional_code);

String linker_flags = "-Wl,-U,_" + entry_symbol;
add_ios_linker_flags(linker_flags);
}
could_export = true;
break;
PackedStringArray tags;
String library_path = NativeExtension::find_extension_library(
p_path, config, [p_features](String p_feature) { return p_features.has(p_feature); }, &tags);
if (!library_path.is_empty()) {
add_shared_object(library_path, tags);

if (p_features.has("iOS") && (library_path.ends_with(".a") || library_path.ends_with(".xcframework"))) {
String additional_code = "extern void register_dynamic_symbol(char *name, void *address);\n"
"extern void add_ios_init_callback(void (*cb)());\n"
"\n"
"extern \"C\" void $ENTRY();\n"
"void $ENTRY_init() {\n"
" if (&$ENTRY) register_dynamic_symbol((char *)\"$ENTRY\", (void *)$ENTRY);\n"
"}\n"
"struct $ENTRY_struct {\n"
" $ENTRY_struct() {\n"
" add_ios_init_callback($ENTRY_init);\n"
" }\n"
"};\n"
"$ENTRY_struct $ENTRY_struct_instance;\n\n";
additional_code = additional_code.replace("$ENTRY", entry_symbol);
add_ios_cpp_code(additional_code);

String linker_flags = "-Wl,-U,_" + entry_symbol;
add_ios_linker_flags(linker_flags);
}
}
if (!could_export) {
Vector<String> tags;
} else {
Vector<String> features_vector;
for (const String &E : p_features) {
tags.append(E);
features_vector.append(E);
}
ERR_FAIL_MSG(vformat("No suitable library found. The libraries' tags referred to an invalid feature flag. Possible feature flags for your platform: %s", p_path, String(", ").join(tags)));
}
Expand All @@ -115,11 +93,11 @@ void GDExtensionExportPlugin::_export_file(const String &p_path, const String &p
config->get_section_keys("dependencies", &dependencies);
}

for (const String &E : libraries) {
Vector<String> tags = E.split(".");
for (const String &E : dependencies) {
Vector<String> dependency_tags = E.split(".");
bool all_tags_met = true;
for (int i = 0; i < tags.size(); i++) {
String tag = tags[i].strip_edges();
for (int i = 0; i < dependency_tags.size(); i++) {
String tag = dependency_tags[i].strip_edges();
if (!p_features.has(tag)) {
all_tags_met = false;
break;
Expand All @@ -129,13 +107,12 @@ void GDExtensionExportPlugin::_export_file(const String &p_path, const String &p
if (all_tags_met) {
Dictionary dependency = config->get_value("dependencies", E);
for (const Variant *key = dependency.next(nullptr); key; key = dependency.next(key)) {
String library_path = *key;
String dependency_path = *key;
String target_path = dependency[*key];
if (!library_path.begins_with("res://")) {
print_line("Skipping export of out-of-project library " + library_path);
continue;
if (dependency_path.is_relative_path()) {
dependency_path = p_path.get_base_dir().path_join(dependency_path);
}
add_shared_object(library_path, tags, target_path);
add_shared_object(dependency_path, dependency_tags, target_path);
}
break;
}
Expand Down
6 changes: 4 additions & 2 deletions editor/project_settings_editor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,10 +274,12 @@ void ProjectSettingsEditor::_add_feature_overrides() {
presets.insert("s3tc");
presets.insert("etc");
presets.insert("etc2");
presets.insert("editor");
presets.insert("template_debug");
presets.insert("template_release");
presets.insert("debug");
presets.insert("release");
presets.insert("editor");
presets.insert("standalone");
presets.insert("template");
presets.insert("32");
presets.insert("64");
presets.insert("movie");
Expand Down
12 changes: 6 additions & 6 deletions tests/core/os/test_os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,14 @@ TEST_CASE("[OS] Feature tags") {
OS::get_singleton()->has_feature("editor"),
"The binary has the \"editor\" feature tag.");
CHECK_MESSAGE(
!OS::get_singleton()->has_feature("standalone"),
"The binary does not have the \"standalone\" feature tag.");
!OS::get_singleton()->has_feature("template"),
"The binary does not have the \"template\" feature tag.");
CHECK_MESSAGE(
OS::get_singleton()->has_feature("debug"),
"The binary has the \"debug\" feature tag.");
!OS::get_singleton()->has_feature("template_debug"),
"The binary does not have the \"template_debug\" feature tag.");
CHECK_MESSAGE(
!OS::get_singleton()->has_feature("release"),
"The binary does not have the \"release\" feature tag.");
!OS::get_singleton()->has_feature("template_release"),
"The binary does not have the \"template_release\" feature tag.");
}

TEST_CASE("[OS] Process ID") {
Expand Down