godotengine
/
godot
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix reload of GDExtension libraries in framework package on macos 

`GDExtension::open_library` has a check in it to see if the library was loaded
from a temp file, and if it was to restore the original name as that is the one
we actually care about. This check is breaking extension reloading on Mac when
the library path is to a framework folder, as the file inside the framework
will not generally be the same name as the folder.

This check also shouldn't be necessary even on Windows, which is the only
platform that uses `generate_temp_files`, since disposal of the created temp
file is handled within `OS_Windows::open_dynamic_library`, and
`GDExtension::open_library` (which is the only function to call
`open_dynamic_library` with a `p_data` argument) only cares about the original
library file path and has to do extra work to remove the name of the temp file.
Instead, I have removed that check and set `OS_Windows::open_dynamic_library`
to return the name of the original file and not the name of the copy.

This fixes GDExtension reloading on macOS. I do not have a Windows machine
available to test that it still works properly on Windows, so someone should
check that before merging this.

(cherry picked from commit f44d6a235f)
This commit is contained in:
Joel Croteau 2024-08-04 11:00:56 -06:00 committed by Rémi Verschelde
parent 6eea970e8c
commit 6e78eec37f
No known key found for this signature in database
GPG Key ID: C3336907360768E1
3 changed files with 17 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
core/extension/gdextension.cpp
View File

@ -781,23 +781,14 @@ Error GDExtension::open_library(const String &p_path, const String &p_entry_symb
}
}
String actual_lib_path;
OS::GDExtensionData data = {
true, // also_set_library_path
&actual_lib_path, // r_resolved_path
&library_path, // r_resolved_path
Engine::get_singleton()->is_editor_hint(), // generate_temp_files
&abs_dependencies_paths, // library_dependencies
};
Error err = OS::get_singleton()->open_dynamic_library(abs_path, library, &data);
if (actual_lib_path.get_file() != abs_path.get_file()) {
// If temporary files are generated, let's change the library path to point at the original,
// because that's what we want to check to see if it's changed.
library_path = actual_lib_path.get_base_dir().path_join(p_path.get_file());
} else {
library_path = actual_lib_path;
}
ERR_FAIL_COND_V_MSG(err == ERR_FILE_NOT_FOUND, err, "GDExtension dynamic library not found: " + abs_path);
ERR_FAIL_COND_V_MSG(err != OK, err, "Can't open GDExtension dynamic library: " + abs_path);

2
drivers/unix/file_access_unix.cpp
View File

@ -383,7 +383,7 @@ uint64_t FileAccessUnix::_get_modified_time(const String &p_file) {
if (!err) {
return status.st_mtime;
} else {
print_verbose("Failed to get modified time for: " + p_file + "");
WARN_PRINT("Failed to get modified time for: " + p_file);
return 0;
}
}

30
platform/windows/os_windows.cpp
View File

@ -379,6 +379,8 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
//this code exists so gdextension can load .dll files from within the executable path
path = get_executable_path().get_base_dir().path_join(p_path.get_file());
}
// Path to load from may be different from original if we make copies.
String load_path = path;
ERR_FAIL_COND_V(!FileAccess::exists(path), ERR_FILE_NOT_FOUND);
@ -387,25 +389,22 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
if (p_data != nullptr && p_data->generate_temp_files) {
// Copy the file to the same directory as the original with a prefix in the name.
// This is so relative path to dependencies are satisfied.
String copy_path = path.get_base_dir().path_join("~" + path.get_file());
load_path = path.get_base_dir().path_join("~" + path.get_file());
// If there's a left-over copy (possibly from a crash) then delete it first.
if (FileAccess::exists(copy_path)) {
DirAccess::remove_absolute(copy_path);
if (FileAccess::exists(load_path)) {
DirAccess::remove_absolute(load_path);
}
Error copy_err = DirAccess::copy_absolute(path, copy_path);
Error copy_err = DirAccess::copy_absolute(path, load_path);
if (copy_err) {
ERR_PRINT("Error copying library: " + path);
return ERR_CANT_CREATE;
}
FileAccess::set_hidden_attribute(copy_path, true);
FileAccess::set_hidden_attribute(load_path, true);
// Save the copied path so it can be deleted later.
path = copy_path;
Error pdb_err = WindowsUtils::copy_and_rename_pdb(path);
Error pdb_err = WindowsUtils::copy_and_rename_pdb(load_path);
if (pdb_err != OK && pdb_err != ERR_SKIP) {
WARN_PRINT(vformat("Failed to rename the PDB file. The original PDB file for '%s' will be loaded.", path));
}
@ -421,21 +420,21 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
DLL_DIRECTORY_COOKIE cookie = nullptr;
if (p_data != nullptr && p_data->also_set_library_path && has_dll_directory_api) {
cookie = add_dll_directory((LPCWSTR)(path.get_base_dir().utf16().get_data()));
cookie = add_dll_directory((LPCWSTR)(load_path.get_base_dir().utf16().get_data()));
}
p_library_handle = (void *)LoadLibraryExW((LPCWSTR)(path.utf16().get_data()), nullptr, (p_data != nullptr && p_data->also_set_library_path && has_dll_directory_api) ? LOAD_LIBRARY_SEARCH_DEFAULT_DIRS : 0);
p_library_handle = (void *)LoadLibraryExW((LPCWSTR)(load_path.utf16().get_data()), nullptr, (p_data != nullptr && p_data->also_set_library_path && has_dll_directory_api) ? LOAD_LIBRARY_SEARCH_DEFAULT_DIRS : 0);
if (!p_library_handle) {
if (p_data != nullptr && p_data->generate_temp_files) {
DirAccess::remove_absolute(path);
DirAccess::remove_absolute(load_path);
}
#ifdef DEBUG_ENABLED
DWORD err_code = GetLastError();
HashSet<String> checekd_libs;
HashSet<String> checked_libs;
HashSet<String> missing_libs;
debug_dynamic_library_check_dependencies(path, path, checekd_libs, missing_libs);
debug_dynamic_library_check_dependencies(load_path, load_path, checked_libs, missing_libs);
if (!missing_libs.is_empty()) {
String missing;
for (const String &E : missing_libs) {
@ -464,7 +463,8 @@ Error OS_Windows::open_dynamic_library(const String &p_path, void *&p_library_ha
}
if (p_data != nullptr && p_data->generate_temp_files) {
temp_libraries[p_library_handle] = path;
// Save the copied path so it can be deleted later.
temp_libraries[p_library_handle] = load_path;
}
return OK;