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

Refactor OS exit code to be EXIT_SUCCESS by default #89229

Merged
1 commit merged into from
Mar 24, 2024
+82 −86
Merged

Refactor OS exit code to be EXIT_SUCCESS by default #89229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 2 additions & 1 deletion core/os/os.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ class OS {
bool _verbose_stdout = false;
bool _debug_stdout = false;
String _local_clipboard;
int _exit_code = EXIT_FAILURE; // unexpected exit is marked as failure
// Assume success by default, all failure cases need to set EXIT_FAILURE explicitly.
int _exit_code = EXIT_SUCCESS;
bool _allow_hidpi = false;
bool _allow_layered = false;
bool _stdout_enabled = true;
Expand Down
96 changes: 43 additions & 53 deletions main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,7 @@ Error Main::test_setup() {

return OK;
}

// The order is the same as in `Main::cleanup()`.
void Main::test_cleanup() {
ERR_FAIL_COND(!_start_success);
Expand Down Expand Up @@ -977,8 +978,9 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
packed_data->add_pack_source(zip_packed_data);
#endif

// Default exit code, can be modified for certain errors.
Error exit_code = ERR_INVALID_PARAMETER;
// Exit error code used in the `goto error` conditions.
// It's returned as the program exit code. ERR_HELP is special cased and handled as success (0).
Error exit_err = ERR_INVALID_PARAMETER;

I = args.front();
while (I) {
Expand Down Expand Up @@ -1028,12 +1030,12 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
} else if (I->get() == "-h" || I->get() == "--help" || I->get() == "/?") { // display help

show_help = true;
exit_code = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
exit_err = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
goto error;

} else if (I->get() == "--version") {
print_line(get_full_version_string());
exit_code = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
exit_err = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
goto error;

} else if (I->get() == "-v" || I->get() == "--verbose") { // verbose output
Expand Down Expand Up @@ -2459,7 +2461,7 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
OS::get_singleton()->finalize_core();
locale = String();

return exit_code;
return exit_err;
}

Error _parse_resource_dummy(void *p_data, VariantParser::Stream *p_stream, Ref<Resource> &r_res, int &line, String &r_err_str) {
Expand Down Expand Up @@ -3139,7 +3141,10 @@ String Main::get_rendering_driver_name() {
// everything the main loop needs to know about frame timings
static MainTimerSync main_timer_sync;

bool Main::start() {
// Return value should be EXIT_SUCCESS if we start successfully
// and should move on to `OS::run`, and EXIT_FAILURE otherwise for
// an early exit with that error code.
int Main::start() {
ERR_FAIL_COND_V(!_start_success, false);

bool has_icon = false;
Expand Down Expand Up @@ -3276,7 +3281,7 @@ bool Main::start() {

{
Ref<DirAccess> da = DirAccess::open(doc_tool_path);
ERR_FAIL_COND_V_MSG(da.is_null(), false, "Argument supplied to --doctool must be a valid directory path.");
ERR_FAIL_COND_V_MSG(da.is_null(), EXIT_FAILURE, "Argument supplied to --doctool must be a valid directory path.");
}

#ifndef MODULE_MONO_ENABLED
Expand Down Expand Up @@ -3311,23 +3316,23 @@ bool Main::start() {
// Create the module documentation directory if it doesn't exist
Ref<DirAccess> da = DirAccess::create_for_path(path);
err = da->make_dir_recursive(path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create directory: " + path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create directory: " + path + ": " + itos(err));

print_line("Loading docs from: " + path);
err = docsrc.load_classes(path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error loading docs from: " + path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error loading docs from: " + path + ": " + itos(err));
}
}

String index_path = doc_tool_path.path_join("doc/classes");
// Create the main documentation directory if it doesn't exist
Ref<DirAccess> da = DirAccess::create_for_path(index_path);
err = da->make_dir_recursive(index_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create index directory: " + index_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create index directory: " + index_path + ": " + itos(err));

print_line("Loading classes from: " + index_path);
err = docsrc.load_classes(index_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error loading classes from: " + index_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error loading classes from: " + index_path + ": " + itos(err));
checked_paths.insert(index_path);

print_line("Merging docs...");
Expand All @@ -3336,20 +3341,19 @@ bool Main::start() {
for (const String &E : checked_paths) {
print_line("Erasing old docs at: " + E);
err = DocTools::erase_classes(E);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error erasing old docs at: " + E + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error erasing old docs at: " + E + ": " + itos(err));
}

print_line("Generating new docs...");
err = doc.save_classes(index_path, doc_data_classes);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error saving new docs:" + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error saving new docs:" + itos(err));

print_line("Deleting docs cache...");
if (FileAccess::exists(EditorHelp::get_cache_full_path())) {
DirAccess::remove_file_or_error(EditorHelp::get_cache_full_path());
}

OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}

// GDExtension API and interface.
Expand All @@ -3364,32 +3368,24 @@ bool Main::start() {
}

if (dump_gdextension_interface || dump_extension_api) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}

if (validate_extension_api) {
Engine::get_singleton()->set_editor_hint(true); // "extension_api.json" should always contains editor singletons.
bool valid = GDExtensionAPIDump::validate_extension_json_file(validate_extension_api_file) == OK;
OS::get_singleton()->set_exit_code(valid ? EXIT_SUCCESS : EXIT_FAILURE);
return false;
return valid ? EXIT_SUCCESS : EXIT_FAILURE;
}
}

#ifndef DISABLE_DEPRECATED
if (converting_project) {
int ret = ProjectConverter3To4(converter_max_kb_file, converter_max_line_length).convert();
if (ret) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return ret ? EXIT_SUCCESS : EXIT_FAILURE;
}
if (validating_converting_project) {
bool ret = ProjectConverter3To4(converter_max_kb_file, converter_max_line_length).validate_conversion();
if (ret) {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return ret ? EXIT_SUCCESS : EXIT_FAILURE;
}
#endif // DISABLE_DEPRECATED

Expand All @@ -3406,7 +3402,7 @@ bool Main::start() {
// this might end up triggered by valid usage, in which case we'll have to
// fine-tune further.
OS::get_singleton()->alert("Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
ERR_FAIL_V_MSG(false, "Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
ERR_FAIL_V_MSG(EXIT_FAILURE, "Couldn't detect whether to run the editor, the project manager or a specific project. Aborting.");
}
#endif

Expand All @@ -3420,15 +3416,10 @@ bool Main::start() {

if (!script.is_empty()) {
Ref<Script> script_res = ResourceLoader::load(script);
ERR_FAIL_COND_V_MSG(script_res.is_null(), false, "Can't load script: " + script);
ERR_FAIL_COND_V_MSG(script_res.is_null(), EXIT_FAILURE, "Can't load script: " + script);

if (check_only) {
if (!script_res->is_valid()) {
OS::get_singleton()->set_exit_code(EXIT_FAILURE);
} else {
OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
}
return false;
return script_res->is_valid() ? EXIT_SUCCESS : EXIT_FAILURE;
}

if (script_res->can_instantiate()) {
Expand All @@ -3440,21 +3431,21 @@ bool Main::start() {
memdelete(obj);
}
OS::get_singleton()->alert(vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
ERR_FAIL_V_MSG(false, vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("Can't load the script \"%s\" as it doesn't inherit from SceneTree or MainLoop.", script));
}

script_loop->set_script(script_res);
main_loop = script_loop;
} else {
return false;
return EXIT_FAILURE;
}
} else { // Not based on script path.
if (!editor && !ClassDB::class_exists(main_loop_type) && ScriptServer::is_global_class(main_loop_type)) {
String script_path = ScriptServer::get_global_class_path(main_loop_type);
Ref<Script> script_res = ResourceLoader::load(script_path);
if (script_res.is_null()) {
OS::get_singleton()->alert("Error: Could not load MainLoop script type: " + main_loop_type);
ERR_FAIL_V_MSG(false, vformat("Could not load global class %s.", main_loop_type));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("Could not load global class %s.", main_loop_type));
}
StringName script_base = script_res->get_instance_base_type();
Object *obj = ClassDB::instantiate(script_base);
Expand All @@ -3464,7 +3455,7 @@ bool Main::start() {
memdelete(obj);
}
OS::get_singleton()->alert("Error: Invalid MainLoop script base type: " + script_base);
ERR_FAIL_V_MSG(false, vformat("The global class %s does not inherit from SceneTree or MainLoop.", main_loop_type));
ERR_FAIL_V_MSG(EXIT_FAILURE, vformat("The global class %s does not inherit from SceneTree or MainLoop.", main_loop_type));
}
script_loop->set_script(script_res);
main_loop = script_loop;
Expand All @@ -3478,15 +3469,15 @@ bool Main::start() {
if (!main_loop) {
if (!ClassDB::class_exists(main_loop_type)) {
OS::get_singleton()->alert("Error: MainLoop type doesn't exist: " + main_loop_type);
return false;
return EXIT_FAILURE;
} else {
Object *ml = ClassDB::instantiate(main_loop_type);
ERR_FAIL_NULL_V_MSG(ml, false, "Can't instance MainLoop type.");
ERR_FAIL_NULL_V_MSG(ml, EXIT_FAILURE, "Can't instance MainLoop type.");

main_loop = Object::cast_to<MainLoop>(ml);
if (!main_loop) {
memdelete(ml);
ERR_FAIL_V_MSG(false, "Invalid MainLoop type.");
ERR_FAIL_V_MSG(EXIT_FAILURE, "Invalid MainLoop type.");
}
}
}
Expand Down Expand Up @@ -3610,7 +3601,7 @@ bool Main::start() {
Error err;

Vector<String> paths = get_files_with_extension(gdscript_docs_path, "gd");
ERR_FAIL_COND_V_MSG(paths.is_empty(), false, "Couldn't find any GDScript files under the given directory: " + gdscript_docs_path);
ERR_FAIL_COND_V_MSG(paths.is_empty(), EXIT_FAILURE, "Couldn't find any GDScript files under the given directory: " + gdscript_docs_path);

for (const String &path : paths) {
Ref<GDScript> gdscript = ResourceLoader::load(path);
Expand All @@ -3625,14 +3616,13 @@ bool Main::start() {

Ref<DirAccess> da = DirAccess::create_for_path(doc_tool_path);
err = da->make_dir_recursive(doc_tool_path);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error: Can't create GDScript docs directory: " + doc_tool_path + ": " + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error: Can't create GDScript docs directory: " + doc_tool_path + ": " + itos(err));

HashMap<String, String> doc_data_classes;
err = docs.save_classes(doc_tool_path, doc_data_classes, false);
ERR_FAIL_COND_V_MSG(err != OK, false, "Error saving GDScript docs:" + itos(err));
ERR_FAIL_COND_V_MSG(err != OK, EXIT_FAILURE, "Error saving GDScript docs:" + itos(err));

OS::get_singleton()->set_exit_code(EXIT_SUCCESS);
return false;
return EXIT_SUCCESS;
}
#endif // MODULE_GDSCRIPT_ENABLED

Expand Down Expand Up @@ -3747,7 +3737,7 @@ bool Main::start() {

if (sep == -1) {
Ref<DirAccess> da = DirAccess::create(DirAccess::ACCESS_FILESYSTEM);
ERR_FAIL_COND_V(da.is_null(), false);
ERR_FAIL_COND_V(da.is_null(), EXIT_FAILURE);

local_game_path = da->get_current_dir().path_join(local_game_path);
} else {
Expand Down Expand Up @@ -3797,7 +3787,7 @@ bool Main::start() {
scene = scenedata->instantiate();
}

ERR_FAIL_NULL_V_MSG(scene, false, "Failed loading scene: " + local_game_path + ".");
ERR_FAIL_NULL_V_MSG(scene, EXIT_FAILURE, "Failed loading scene: " + local_game_path + ".");
sml->add_current_scene(scene);

#ifdef MACOS_ENABLED
Expand Down Expand Up @@ -3870,7 +3860,7 @@ bool Main::start() {
OS::get_singleton()->benchmark_end_measure("Startup", "Total");
OS::get_singleton()->benchmark_dump();

return true;
return EXIT_SUCCESS;
}

/* Main iteration
Expand Down Expand Up @@ -3900,10 +3890,10 @@ static uint64_t physics_process_max = 0;
static uint64_t process_max = 0;
static uint64_t navigation_process_max = 0;

// Return false means iterating further, returning true means `OS::run`
// will terminate the program. In case of failure, the OS exit code needs
// to be set explicitly here (defaults to EXIT_SUCCESS).
bool Main::iteration() {
//for now do not error on this
//ERR_FAIL_COND_V(iterating, false);

iterating++;

const uint64_t ticks = OS::get_singleton()->get_ticks_usec();
Expand Down
2 changes: 1 addition & 1 deletion main/main.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class Main {
static Error test_setup();
static void test_cleanup();
#endif
static bool start();
static int start();

static bool iteration();
static void force_redraw();
Expand Down
11 changes: 6 additions & 5 deletions platform/ios/godot_ios.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,15 +102,16 @@ int ios_main(int argc, char **argv) {

Error err = Main::setup(fargv[0], argc - 1, &fargv[1], false);

if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
} else if (err != OK) {
return 255;
if (err != OK) {
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return EXIT_SUCCESS;
}
return EXIT_FAILURE;
}

os->initialize_modules();

return 0;
return os->get_exit_code();
Copy link
Member Author

Choose a reason for hiding this comment

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

FYI @bruvzg, I changed this to harmonize with other platforms, but I don't know if this can have unwanted implications.

}

void ios_finish() {
Expand Down
13 changes: 7 additions & 6 deletions platform/linuxbsd/godot_linuxbsd.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,18 +72,19 @@ int main(int argc, char *argv[]) {
char *ret = getcwd(cwd, PATH_MAX);

Error err = Main::setup(argv[0], argc - 1, &argv[1]);

if (err != OK) {
free(cwd);

if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
return EXIT_SUCCESS;
}
return 255;
return EXIT_FAILURE;
}

if (Main::start()) {
os.set_exit_code(EXIT_SUCCESS);
os.run(); // it is actually the OS that decides how to run
if (Main::start() == EXIT_SUCCESS) {
os.run();
} else {
os.set_exit_code(EXIT_FAILURE);
}
Main::cleanup();

Expand Down
19 changes: 11 additions & 8 deletions platform/macos/godot_main_macos.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,18 +69,21 @@ int main(int argc, char **argv) {
err = Main::setup(argv[0], argc - first_arg, &argv[first_arg]);
}

if (err == ERR_HELP) { // Returned by --help and --version, so success.
return 0;
} else if (err != OK) {
return 255;
if (err != OK) {
if (err == ERR_HELP) { // Returned by --help and --version, so success.
return EXIT_SUCCESS;
}
return EXIT_FAILURE;
}

bool ok;
int ret;
@autoreleasepool {
ok = Main::start();
ret = Main::start();
}
if (ok) {
os.run(); // It is actually the OS that decides how to run.
if (ret) {
os.run();
} else {
os.set_exit_code(EXIT_FAILURE);
}

@autoreleasepool {
Expand Down
Loading
Loading