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.

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

Fix exit code of --help and --version, and test them in CI #62618

Merged
merged 1 commit into from
Jul 2, 2022

Conversation

Bromeon
Copy link
Contributor

@Bromeon Bromeon commented Jul 1, 2022

Corrects prior regression from #62550, which caused ERROR: output and exit code 1.

Also invokes both --help and --version in GitHub Actions to ensure exit code 0.

Corrects prior regression which caused ERROR output and exit code of 1.
@Bromeon Bromeon requested review from a team as code owners July 1, 2022 23:19
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I have a test run on https://github.com/akien-mga/godot/actions/runs/2599518766 from before this PR but with CI checks added to try to confirm that it would actually mark the run as failing.
Seems to work:

Run ./bin/godot.linuxbsd.tools.64.llvm.san --version
4.0.alpha.custom_build.e458311a4
ERROR: Condition "!_start_success" is true. Returning: false
   at: start (main/main.cpp:2089)
ERROR: Condition "!_start_success" is true.
   at: cleanup (main/main.cpp:2936)
Error: Process completed with exit code 1.

@akien-mga akien-mga added this to the 4.0 milestone Jul 1, 2022
@@ -691,12 +691,12 @@ Error Main::setup(const char *execpath, int argc, char *argv[], bool p_second_ph
if (I->get() == "-h" || I->get() == "--help" || I->get() == "/?") { // display help

show_help = true;
exit_code = OK;
exit_code = ERR_HELP; // Hack to force an early exit in `main()` with a success code.
Copy link
Member

Choose a reason for hiding this comment

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

For the record we discussed this on chat, and chose to go with this hack using the pre-existing ERR_HELP (which nothing uses otherwise in the codebase) to convey the meaning of early exit but success.

It's not the prettiest but the whole main setup is pretty messy already and due a full refactor/rewrite, so in the meantime this is good enough because it's simple/non-intrusive.

@akien-mga akien-mga merged commit ec1348a into godotengine:master Jul 2, 2022
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Jul 2, 2022
@Bromeon Bromeon deleted the bugfix/exit-code branch July 2, 2022 09:07
@akien-mga
Copy link
Member

akien-mga commented Jul 2, 2022

Cherry-picked for 3.5.

Edit: Seems like I messed up locally and forgot to push this back then. Fixed on 2022-07-08 with 861e155

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants