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: build Linux x86-64 binaries on older Linux #858

Merged
merged 2 commits into from
Jan 29, 2024

Conversation

DeeDeeG
Copy link
Member

@DeeDeeG DeeDeeG commented Jan 13, 2024

Issue or RFC Endorsed by Atom's Maintainers

Resolves #733
Resolves #747

Description of the Change

Short summary: Build Pulsar's Linux x86-64 binaries on an older version of Linux, in a Docker container. Appears to allow older Linux distros (with older glibc) to run Pulsar.

Details:

This gets us building against older glibc, which is forward-compatible with modern distros (great) but also compatible with contemporaneous distris from when that older glibc was the latest (notably: older Red Hat Enterprise Linux and RHEL-alike distros, which have very long support windows and don't update their glibc within a major distro version.) This compatibility with RHEL-alike distros is the main purpose of this PR. Since those are the uswers who were asking for it. Should also help with Debian 10 users, etc. See the above-linked issues for context and the requests from users for this change.

I also added a separate job that waits for the build job to finish, that will extract and test the built Linux binaries in a normal ubuntu-latest GitHub Actions runner environment (that is: outside of any Docker container). I wasn't able to get the tests to run in the Debian 10 Docker container (playwright couldn't successfully launch Pulsar in the Docker container -- ???), so this added complexity is the cost of still doing the integration (visual editor) tests on Linux.

(Tangent: Since the macOS visual editor tests may or may not even be running, I think this is the only way we run the visual tests at all. I like the visual tests! We should keep running them. I wish I could figure out how to do so in Docker, but it wasn't working for me. I'm not sure whether Playwright has ever played nice with Debian 10 or Docker, since Debian 11 support was the first time Playwright's release notes mentioned any deliberate Debian support. Oh, well.)

Alternate Designs

Could build on an even older Linux distro, for compatiblity with such older distros, but I would rather keep it to fully first-party officially supported distros, personally. It's easier for us to deal with distros, during our build, which are fully supported, than mess around with separate LTS infrastructure you have to opt-into. See this discussion for some context and my thoughts when asked about building for even older Debian: #733 (comment)

On another note: I would really like to run the tests in the same Docker container the binaries are built on. So we don't need two separate jobs just to build (1) and test (2) the Linux binaries. But I couldn't get testing working within the Docker container (job 1) so I needed a second job (job 2) outside the Docker container.

Possible Drawbacks

I don't feel qualified to assess what building on older distros would do for the binary -- maybe slightly less optimization in the native C/C++ code, due to older glibc and older compiler stack? But we're a mostly JS stack, so unclear how much this would even matter if true.

I expect any difference would not be noticeable to end-users. Could be wrong, honestly.

Verification Process

Folks on older distros tested binaries generated this way, and it worked for them:

Also basically works fine for me on a current Ubuntu-based distro (Linux Mint 21, based on Ubuntu 22.04).

Release Notes

Build Linux binaries on Debian 10, for older glibc and compatibility with older Linux distros

@DeeDeeG

This comment was marked as outdated.

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Jan 16, 2024

Update: The commit log showing all the things I had to try to get here is at https://github.com/DeeDeeG/pulsar/commits/CI-build-on-older-Linux-v1 (current tip of that branch is commit 21ad8e3).

I'll be squashing this PR's branch so the commit history here is neater, but anyone curious about why I implemented this this way can check the things I tried in the commit history of that original branch. Also rebased on master branch to confirm this still passes with the latest code.

For compatibility with older Linux distros

Debian 8 is pretty much end-of-life now.
Debian 10 should be old enough to have compatible glibc for RHEL 8.
Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

Overall this looks fantastic!

A seriously impressive amount of CI wizardry is stuffed into this change.

Overall everything seems to be doing exactly as advertised which is awesome, and the only questions/requests I have are very minor, so feel free to disagree if you feel the need.

Firstly, it might be a good idea to comment at the top of test-and-upload-Linux just to quickly document the reason it exists outside of the regular run, just so we can quickly remember it's a visual test issue.
Secondly, I would wish we can keep our single declaration for Python versioning. Since by removing the Python action and having Python stuffed into the larger Install build dependencies - Linux command, I feel like it sorta gets lost.

Think there's anyway we could utilize the environ values to specify the version still, maybe by utilizing that to construct the install command? Or I think I'd feel better with just creating a Setup Python - Linux where we can run the ap install. But what do you think?


Sorry the review has taken so long, thanks a ton for this one

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Jan 25, 2024

Thank you for the review! (Anything this early in a cycle is pretty chill, IMO, and we've all got stuff going on, so no worries there.)

Firstly, it might be a good idea to comment at the top of test-and-upload-Linux just to quickly document the reason it exists outside of the regular run, just so we can quickly remember it's a visual test issue.

Good idea, done!

Secondly, I would wish we can keep our single declaration for Python versioning.

I added a comment to explain why I had to not do that. Debian 10 is old. Old enough that software built with newer glibc won't run on it, ergo this PR to get Pulsar running on older Linux. The copy of Python we'd get from actions/setup-python is also compiled against too new glibc to run on Debian 10.

Getting newer Python on older Debian is kind of a pain, so I just let the Linux build use Python 3.7 (!!!) as the latest version available from Debian 10 "Buster"'s package repos: https://packages.debian.org/buster/python3. Alternative would be to maybe use a PPA like deadsnakes. https://unix.stackexchange.com/a/188819. Kind of a pain and not worth it, IMO, personally.

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Jan 25, 2024

The binary from this PR is working for me on Linux Mint 21 (Ubuntu 22.04).

Can't tell if it's a hair slower or not, feels similar to a binary built on Ubuntu-latest. We're a mostly JS codebase anyway, not a ton of direct C/C++ stuff. Overall usable on a quite old/slow machine, even on an HDD. (Though I'd definitely recommend to be on an SSD to anyone who can.)

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

@DeeDeeG Thanks for addressing my concerns! This looks awesome from here, and I think we are good to get it merged!

@confused-Techie confused-Techie merged commit e999387 into master Jan 29, 2024
103 checks passed
@Charadon
Copy link

Charadon commented Jan 29, 2024

Hey, just wanted to mention. Since this is based on Debian 10, and Debian 10's main support ends in June. Don't forget to add the Freexian repositories for extended support when that time comes: https://www.freexian.com/

It's officially endorsed by Debian themselves. (https://wiki.debian.org/LTS/Extended)

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

Successfully merging this pull request may close these issues.

Pulsar not working in RockyLinux9 Can't run on Almalinux 8
3 participants