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

CI: use ubuntu 24.04 over ubuntu 20.04 in linux tiles builds #75992

Closed
wants to merge 1 commit into from

Conversation

NetSysFire
Copy link
Member

Summary

Build "Use the newer ubuntu24.04 image in Linux tiles builds"

Purpose of change

Please do not merge until people had time to review this.

Fixes #54886
Fixes #73667

This hopefully fixes those arcane and cursed issues. GCC 9 is unsupported anyways.
Special thanks to #gcc on libera.chat. Your mystic ways fascinate me.

Describe the solution

CC @akrieger.

So why am I doing this?

  1. I can very reliably reproduce the crashing on any of our Linux tiles builds, which are built using a ubuntu 20.04 image.
  2. I can not reproduce on my own build on Arch Linux, with gcc 14.2.1+r32+geccf707e5ce-1.
  3. I can not reproduce on the build I made the CI in my fork produce using this ubuntu 24.04 image.

I tried compiling gcc9 myself on Arch but after 10h of gcc failing to build because of random reasons, I stopped. I also spent 6h of compiling cdda over and over and over again with different flags because I initially thought that the Arch Linux flag defaults may interfere with that heap overflow because e.g FORTIFY_SOURCE may prevent them. No change in behavior when I disabled all of them to have a completely plain default build.

The wise folks in #gcc eventually pointed me at the fact that I am using completely a different compiler version than you. So I unsuccessfully tried to compile gcc9 and then went to say fuck it, lets just make the CI use a newer version of gcc and that works.

Describe alternatives you've considered

Testing

CI in my fork built it successfully. It runs and does not crash.

The only thing I am worried about is that on Arch, I need -Wno-error=maybe-uninitialized. I do not know what part in the chain complained about this specifically and because it compiled just fine on the CI, it might not apply to us at all.

Additional context

@github-actions github-actions bot added Code: Tooling Tooling that is not part of the main game but is part of the repo. <Bugfix> This is a fix for a bug (or closes open issue) Code: Build Issues regarding different builds and build environments astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Aug 27, 2024
@harakka
Copy link
Member

harakka commented Aug 27, 2024

For the record, the reason we use 20.04 in CI is that during the initial imgui implementation, this also came up due to 20.4 having too old SDL version. In #72026 the argument was made against updating to 22.04:

this breaks the implicit promise of supporting ubuntu LTS releases. Maybe nobody ever said the words "we promise to support ubuntu LTS releases", but the tooling decisions, build environment, and project history set the expectation that we should.

On this argument the fact that gcc9 is EOL doesn't matter because it's still shipped in a non EOL Ubuntu LTS release, so it's a build and running env and toolchain we want to support in principle. I don't know if we've had an in depth discussion about this as a project policy but we probably should.

Also ftr the currently supposedly supported compilers are listed here: https://github.com/CleverRaven/Cataclysm-DDA/blob/master/doc/COMPILING/COMPILER_SUPPORT.md

This is not meant to be an argument for or against but I want it to be on record that this came up previously and what the circumstances and decision were then.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Aug 27, 2024
@esotericist
Copy link
Contributor

this is a heavy handed approach to what is almost certainly a flaw in our code somewhere.

the nature of the problem on display suggests somewhere we are engaging in undefined behavior, and just shuffling over to a different toolchain version is just papering it over rather than actually finding and fixing the problem. it could recur somewhere else, down the road, if it's sufficiently temporally displaced, it might look like a 'new' problem, not being correctly connected to previous attempts to diagnose.

there is no evidence on display that gcc 9 itself is somehow responsible, and i'm also not seeing an argument for why we should drop a still supported distro version. and no "gcc 9 is eol" isn't an argument to that end.

@NetSysFire
Copy link
Member Author

Then I invite you to actually reproduce the issue and help me with debugging it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) Code: Build Issues regarding different builds and build environments Code: Tooling Tooling that is not part of the main game but is part of the repo. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

imgui related crash when quitting during a blocking activity (falling asleep) Crash on quit without saving
3 participants