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

[3.x] Fix the logic to restart the Godot application #61332

Merged
merged 1 commit into from
May 23, 2022

Conversation

m4gr3d
Copy link
Contributor

@m4gr3d m4gr3d commented May 23, 2022

Fixes #53716

Bugsquad edit: 3.x version of #61333.

@m4gr3d m4gr3d added this to the 3.5 milestone May 23, 2022
@m4gr3d m4gr3d requested review from a team as code owners May 23, 2022 19:42
@m4gr3d m4gr3d changed the title Fix the logic to restart the Godot application [3.x] Fix the logic to restart the Godot application May 23, 2022
@akien-mga
Copy link
Member

You should document that file's licensing in COPYRIGHT.txt like this:

diff --git a/COPYRIGHT.txt b/COPYRIGHT.txt
index 52cdbd321f..765d820fb7 100644
--- a/COPYRIGHT.txt
+++ b/COPYRIGHT.txt
@@ -80,10 +80,9 @@ Comment: The Android Open Source Project
 Copyright: 2008-2013, The Android Open Source Project
 License: Apache-2.0
 
-Files: ./platform/android/java/src/com/android/vending/licensing/util/Base64.java
- ./platform/android/java/src/com/android/vending/licensing/util/Base64DecoderException.java
-Comment: The Android Open Source Project
-Copyright: 2002, Google Inc.
+Files: ./platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
+Comment: Process Phoenix
+Copyright: 2014, Jake Wharton.
 License: Apache-2.0
 
 Files: ./platform/android/power_android.cpp

(Also cleaned up two files where the paths were outdated - the files still existing within com/android/vending/licensing/ but I no longer thing we need to go so deep documenting copyright, I think it's good enough to keep those included as part of the general licensing library's copyright statement.)

@akien-mga
Copy link
Member

Also, while this can't go in thirdparty/ like other thirdparty libraries, it might be worth adding a comment in the file that documents its provenance and the commit that was copied over (and if relevant any downstream changes).

@akien-mga
Copy link
Member

And you'll want to exclude the file from the clang-format script:

diff --git a/misc/hooks/pre-commit-clang-format b/misc/hooks/pre-commit-clang-format
index e8e62e6470..1277ce7628 100755
--- a/misc/hooks/pre-commit-clang-format
+++ b/misc/hooks/pre-commit-clang-format
@@ -130,6 +130,9 @@ do
     if grep -q "platform/android/java/lib/src/com" <<< $file; then
         continue;
     fi
+    if grep -q "platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java" <<< $file; then
+        continue;
+    fi
     if grep -q "\-so_wrap." <<< $file; then
         continue;
     fi
diff --git a/misc/scripts/clang_format.sh b/misc/scripts/clang_format.sh
index cb007b0c6b..97daf5fcb0 100755
--- a/misc/scripts/clang_format.sh
+++ b/misc/scripts/clang_format.sh
@@ -35,6 +35,8 @@ while IFS= read -rd '' f; do
                 continue 2
             elif [[ "$f" == "platform/android/java/lib/src/org/godotengine/godot/gl/EGLLogWrapper"* ]]; then
                 continue 2
+            elif [[ "$f" == "platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java" ]]; then
+                continue 2
             fi
             python misc/scripts/copyright_headers.py "$f"
             continue 2

Or maybe use /* clang-format off */ in the file to turn it off.

@m4gr3d
Copy link
Contributor Author

m4gr3d commented May 23, 2022

@akien-mga Thanks for the info! I've applied the updates you mentioned.

COPYRIGHT.txt Outdated
@@ -86,6 +86,11 @@ Comment: The Android Open Source Project
Copyright: 2002, Google Inc.
License: Apache-2.0

Files: ./platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
Comment: ProcessPhoenix (commit #12cb27c2cc9c3fc555e97f2db89e571667de82c4)
Copy link
Member

Choose a reason for hiding this comment

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

I would put the commit in the .java file itself. This comment is used as title for the thirdparty licenses in the About dialog, such a long name will look pretty bad.

Copy link
Member

Choose a reason for hiding this comment

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

Something like this:

diff --git a/COPYRIGHT.txt b/COPYRIGHT.txt
index 17ddcadfe0..64d9a93b9f 100644
--- a/COPYRIGHT.txt
+++ b/COPYRIGHT.txt
@@ -87,7 +87,7 @@ Copyright: 2002, Google Inc.
 License: Apache-2.0
 
 Files: ./platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
-Comment: ProcessPhoenix (commit #12cb27c2cc9c3fc555e97f2db89e571667de82c4)
+Comment: ProcessPhoenix
 Copyright: 2015, Jake Wharton
 License: Apache-2.0
 
diff --git a/platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java b/platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
index 900cf254f0..2cc37b627a 100644
--- a/platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
+++ b/platform/android/java/lib/src/org/godotengine/godot/utils/ProcessPhoenix.java
@@ -1,5 +1,10 @@
 // clang-format off
 
+/* Third-party library.
+ * Upstream: https://github.com/JakeWharton/ProcessPhoenix
+ * Commit: 12cb27c2cc9c3fc555e97f2db89e571667de82c4
+ */
+
 /*
  * Copyright (C) 2014 Jake Wharton
  *

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

@akien-mga akien-mga merged commit e0ddaf7 into godotengine:3.x May 23, 2022
@m4gr3d m4gr3d deleted the fix_restart_logic_3x branch May 23, 2022 21:45
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