-
-
Notifications
You must be signed in to change notification settings - Fork 21.2k
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
Web: Workaround Emscripten 3.1.42+ LTO regression #81340
Web: Workaround Emscripten 3.1.42+ LTO regression #81340
Conversation
@@ -16,7 +16,7 @@ concurrency: | |||
|
|||
jobs: | |||
web-template: | |||
runs-on: "ubuntu-20.04" | |||
runs-on: "ubuntu-22.04" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't strictly needed, just a drive-by change as Ubuntu 20.04 will be EOL'ed by GH Actions eventually, and we have no reason to stick to the old version for Web builds.
cc_version = get_compiler_version(env) | ||
cc_semver = (int(cc_version["major"]), int(cc_version["minor"]), int(cc_version["patch"])) | ||
if cc_semver >= (3, 1, 42): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll refactor this logic in #80719 after this is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed in the 3.x
branch that we use tuple(get_compiler_version(env))
there, which is nicer. I'm changing it to that when cherry-picking, and for master
I'll refactor later.
Edit: That can't be done for 4.x, there's more than just (major, minor, patch) in the dict.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 !
Cherry-picked for 3.6. |
Cherry-picked for 3.5.3. |
The check needs to happen after we set `env["CXX"]`. Follow-up to godotengine#81340.
Cherry-picked for 4.1.2. |
The check needs to happen after we set `env["CXX"]`. Follow-up to godotengine#81340. (cherry picked from commit 5016180)
The check needs to happen after we set `env["CXX"]`. Follow-up to godotengine#81340.
The check needs to happen after we set `env["CXX"]`. Follow-up to godotengine#81340. (cherry picked from commit 5016180)
Fixes #80010.
Tested with Emscripten 3.1.45 and
scons p=web target=template_release production=yes verbose=yes
.