-
Notifications
You must be signed in to change notification settings - Fork 30
ci: get rid of ci-docker-image #1632
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
base: master
Are you sure you want to change the base?
Conversation
6545b61 to
241f64c
Compare
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughRemoved container-based CI wrappers, scripts, and docs; migrated several GitHub Actions to run on host Ubuntu runners with explicit host provisioning and simplified action steps. Changes
Sequence Diagram(s)mermaid GH->>OldScript: invoke wrapper script mermaid GH->>Host: start job on host runner Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/actions/tox-preinstall/action.yml (1)
16-22: LGTM! Conditional tox installation is well-implemented.The logic correctly ensures tox is available before attempting to use it, which aligns well with the shift to native ubuntu-slim runners.
Optional: Use `command -v` with output redirection for cleaner logs
The uppercase
-Vflag produces verbose output that may clutter CI logs. Consider using the lowercase-vwith output redirection:- - shell: bash - run: | - if ! command -V tox - then - sudo apt-get update - sudo apt-get install -y --no-install-recommends tox - fi + - shell: bash + run: | + if ! command -v tox > /dev/null 2>&1 + then + sudo apt-get update + sudo apt-get install -y --no-install-recommends tox + fi
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
.github/actions/container-test/action.yml.github/actions/pip-compile/action.yml.github/actions/tox-preinstall/action.yml.github/workflows/ci.yml.gitignoredo-in-docker.shdo-in-podman.shdocs/Build.mddocs/BuildInContainer.mddocs/CONTRIBUTING.mdrequirements/pip-compile.shtests/pict/gen.sh
💤 Files with no reviewable changes (8)
- .gitignore
- do-in-docker.sh
- docs/Build.md
- docs/CONTRIBUTING.md
- docs/BuildInContainer.md
- do-in-podman.sh
- requirements/pip-compile.sh
- tests/pict/gen.sh
🔇 Additional comments (8)
.github/actions/pip-compile/action.yml (1)
27-27: LGTM!The ignore pattern correctly filters out autogenerated pip-compile header comments that vary only by Python version, reducing false positives in the diff check.
.github/actions/container-test/action.yml (2)
18-21: LGTM!Simplified container pull step aligns with the PR's goal to eliminate wrapper script complexity.
23-33: LGTM!The streamlined test execution with direct bash invocation, junit XML output, and explicit container parameter improves clarity and CI integration.
.github/workflows/ci.yml (5)
159-159: Verify the ubuntu-slim runner availability.Same concern as the lint job - confirm
ubuntu-slimis available as a runner in your environment.
225-225: LGTM - Container image change aligns with PR objectives.Switching to
ghcr.io/weblateorg/base:2025.50.0removes the dependency on the custom CI docker image, consistent with the PR's goal to eliminate container-based CI infrastructure.
241-241: No action required. The-Dtypelib_installer=falseflag has no impact on translation operations.The
typelib_installeroption controls generation of a GObject introspection typelib installer tool and is entirely separate from translation handling. Translation targets (potfiles,pot,pot-set-year) depend only onxgettextand gettext tools, not on typelib infrastructure. Disabling this flag in theupdate-translationsjob is an appropriate performance optimization.Likely an incorrect or invalid review comment.
106-106: No action needed—ubuntu-slimis a valid GitHub-hosted runner.
ubuntu-slimis an official GitHub-hosted runner label, not a custom or self-hosted runner. It provides a lightweight, single-CPU container-based option designed for short jobs and automation tasks. With a 15-minute job timeout, the lint job's 5-minute timeout is well within limits.
228-230: All specified packages (tox, colorized-logs, gettext, libglib2.0-bin, ninja-build) are standard Debian/Ubuntu packages available in the container's repositories. The installation commands are correct and will succeed.
| - run: sudo apt-get update | ||
| - run: sudo apt-get install -y --no-install-recommends tox colorized-logs gjs gettext desktop-file-utils ninja-build | ||
|
|
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.
🧩 Analysis chain
🌐 Web query:
colorized-logs Ubuntu package availability
💡 Result:
Yes — the colorized-logs package is available in Ubuntu’s universe repository for multiple releases (e.g. Bionic, Focal, Jammy, Noble/24.04). Install with: sudo apt update && sudo apt install colorized-logs. [1][2][3]
Sources:
- Ubuntu (Focal) package page. [1]
- Ubuntu (Jammy) source on Launchpad (2.6-1). [2]
- UbuntuUpdates (Bionic). [3]
🏁 Script executed:
cat -n .github/workflows/ci.yml | sed -n '160,180p'Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 918
🏁 Script executed:
cat -n .github/workflows/ci.yml | head -50Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 1402
🏁 Script executed:
cat -n .github/workflows/ci.yml | sed -n '140,175p'Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 1551
🌐 Web query:
ubuntu-slim github actions runner default repositories universe
💡 Result:
Short answer: The ubuntu-slim GitHub Actions runner is a minimal image with limited preinstalled packages and incomplete public documentation; it may not have the "universe" archive enabled by default, so you should check /etc/apt/sources.list in the runner and enable/add "universe" in your workflow if you need packages from it. Recommend adding a step that enables universe (sudo add-apt-repository universe or echo the appropriate deb line) and running sudo apt-get update before apt install. [1][2][3]
Sources
- actions/runner-images repo (ubuntu-slim discussed; request for included-software list). [1]
- GitHub Docs: Customizing GitHub-hosted runners (install packages / run apt in workflow). [2]
- Issue reporting lack of ubuntu-slim documentation (shows ubuntu-slim is not documented like other images). [3]
🏁 Script executed:
grep -r "add-apt-repository\|universe" .github/workflows/Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 61
Add universe repository before installing colorized-logs.
The colorized-logs package exists in Ubuntu's universe repository, but ubuntu-slim runners may not have this repository enabled by default. Add sudo add-apt-repository universe before the apt-get install command, or ensure the universe repository is available in the runner environment.
🤖 Prompt for AI Agents
.github/workflows/ci.yml around lines 167 to 169: the workflow installs
colorized-logs which lives in Ubuntu's universe repo but ubuntu-slim runners may
not have it enabled; before the apt-get install line, add a step to enable the
universe repository (e.g., run sudo add-apt-repository -y universe) and then run
sudo apt-get update so the package can be found, or otherwise ensure the runner
environment has the universe repo enabled prior to installing colorized-logs.
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.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @.github/workflows/ci.yml:
- Around line 222-230: The workflow uses the ghcr.io image; replace the
container image value under container:image (the current
ghcr.io/weblateorg/wlc:1.16.1@sha256:...) with the official Docker Hub image
docker.io/weblate/wlc:1.16.1 and verify/update the sha256 digest to match the
Docker Hub manifest (or omit the digest if you prefer pinning by tag only);
ensure the change is made in the CI job block that sets
runs-on/timeout-minutes/container so subsequent apt-get installs run against the
official weblate/wlc base image.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.github/actions/container-test/action.yml.github/actions/pip-compile/action.yml.github/actions/tox-preinstall/action.yml.github/workflows/ci.yml.gitignoredo-in-docker.shdo-in-podman.shdocs/Build.mddocs/BuildInContainer.mddocs/CONTRIBUTING.mdrequirements/pip-compile.shtests/pict/gen.sh
💤 Files with no reviewable changes (8)
- do-in-podman.sh
- docs/CONTRIBUTING.md
- tests/pict/gen.sh
- .gitignore
- docs/Build.md
- docs/BuildInContainer.md
- do-in-docker.sh
- requirements/pip-compile.sh
🔇 Additional comments (6)
.github/actions/tox-preinstall/action.yml (1)
16-22: LGTM! Appropriate tox availability check and installation.The conditional installation logic correctly ensures tox is available in the host environment before proceeding with environment setup.
.github/actions/container-test/action.yml (2)
18-21: LGTM! Container pull step is correct.
23-33: LGTM! Test execution properly configured.The pytest invocation correctly passes the container configuration and generates the required reports.
.github/workflows/ci.yml (2)
106-106: LGTM! Appropriate runner for lint job.Using ubuntu-slim for the lightweight lint job is a good optimization.
241-241: No action needed —-Dtypelib_installer=falseis correctly applied.The flag appropriately disables the gjs-typelib-installer subproject for the translation-only job. Since the shebang is explicitly overridden with
/usr/bin/env gjs, thetypelib_installeroption is not required for the gjs dependency to be satisfied. The job only updates translation files (POTFILES.in and .pot), which do not need typelib installation functionality. This configuration reduces unnecessary dependencies and build overhead for the weblate container environment and is also used in the package build (PKGBUILD), confirming it is a valid and tested setup..github/actions/pip-compile/action.yml (1)
27-27: The ignore pattern correctly matches pip-compile output format.The pattern
^# This file is autogenerated by pip-compile with Pythonsuccessfully ignores version-specific comments from pip-compile, as confirmed by actual requirements files which contain comments like# This file is autogenerated by pip-compile with Python 3.14.
| runs-on: ubuntu-24.04 | ||
| timeout-minutes: 5 | ||
| container: | ||
| image: ghcr.io/ddterm/ci-docker-image:2025.12.05.0 | ||
| image: ghcr.io/weblateorg/wlc:1.16.1@sha256:6c6d4977a8377a69f0727963d8ff4f31b49f429102a1fb99c0843b941aeb1006 | ||
|
|
||
| steps: | ||
| - run: apt-get update | ||
| - run: apt-get install -y --no-install-recommends tox colorized-logs gettext libglib2.0-bin ninja-build | ||
|
|
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.
🧩 Analysis chain
🌐 Web query:
weblate wlc container image 1.16.1 ghcr.io
💡 Result:
Short answer: Weblate does not appear to publish an official wlc container image on ghcr.io. The official wlc container is published on Docker Hub (weblate/wlc); wlc releases (including v1.16.1) are available via PyPI/GitHub releases and distributions. [1][2][3]
Sources:
- Weblate wlc GitHub repository (no ghcr.io image referenced; Docker Hub used). [1]
- Docker Hub weblate/wlc (official image). [2]
- PyPI / release info for wlc (shows versions such as 1.16.1). [3]
🌐 Web query:
debian ubuntu packages tox colorized-logs gettext libglib2.0-bin ninja-build
💡 Result:
All five packages are available from Debian/Ubuntu apt repositories. Example apt command:
sudo apt update && sudo apt install colorized-logs tox gettext libglib2.0-bin ninja-build
Sources: colorized-logs [1], tox [2], gettext [3], libglib2.0-bin [4], ninja-build [5].
References
[1] Debian: colorized-logs package details.
[2] Debian: tox package details.
[3] Debian: gettext package search/details.
[4] Debian: libglib2.0-bin package details.
[5] Debian: ninja-build package details.
🏁 Script executed:
# Check if we can find any reference to this container or workflow in the repo
git ls-files | grep -E "workflows|ci\.yml"Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 187
🏁 Script executed:
cat -n .github/workflows/ci.yml | sed -n '222,230p'Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 471
🌐 Web query:
ghcr.io/weblateorg container image legitimacy official
💡 Result:
Yes — images under ghcr.io/weblateorg (e.g. ghcr.io/weblateorg/weblate) are the official Weblate releases. Weblate documents that its containers are published to Docker Hub and the GitHub Packages (GHCR) registry, and the WeblateOrg/docker package page shows the ghcr.io/weblateorg/weblate images/tags. [1][2][3]
Sources:
- Weblate docs — Choosing Docker image registry (mentions GitHub Packages). [1]
- WeblateOrg/docker GitHub package page (shows ghcr.io/weblateorg/weblate tags). [2]
- Weblate Docker Hub organization (official weblate images). [3]
🌐 Web query:
"ghcr.io/weblateorg/wlc" official weblate command line
💡 Result:
Short answer: the official Weblate command‑line client is called "wlc" (invoked as wlc). It's published as a Python package (pip install wlc) and as a Docker image under weblate/wlc on Docker Hub; canonical sources are the wlc GitHub repo and the Weblate docs. Example Docker run: docker run --rm weblate/wlc --url https://hosted.weblate.org/api/ list-projects. [1][2][3]
Sources:
- wlc GitHub repo (README). [1]
- Weblate documentation (wlc page). [2]
- weblate/wlc Docker Hub page. [3]
Use the official wlc Docker image from Docker Hub instead of the ghcr.io image.
The wlc official image is published to Docker Hub as weblate/wlc, not ghcr.io/weblateorg/wlc. Change the image reference to docker.io/weblate/wlc:1.16.1 and verify the digest. The package installation should work fine once the correct base image is used.
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 222 - 230, The workflow uses the
ghcr.io image; replace the container image value under container:image (the
current ghcr.io/weblateorg/wlc:1.16.1@sha256:...) with the official Docker Hub
image docker.io/weblate/wlc:1.16.1 and verify/update the sha256 digest to match
the Docker Hub manifest (or omit the digest if you prefer pinning by tag only);
ensure the change is made in the CI job block that sets
runs-on/timeout-minutes/container so subsequent apt-get installs run against the
official weblate/wlc base image.
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.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.github/actions/tox-preinstall/action.yml:
- Around line 16-22: Replace the non-portable command existence check using
"command -V tox" with the POSIX-standard "command -v tox" in the shell block
(the conditional in the action.yml shell run snippet) so the if-test uses
"command -v" to detect tox before attempting apt-get install; keep the same
logic and surrounding commands (sudo apt-get update / install) unchanged.
In @.github/workflows/ci.yml:
- Around line 226-230: In the update-translations job you should install gettext
via apt like the bundle job does instead of using Homebrew and PATH hacks:
remove the lines that echo /home/linuxbrew/.linuxbrew/bin >>"$GITHUB_PATH" and
brew install gettext and replace them with a sudo apt-get update (if not already
present in that job) and sudo apt-get install -y --no-install-recommends gettext
so the job uses the distro package manager consistently.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
.github/actions/container-test/action.yml.github/actions/pip-compile/action.yml.github/actions/tox-preinstall/action.yml.github/workflows/ci.yml.gitignoredo-in-docker.shdo-in-podman.shdocs/Build.mddocs/BuildInContainer.mddocs/CONTRIBUTING.mdrequirements/pip-compile.shtests/pict/gen.sh
💤 Files with no reviewable changes (8)
- docs/BuildInContainer.md
- docs/CONTRIBUTING.md
- do-in-docker.sh
- tests/pict/gen.sh
- .gitignore
- docs/Build.md
- requirements/pip-compile.sh
- do-in-podman.sh
🔇 Additional comments (5)
.github/actions/pip-compile/action.yml (1)
25-31: LGTM! Smart approach to ignore Python version differences.The ignore pattern effectively prevents false positives when the pip-compile autogenerated comment changes due to Python version updates. The regex syntax is correct and appropriately anchored.
.github/actions/container-test/action.yml (1)
18-33: Excellent simplification!The removal of complex cgroup mounting and ownership management significantly reduces complexity while maintaining functionality. The direct bash execution with tox is cleaner and easier to maintain.
.github/workflows/ci.yml (3)
105-157: Clean migration to ubuntu-slim runner.The lint job successfully migrates to host-based execution. The
./.github/actions/tox-preinstallaction (reviewed in another file) now ensures tox is available when needed.
238-241: Appropriate build configuration for translation-only workflow.The
-Dtypelib_installer=falseflag correctly optimizes the build configuration for this job's specific purpose (updating translations), avoiding unnecessary typelib generation.
276-312: Test job migration looks solid.The test job successfully migrates to ubuntu-24.04 host execution while still running tests inside containers via the simplified
container-testaction. The dependency installation throughtox-preinstallis appropriate.
| - shell: bash | ||
| run: | | ||
| if ! command -V tox | ||
| then | ||
| sudo apt-get update | ||
| sudo apt-get install -y --no-install-recommends tox | ||
| fi |
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.
🧹 Nitpick | 🔵 Trivial
Good defensive setup for host-based execution.
Installing tox on-demand ensures it's available on ubuntu-slim runners without assuming pre-installed packages. The conditional check prevents unnecessary reinstalls.
💡 Optional: Consider using `command -v` for POSIX portability
While command -V works fine in bash, command -v is more portable:
- if ! command -V tox
+ if ! command -v toxBoth work here, but -v is the POSIX standard flag for checking command existence.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - shell: bash | |
| run: | | |
| if ! command -V tox | |
| then | |
| sudo apt-get update | |
| sudo apt-get install -y --no-install-recommends tox | |
| fi | |
| - shell: bash | |
| run: | | |
| if ! command -v tox | |
| then | |
| sudo apt-get update | |
| sudo apt-get install -y --no-install-recommends tox | |
| fi |
🤖 Prompt for AI Agents
In @.github/actions/tox-preinstall/action.yml around lines 16 - 22, Replace the
non-portable command existence check using "command -V tox" with the
POSIX-standard "command -v tox" in the shell block (the conditional in the
action.yml shell run snippet) so the if-test uses "command -v" to detect tox
before attempting apt-get install; keep the same logic and surrounding commands
(sudo apt-get update / install) unchanged.
| - run: sudo apt-get update | ||
| - run: sudo apt-get install -y --no-install-recommends tox colorized-logs | ||
|
|
||
| - run: echo /home/linuxbrew/.linuxbrew/bin >>"$GITHUB_PATH" | ||
| - run: brew install gettext |
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for gettext version constraints in the project
rg -i "gettext" --type-list | head -20
fd . -type f -name "*.build" -o -name "meson.build" -o -name "*.py" -o -name "*.md" | head -20Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 639
🏁 Script executed:
#!/bin/bash
# Search for gettext references and version requirements
rg "gettext" --type md --type txt --type python -A 3 -B 3 | head -100Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 109
🏁 Script executed:
#!/bin/bash
# Check meson.build for gettext configuration
find . -name "meson.build" -type f -exec cat {} \; | grep -A 5 -B 5 "gettext" 2>/dev/nullRepository: ddterm/gnome-shell-extension-ddterm
Length of output: 2455
🏁 Script executed:
#!/bin/bash
# Look for version specifications in the workflow and related files
rg -A 5 -B 5 "gettext|colorized" .github/workflows/ci.yml | head -50Repository: ddterm/gnome-shell-extension-ddterm
Length of output: 905
Install gettext via apt-get for consistency with other jobs.
The bundle job installs gettext via apt (line 163), but the update-translations job uses Homebrew instead (lines 229-230). Since both jobs run on ubuntu-24.04 and the package is available in Ubuntu's repositories, install it via apt-get to be consistent and avoid the unnecessary Homebrew PATH manipulation.
Suggested change
- run: sudo apt-get update
- - run: sudo apt-get install -y --no-install-recommends tox colorized-logs
-
- - run: echo /home/linuxbrew/.linuxbrew/bin >>"$GITHUB_PATH"
- - run: brew install gettext
+ - run: sudo apt-get install -y --no-install-recommends tox colorized-logs gettext📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - run: sudo apt-get update | |
| - run: sudo apt-get install -y --no-install-recommends tox colorized-logs | |
| - run: echo /home/linuxbrew/.linuxbrew/bin >>"$GITHUB_PATH" | |
| - run: brew install gettext | |
| - run: sudo apt-get update | |
| - run: sudo apt-get install -y --no-install-recommends tox colorized-logs gettext |
🤖 Prompt for AI Agents
In @.github/workflows/ci.yml around lines 226 - 230, In the update-translations
job you should install gettext via apt like the bundle job does instead of using
Homebrew and PATH hacks: remove the lines that echo
/home/linuxbrew/.linuxbrew/bin >>"$GITHUB_PATH" and brew install gettext and
replace them with a sudo apt-get update (if not already present in that job) and
sudo apt-get install -y --no-install-recommends gettext so the job uses the
distro package manager consistently.
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.