ci: add deepin & archlinux build CI #66
Merged
+223
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This may work as intended.
It's informative and suitable for everyone that want to have look to this project.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: calsys456 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @calsys456. Thanks for your PR. I'm waiting for a linuxdeepin member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Reviewer's GuideAdds two new GitHub Actions workflows to build and package ddm on Arch Linux and Deepin Crimson, including building treeland-protocols from source and uploading build artifacts. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 5 issues, and left some high level feedback:
- In the Arch Linux workflow you both install
treeland-protocolsviapacmanand then build/install it from source; consider using only the source build (or only the package) to avoid redundant work and potential conflicts. - The Arch Linux job runs
pacman -Syumultiple times (initial update, then in base deps, then again just to installzip); you can significantly speed up and stabilize the job by doing a single full upgrade and then usingpacman -S --neededfor additional packages. - In the Arch Linux CMake invocation you pass
-DCMAKE_CXX_FLAGS='${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror', but the single quotes prevent any expansion and${CMAKE_CXX_FLAGS}is a CMake variable, not a shell one; prefer something like-DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror"or set these flags directly in the CMakeLists instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the Arch Linux workflow you both install `treeland-protocols` via `pacman` and then build/install it from source; consider using only the source build (or only the package) to avoid redundant work and potential conflicts.
- The Arch Linux job runs `pacman -Syu` multiple times (initial update, then in base deps, then again just to install `zip`); you can significantly speed up and stabilize the job by doing a single full upgrade and then using `pacman -S --needed` for additional packages.
- In the Arch Linux CMake invocation you pass `-DCMAKE_CXX_FLAGS='${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror'`, but the single quotes prevent any expansion and `${CMAKE_CXX_FLAGS}` is a CMake variable, not a shell one; prefer something like `-DCMAKE_CXX_FLAGS="-Wall -Wextra -Werror"` or set these flags directly in the CMakeLists instead.
## Individual Comments
### Comment 1
<location> `.github/workflows/ddm-archlinux-build.yml:44-48` </location>
<code_context>
+ with:
+ fetch-depth: 0
+
+ - name: Configure and Build ddm with ci preset
+ run: |
+ echo "Working directory:" $PWD
+ echo "Configuring ddm..."
+ cmake -B build -G Ninja \
+ -DCMAKE_INSTALL_PREFIX=/usr \
+ -DCMAKE_BUILD_TYPE=Release \
</code_context>
<issue_to_address>
**issue (bug_risk):** CMAKE_CXX_FLAGS is not expanded due to single-quoted string and will be passed literally to CMake.
Because the value is single-quoted:
```yaml
-DCMAKE_CXX_FLAGS='${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror'
```
`${CMAKE_CXX_FLAGS}` will not be expanded by the shell and will be passed to CMake literally. If you intend to append to the environment variable, use double quotes instead:
```yaml
-DCMAKE_CXX_FLAGS="${CMAKE_CXX_FLAGS} -Wall -Wextra -Werror"
```
Or set the full flag string directly if you don’t need the existing value.
</issue_to_address>
### Comment 2
<location> `.github/workflows/ddm-archlinux-build.yml:16-20` </location>
<code_context>
+ runs-on: ubuntu-latest
+ container: archlinux:latest
+ steps:
+ - name: Run in container
+ run: |
+ cat /etc/pacman.d/mirrorlist
+ pacman-key --init
+ pacman --noconfirm --noprogressbar -Syu
+
+ - name: Install base dependencies
</code_context>
<issue_to_address>
**suggestion (performance):** You are running a full pacman -Syu twice, which adds unnecessary time and risk in CI.
The first step runs `pacman --noconfirm --noprogressbar -Syu`, and the `Install base dependencies` step also runs `pacman -Syu --noconfirm --noprogressbar ...`, causing a second full upgrade that slows CI and can introduce breaking changes mid-job.
You can usually drop the initial `-Syu` and rely on the one in the dependency install step, or change the first command to `pacman -Sy` if you only need to refresh package databases.
```suggestion
- name: Run in container
run: |
cat /etc/pacman.d/mirrorlist
pacman-key --init
```
</issue_to_address>
### Comment 3
<location> `.github/workflows/ddm-archlinux-build.yml:70-71` </location>
<code_context>
+ find /tmp/ddm-install -type f | head -20
+ echo "Total files installed: $(find /tmp/ddm-install -type f | wc -l)"
+
+ - name: Create ddm installation package
+ run: |
+ cd /tmp/ddm-install
+
+ # Install zip if not available
+ pacman -Syu --noconfirm zip
+
+ # Create package info
</code_context>
<issue_to_address>
**suggestion (performance):** Re-running pacman -Syu just to install zip is overkill; consider installing zip alongside the other base dependencies.
This command performs a second full system upgrade mid-workflow. To avoid unnecessary work and keep the environment consistent, install `zip` with the earlier base dependencies, or if you only need the package here, use `pacman -S --noconfirm zip` instead.
```suggestion
# Install zip if not available
pacman -S --noconfirm zip
```
</issue_to_address>
### Comment 4
<location> `.github/workflows/ddm-deepin-build.yml:45-38` </location>
<code_context>
+ - name: Build ddm deb package
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The workflow continues to dpkg-buildpackage even when dpkg-checkbuilddeps reports unmet dependencies.
Here you capture `dpkg-checkbuilddeps`’s return code and print diagnostics, but always continue to `dpkg-buildpackage`. When build-deps are missing, the build will likely fail later with a less clear error. After printing the analysis, explicitly exit (e.g. `exit 1`) when `BUILDDEPS_RC != 0` so the job fails at the dependency-check step.
Suggested implementation:
```
- name: Build ddm deb package
run: |
set -euxo pipefail
# Check build dependencies and diagnose missing packages
set +e
dpkg-checkbuilddeps 2> /tmp/ddm-builddeps.err
BUILDDEPS_RC=$?
set -e
if [ "$BUILDDEPS_RC" -ne 0 ]; then
# If build dependencies are missing, fail the job after printing diagnostics
```
Inside the `if [ "$BUILDDEPS_RC" -ne 0 ]; then` block (after whatever logic prints/analyses `/tmp/ddm-builddeps.err` and before the corresponding `fi`), add a line:
```bash
exit 1
```
This will cause the workflow to fail immediately at the dependency-check step, instead of proceeding to `dpkg-buildpackage` with missing build-deps.
</issue_to_address>
### Comment 5
<location> `.github/workflows/ddm-deepin-build.yml:62-63` </location>
<code_context>
+ # Extract and list each missing package with version requirements
+ echo ""
+ echo "Detailed missing packages analysis:"
+ grep -oP '(?<=Unmet build dependencies: ).*' /tmp/ddm-builddeps.err | tr ' ' '\n' | while read pkg; do
+ if [ -n "$pkg" ]; then
+ # Check if package exists in repos
+ apt-cache policy "$pkg" 2>/dev/null | head -20 || echo "Package '$pkg' not found in any repository"
</code_context>
<issue_to_address>
**issue:** The parsing of dpkg-checkbuilddeps output splits on spaces and will mangle package names with version constraints.
Splitting the error line on spaces means versioned deps are broken into invalid tokens (e.g. `libfoo-dev (>= 1.2), libbar-dev` becomes `libfoo-dev`, `(>=`, `1.2),`, `libbar-dev`), and `apt-cache policy` is run on the fragments, producing misleading output.
Consider splitting on commas, then stripping version constraints and whitespace before calling `apt-cache`, e.g.:
```bash
deps=$(grep -oP '(?<=Unmet build dependencies: ).*' /tmp/ddm-builddeps.err)
printf '%s
' "$deps" | tr ',' '\n' | while read pkg; do
pkg="$(echo "$pkg" | sed 's/(.*)//' | xargs)"
[ -n "$pkg" ] || continue
apt-cache policy "$pkg" 2>/dev/null | head -20 || \
echo "Package '$pkg' not found in any repository"
done
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
calsys456
force-pushed
the
fix
branch
2 times, most recently
from
f4763a1 to
9454258
Compare
zccrs
approved these changes
Jan 15, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This may work as intended.
Summary by Sourcery
Add CI workflows to build ddm on Arch Linux and Deepin containers and publish build artifacts.
CI: