Skip to content

Convert actions to Ubuntu 24.04#3664

Open
Nateowami wants to merge 1 commit intomasterfrom
test/2024-04-actions
Open

Convert actions to Ubuntu 24.04#3664
Nateowami wants to merge 1 commit intomasterfrom
test/2024-04-actions

Conversation

@Nateowami
Copy link
Collaborator

@Nateowami Nateowami commented Feb 2, 2026

This PR exists to test what happens when we start running builds on Ubuntu 24.04. It exists as a test, rather than something we actually intend to merge.

This change is Reviewable


Open with Devin

@Nateowami Nateowami added the e2e Run e2e tests for this pull request label Feb 2, 2026
@Nateowami Nateowami force-pushed the test/2024-04-actions branch 3 times, most recently from 915370f to ea95774 Compare February 3, 2026 02:49
@codecov
Copy link

codecov bot commented Feb 3, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.32%. Comparing base (a3e53c4) to head (44e3440).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3664   +/-   ##
=======================================
  Coverage   81.32%   81.32%           
=======================================
  Files         620      620           
  Lines       39086    39086           
  Branches     6375     6351   -24     
=======================================
  Hits        31787    31787           
- Misses       6316     6329   +13     
+ Partials      983      970   -13     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Nateowami Nateowami force-pushed the test/2024-04-actions branch from ea95774 to 414661d Compare February 5, 2026 15:30
@Nateowami Nateowami marked this pull request as ready for review March 18, 2026 17:08
@Nateowami Nateowami force-pushed the test/2024-04-actions branch from 414661d to f84959b Compare March 18, 2026 17:08
@Nateowami
Copy link
Collaborator Author

Given that servers have been upgraded, it probably makes sense to now review and merge this.

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

View 1 additional finding in Devin Review.

Open in Devin Review

strategy:
matrix:
os: ["ubuntu-22.04"]
os: ["ubuntu-24.04"]
Copy link

@devin-ai-integration devin-ai-integration bot Mar 18, 2026

Choose a reason for hiding this comment

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

🚩 chromium-browser references may need updating for Ubuntu 24.04 compatibility

The pre-build report steps in build-and-test.yml:65-66 and build-and-test.yml:180-181 run which chromium-browser and chromium-browser --version under set -xueo pipefail. On standard Ubuntu 24.04, chromium-browser is a transitional package that redirects to the snap chromium package, and the binary name is typically chromium rather than chromium-browser. GitHub Actions runner images for ubuntu-24.04 may provide chromium-browser through a third-party PPA (e.g., chromium-team/stable), which would make the command available. However, this depends on the runner image's specific configuration and PPA support for Noble (24.04). If chromium-browser is not available, the step will fail due to set -e, blocking the entire CI pipeline. Additionally, the test:gha npm script at src/SIL.XForge.Scripture/ClientApp/package.json:17 uses xForgeChromiumHeadless (based on ChromiumHeadless from karma-chrome-launcher), which also looks for chromium-browser in PATH. Worth verifying these commands work on the new runner image.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's a problem. And the E2E build passed.

@marksvc marksvc self-assigned this Mar 20, 2026
Copy link
Collaborator

@marksvc marksvc left a comment

Choose a reason for hiding this comment

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

@marksvc reviewed 3 files and all commit messages, made 3 comments, and resolved 2 discussions.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on Nateowami).

strategy:
matrix:
os: ["ubuntu-22.04"]
os: ["ubuntu-24.04"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think it's a problem. And the E2E build passed.

Copy link
Collaborator Author

@Nateowami Nateowami left a comment

Choose a reason for hiding this comment

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

@Nateowami made 1 comment and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Nateowami).

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 1 additional finding in Devin Review.

Open in Devin Review

npm_version: "11.11.0"
# When bumping OS version, the server will need to authorize the new default rsync args.
os: "ubuntu-22.04"
os: "ubuntu-24.04"

Choose a reason for hiding this comment

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

🚩 rsync compatibility warning: server may need reconfiguration for ubuntu-24.04

The existing comments at release-live.yml:120 and release-qa.yml:141 explicitly warn: "When bumping OS version, the server will need to authorize the new default rsync args." This PR bumps the OS version from ubuntu-22.04 to ubuntu-24.04, which is exactly the scenario the comment warns about. Ubuntu 24.04 ships rsync 3.2.7+ which may use different default arguments (e.g., different compression or checksum algorithms like xxhash) compared to Ubuntu 22.04's rsync. If the deployment server uses rrsync or other restrictions on allowed rsync arguments, the deploy step in release.yml:163 (scripts/build-and-ship) could fail. The server-side configuration should be verified before deploying from an ubuntu-24.04 runner.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@marksvc marksvc added the do not merge See PR description and/or comments for explanation label Mar 23, 2026
Copy link
Collaborator

@marksvc marksvc left a comment

Choose a reason for hiding this comment

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

@marksvc reviewed 3 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on Nateowami).

@marksvc
Copy link
Collaborator

marksvc commented Mar 23, 2026

(I added do-not-merge tag related to a comment I made about not merging until after a coming production release.)

Copy link

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

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

Labels

do not merge See PR description and/or comments for explanation e2e Run e2e tests for this pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants