-
-
Notifications
You must be signed in to change notification settings - Fork 66
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
Upgrade base to 17.1.1 and resolve problems with building librespot 0.6.0 #281
Conversation
Warning Rate limit exceeded@frenck has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 12 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request focuses on upgrading the Spotify add-on's build environment by updating the base image and multiple system dependencies. The changes primarily involve moving from version 14.3.3 to 17.1.0 of the base image across different architectures. Several packages have been updated, including Rust's cargo, Git, protobuf, PulseAudio, and new dependencies like CMake and Clang have been introduced. Additionally, the installation process for Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
spotify/Dockerfile
(3 hunks)spotify/build.yaml
(1 hunks)
🔇 Additional comments (4)
spotify/build.yaml (1)
3-5
: Verify the base image version upgrade.The upgrade from version 14.3.3 to 17.1.0 is a significant jump that might introduce breaking changes.
Let's verify the base image version and its changelog:
✅ Verification successful
Base image upgrade to v17.1.0 appears safe to proceed
The upgrade primarily contains routine updates to Alpine Linux (v3.21.2) and home-assistant/tempio. No breaking changes are documented in the release notes for v17.0.0 through v17.1.0, and all versions are stable releases.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify base image version and changelog # Check if the image exists for all architectures for arch in aarch64 amd64 armv7; do echo "Checking $arch..." gh api \ -H "Accept: application/vnd.oci.image.index.v1+json" \ "/v2/hassio-addons/base/manifests/17.1.0" || echo "Image not found" done # Get the latest releases to verify if 17.1.0 is the latest stable version gh release list --repo hassio-addons/addon-base --limit 5Length of output: 1843
Script:
#!/bin/bash # Get release notes for v17.1.0 gh release view v17.1.0 --repo hassio-addons/addon-base # Also check if there's a changelog file in the repository fd -t f -e md -e txt changelog -iLength of output: 479
Script:
#!/bin/bash # Get release notes for v17.0.0 to check for breaking changes gh release view v17.0.0 --repo hassio-addons/addon-baseLength of output: 559
spotify/Dockerfile (3)
1-1
: Overall changes look good!The dependency updates and build process changes appear well-structured and appropriate for supporting librespot 0.6.0. The cleanup process is properly maintained to keep the image size minimal.
22-22
: Verify pulseaudio runtime version compatibility.The runtime pulseaudio package has been updated to match the dev package version.
Let's check for any breaking changes in PulseAudio 17.0:
✅ Verification successful
PulseAudio 17.0 runtime version is compatible
The version update primarily includes feature additions and improvements with no breaking changes that would affect runtime compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check PulseAudio changelog for breaking changes echo "Checking PulseAudio changelog..." curl -s "https://www.freedesktop.org/wiki/Software/PulseAudio/Notes/17.0/" || echo "Changelog not found"Length of output: 17794
Line range hint
24-36
: Review bindgen-cli installation process.The addition of bindgen-cli installation and subsequent removal is a new build step. This appears to be related to resolving build issues with librespot 0.6.0.
Let's verify if this is a required dependency for librespot 0.6.0:
cargo=1.83.0-r0 \ | ||
git=2.47.2-r0 \ | ||
protobuf-dev=24.4-r4 \ | ||
pulseaudio-dev=17.0-r4 \ | ||
cmake=3.31.1-r0 \ | ||
clang19-libclang=19.1.4-r0 \ |
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.
💡 Codebase verification
The specified package versions appear to be incorrect or unavailable:
git=2.47.2-r0
- Version number is too high (latest is 2.43.x)cmake=3.31.1-r0
- Version number is too high (latest is 3.27.x)- Other package versions were not found in any Alpine repository (main, community, or testing)
Please verify the correct package versions available in Alpine Linux repositories and update accordingly.
🔗 Analysis chain
Verify package version compatibility.
The PR updates multiple build dependencies to newer versions and adds new ones (cmake, clang19-libclang). Let's verify these versions are available in the Alpine package repository for the base image's Alpine version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify package versions in Alpine repository
# Extract Alpine version from base image
echo "Checking Alpine version..."
gh api \
-H "Accept: application/vnd.docker.distribution.manifest.v2+json" \
"/v2/hassio-addons/base/manifests/17.1.0" | jq -r '.config.digest' | xargs -I {} \
gh api \
-H "Accept: application/vnd.docker.container.image.v1+json" \
"/v2/hassio-addons/base/blobs/{}" | jq -r '.config.Labels["org.opencontainers.image.version"]'
# Check if packages exist with specified versions
packages=(
"cargo=1.83.0-r0"
"git=2.47.2-r0"
"protobuf-dev=24.4-r4"
"pulseaudio-dev=17.0-r4"
"cmake=3.31.1-r0"
"clang19-libclang=19.1.4-r0"
)
for pkg in "${packages[@]}"; do
echo "Checking $pkg..."
# Use curl to check Alpine package repository
# Note: This is a simplified check, actual repository URL should match Alpine version
curl -s "https://dl-cdn.alpinelinux.org/alpine/edge/main/x86_64/APKINDEX.tar.gz" | tar -xz -O | grep -A1 "${pkg%%=*}"
done
Length of output: 2306
Script:
#!/bin/bash
# Get Alpine version from Dockerfile
echo "Base image version:"
grep "FROM ghcr.io/hassio-addons/base" spotify/Dockerfile
# Function to check package in repository
check_package() {
local pkg=$1
local ver=$2
local repo=$3
echo "Checking $pkg=$ver in $repo..."
curl -s "https://dl-cdn.alpinelinux.org/alpine/v3.19/$repo/x86_64/APKINDEX.tar.gz" | tar -xz -O | awk -v pkg="$pkg" -v ver="$ver" '
$1 == "P:" && $2 == pkg { p=1; next }
p && $1 == "V:" { if ($2 == ver) print "✓ Found exact version"; else print "× Version mismatch, found: "$2; p=0 }
'
}
# Check each package in main, community, and testing
packages=(
"cargo:1.83.0-r0"
"git:2.47.2-r0"
"protobuf-dev:24.4-r4"
"pulseaudio-dev:17.0-r4"
"cmake:3.31.1-r0"
"clang19-libclang:19.1.4-r0"
)
for pkg_info in "${packages[@]}"; do
pkg=${pkg_info%:*}
ver=${pkg_info#*:}
for repo in main community testing; do
check_package "$pkg" "$ver" "$repo"
done
echo "---"
done
Length of output: 11284
Script:
#!/bin/bash
# Get base image version
echo "Base image version:"
head -n1 spotify/Dockerfile
# Function to check package in repository
check_package() {
local pkg=$1
local ver=$2
local branch=$3
local repo=$4
echo "Checking $pkg=$ver in $branch/$repo..."
curl -sf "https://dl-cdn.alpinelinux.org/alpine/$branch/$repo/x86_64/APKINDEX.tar.gz" | tar -xz -O 2>/dev/null | awk -v pkg="$pkg" -v ver="$ver" '
$1 == "P:" && $2 == pkg { p=1; next }
p && $1 == "V:" { if ($2 == ver) print "✓ Found exact version"; else print "× Version mismatch, found: "$2; p=0 }
'
}
# Check each package in edge repositories
packages=(
"cargo:1.83.0-r0"
"git:2.47.2-r0"
"protobuf-dev:24.4-r4"
"pulseaudio-dev:17.0-r4"
"cmake:3.31.1-r0"
"clang19-libclang:19.1.4-r0"
)
for pkg_info in "${packages[@]}"; do
pkg=${pkg_info%:*}
ver=${pkg_info#*:}
echo "Package: $pkg"
# Check in edge repositories
for repo in main community testing; do
check_package "$pkg" "$ver" "edge" "$repo"
done
echo "---"
done
Length of output: 11587
This PR is mostly a duplicate of #253, and shows the same issue. armv7 & aarch64 not passing. |
I'm not 100% certain, however, I think that is the issue of building images, on emulated environment. Will try to build it on arm-native machine. |
|
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.
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.
Unfortunately, the CI is still failing.
Since image builds successfully on arm based systems, we can conclude that it isn't necessarily a code error. I think the issue cannot be fixed without switching to either:
|
That one was on my todolist for quite some time now, this was the final push for doing that :) All add-on workflows now use the arm runners for all arm arch add-ons. Builds passes :) ../Frenck |
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.
Thanks again @ImJustToNy 🙏
../Frenck
Proposed Changes
Hi, foremost, thanks for this add-on :)
I believe this PR should fix build issues by addressing dependency problems.
It's my first time contributing to hassio-addons, so please forgive me if I made any issues.
Wish you a good day!
Summary by CodeRabbit